[resteasy-dev] resteasy jax-rs fail fast on entity validation exception
Ron Sigal
rsigal at redhat.com
Thu Nov 16 14:03:46 EST 2017
I think we should take the discussion to either RESTEASY-1749. Also, I
just found a related, ongoing discussion at
https://developer.jboss.org/thread/276505.
On 11/15/2017 10:10 PM, Ron Sigal wrote:
>
> Hi John,
>
> Looking a little deeper, I see a remaining problem. I've got a test
> with three invocations:
>
> 1. field and parameter violations
>
> 2. no violations
>
> 3. field and parameter violations
>
> On the third invocation, the parameter violation is found by Resteasy,
> and then the EJB subsystem finds it again and throws a
> ConstaintViolationException. I unwrap the exception, collect the two
> exceptions (as I mentioned below - it turns out that
> org.hibernate.validator.internal.engine.ConstraintViolationImpl.hashCode()
> refers to the class in which the violation occurs, and I've got a
> plain class and a proxy class, so two essentially identical violations
> appear to not be equal), and send them back to the client. The problem
> is that, by throwing an exception, EJB is interrupting the normal flow
> and preventing org.jboss.resteasy.cdi.JaxrsInjectionTarget from
> finding field/property violations. That is, it's worse than just
> reporting duplicate violations - it's losing violations.
>
> I can probably do some contorted thing to ignore the fact that EJB is
> throwing an exception in order to continue with the normal flow, but
> I'm thinking it would be much better to be able to turn off the
> validation in EJB. You were going to look into the log level of EJB
> exceptions. Have you seen anything about turning off validation? Or,
> if not, do you know someone who might know?
>
> -Ron
>
>
> On 11/14/2017 07:08 AM, John O'Hara wrote:
>> 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
>> <mailto: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
>> <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
>>> <mailto: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/projects/RESTEASY/issues/RESTEASY-1749
>>>> <https://issues.jboss.org/projects/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 <mailto: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 @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, butI 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
>>>>> <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 <mailto: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 <mailto: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
>>>>>
>>>>> 2.
>>>>>
>>>>> The validator is not called in
>>>>> ResourceMethodInvoker.invokeOnTarget() [3]
>>>>> as isValidatable is false [4], a
>>>>> ResteasyViolationException is not thrown
>>>>>
>>>>> 3.
>>>>>
>>>>> 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-jaxrs/src/main/java/org/jboss/resteasy/core/ResourceMethodInvoker.java#L305
>>>>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/ResourceMethodInvoker.java#L305>
>>>>>
>>>>> [4] -
>>>>> https://github.com/resteasy/Resteasy/blob/master/providers/resteasy-validator-provider-11/src/main/java/org/jboss/resteasy/plugins/validation/GeneralValidatorImpl.java#L223
>>>>> <https://github.com/resteasy/Resteasy/blob/master/providers/resteasy-validator-provider-11/src/main/java/org/jboss/resteasy/plugins/validation/GeneralValidatorImpl.java#L223>
>>>>>
>>>>> [5] -
>>>>> https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L119
>>>>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L119>
>>>>>
>>>>> [6] -
>>>>> https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/MethodInjectorImpl.java#L140
>>>>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/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 list
>>>>> resteasy-dev at lists.jboss.org
>>>>> <mailto:resteasy-dev at lists.jboss.org>
>>>>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>>>> <https://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
>>>> <mailto:resteasy-dev at lists.jboss.org>
>>>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>>> <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
>>>> <mailto:resteasy-dev at lists.jboss.org>
>>>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>>> <https://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
>>> <mailto:resteasy-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>> <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 <mailto:resteasy-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>>> <https://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
>> <mailto:resteasy-dev at lists.jboss.org>
>> https://lists.jboss.org/mailman/listinfo/resteasy-dev
>> <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>
> --
> My company's smarter than your company (unless you work for Red Hat)
--
My company's smarter than your company (unless you work for Red Hat)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/resteasy-dev/attachments/20171116/2c3f28b3/attachment-0001.html
More information about the resteasy-dev
mailing list