[resteasy-dev] resteasy jax-rs fail fast on entity validation exception
John O'Hara
johara at redhat.com
Mon Oct 30 07:30:09 EDT 2017
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
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/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/20171030/3da4563a/attachment-0001.html
More information about the resteasy-dev
mailing list