[Pki-devel] [PATCH] 411 Added CLI parameter to select message format.

[Ade Lee]

For the most part, this looks good.

I have only one question - what happens when a client passes in either
no Accepts or no Content-Type header - or is this even possible?

Thanks, 
Ade

On Mon, 2014-03-03 at 21:56 -0500, John Magne wrote:
> Looks like this patch addresses alee's concerns.
> 
> If its working fine ACK from me, but alee may have more 
> specialized knowledge of this stuff, so consult him as well.
> 
> ----- Original Message -----
> From: "Endi Sukma Dewata" <edewata at redhat.com>
> To: alee at redhat.com
> Cc: "pki-devel" <pki-devel at redhat.com>
> Sent: Monday, March 3, 2014 4:18:59 PM
> Subject: Re: [Pki-devel] [PATCH] 411 Added CLI parameter to select message format.
> 
> New patch #411-2 is attached. It's now resolving the media types 
> specified by the client (which may contain wildcards) into specific 
> formats supported by the server.
> 
> On 3/3/2014 8:31 AM, Endi Sukma Dewata wrote:
> > New patch #411-1 is attached. Patch #412-2 is unchanged.
> >
> > On 2/27/2014 10:40 AM, Ade Lee wrote:
> >> 1. Typo in MainCLI.java : Mesage format
> >
> > Fixed.
> >
> >> 2. There appears to be no code to enforce the input to be "json or xml".
> >> What if someone puts in "XML" or "Json" or "foo" in the CLI command
> >> line?
> >
> > The original patch was case insensitive. If someone puts a value that
> > doesn't translate to a valid MIME media type it will be rejected by the
> > HTTP client library.
> >
> > The new patch now only accepts "xml" or "json", case sensitive.
> >
> >> 3. So, it looks like we have decided to remove all the Produces/Consumes
> >> and basically provide all servlets as XML and JSON by default.  What
> >> that means though is that we are taking the logic of deciding which
> >> format to return from the framework code.
> >>
> >> I'm a little confused as to why this is needed.  Doesn't the framework
> >> already do this work for us?
> >
> > As discussed on IRC, the hardcoded @Provides and @Consumes in the
> > interface prevent java client from selecting the message format on
> > demand. There are other options that have been considered (e.g. creating
> > separate methods for each format, creating separate interfaces for
> > client & server, moving the annotations to service classes, adding media
> > type parameter to each method, adding enhancements to RESTEasy), but so
> > far removing @Provides and @Consumes seems to be one solution that works
> > with less changes to the server & client code and is available now.
> >
> >> What if the client requests a format like "application/atom-xml" - and
> >> nothing else.  If we follow your code, we will end up returning the
> >> results in whatever format the request came in as.  This seems wrong -
> >> we should reject the request with a code 406 (not acceptable).  And more
> >> importantly - we should reject this prior to processing which is
> >> presumably what the framework would do - rather than post-processing
> >> when generating the response which is what your code does.
> >
> > The new patch utilizes a new interceptor to validate the acceptable
> > message format before the operation is executed. It will throw error 415
> > on invalid request format and 406 on invalid response format, which is
> > the same behavior in the existing code.
>