Re: [resteasy-dev] resteasy jax-rs fail fast on entity validation exception

On Thu, Nov 2, 2017 at 8:21 AM, John O'Hara <johara(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; 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(&quot;/policyholder&quot;)@Produces(MediaType.APPLICATION_JSON)(a)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(a)redhat.com&gt; >>> 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(a)redhat.com&gt; >>>> 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@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(a)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@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(a)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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/resteasy-dev > >