Jaxrs-api: Clarify documentation ambiguities

[glassfishrobot]

A number of Resteasy JIRA issues turn out to arise from ambiguities in the JAX-RS specification.

1. From https://issues.jboss.org/browse/RESTEASY-1162 "Response.created doesn't work as advertised with relative URIs":

The javadoc for javax.ws.rs.core.Response.created() says, "If a relative URI is supplied it will be converted into an absolute URI by resolving it relative to the request URI" However, the implementation of Response.created() is

public static ResponseBuilder created(URI location) {
        return status(Status.CREATED).location(location);
    }

and the javadoc for javax.ws.rs.core.Response$ResponseBuilder.location() says, "If a relative URI is supplied it will be converted into an absolute URI by resolving it relative to the base URI of the application".

2. From https://issues.jboss.org/browse/RESTEASY-1505 "@DefaultValue on @PathParameter not working":

The javadoc for @DefaultValue says, "Defines the default value of request meta-data that is bound using one of the following annotations: javax.ws.rs.PathParam, javax.ws.rs.QueryParam, javax.ws.rs.MatrixParam, javax.ws.rs.CookieParam, javax.ws.rs.FormParam, or javax.ws.rs.HeaderParam. "

On the other hand, Appendix A of the specification "JAX-RS: Java API for RESTful Web Services", Version 2.0, says that @DefaultValue "Specifies a default value for a field, property or method parameter annotated with @QueryParam , @MatrixParam , @CookieParam , @FormParam or @HeaderParam." Note that the spec omits @PathParam.

I think that the problem is the use of something like @Path("/a/b/{c}"). If the value for "c" is missing, then the annotated resource method just isn't going to get called. But what about @Path(/a/b/c{d}e)?

3. From https://issues.jboss.org/browse/RESTEASY-1516 "Cookies sent by resteasy-client are not spec compliant":

Not exactly an ambiguity. I'll just refer to #435.

Affected Versions

[2.0]

[glassfishrobot]

• Issue Imported From: https://github.com/jax-rs/api/issues/554
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @glassfishrobot

[glassfishrobot]

@glassfishrobot Commented
Reported by ronsigal

[glassfishrobot]

@glassfishrobot Commented
This issue was imported from java.net JIRA JAX_RS_SPEC-549

[ronsigal]

For the first problem, Response.created(), I would suggest turning the javadoc for created() to

    /**
     * Create a new ResponseBuilder for a created resource, set the location
     * header using the supplied value.
     *
     * @param location the URI of the new resource. If a relative URI is
     *                 supplied it will be converted into an absolute URI by resolving it
     *                 relative to the base URI of the application (see {@link UriInfo#getBaseUri}).
     * @return a new response builder.
     * @throws java.lang.IllegalArgumentException
     *          if location is {@code null}.
     */

There are two reasons for this change:

1. It makes the semantics of Response.created() consistent with the semantics of ResponseBuilder.location().
2. It's likely that the more common usage of created() would be to extend the base URI rather than include the path to the resource method.

If this solution is accepted, I could create a pull request with the javadoc change plus a new TCK test.

[ronsigal]

For the second problem, I think it's reasonable to update the javadoc for @DefaultValue to "Defines the default value of request meta-data that is bound using one of the following annotations: javax.ws.rs.QueryParam, javax.ws.rs.MatrixParam, javax.ws.rs.CookieParam, javax.ws.rs.FormParam, or javax.ws.rs.HeaderParam.", omitting @PathParam. I just don't see how to make sense of a missing path segment. I would be happy to submit a pull request updating the javadoc.

[chkal]

@ronsigal I agree that @DefaultValue doesn't make much sense for @PathParam. +1 for change the wording as proposed.

[ronsigal]

Thanks, @chkal. Do you have a reaction to the Response.created() problem?

[chkal]

@ronsigal I agree that your proposed solution for the Response.created() issue is good. It would be more consistent if relative URIs are always resolved relative to base URI of the application, regardless of which method on Response or ResponseBuilder is used. However, this is also a non-compatible change, although this method never really worked as expected.

@eclipse-ee4j/eclipse-jaxrs Other thoughts?

[spericas]

@ronsigal @chkal I believe the Response::created issue is really a bug in the spec. However, as stated, the proposed and correct behavior is not compatible. Perhaps we need to (1) fix the Javadoc to reflect actual behavior and (2) add a new method.

[ronsigal]

@spericas, @chkal, ok, I'll work on something that maintains backwards compatibility.

[ronsigal]

@spericas, @chkal Now that I'm thinking about this issue again, I think my proposal is just right.

1. What I suggested was to change the Response.created() javadoc to "reflect actual behavior". That is, Response.created() calls ResponseBuilder.location(), which resolves relative to the base URI. So the change satisflies @spericas' (1). Note, by the way, that the actual semantics of Response.created() accidentally happens to be what is likely the desired behavior.

2. We could also consider adding a method whose semantics matches the current Response.created() javadoc. That is, add a version of created() that resolves relative to the request URI. But

a. It seems less useful that the current created().
b. The request URI can be obtained from @Context UriInfo, so the new method would be a convenience method for something that's probably not that useful.

Having said that, I have no problem adding a new method if you think it's worthwhile.

WDYT?

-Ron

[chkal]

I think we definitely need to fix (1), but I don't see much benefit of adding another method just get a different behavior for relative URI resolving.

[spericas]

@ronsigal I'm fine with addressing just (1) at this point

[ronsigal]

Ok. Thanks, @spericas.

[ronsigal]

I think that, if I live to be 120, I'll still find something new in the JAX-RS spec. I withdraw my objection to the use of @DefaultValue for @PathParam, for two reasons:

1. The spec says (Section 3.4 "URI Templates"): "Note that if a URI template is used on a method, a path parameter injected in a field or property may not be available (set to null )." The example given has

@PathParam("id") String id;

where {id} appears in the @Path of one resource method but not another.

1. The TCK has the following class:

public class PathBeanParamEntity {
  @DefaultValue(Constants.DEFAULT_VALUE)
  @PathParam(Constants.PARAM_ENTITY_WITH_CONSTRUCTOR)
  public ParamEntityWithConstructor paramEntityWithConstructor;
  ...
}

-Ron

[ronsigal-tmp]

I think that the Description of @DefaultValue in Appendix A could also mention @PathParam:

Specifies a default value for a field, property or method
parameter annotated with @QueryParam ,
@MatrixParam , @CookieParam , @FormParam or
@HeaderParam . Also, specifies a default value for a field
or property annotated with @PathParam.
The specified value will be used if the
corresponding query or matrix parameter is not present in
the request URI, if the corresponding form parameter is
not in the request entity body, or if the corresponding
HTTP header is not included in the request.

[Adding the italicized sentence.]

[ronsigal]

For issue 1 (Response.created()), I've created two pull requests:

1. https://github.com/eclipse-ee4j/jaxrs-api/pull/810
2. https://github.com/eclipse-ee4j/jakartaee-tck/pull/135

-Ron

[arjantijms]

Issue 1 has been addressed and merged. I think this can be closed now, especially as 3.0 has been delivered. Please re-open and retarget if more work needed to be done.

[mkarg]

Do we need to mention this in the spec, at least in the history appendix?