[resteasy-dev] resteasy jax-rs fail fast on entity validation exception
John O'Hara
johara at redhat.com
Tue Nov 14 07:08:30 EST 2017
Ron,
I have checked the fix in the
https://github.com/ronsigal/Resteasy/tree/3.0_1749 branch and it does now
pass the unit test and we see the correct response in the benchmark I am
running.
I still see the Exception being logged in the ejb3 subsystem as an ERROR,
but I will investigate whether this should be logged as an error there, or
whether a different log level would be more appropriate.
Thanks
John
On Sat, Nov 4, 2017 at 10:31 PM, Ron Sigal <rsigal at redhat.com> wrote:
> Hi John,
>
> Well, I thought I would have wrapped this up by now. I started by
> experimenting with @ApplicationScoped resources, but I just wasn't seeing
> what I expected. Then I started playing with @Stateless bean resources, and
> I still didn't see what I expected. That was all on the master branch.
> Today, I played with @Stateless resources on branch 3.0, and I finally
> reproduced what you've been seeing. It's vaguely familiar - EJB is doing
> its own validation in an interceptor. Anyway, I've updated
> GeneralValidatorImpl.checkForConstraintViolations(), and I've gotten rid
> of the 500 status by unwinding thrown Exceptions until I get a
> ConstraintViolationException. One problem I still have is that I'm getting
> two copies of the violations when I combine a couple of sources, even
> though there is an equals() method that I think should prevent that.
>
> Anyway, I'm telling you now about a not quite complete solution because
> I'm going to take off next week. The solution is on branch 3.0_1749 in my
> personal github repository: https://github.com/ronsigal/
> Resteasy/tree/3.0_1749.
>
> I'll clean things up when I get back. If you get a chance, let me know if
> the updated version fixes your problem.
>
> -Ron
>
> On 11/02/2017 03:21 AM, John O'Hara 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.
>
> 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?
>
> 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/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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/resteasy-dev/attachments/20171114/94b64513/attachment-0001.html
More information about the resteasy-dev
mailing list