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

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(a)redhat.com <mailto:rsigal@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(a)redhat.com > <mailto:rsigal@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(a)redhat.com <mailto:rsigal@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/... >>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> On Fri, Oct 27, 2017 at 8:21 AM, John O'Hara >>> <johara(a)redhat.com <mailto:johara@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(a)redhat.com <mailto:johara@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/... >>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> >>> [4] - >>> https://github.com/resteasy/Resteasy/blob/master/providers/resteasy-valid... >>> <https://github.com/resteasy/Resteasy/blob/master/providers/resteasy-valid... >>> >>> [5] - >>> https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> >>> [6] - >>> https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> <https://github.com/resteasy/Resteasy/blob/master/resteasy-jaxrs/src/main/... >>> >>> -- >>> >>> 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(a)lists.jboss.org >>> <mailto:resteasy-dev@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(a)lists.jboss.org >> <mailto:resteasy-dev@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(a)lists.jboss.org >> <mailto:resteasy-dev@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(a)lists.jboss.org > <mailto:resteasy-dev@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(a)lists.jboss.org <mailto:resteasy-dev@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(a)lists.jboss.org <mailto:resteasy-dev@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>