Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate ORM REST Data Panache with Hibernate Validator #13307

Closed
Sgitario opened this issue Nov 16, 2020 · 10 comments · Fixed by #15447
Closed

Integrate ORM REST Data Panache with Hibernate Validator #13307

Sgitario opened this issue Nov 16, 2020 · 10 comments · Fixed by #15447
Labels
area/hibernate-validator Hibernate Validator area/panache kind/bug Something isn't working
Milestone

Comments

@Sgitario
Copy link
Contributor

Description
I think it would really useful to integrate the ORM REST Data Panache module with the Hibernate Validator extension.

Implementation ideas
I've tried to implement a workaround by using the annotations, but I could not make this integration worked.

The modules that I used are:

<dependency>
    <groupId>io.quarkus</groupId>
    <artifactId>quarkus-hibernate-orm-rest-data-panache</artifactId>
</dependency>

<dependency>
    <groupId>io.quarkus</groupId>
    <artifactId>quarkus-hibernate-validator</artifactId>
</dependency>

My resource:

public interface ApplicationResource extends PanacheEntityResource<ApplicationEntity, Long> {
}

And my entity:

@Entity(name = "application")
public class ApplicationEntity extends PanacheEntity {
    @NotEmpty
    @Column(unique = true, nullable = false)
    public String name;
}

When I add an application with an empty name, I got a 500 Internal Server Error because the name is not nullable which is expected, but not nice as I annotated the name field as not empty, therefore I should have received 400 Bad Request.

The only current way to implement this is manually using the validation service, but I would be nice to have something like:

public interface ApplicationResource extends PanacheEntityResource<ApplicationEntity, Long> {
    @MethodProperties(validatable = true)
    ApplicationEntity add(ApplicationEntity entity)
}

Or even validate the entities always only if the hibernate-validator extension does exist in the classpath.

@Sgitario Sgitario added the kind/enhancement New feature or request label Nov 16, 2020
@ghost ghost added area/hibernate-validator Hibernate Validator area/panache labels Nov 16, 2020
@ghost
Copy link

ghost commented Nov 16, 2020

/cc @FroMage, @gsmet, @loicmathieu

@gsmet
Copy link
Member

gsmet commented Nov 16, 2020

Hibernate Validator ORM integration should be automatically enabled if present.

Can you build a small reproducer? Thanks!

@Sgitario
Copy link
Contributor Author

Hibernate Validator ORM integration should be automatically enabled if present.

Can you build a small reproducer? Thanks!

Find the reproducer in this repository. Concretely, I have a small test that is failing because of this issue: https://github.com/Sgitario/beefy-scenarios/blob/baebdbde9d97f173ab02ac604b0ef133fba868b7/011-quarkus-panache-rest-flyway/src/test/java/io/quarkus/qe/PostgreSqlApplicationResourceTest.java#L68

Let me know if this is all you need.

@gsmet
Copy link
Member

gsmet commented Nov 16, 2020

I have the following message:

2020-11-16 12:04:49,485 WARN  [com.arj.ats.arjuna] (executor-thread-1) ARJUNA012125: TwoPhaseCoordinator.beforeCompletion - failed for SynchronizationImple< 0:ffffc0a80111:a85d:5fb25cd0:4e, org.hibernate.resource.transaction.backend.jta.internal.synchronization.RegisteredSynchronization@138dc5e5 >: javax.validation.ConstraintViolationException: Validation failed for classes [io.quarkus.qe.ApplicationEntity] during persist time for groups [javax.validation.groups.Default, ]
List of constraint violations:[
	ConstraintViolationImpl{interpolatedMessage='must not be empty', propertyPath=name, rootBeanClass=class io.quarkus.qe.ApplicationEntity, messageTemplate='{javax.validation.constraints.NotEmpty.message}'}
]
	at org.hibernate.cfg.beanvalidation.BeanValidationEventListener.validate(BeanValidationEventListener.java:140)
	at org.hibernate.cfg.beanvalidation.BeanValidationEventListener.onPreInsert(BeanValidationEventListener.java:80)
	at org.hibernate.action.internal.EntityInsertAction.preInsert(EntityInsertAction.java:227)
	at org.hibernate.action.internal.EntityInsertAction.execute(EntityInsertAction.java:100)
	at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:604)
	at org.hibernate.engine.spi.ActionQueue.lambda$executeActions$1(ActionQueue.java:478)

so the Hibernate Validator integration is properly triggered.

But you end up with a deeply wrapped exception:

2020-11-16 12:18:28,994 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /application failed, error id: eea5d19e-9baf-4dd0-baae-1f837adc6076-1: org.jboss.resteasy.spi.UnhandledException: io.quarkus.arc.ArcUndeclaredThrowableException: Error invoking subclass method
	at org.jboss.resteasy.core.ExceptionHandler.handleApplicationException(ExceptionHandler.java:106)
	at org.jboss.resteasy.core.ExceptionHandler.handleException(ExceptionHandler.java:372)
	at org.jboss.resteasy.core.SynchronousDispatcher.writeException(SynchronousDispatcher.java:218)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:519)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:261)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:161)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
	at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:164)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:247)
	at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:133)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.access$000(VertxRequestHandler.java:38)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$1.run(VertxRequestHandler.java:95)
	at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:2046)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1578)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1452)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at java.lang.Thread.run(Thread.java:748)
	at org.jboss.threads.JBossThread.run(JBossThread.java:479)
Caused by: io.quarkus.arc.ArcUndeclaredThrowableException: Error invoking subclass method
	at io.quarkus.qe.ApplicationResourceJaxRs_9c0f1e0e8e71d85c62e2a9d955dc3bb6609f7788_Subclass.add(ApplicationResourceJaxRs_9c0f1e0e8e71d85c62e2a9d955dc3bb6609f7788_Subclass.zig:484)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:170)
	at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:130)
	at org.jboss.resteasy.core.ResourceMethodInvoker.internalInvokeOnTarget(ResourceMethodInvoker.java:643)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTargetAfterFilter(ResourceMethodInvoker.java:507)
	at org.jboss.resteasy.core.ResourceMethodInvoker.lambda$invokeOnTarget$2(ResourceMethodInvoker.java:457)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:459)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:419)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:393)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:68)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:492)
	... 17 more
Caused by: javax.transaction.RollbackException: ARJUNA016053: Could not commit transaction.
	at com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.commitAndDisassociate(TransactionImple.java:1307)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.BaseTransaction.commit(BaseTransaction.java:126)
	at io.quarkus.narayana.jta.runtime.CDIDelegatingTransactionManager.commit(CDIDelegatingTransactionManager.java:97)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.endTransaction(TransactionalInterceptorBase.java:311)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:160)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:100)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.doIntercept(TransactionalInterceptorRequired.java:32)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.intercept(TransactionalInterceptorBase.java:53)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.intercept(TransactionalInterceptorRequired.java:26)
	at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired_Bean.intercept(TransactionalInterceptorRequired_Bean.zig:340)
	at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:41)
	at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:41)
	at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:32)
	at io.quarkus.qe.ApplicationResourceJaxRs_9c0f1e0e8e71d85c62e2a9d955dc3bb6609f7788_Subclass.add(ApplicationResourceJaxRs_9c0f1e0e8e71d85c62e2a9d955dc3bb6609f7788_Subclass.zig:462)
	... 32 more
Caused by: javax.validation.ConstraintViolationException: Validation failed for classes [io.quarkus.qe.ApplicationEntity] during persist time for groups [javax.validation.groups.Default, ]
List of constraint violations:[
	ConstraintViolationImpl{interpolatedMessage='must not be empty', propertyPath=name, rootBeanClass=class io.quarkus.qe.ApplicationEntity, messageTemplate='{javax.validation.constraints.NotEmpty.message}'}
]
	at org.hibernate.cfg.beanvalidation.BeanValidationEventListener.validate(BeanValidationEventListener.java:140)
	at org.hibernate.cfg.beanvalidation.BeanValidationEventListener.onPreInsert(BeanValidationEventListener.java:80)
	at org.hibernate.action.internal.EntityInsertAction.preInsert(EntityInsertAction.java:227)
	at org.hibernate.action.internal.EntityInsertAction.execute(EntityInsertAction.java:100)
	at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:604)
	at org.hibernate.engine.spi.ActionQueue.lambda$executeActions$1(ActionQueue.java:478)
	at java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
	at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:475)
	at org.hibernate.event.internal.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:348)
	at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:40)
	at org.hibernate.event.service.internal.EventListenerGroupImpl.fireEventOnEachListener(EventListenerGroupImpl.java:102)
	at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1362)
	at org.hibernate.internal.SessionImpl.managedFlush(SessionImpl.java:453)
	at org.hibernate.internal.SessionImpl.flushBeforeTransactionCompletion(SessionImpl.java:3212)
	at org.hibernate.internal.SessionImpl.beforeTransactionCompletion(SessionImpl.java:2380)
	at org.hibernate.engine.jdbc.internal.JdbcCoordinatorImpl.beforeTransactionCompletion(JdbcCoordinatorImpl.java:447)
	at org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorImpl.beforeCompletion(JtaTransactionCoordinatorImpl.java:355)
	at org.hibernate.resource.transaction.backend.jta.internal.synchronization.SynchronizationCallbackCoordinatorNonTrackingImpl.beforeCompletion(SynchronizationCallbackCoordinatorNonTrackingImpl.java:47)
	at org.hibernate.resource.transaction.backend.jta.internal.synchronization.RegisteredSynchronization.beforeCompletion(RegisteredSynchronization.java:37)
	at com.arjuna.ats.internal.jta.resources.arjunacore.SynchronizationImple.beforeCompletion(SynchronizationImple.java:76)
	at com.arjuna.ats.arjuna.coordinator.TwoPhaseCoordinator.beforeCompletion(TwoPhaseCoordinator.java:360)
	at com.arjuna.ats.arjuna.coordinator.TwoPhaseCoordinator.end(TwoPhaseCoordinator.java:91)
	at com.arjuna.ats.arjuna.AtomicAction.commit(AtomicAction.java:162)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.commitAndDisassociate(TransactionImple.java:1295)

I think there are several issues here:

  • I already mentioned elsewhere that we shouldn't have a warning logged by Arjuna here, it's counter productive;
  • then Arjuna is wrapping our ConstraintViolationException into a javax.transaction.RollbackException (the javax.persistence one is runtime but not the javax.transaction one) which is not a runtime one and gets wrapped by ArC.

@Sanne I think we should try to have someone from Arjuna to explain us why the warning?

Then I don't really know what we could do to improve the wrapping... We could have all the default REST Panache methods throw a javax.transaction.RollbackException but that would end up being visible to the users if they want to customize the methods. Not very user friendly... Or maybe we could transform this exception to something runtime in our Quarkus layers?

Another option could be to catch RollbackException in the generated Panache Rest code and if the cause is ConstraintViolationException, we could unwrap it and throw this one.

That being said, that would be a change in behavior compared to how things usually work.

I don't really know if, in this case, the important thing is that we violated a constraint or if the transaction wasn't committed. The fact is that the constraints might be internal information that we don't really want to expose to the user.

/cc @Sanne @FroMage @gytis I think this warrants a discussion.

@FroMage
Copy link
Member

FroMage commented Nov 16, 2020

1/ ArC doesn't have to wrap, it can rethrow even checked exceptions.
2/ Is there any good reason why Arjuna even wraps exceptions into RollbackException? That doesn't seem useful. @mmusgrov do you know?

@gsmet
Copy link
Member

gsmet commented Nov 16, 2020

1/ ArC doesn't have to wrap, it can rethrow even checked exceptions.

ArC does that only if they are in the list of thrown exceptions of the method. So if we keep it that way, as I mentioned it above, that means adding the RollbackException to the REST Panache methods.

@FroMage
Copy link
Member

FroMage commented Nov 16, 2020

You're telling me what it does ATM, and I'm saying it doesn't have to. If the method throws an undeclared checked exception, it is most likely due to an interceptor. Either it's wrong and the interceptor should be corrected, or it's fine, and ArC should not get in the way and wrap it.

@rsvoboda
Copy link
Member

This has kind/enhancement label present, should it be changed to kind/bug + description updated a bit?

@Sanne Sanne added kind/bug Something isn't working and removed kind/enhancement New feature or request labels Nov 18, 2020
@Sanne
Copy link
Member

Sanne commented Nov 18, 2020

@rsvoboda +1, done for the labels. Need to think about the description, this one is complicated.

@gytis
Copy link

gytis commented Feb 24, 2021

I have recently updated rest-data-panache to wrap all exceptions that happened to be throw during the persistence to allow easier error mapping. And added a custom mapping for the Hibernate constraint violation exception as per #14281.

I could similarly map a validation constraint violation to HTTP 400 error here: https://github.com/quarkusio/quarkus/blob/master/extensions/panache/hibernate-orm-rest-data-panache/runtime/src/main/java/io/quarkus/hibernate/orm/rest/data/panache/runtime/RestDataPanacheExceptionMapper.java#L31.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-validator Hibernate Validator area/panache kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants