Sorry for the slow reply, responses interspersed below:
On Jan 28, 2008, at 7:33 AM, Stephan Koops wrote:
>
> UriBuilder
> for the UriBuilder I think it should be possible to call the
> "setter" methods with a given encode state, independent of the
> current instance variable state, e.g. in a utility method. There are
> two solutions in my mind:
> • add for every method (host(.), port(.), query(.), ...) another
> method with an additional boolean parameter encode, where the
> application developer can set this property independant of the
> internal encode state of the UriBuilder or
We did try this approach originally but the doubling of the number of
methods was unfortunate so we adopted the single encode() method
approach.
> • add a method getEncode() to the UriBuilder, so you can request
> the current encode state, set a new one and later switch back to the
> old one.
I assume you want to be able to do something like:
UriBuilder b = ...
appendToBuilder(b, ...)
// do something else to the builder
and have the appendToBuilder method be able to restore the original
encoding state before it returns ?
I can see the utility of this and will add a boolean isEncode() method.
> Another point for the UriBuilder: IMO it should be more precised
> what should be encoded in some methods in the javadoc (or whereever)
> • in replacePath(String) the slash must not be encoded, of
> course.What about ";" and "="? It could happe, that someone sets
> some matrix parameters here. If this is not allowed, it should be
> documented in the javadoc.
Only illegal characters would be subject to encoding, since ";" and
"=" are permitted in a path segment they wouldn't be encoded. The
javadoc references RFC3986 and the application/x-www-form-urlencoded
media type for details of what is legal where and I'd rather not get
into restating those rules.
>
> • in replaceQueryParams "=" and "?" must not be encoded.
> • in replaceMatrixParams ";" and "=" must not be encoded.
As above, the characters you mention are legal in their respective URI
components so they wouldn't be encoded.
> • userInfo: should the ":" be encoded, for a setting of
> "username:password".
No, since ":" is a legal char in the userInfo production
> Perhaps we can change the signature to userInfo(String ...). The
> array args could internally separated by a ":" (see http://tools.ietf.org/html/rfc3986#section-3.2.1)
> .
That might be OK but I don't know that userInfo with multiple segments
is used all that widely and therefore whether it deserves its own
method ?
> This (String ...) is perhaps useful for all replace*() methods.
I can see it might be useful for replacePath, less so for query and
matrix parameters though.
>
> The encode specialities should be clarified in the javadoc, IMO.
I will add some additional text to the class documentation. As I said
above I don't want to repeat the rules from the referenced
specifications but I can make it clearer that the encoding rules are
specific to particular URI components as specified in those
specifications.
> I suggest also to allow null as parameter in this methods for clear
> the path respectively the query parameters or the matrix parameters.
> It could be also documented in the javadoc that the parameter ""
> will clear the path and so on.
>
OK, that fits with URI where undefined components return null.
> MultivaluedMap of query / matrix parameters
> I think it should be documented if the MultivaluedMap should be
> modifiable or not for the query parameteres and for the matrix
> parameters. Unmodifiable is the right think in theory, but sometimes
> it is useful to allow the modification.
>
OK. My preference is for unmodifiable for request data, I'll clarify
the javadoc.
> ResponseBuilder.tag(String)
> The javadoc of method ResponseBuilder.tag(String) says that this
> method is used to set a strong entity tag. It is not documented what
> should happens, if a weak tag is set. Should it be rejected with an
> IllegalArgumentException or should it be allowed as a strong
> entityTag?
>
The parameter javadoc says it is "the string content of a strong
entity tag" so:
tag(String) will result in ETag: "String", e.g. tag("W/foo") would
result in ETag: "W/foo" rather than ETag: W/"foo".
I'll try to be more explicit about this in the javadoc. To set a weak
entity tag you'd have to use tag(EntityTag) instead.
> Request.evaluatePreconditions()
> RFC 2616, Section 10.3.5 requires, that the headers Content-
> Location, Expires, Cache-Control and/or vary must be under some
> conditions. How it s possible with the current spec? Perhaps the
> method should return a ResponseBuilder instead of a Response. So the
> application developer can add it.
>
Good suggestion. I think its possible for an implementation to create
a Vary header if the selectVariant method was previously called but I
can't see any way for it to know the values of the other headers. I'll
make this change and add a reference to the above section to highlight
the need to add these headers if appropriate.
Thanks for the feedback.
Marc.
---
Marc Hadley <marc.hadley at sun.com>
CTO Office, Sun Microsystems.