[resteasy-dev] resteasy jax-rs fail fast on entity validation exception
John O'Hara
johara at redhat.com
Thu Nov 2 07:48:59 EDT 2017
I have created a test here;
https://github.com/johnaohara/Resteasy/commits/RESTEASY-1749
Do you want me to create a PR for the testcase?
Thanks
John
On Thu, Nov 2, 2017 at 9:13 AM, Alessio Soldano <asoldano at redhat.com> wrote:
>
>
> 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/MethodInjectorIm
>>>> pl.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
>>
>>
>
--
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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/resteasy-dev/attachments/20171102/a0803b61/attachment-0001.html
More information about the resteasy-dev
mailing list