Detailed disposition of comments interspersed below. Note that many of
the comments have been addressed in a new editors draft which can be
found at:
https://jsr311.dev.java.net/drafts/jsr311_20071204.pdf
On Nov 23, 2007, at 4:56 PM, Georg Sauer-Limbach wrote:
> A. General Comments
>
> A.1 The specification text should use the fully-qualified class
> names
> for all classes of the framework; this supports clarity and ease of
> reading.
>
I experimented with this but found that using the fully qualified
class names actually diminished readability somewhat.
> A.2 The HTTP-centricness should be resembled in the name of the
> API in
> some way, since this API is so closely linked with HTTP.
>
The title and package name were the subject of some fairly intense
discussion, I'd rather not re-open that.
> A.3 I did not fully understand how the services defined by the API
> would fit into an existing web application. Is it obligatory that
> these
> services are run in a separate web application of their own, or can
> they be incorporated in a web application which also defines
> other web
> resources (servlets) in the standard way, i.e. not using the JAX-RS
> framework. If they can, how are conflicts in the URL space
> handled? The
> framework would have to output a fragment of the web application
> deployment descriptor which would have to be merged into the
> existing
> deployment descriptor.
>
The intention is that JAX-RS resources (and their associated servlet)
can be mixed with other resources in the same web application. The URI
paths of resource classes are relative, in the case of Servlet they
are relative to the URI of the servlet and it is the responsibility of
the developer to avoid clashes, e.g. by assigning a unique prefix to
the JAX-RS servlet.
> A.4 Has it been considered to add a more high-level support for
> usage
> of the HTTP Basic Authentification with the JAX-RS API? I believe
> that
> a significant share of applications will decide to implement HTTP
> Basic
> Authentification not on the web server but on the application
> server.
> With the current specification, this could be realised using the
> @HeaderParam annotation for the Authorization header, plus using
> Response return types, thereby being able to set the required 401
> return code etc. Maybe it would be preferable to have a class-level
> annotation for this, though the details need to be worked out.
>
The intention is to support application-server-provided authentication
mechanisms, e.g. using JSR 196 providers. We plan to allow JSR 250
annotations to support specification of role-based access and to add a
security context that can be injected for non-declarative use-cases.
Handling of authentication within an application should also be
possible as you outlined and I note that Servlet 3.0 (
http://jcp.org/en/jsr/detail?id=315
) plans to address similar concerns.
> A.5 The specification should contain an overview of all existing
> annotations and their targets!
>
I will add a summary as requested.
> A.6 The specification could go into a bit more length and provide
> some more sound explanations. More examples would also greatly help
> understanding.
>
Agreed, the specification will be expanded as the API settles down.
>
> B. Specific Comments on the Specification
>
> B.1 Page 5, Lines 19-23. @UriTemplate( encode )
>
> The specification should state that the curly braces are handled
> specially. It should be pointed out that the curly braces cannot be
> made part of a URI, because they will always be taken as meta
> characters when they appear in a URITemplate.
>
Done.
> B.2 Page 5, Lines 19-23. @UriTemplate( encode )
>
> Possibly, the encode property of @UriTemplate should be dropped.
> I do
> not see a case where it adds functionality. Is there a reason to
> note a
> URI already encoded?
>
See the thread starting at:
https://jsr311.dev.java.net/servlets/ReadMsg?list=dev&msgNo=477
The current API seems to offer a reasonable compromise between
capability and complexity.
> B.3 Page 5, Line 26. @UriTemplate( encode )
>
> Draft: "... followed by an arbitrary length path."
> Since the length of the path is not the key point here, the passage
> should rather be: "... followed by a path consisting of arbitrarily
> many path segments."
>
Done.
> The spec should add an example.
>
Done.
> B.4 Chapter 2
>
> Since resource classes and resource methods are the key parts of
> the
> API, chapter 2 should start with a section on "Resource classes"
> (even if the whole chapter is already named like this), directly
> followed by a section on "Resource Methods".
>
Done, chapter 2 has been reorganized accordingly.
> B.5 Page 6, Line 2. Section 2.1.1 Sub Resources
>
> The section starts like this: "Resource class methods can also be
> annotated with @UriTemplate. The effect ... depends on ..."
> This could be much clearer be stated like this:
> "Methods of a resource class that are annotated with @UriTemplate
> are sub-resource locators or sub-resource methods."
>
Done.
> B.6 Page 6, Lines 4 to 11.
>
> Change the order of the two cases to align with the order of the
> methods in the example below.
>
Done.
> B.7 Page 6, Lines 24-26
>
> In the example, "offers" works like a special case of the {id}
> variable only. The URI widgets/offfers would result in an
> invocation
> of the findWidget method with id='offfers'.
>
Fixed, the algorithm (now section 2.5) has been expanded to address
this issue.
> B.8 Page 6, Lines 4 and 8.
>
> Let the definitions of the terms "sub-resource locators" and "sub-
> resource methods" typographically stay out in the text.
>
Done.
> B.9 Page 7, Line 7. Wording "resource class methods"
>
> I find the wording "resource class methods" irritating. Methods
> generally belong to classes, so saying "class methods" is a
> redundancy which serves no purpose. Instead, it can be confused
> with
> the term "class fields" which means a static field of a class,
> though the term "class method" is more rarely used to mean static
> methods.
>
> So maybe, for clarity, change the sentence "Resource methods are
> resource class methods ..." into "Resource methods are methods of a
> resource class ...".
>
Done.
> B.10 Page 7, Line 9. Term "Request method".
>
> I would rather prefer the term "HTTP method" because this makes the
> meaning much clearer. (There are no "response methods", so it does
> not really make sense to speak of "request methods", although the
> HTTP 1.1 RFC does so, though only in few places.)
>
I don't have a strong opinion on this one. I've left it as-is for now.
> B.11 Page 7, Lines 25-26.
>
> Replace the enumeration of the five annotations by "any of the
> above-
> listed annotations".
>
Done.
> B.12 Page 7, line 32.
>
> "instanceof Response" should be replaced by "Response"; placing the
> keyword "instanecof" to point out that the actual type of the
> returned object can be a subclass of Response is highly non-
> standard,
> and can be expected to be well-known by every reader.
>
Done.
> Further, for clarity and ease of reading, use the fully-qualified
> class name javax.ws.rs.core.Response instead of just "Response".
>
Not done for readability, I think the spec provides sufficient context
for the package name to be obvious.
> B.13 Page 8, Section 2.4.3 Exceptions
>
> I do not support the idea that unchecked exceptions that are
> propagating out of resource methods should be propagated to the
> web container. The framework should be regarded as authoritative in
> regard of the resource methods. Anything resulting from the
> invocation of a resource method should be dealt with by the JAX-RS
> framework, since the framework is the component which decides that
> a resource method is invoked at all.
>
> So any unchecked exception that is propagating out of a resource
> method should be catched by the framework and turned into a code
> 500
> response.
>
> B.14 Page 8, Lines 12-14
>
> In response to the editors' note: The resource methods could be
> defined to throw javax.servlet.ServletException and possibly also
> java.io.IOException as the only checked exceptions, like doGet()
> etc. of javax.servlet.http.HttpServlet? (Is it possible to enforce
> this by annotations?) This is the reasonable way that works for the
> Servlet Specification.
>
I've created an issue to track the above two points:
https://jsr311.dev.java.net/issues/show_bug.cgi?id=26
> B.15 Page 8, Line 27.
>
> "Declarations on a resource class method override any on the
> resource
> class;". This statement, in its generality, is misleading, and
> false
> if taken out of the context. It is true for the before-mentioned
> annotations @ProduceMime and @ConsumeMime, but certainly not for,
> say, @UriTemplate. Better make this clear by reforming the
> paragraph.
>
Done.
> B.16 Page 9, line 42.
>
> "Select the first matching class..." Which order is applied here?
>
Fixed.
> B.17 Page 10, line 39.
>
> "... URI encoding any invalid characters ..." It does not make
> sense
> to talk about "invalid characters". There are characters that are
> mapped into a sequence of other characters under the encoding, and
> there are some characters that are mapped into themselves. None
> of the
> characters are invalid, just some are transformed and some are not.
> I'd just omit "any invalid characters", leaving just "... URI
> encoding the template ..." which says it all.
>
Done.
> B.18 Page 11, line 3. Typo in regexp.
>
> I think the 'w' inside the square brackets needs a backslash in
> front of it, otherwise is stands for the character 'w' and not for
> any word character.
>
Fixed.
> B.19 Page 15, section 3.1.5 "Transfer Encoding"
>
> I do not see why there should not be a way to specify the encoding
> of the response body from within a resource method or
> EntityProvider.
> What are the reasons to leave this to the container or the runtime?
>
The spec has been clarified as follows:
"Transfer encoding for inbound data is handled by a component of the
container or the JAX-RS runtime.
MessageBodyReader providers always operate on the decoded HTTP entity
body rather than directly on the HTTP message body.
A JAX-RS runtime or container MAY transfer encode outbound data or
this MAY be done by an application returning a suitable Response
instance. "
>
> C. Specific Comment on the Javadoc
>
> C.1 The docs often use the term "Java method" when it should be
> perfectly clear that it is talking about Java, like in "Indicates
> that
> the annotated Java method should be used to handle..." - I believe
> "method" would do in all of these cases.
>
The Javadoc will be overhauled accordingly in due course.
Thanks again for the detailed feedback.
Marc.
---
Marc Hadley <marc.hadley at sun.com>
CTO Office, Sun Microsystems.