JsonRpcServlet postBody handling

[Robert Evans <bo...@google.com>]

I'd like to log parts of the post body at the end of the request
handling. The way it's currently being done, the servlet pulls the
content out of the post body stream, instantiates json objects from
it, and throws it away. The stream on the request can't be reset,
because it wasn't marked, and hence the post body content is no longer
available.

I see two options to retain the post body:

1) (Token bad suggestion) Have a servlet filter that marks the stream,
reads the stream, grabs off what it wants, makes a request attribute
and then resets the stream for later processing by the JsonRpcServlet.
Sounds redundant and inefficient as it has the stream being read
twice.

2) (What I propose) Have a servlet filter that reads the stream, and
stashes the post body as a request attribute in a filter. Modify
JsonRpcServlet to retrieve the attribute the same way that my
end-of-request logging will once the post body as attribute is
available.

Is this cool with everyone? If so, I can whip up a patch. Other
suggestions are welcome if it is not.

Thanks,
Bob Evans

Re: JsonRpcServlet postBody handling

[Louis Ryan <lr...@google.com>]

This is kind of what I had in mind. In addition making many of the
implementation methods protected allows for subclasses to intercept calls
they are interested in and still utilize the behavior of the base impl which
is a fairly common pattern in the gadgets code. You could reasonably make an
argument for a listener interface that can be attached to the impl instead
of subclassing to allow for finer grained event tracking and protecting
implementors from unintended refactoring consequences in the base but that
may be overkill.

One advantage of refactoring with subclassing in mind is that it generally
forces the base impl to modularize itself into usefully composeable methods
which is generally a good design goal anyway...

Maybe some of the Hi5 folks have feelings on this as I hear they have a
pretty impressive stats reporting solution....

On Wed, Sep 3, 2008 at 10:18 PM, Kevin Brown <et...@google.com> wrote:

> One painful lesson we had to learn when doing the gadget rendering code is
> that doing any non-trivial work in servlets is inflexible and leads to
> problems down the road. I'd suggest making the servlet parts little more
> than input handling with an adapter for providing the "real" interface that
> the handling code deals with.
> This also makes it a lot easier for people who aren't using servlets to
> reuse this code, including such use cases as implementing data pipelining
> for opensocial templates.
>
> On Wed, Sep 3, 2008 at 7:54 PM, Robert Evans <bo...@google.com> wrote:
>
> > Which refactorings are you thinking of? I could be into implementing
> them.
> >
> > For my case, I wasn't going to re-jsonify the post body, I was just
> > going to regex the method name/pair.
> >
> > In general, though, it seems like the post body should be turned into
> > what it is and then be available for anyone to work with. So, I could
> > imagine a filter that grabs the post body, recognizes application/json
> > content type and runs the JSON.createObject calls and then stuffs that
> > in the attributes. Then whoever wanted to use that json, or inspect
> > its properties would have a tidy little structure to do it.
> >
> > Is that the refactoring of which you were thinking?
> >
> >
> > On Wed, Sep 3, 2008 at 7:45 PM, Louis Ryan <lr...@google.com> wrote:
> > > As you rightly point out one of the issues with the existing API
> servlets
> > is
> > > they dont allow for cross-cuts so that implementers can introduce
> logging
> > > features and statistics gathering. How do you feel about some
> refactoring
> > > love in the servlets rather than stashing the POST body away which you
> > would
> > > have to re-parse?
> > >
> > > On Wed, Sep 3, 2008 at 7:31 PM, Robert Evans <bo...@google.com>
> > wrote:
> > >
> > >> I'm interested in logging the types of requests that get processed by
> > >> our implementation of OpenSocial. For instance, in the jsonrpc
> > >> requests, I want the 'method' property of the post body so that I can
> > >> see how many of each type of request we are handling at the point when
> > >> my logger gets called.
> > >>
> > >> On Wed, Sep 3, 2008 at 6:20 PM, Louis Ryan <lr...@google.com> wrote:
> > >> > Just out curiosity what are you interested in logging?
> > >> >
> > >> > On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com>
> > >> wrote:
> > >> >
> > >> >> I'd like to log parts of the post body at the end of the request
> > >> >> handling. The way it's currently being done, the servlet pulls the
> > >> >> content out of the post body stream, instantiates json objects from
> > >> >> it, and throws it away. The stream on the request can't be reset,
> > >> >> because it wasn't marked, and hence the post body content is no
> > longer
> > >> >> available.
> > >> >>
> > >> >> I see two options to retain the post body:
> > >> >>
> > >> >> 1) (Token bad suggestion) Have a servlet filter that marks the
> > stream,
> > >> >> reads the stream, grabs off what it wants, makes a request
> attribute
> > >> >> and then resets the stream for later processing by the
> > JsonRpcServlet.
> > >> >> Sounds redundant and inefficient as it has the stream being read
> > >> >> twice.
> > >> >>
> > >> >> 2) (What I propose) Have a servlet filter that reads the stream,
> and
> > >> >> stashes the post body as a request attribute in a filter. Modify
> > >> >> JsonRpcServlet to retrieve the attribute the same way that my
> > >> >> end-of-request logging will once the post body as attribute is
> > >> >> available.
> > >> >>
> > >> >> Is this cool with everyone? If so, I can whip up a patch. Other
> > >> >> suggestions are welcome if it is not.
> > >> >>
> > >> >> Thanks,
> > >> >> Bob Evans
> > >> >>
> > >> >
> > >>
> > >
> >
>

Re: JsonRpcServlet postBody handling

[Kevin Brown <et...@google.com>]

One painful lesson we had to learn when doing the gadget rendering code is
that doing any non-trivial work in servlets is inflexible and leads to
problems down the road. I'd suggest making the servlet parts little more
than input handling with an adapter for providing the "real" interface that
the handling code deals with.
This also makes it a lot easier for people who aren't using servlets to
reuse this code, including such use cases as implementing data pipelining
for opensocial templates.

On Wed, Sep 3, 2008 at 7:54 PM, Robert Evans <bo...@google.com> wrote:

> Which refactorings are you thinking of? I could be into implementing them.
>
> For my case, I wasn't going to re-jsonify the post body, I was just
> going to regex the method name/pair.
>
> In general, though, it seems like the post body should be turned into
> what it is and then be available for anyone to work with. So, I could
> imagine a filter that grabs the post body, recognizes application/json
> content type and runs the JSON.createObject calls and then stuffs that
> in the attributes. Then whoever wanted to use that json, or inspect
> its properties would have a tidy little structure to do it.
>
> Is that the refactoring of which you were thinking?
>
>
> On Wed, Sep 3, 2008 at 7:45 PM, Louis Ryan <lr...@google.com> wrote:
> > As you rightly point out one of the issues with the existing API servlets
> is
> > they dont allow for cross-cuts so that implementers can introduce logging
> > features and statistics gathering. How do you feel about some refactoring
> > love in the servlets rather than stashing the POST body away which you
> would
> > have to re-parse?
> >
> > On Wed, Sep 3, 2008 at 7:31 PM, Robert Evans <bo...@google.com>
> wrote:
> >
> >> I'm interested in logging the types of requests that get processed by
> >> our implementation of OpenSocial. For instance, in the jsonrpc
> >> requests, I want the 'method' property of the post body so that I can
> >> see how many of each type of request we are handling at the point when
> >> my logger gets called.
> >>
> >> On Wed, Sep 3, 2008 at 6:20 PM, Louis Ryan <lr...@google.com> wrote:
> >> > Just out curiosity what are you interested in logging?
> >> >
> >> > On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com>
> >> wrote:
> >> >
> >> >> I'd like to log parts of the post body at the end of the request
> >> >> handling. The way it's currently being done, the servlet pulls the
> >> >> content out of the post body stream, instantiates json objects from
> >> >> it, and throws it away. The stream on the request can't be reset,
> >> >> because it wasn't marked, and hence the post body content is no
> longer
> >> >> available.
> >> >>
> >> >> I see two options to retain the post body:
> >> >>
> >> >> 1) (Token bad suggestion) Have a servlet filter that marks the
> stream,
> >> >> reads the stream, grabs off what it wants, makes a request attribute
> >> >> and then resets the stream for later processing by the
> JsonRpcServlet.
> >> >> Sounds redundant and inefficient as it has the stream being read
> >> >> twice.
> >> >>
> >> >> 2) (What I propose) Have a servlet filter that reads the stream, and
> >> >> stashes the post body as a request attribute in a filter. Modify
> >> >> JsonRpcServlet to retrieve the attribute the same way that my
> >> >> end-of-request logging will once the post body as attribute is
> >> >> available.
> >> >>
> >> >> Is this cool with everyone? If so, I can whip up a patch. Other
> >> >> suggestions are welcome if it is not.
> >> >>
> >> >> Thanks,
> >> >> Bob Evans
> >> >>
> >> >
> >>
> >
>

Re: JsonRpcServlet postBody handling

[Robert Evans <bo...@google.com>]

Which refactorings are you thinking of? I could be into implementing them.

For my case, I wasn't going to re-jsonify the post body, I was just
going to regex the method name/pair.

In general, though, it seems like the post body should be turned into
what it is and then be available for anyone to work with. So, I could
imagine a filter that grabs the post body, recognizes application/json
content type and runs the JSON.createObject calls and then stuffs that
in the attributes. Then whoever wanted to use that json, or inspect
its properties would have a tidy little structure to do it.

Is that the refactoring of which you were thinking?


On Wed, Sep 3, 2008 at 7:45 PM, Louis Ryan <lr...@google.com> wrote:
> As you rightly point out one of the issues with the existing API servlets is
> they dont allow for cross-cuts so that implementers can introduce logging
> features and statistics gathering. How do you feel about some refactoring
> love in the servlets rather than stashing the POST body away which you would
> have to re-parse?
>
> On Wed, Sep 3, 2008 at 7:31 PM, Robert Evans <bo...@google.com> wrote:
>
>> I'm interested in logging the types of requests that get processed by
>> our implementation of OpenSocial. For instance, in the jsonrpc
>> requests, I want the 'method' property of the post body so that I can
>> see how many of each type of request we are handling at the point when
>> my logger gets called.
>>
>> On Wed, Sep 3, 2008 at 6:20 PM, Louis Ryan <lr...@google.com> wrote:
>> > Just out curiosity what are you interested in logging?
>> >
>> > On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com>
>> wrote:
>> >
>> >> I'd like to log parts of the post body at the end of the request
>> >> handling. The way it's currently being done, the servlet pulls the
>> >> content out of the post body stream, instantiates json objects from
>> >> it, and throws it away. The stream on the request can't be reset,
>> >> because it wasn't marked, and hence the post body content is no longer
>> >> available.
>> >>
>> >> I see two options to retain the post body:
>> >>
>> >> 1) (Token bad suggestion) Have a servlet filter that marks the stream,
>> >> reads the stream, grabs off what it wants, makes a request attribute
>> >> and then resets the stream for later processing by the JsonRpcServlet.
>> >> Sounds redundant and inefficient as it has the stream being read
>> >> twice.
>> >>
>> >> 2) (What I propose) Have a servlet filter that reads the stream, and
>> >> stashes the post body as a request attribute in a filter. Modify
>> >> JsonRpcServlet to retrieve the attribute the same way that my
>> >> end-of-request logging will once the post body as attribute is
>> >> available.
>> >>
>> >> Is this cool with everyone? If so, I can whip up a patch. Other
>> >> suggestions are welcome if it is not.
>> >>
>> >> Thanks,
>> >> Bob Evans
>> >>
>> >
>>
>

Re: JsonRpcServlet postBody handling

[Louis Ryan <lr...@google.com>]

As you rightly point out one of the issues with the existing API servlets is
they dont allow for cross-cuts so that implementers can introduce logging
features and statistics gathering. How do you feel about some refactoring
love in the servlets rather than stashing the POST body away which you would
have to re-parse?

On Wed, Sep 3, 2008 at 7:31 PM, Robert Evans <bo...@google.com> wrote:

> I'm interested in logging the types of requests that get processed by
> our implementation of OpenSocial. For instance, in the jsonrpc
> requests, I want the 'method' property of the post body so that I can
> see how many of each type of request we are handling at the point when
> my logger gets called.
>
> On Wed, Sep 3, 2008 at 6:20 PM, Louis Ryan <lr...@google.com> wrote:
> > Just out curiosity what are you interested in logging?
> >
> > On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com>
> wrote:
> >
> >> I'd like to log parts of the post body at the end of the request
> >> handling. The way it's currently being done, the servlet pulls the
> >> content out of the post body stream, instantiates json objects from
> >> it, and throws it away. The stream on the request can't be reset,
> >> because it wasn't marked, and hence the post body content is no longer
> >> available.
> >>
> >> I see two options to retain the post body:
> >>
> >> 1) (Token bad suggestion) Have a servlet filter that marks the stream,
> >> reads the stream, grabs off what it wants, makes a request attribute
> >> and then resets the stream for later processing by the JsonRpcServlet.
> >> Sounds redundant and inefficient as it has the stream being read
> >> twice.
> >>
> >> 2) (What I propose) Have a servlet filter that reads the stream, and
> >> stashes the post body as a request attribute in a filter. Modify
> >> JsonRpcServlet to retrieve the attribute the same way that my
> >> end-of-request logging will once the post body as attribute is
> >> available.
> >>
> >> Is this cool with everyone? If so, I can whip up a patch. Other
> >> suggestions are welcome if it is not.
> >>
> >> Thanks,
> >> Bob Evans
> >>
> >
>

Re: JsonRpcServlet postBody handling

[Robert Evans <bo...@google.com>]

I'm interested in logging the types of requests that get processed by
our implementation of OpenSocial. For instance, in the jsonrpc
requests, I want the 'method' property of the post body so that I can
see how many of each type of request we are handling at the point when
my logger gets called.

On Wed, Sep 3, 2008 at 6:20 PM, Louis Ryan <lr...@google.com> wrote:
> Just out curiosity what are you interested in logging?
>
> On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com> wrote:
>
>> I'd like to log parts of the post body at the end of the request
>> handling. The way it's currently being done, the servlet pulls the
>> content out of the post body stream, instantiates json objects from
>> it, and throws it away. The stream on the request can't be reset,
>> because it wasn't marked, and hence the post body content is no longer
>> available.
>>
>> I see two options to retain the post body:
>>
>> 1) (Token bad suggestion) Have a servlet filter that marks the stream,
>> reads the stream, grabs off what it wants, makes a request attribute
>> and then resets the stream for later processing by the JsonRpcServlet.
>> Sounds redundant and inefficient as it has the stream being read
>> twice.
>>
>> 2) (What I propose) Have a servlet filter that reads the stream, and
>> stashes the post body as a request attribute in a filter. Modify
>> JsonRpcServlet to retrieve the attribute the same way that my
>> end-of-request logging will once the post body as attribute is
>> available.
>>
>> Is this cool with everyone? If so, I can whip up a patch. Other
>> suggestions are welcome if it is not.
>>
>> Thanks,
>> Bob Evans
>>
>

Re: JsonRpcServlet postBody handling

[Louis Ryan <lr...@google.com>]

Just out curiosity what are you interested in logging?

On Wed, Sep 3, 2008 at 5:53 PM, Robert Evans <bo...@google.com> wrote:

> I'd like to log parts of the post body at the end of the request
> handling. The way it's currently being done, the servlet pulls the
> content out of the post body stream, instantiates json objects from
> it, and throws it away. The stream on the request can't be reset,
> because it wasn't marked, and hence the post body content is no longer
> available.
>
> I see two options to retain the post body:
>
> 1) (Token bad suggestion) Have a servlet filter that marks the stream,
> reads the stream, grabs off what it wants, makes a request attribute
> and then resets the stream for later processing by the JsonRpcServlet.
> Sounds redundant and inefficient as it has the stream being read
> twice.
>
> 2) (What I propose) Have a servlet filter that reads the stream, and
> stashes the post body as a request attribute in a filter. Modify
> JsonRpcServlet to retrieve the attribute the same way that my
> end-of-request logging will once the post body as attribute is
> available.
>
> Is this cool with everyone? If so, I can whip up a patch. Other
> suggestions are welcome if it is not.
>
> Thanks,
> Bob Evans
>