[resteasy-dev] resteasy jax-rs fail fast on entity validation exception

Alessio Soldano asoldano at redhat.com
Thu Nov 2 05:13:38 EDT 2017 

On Thu, Nov 2, 2017 at 8:21 AM, John O'Hara <johara at redhat.com> wrote:

> Hi,
>
> Does the test need to be in its own class, or can I add it to an existing
> class?  Should the test be in the integration testsuite, or is there a
> better module to put it in? org.jboss.resteasy.test.cdi.validation.ValidationWithCDITest
> seems to me to be a logical place to put it.
>

I'm fine with you enhancing an existing test.


>
> Also, when I run the integration testsuite, it is run against an existing
> WF runtime, afaics the tests use the version of resteasy jaxrs that is
> bundled with the runtime, are there any tests that run with the build
> artefacts?
>

Actually the resteasy libraries are updated in the WF runtime before
running the testsuite.

Cheers
Alessio


>
> Thanks
>
> John
>
>
> On Tue, Oct 31, 2017 at 1:18 AM, Ron Sigal <rsigal at redhat.com> wrote:
>
>> Hi John,
>>
>> Ok, I'll start playing with RESTEASY-1749. Thanks.
>>
>> Yes, master branch.
>>
>> -Ron
>>
>> On 10/30/2017 07:30 AM, John O'Hara wrote:
>>
>> Ron,
>>
>> I agree with your thinking. I think that we do need to check the
>> parameters when the endpoint is called, and throw an appropriate exception.
>>
>> I have opened a jira : https://issues.jboss.org/pro
>> jects/RESTEASY/issues/RESTEASY-1749
>>
>> I can create a test in the testsuite, would this need to be in the
>> current master branch?
>>
>>
>> John
>>
>> On Fri, Oct 27, 2017 at 5:21 PM, Ron Sigal <rsigal at redhat.com> wrote:
>>
>>> On 10/27/2017 08:33 AM, John O'Hara wrote:
>>>
>>> After some more digging, I have found that this issue occurs if the the
>>> rest endpoint is also defined as an ejb managed bean. e.g.
>>>>
>>>> @Stateless at Path("/policyholder")@Produces(MediaType.APPLICATION_JSON)@Consumes(MediaType.APPLICATION_JSON)public class PolicyHolderEndpoint {
>>>>
>>>> In this case the JaxrsInjectionTarget.inject() is only called on the
>>> first endpoint invocation, and not on subsequent calls.
>>>
>>> Hmmm. @Stateless, so it doesn't have @Request scope; i.e., it's created
>>> just once. I conjecture you wouldn't see this problem for a @Stateful
>>> session bean.
>>>
>>> In the application the pusdo PolicyHolderEndpoint class is also a
>>> managed bean that is injected into other services as a single entry point
>>> for managing policyHolders.
>>> Is it reasonable to expect a rest endpoint can also be defined as an ejb
>>> managed bean?
>>>
>>> According to the JAX-RS spec:
>>>
>>> 10.2.4 Enterprise Java Beans (EJBs)
>>>
>>> In a product that supports EJBs, an implementation MUST support the use
>>> of stateless and singleton ses- sion beans as root resource classes,
>>> providers and Application subclasses. JAX-RS annotations can be applied to
>>> methods in an EJB’s local interface or directly to methods in a
>>> no-interface EJB. Resource class annotations (like @Path ) MUST be applied
>>> to an EJB’s class directly following the annotation inheritance rules
>>> defined in Section 3.6.
>>>
>>> I need to look into why weld isn't injecting the JaxrsInjectionTarget,
>>> but I think that my original question still stands in this use case.
>>> If we are running in a CDI context, we already ascertain in MethodInjectorImpl.invoke()
>>> whether there are validation exceptions by calling [1]. Can we fail at this
>>> point, rather than delegating the validation to the CDI manager and waiting
>>> for a callback?
>>>
>>> The CDI spec says we have to accumulate as many violations as possible
>>> before throwing an exception,so, in general, we still have to proceed to
>>> check field / property violations. BUT ... I think the scope is the source
>>> of the problem. If an EJB session bean is created just once, then field /
>>> parameter injections should be checked only once. On subsequent calls, it
>>> does, as you say, make sense to stop and throw an exception after checking
>>> parameters. SO ... if my reasoning makes sense, this is something that
>>> should be handled in Resteasy. If you agree, do you want to create a JIRA
>>> and I'll work on it? What do you think? -Ron
>>>
>>> Thanks
>>> [1] - https://github.com/resteasy/Resteasy/blob/master/resteasy-
>>> jaxrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L119
>>> On Fri, Oct 27, 2017 at 8:21 AM, John O'Hara <johara at redhat.com> wrote:
>>>>
>>>> Ron,
>>>> I re-read your email and double checked the call
>>>> to GeneralValidatorImpl.checkViolationsfromCDI().
>>>> I only see this being called once, on the first request.
>>>> I will dig some more to see why it is not being called on all requests
>>>> Thanks
>>>> John
>>>> On Fri, Oct 27, 2017 at 8:03 AM, John O'Hara <johara at redhat.com>
>>>> wrote:
>>>>>
>>>>> Thank you for your responses. If I fill you in on a bit of background
>>>>> about what I have experienced, then it might help clarify the issue I see.
>>>>>
>>>>> Background
>>>>>
>>>>> I am running a JEE benchmark on EAP7.1 (using resteasy 3.0.24.Final).
>>>>> One of rest endpoints has a @Valid annotation on a method parameter for a
>>>>> complex object.
>>>>>
>>>>> @POST
>>>>>
>>>>> @Path("/foo")
>>>>>
>>>>> @Consumes(MediaType.APPLICATION_JSON)
>>>>>
>>>>> public void addFoo(@Valid Foo bar) throws Exception {
>>>>>
>>>>> The Foo class contains different validation annotations, such as
>>>>> @NotNull, @Size, @Pattern and a custom validation annotation.
>>>>>
>>>>> One test determines how WF/EAP handles invalid requests. When I
>>>>> invoke the endpoint above I see a non-deterministic response from the app
>>>>> server.  If I invoke the endpoint with an invalid json object I receive a
>>>>> HTTP 400 exception as expected, but if I invoke the endpoint with a valid
>>>>> object, and *then* invoke the endpoint with an invalid object I
>>>>> receive a HTTP 500 error.
>>>>>
>>>>> This exception only occurs on some property validation annotations and
>>>>> not all (e.g. @NotNull works as expected)
>>>>>
>>>>> The test is being run in the context of CDI, and the HTTP 500 error is
>>>>> coming from the ejb3 subsystem. The ejb3 subsystem is wrapping the
>>>>> ConstraintValidationException raised by hibernate-validator.
>>>>>
>>>>> Observations
>>>>>
>>>>> In the call stack I capture [1], and WF thread dump [2] I can see;
>>>>>
>>>>>    1.
>>>>>
>>>>>    The test is run in the context of CDI. The HTTP 500 response code
>>>>>    message is caused by the ejb3 subsystem wrapping a
>>>>>    ConstraintValidationException raised by hibernate-validator
>>>>>
>>>>>
>>>>>    1.
>>>>>
>>>>>    The validator is not called in ResourceMethodInvoker.invokeOnTarget()
>>>>>    [3] as isValidatable is false [4], a ResteasyViolationException is not
>>>>>    thrown
>>>>>
>>>>>
>>>>>    1.
>>>>>
>>>>>    The request is validated again here [5], the validation exceptions
>>>>>    are not acted upon in this method call and the reflected method (which
>>>>>    happens to be the Foo class constructor) is called here [6]
>>>>>
>>>>> Questions
>>>>>
>>>>>    1.
>>>>>
>>>>>    Why is ResourceMethodInvoker.isValidatable false in WF/EAP?
>>>>>    2.
>>>>>
>>>>>    If isValidatable is false, and validation occurs again at
>>>>>    MethodInjectorImpl.invoke() can we not check for validation errors and
>>>>>    raise the correct exception there as well?
>>>>>
>>>>> I have a standalone test that I will clean up and push out. There are
>>>>> some aspects of the test that can not be published publicly, so I need to
>>>>> modify them first.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> John
>>>>>
>>>>> [1] - see failing_4.stack
>>>>>
>>>>> [2] - see WF_stackTrace.out
>>>>>
>>>>> [3] - https://github.com/resteasy/Resteasy/blob/master/resteasy-ja
>>>>> xrs/src/main/java/org/jboss/resteasy/core/ResourceMethodInvo
>>>>> ker.java#L305
>>>>>
>>>>> [4] - https://github.com/resteasy/Resteasy/blob/master/providers/r
>>>>> esteasy-validator-provider-11/src/main/java/org/jboss/restea
>>>>> sy/plugins/validation/GeneralValidatorImpl.java#L223
>>>>>
>>>>> [5] - https://github.com/resteasy/Resteasy/blob/master/resteasy-ja
>>>>> xrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L119
>>>>>
>>>>> [6] - https://github.com/resteasy/Resteasy/blob/master/resteasy-ja
>>>>> xrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L140
>>>>>
>>>> --
>>>>
>>>> JOHN O'HARA
>>>>
>>>> PRINCIPAL SOFTWARE ENGINEER
>>>>
>>>> Red Hat <https://www.redhat.com/>
>>>> <https://red.ht/sig>
>>>> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
>>>> @redhatway <https://twitter.com/redhatway>   @redhatinc
>>>> <https://instagram.com/redhatinc>   @redhatsnaps
>>>> <https://snapchat.com/add/redhatsnaps>
>>>>
>>> --
>>>
>>> JOHN O'HARA
>>>
>>> PRINCIPAL SOFTWARE ENGINEER
>>>
>>> Red Hat <https://www.redhat.com/>
>>> <https://red.ht/sig>
>>> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
>>> @redhatway <https://twitter.com/redhatway>   @redhatinc
>>> <https://instagram.com/redhatinc>   @redhatsnaps
>>> <https://snapchat.com/add/redhatsnaps>
>>>
>>> _______________________________________________
>>> resteasy-dev mailing listresteasy-dev at lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/resteasy-dev
>>>
>>> --
>>> My company's smarter than your company (unless you work for Red Hat)
>>>
>>> _______________________________________________ resteasy-dev mailing
>>> list resteasy-dev at lists.jboss.org https://lists.jboss.org/mailma
>>> n/listinfo/resteasy-dev
>>
>> --
>>
>> JOHN O'HARA
>>
>> PRINCIPAL SOFTWARE ENGINEER
>>
>> Red Hat <https://www.redhat.com/>
>> <https://red.ht/sig>
>> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
>> @redhatway <https://twitter.com/redhatway>   @redhatinc
>> <https://instagram.com/redhatinc>   @redhatsnaps
>> <https://snapchat.com/add/redhatsnaps>
>>
>> _______________________________________________
>> resteasy-dev mailing listresteasy-dev at lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/resteasy-dev
>>
>> --
>> My company's smarter than your company (unless you work for Red Hat)
>>
>>
>> _______________________________________________
>> resteasy-dev mailing list
>> resteasy-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>
>>
>
>
> --
>
> JOHN O'HARA
>
> PRINCIPAL SOFTWARE ENGINEER
>
> Red Hat <https://www.redhat.com/>
> <https://red.ht/sig>
> TRIED. TESTED. TRUSTED. <https://redhat.com/trusted>
> @redhatway <https://twitter.com/redhatway>   @redhatinc
> <https://instagram.com/redhatinc>   @redhatsnaps
> <https://snapchat.com/add/redhatsnaps>
>
> _______________________________________________
> resteasy-dev mailing list
> resteasy-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/resteasy-dev/attachments/20171102/1ef4c41a/attachment-0001.html

More information about the resteasy-dev mailing list