Request.ReadEntity does not cache the entity for rereading about go-restful HOT 9 CLOSED

So this would mean the Request will keep around its payload for the complete request-response flow. I guess that should not put an extra strain on the processing or memory consumption; the request will do a ioutil.ReadAll(r.Request.Body) anyway. In addition, route functions (or filters) must call ReadEntity explicitly so those that need streaming access are not affected.

Alternatively, you could store the entity in a request attributes (using SetAttribute) and pick it up later in the function (Get)

Putting the implementation aside, I am curious why a filter would need the payload ahead of the route function. Can you give an example?

from go-restful.

Example: If you are using a filter to check for whether a particular action( i.e. a POST) and validate the request can be properly created before the particular action is executed.

I see your point about setAttribute, but if you are creating a set of "mix and match" filters and functions, it seems to me you would not necessarily know if read entity had been called already. having to check and then calling it yourself and using setAttribute would get a little tedious and annoying after a while.

from go-restful.

Agree, using your solution the route functions do not need to be aware of the fact that the entity may have been read earlier in the process.

I have merged your changes. Thanks.

from go-restful.

Recently, I got a merge request that invalidates this feature ( caching the resource for re-reading) for performance reasons (imagine what impact large content could have). Instead, I would like to propose to use the request attribute for this special case. ( see earlier discussion).

from go-restful.

That totally makes sense. It's a performance issue for some folks. There's now the issue of backwards compatibility, though.

Also, this would lead to a lot of boilerplate code like the following:

data := request.Attribute("cachedData")
if data == nil {
  err := request.ReadEntity(data)
  if err != nil {
    return err
  }
  request.SetAttribute("cachedData", data)
}
// Now I can use data.

We could add a method that does this boilerplate:
func (req *Request) ReadCachedEntity(interface{}) (error)
This could be implemented internally using attributes, or by adding an interface{} field to the Request struct. Would need some documentation to explain that folks should either use this or ReadEntity, but not both.

Thing is, that's exactly what ReadEntity does today. It's also what clients have come to expect.

Another option would be to provide a new method that does the performant thing by not caching for folks who don't want to use the caching scheme:
func (req *Request) ReadEntityWithoutCaching(interface{}) (error)
Then encourage folks who don't need caching to use that. I believe that may be the lightest burden on existing users.

Thoughts?

from go-restful.

Breaking the API is indeed not an option. Thank you for your suggestions to handle this. Give me some time to think about the solutions.

from go-restful.

How about this alternative. We introduce a setting that can be changed with SetCacheReadEntity() and make ReadEntity behave accordingly. Default value would be true so not change w.r.t current status

from go-restful.

Sounds great!

On Tue, Jun 17, 2014 at 11:22 PM, Ernest Micklei [email protected]
wrote:

How about this alternative. We introduce a setting that can be changed
with SetCacheReadEntity() and make ReadEntity behave accordingly. Default
value would be true so not change w.r.t current status

—
Reply to this email directly or view it on GitHub
#74 (comment).

Jeff Carollo
Software Engineer

usermind
p 206.535.2867 x110
m 979.574.1534

from go-restful.

So what should we do now? is there any way to re-read request?

from go-restful.