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

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(a)redhat.com&gt; To: alee(a)redhat.com Cc: "pki-devel" <pki-devel(a)redhat.com&gt; 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.