Hi Andy

I applied a logging level update patch, thanks.

Re the case below, I agree it is a stretch, while I can imagine how it can work I think it is unlikely having the WARNING will be acceptable in the production, besides it will be good if a getName will be implemented with an 'ar.resume' call, having a return type can make a developer think returning the value is all what is needed: so in the best case one would get a WARNING with a working GET handler - and in another case a WARNING with a non-functional GET handler :-)

I'll let you make the final decision - please do another patch if it is important for the integration project's compliance, etc, I suppose having such candidates will happen very rarely if ever in practice

Thanks, Sergey

On 01/11/16 22:12, Andy McCright wrote:
Hi Sergey,

Sure, I can open both JIRAs.

As for use cases, the only one I can think of is a stretch, but what if a
user wanted to re-use an async method -- something like this:

@GET
public String getName(@QueryParam int id, @Suspended AsyncResponse ar) {
    String result = getNameFromDatabase(id);
    if (ar != null) {
        ar.resume(result);
    }
    return result;
}

@PUT
String getAndUpdateName(int id, String newName) {
    String result = getName(id, null);
    if ( updateDatabaseWithNewName(id, newName) ) {
        return "Was " + result + " but now is " + newName;
    } else {
        throw new DBException("failed to update name ...");
    }
}

I'll start work on the first JIRA and patch (changing to warning) now.  If
this use case convinces you, we can always add to the patch to keep the
method in the list of candidates.

Thanks for feedback,

Andy

On Tue, Nov 1, 2016 at 4:45 PM, Sergey Beryozkin <[email protected]>
wrote:

IMHO it is always worth looking critically at the spec API, not to be
negative but to see what may not be quite right and can be improved going
forward.
As such, I can only imagine that the reason @Suspended docs say such
method responses must be ignored but do not go further and recommend not
adding them as the candidates is about supporting a "just in case" case or
a case where a test has not been provided with a WARNING being the last
resort warning.

I believe at this stage it is safe and good to ignore such methods
completely unless I'm missing something obvious.

Andy, can you please start with a patch to move from FINE to WARNING and
open another JIRA (minor Bug is probably OK) about such methods being
removed from the list of candidates - we would link it to a JAX-RS 2.1
issue where some extra justifications about the reason why such methods are
still candidates can be sought - please open this JAX-RS 2.1 issue too or I
can open it.

Does this plan work for you ?

Thanks, Sergey


On 01/11/16 16:25, Sergey Beryozkin wrote:

Hi Andy
On 01/11/16 15:50, Andy McCright wrote:

Hi All,

When we moved to 3.1.7 a few months ago, we started seeing a test failure
in one of our system tests.  The tester had a JAX-RS resource with the
following method signature:

  @GET
  @Produces(MediaType.APPLICATION_JSON)
  public JSONObject getMessage(@QueryParam("id") int i, @Suspended
AsyncResponse async)

Prior to 3.1.7, this method could be invoked and would return a 200
response with a JSON message.  After the upgrade, the same request
results
in a 405 response, and no warning message.

When I read the javadoc text for the @Suspended annotation (
http://docs.oracle.com/javaee/7/api/javax/ws/rs/container/Suspended.html
),
it makes me think that the method should be available (and requests
should
still succeed), but that the return value of the method should be
ignored.
I also think we should be logging a warning rather than a fine message.

Does that sound good?  If no objections, I'll plan to open a JIRA and
provide a patch.

I agree it should be a warning instead of a fine message.

However I'm not sure what we will achieve by keeping such method
candidates. The developer who writes this code will expect the value be
returned to the user but the test validating that some value is returned
will fail anyway.

Thus such methods can only add to the cost of the selection process if
they are added to the list of the candidates and will confuse the users
(ex, suspended GET will be still supported but the GET user will
eventually get an empty page...).

Can you see any practical scenarios where keeping such method candidates
can you actually make these methods work ?

Thanks, Sergey


Thanks, Andy




--
Sergey Beryozkin

Talend Community Coders
http://coders.talend.com/



Reply via email to