[resteasy-dev] resteasy jax-rs fail fast on entity validation exception
Ron Sigal
rsigal at redhat.com
Sat Nov 4 18:31:06 EDT 2017
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
> <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
> https://lists.jboss.org/mailman/listinfo/resteasy-dev
--
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/20171104/5881f30c/attachment-0001.html
More information about the resteasy-dev
mailing list