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

Cannot add entity to lazy loaded collection with Quarkus Reactive Panache - org.hibernate.LazyInitializationException #23757

Closed
mklueh opened this issue Feb 16, 2022 · 24 comments
Assignees
Labels

Comments

@mklueh
Copy link
Contributor

mklueh commented Feb 16, 2022

Describe the bug

When I'm trying to insert into a child collection of type OneToMany, I'm running into this error:

I've tried multiple ways of inserting, and all result in the same behavior. Also tried using Mutiny.SessionFactory with no change.

@Slf4j
@ApplicationScoped
public class LazyChildrenService {

    @Inject
    ParentRepository parentRepository;

    @Inject
    ChildRepository childRepository;

    public Uni<Parent> saveParentWithLazyChildren() {
        return Panache.withTransaction(() -> parentRepository.findAll().firstResult()
                .onItem().transformToUni(parent -> parentRepository.persist(parent.addLazy(new Child()))
                        .onItem().transform(unused -> parent)));
    }

    public Uni<Parent> saveLazyChild() {
        return Panache.withTransaction(() -> parentRepository.findAll().firstResult()
                .onItem().transformToUni(parent -> {
                    Child child = new Child();
                    parent.addLazy(child);
                    return childRepository.persist(child)
                            .onItem().transform(unused -> parent);
                }));
    }

    @ReactiveTransactional
    public Uni<Parent> saveParentWithLazyChildrenAndAnnotation() {
        return parentRepository.findAll().firstResult()
                .onItem().transformToUni(parent -> parentRepository.persist(parent.addLazy(new Child()))
                        .onItem().transform(unused -> parent));
    }

}

Results in error:

org.hibernate.LazyInitializationException: HR000056: Collection cannot be initialized: com.example.Parent.lazyChildren
	at org.hibernate.reactive.session.impl.ReactiveSessionImpl.initializeCollection(ReactiveSessionImpl.java:370)
	at org.hibernate.collection.internal.AbstractPersistentCollection$4.doWork(AbstractPersistentCollection.java:595)
	at org.hibernate.collection.internal.AbstractPersistentCollection.withTemporarySessionIfNeeded(AbstractPersistentCollection.java:264)
	at org.hibernate.collection.internal.AbstractPersistentCollection.initialize(AbstractPersistentCollection.java:591)
	at org.hibernate.collection.internal.AbstractPersistentCollection.read(AbstractPersistentCollection.java:149)
	at org.hibernate.collection.internal.AbstractPersistentCollection$3.doWork(AbstractPersistentCollection.java:347)
	at org.hibernate.collection.internal.AbstractPersistentCollection$3.doWork(AbstractPersistentCollection.java:335)
	at org.hibernate.collection.internal.AbstractPersistentCollection.withTemporarySessionIfNeeded(AbstractPersistentCollection.java:264)
	at org.hibernate.collection.internal.AbstractPersistentCollection.readElementExistence(AbstractPersistentCollection.java:334)
	at org.hibernate.collection.internal.PersistentSet.add(PersistentSet.java:208)
	at com.example.Parent.addLazy(Parent.java:28)
	at com.example.LazyChildrenService.lambda$saveLazyChild$4(LazyChildrenService.java:34)
	at io.smallrye.context.impl.wrappers.SlowContextualFunction.apply(SlowContextualFunction.java:21)
	at io.smallrye.mutiny.operators.uni.UniOnItemTransformToUni$UniOnItemTransformToUniProcessor.performInnerSubscription(UniOnItemTransformToUni.java:68)
	at io.smallrye.mutiny.operators.uni.UniOnItemTransformToUni$UniOnItemTransformToUniProcessor.onItem(UniOnItemTransformToUni.java:57)
	at io.smallrye.mutiny.operators.uni.UniOnItemTransformToUni$UniOnItemTransformToUniProcessor.onItem(UniOnItemTransformToUni.java:60)
	at io.smallrye.mutiny.operators.uni.UniOnItemTransform$UniOnItemTransformProcessor.onItem(UniOnItemTransform.java:43)
	at io.smallrye.mutiny.operators.uni.UniOperatorProcessor.onItem(UniOperatorProcessor.java:46)
	at io.smallrye.mutiny.operators.uni.builders.UniCreateFromCompletionStage$CompletionStageUniSubscription.forwardResult(UniCreateFromCompletionStage.java:63)
	at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
	at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)
	at io.vertx.core.Future.lambda$toCompletionStage$2(Future.java:360)
	at io.vertx.core.impl.future.FutureImpl$3.onSuccess(FutureImpl.java:141)
	at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:60)
	at io.vertx.core.impl.future.FutureImpl.tryComplete(FutureImpl.java:211)
	at io.vertx.core.impl.future.PromiseImpl.tryComplete(PromiseImpl.java:23)
	at io.vertx.sqlclient.impl.QueryResultBuilder.tryComplete(QueryResultBuilder.java:102)
	at io.vertx.sqlclient.impl.QueryResultBuilder.tryComplete(QueryResultBuilder.java:35)
	at io.vertx.core.Promise.complete(Promise.java:66)
	at io.vertx.core.Promise.handle(Promise.java:51)
	at io.vertx.core.Promise.handle(Promise.java:29)
	at io.vertx.core.impl.future.FutureImpl$3.onSuccess(FutureImpl.java:141)
	at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:60)
	at io.vertx.core.impl.future.FutureImpl.tryComplete(FutureImpl.java:211)
	at io.vertx.core.impl.future.PromiseImpl.tryComplete(PromiseImpl.java:23)
	at io.vertx.core.impl.future.PromiseImpl.onSuccess(PromiseImpl.java:49)
	at io.vertx.core.impl.future.PromiseImpl.handle(PromiseImpl.java:41)
	at io.vertx.sqlclient.impl.TransactionImpl.lambda$wrap$0(TransactionImpl.java:72)
	at io.vertx.core.impl.future.FutureImpl$3.onSuccess(FutureImpl.java:141)
	at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:60)
	at io.vertx.core.impl.future.FutureImpl.tryComplete(FutureImpl.java:211)
	at io.vertx.core.impl.future.PromiseImpl.tryComplete(PromiseImpl.java:23)
	at io.vertx.core.impl.future.PromiseImpl.onSuccess(PromiseImpl.java:49)
	at io.vertx.core.impl.future.PromiseImpl.handle(PromiseImpl.java:41)
	at io.vertx.core.impl.future.PromiseImpl.handle(PromiseImpl.java:23)
	at io.vertx.sqlclient.impl.command.CommandResponse.fire(CommandResponse.java:46)
	at io.vertx.sqlclient.impl.SocketConnectionBase.handleMessage(SocketConnectionBase.java:287)
	at io.vertx.pgclient.impl.PgSocketConnection.handleMessage(PgSocketConnection.java:96)
	at io.vertx.sqlclient.impl.SocketConnectionBase.lambda$init$0(SocketConnectionBase.java:99)
	at io.vertx.core.impl.EventLoopContext.emit(EventLoopContext.java:50)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:274)
	at io.vertx.core.impl.EventLoopContext.emit(EventLoopContext.java:22)
	at io.vertx.core.net.impl.NetSocketImpl.handleMessage(NetSocketImpl.java:394)
	at io.vertx.core.net.impl.ConnectionBase.read(ConnectionBase.java:156)
	at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:153)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
	at io.vertx.pgclient.impl.codec.PgEncoder.lambda$write$0(PgEncoder.java:87)
	at io.vertx.pgclient.impl.codec.PgCommandCodec.handleReadyForQuery(PgCommandCodec.java:139)
	at io.vertx.pgclient.impl.codec.PgDecoder.decodeReadyForQuery(PgDecoder.java:237)
	at io.vertx.pgclient.impl.codec.PgDecoder.channelRead(PgDecoder.java:96)
	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:722)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:658)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:584)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:496)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)

How to Reproduce?

Reproducer: https://github.com/mklueh/quarkus-reproducer-insert-into-lazy-collection

Quarkus version or git rev

2.7.1

Am I missing something crucial or is this a bug?

@mklueh mklueh added the kind/bug Something isn't working label Feb 16, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 16, 2022

/cc @FroMage, @Sanne, @gsmet, @loicmathieu, @yrodiere

@mklueh
Copy link
Contributor Author

mklueh commented Feb 16, 2022

Might be related to this hibernate/hibernate-reactive#663

@yrodiere yrodiere removed the area/hibernate-orm Hibernate ORM label Feb 16, 2022
@DavideD DavideD self-assigned this Feb 16, 2022
@DavideD
Copy link
Contributor

DavideD commented Feb 16, 2022

This should work:

    public Uni<Parent> saveLazyChild() {
		return Panache.withTransaction( () -> parentRepository
				.find( "from Parent p left join fetch p.lazyChildren" ).firstResult()
				.call( parent -> {
					Child child = new Child();
					parent.addLazy( child );
					return childRepository.persist( child );
				} ) );
    }

The problem is that one needs to fetch lazy associations explicitly when using Hibernate Reactive.

You could also use the Hibernate Reactive Mutiny#fetch method:

    import org.hibernate.reactive.mutiny.Mutiny;

    public Uni<Parent> saveLazyChild() {
		return Panache
				.withTransaction( () -> parentRepository
					.findAll().firstResult()
					.call( parent -> Mutiny
							.fetch( parent.getLazyChildren() )
							.call( lazyChildren -> {
								Child child = new Child();
								lazyChildren.add( child );
								return childRepository.persist( child );
							} )
					)
				);
    }

@mklueh
Copy link
Contributor Author

mklueh commented Feb 16, 2022

@DavideD oh, thank you. In my real-world application, my entity has two ManyToOne relations that have to be updated at the same time. This means I'd have to explicitly fetch two collections sequentially, waiting for both to complete and only then persist the child.

It sounds complicated to me and I would have no clue how to do that, but this got me to rethink if Hibernate is the right choice.... I think I just go with plain SessionFactory and skip the fetching of the collections, which is unnecessary anyway. I just want to insert and don't care about the child.

Thank you anyway, I keep that solution in mind for the next time :)

@DavideD
Copy link
Contributor

DavideD commented Feb 17, 2022

This means I'd have to explicitly fetch two collections sequentially, waiting for both to complete and only then persist the child.

You know, I think this might be a bug in Hibernate Reactive. We should be able to notice that in this case one doesn't need an additional query for the association and a reference to the parent is what we need.

By the way, this is not what the docs say but, unofficially, in this specific case, it should work if you only update the owning side of the association, like this:

.withTransaction( () -> parentRepository
    .findAll().firstResult()
    .call( parent -> {
        Child child = new Child();
        child.setParent(parent);
        return childRepository.persist( child );
    })
);

Updating both sides of the association is the correct approach, though.

@DavideD
Copy link
Contributor

DavideD commented Feb 17, 2022

I will keep this issue open because I want to check what's going on

@mklueh
Copy link
Contributor Author

mklueh commented Feb 17, 2022

Just for completeness, I haven't tested it, it was just an assumption based on the Mutiny.fetch call that I'd have to do it two times before I could do what I wanted to do.

@DavideD
Copy link
Contributor

DavideD commented Feb 17, 2022

You need to call fetch for all the lazy associations you intend to update, but in this case it shouldn't result in a query on the db because the info on the child should be enough. I need to check what happens in Hibernate ORM

@DavideD
Copy link
Contributor

DavideD commented Feb 17, 2022

I've created an issue for Hibernate Reactive: hibernate/hibernate-reactive#1207

@StephenOTT
Copy link

Is there a final solution for this? 1207 is closed in hibernate-reactive. But as of latest releases of Quarkus, you still have to manually fetch the relationships. This further breaks when you return Entities in rest responses where the fetching does not occur and you get the error: Fetch the collection using 'Mutiny.fetch', 'Stage.fetch', or 'fetch join' in HQL

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

I don't know why this issue is still open, I will have a second look at it later.

hibernate/hibernate-reactive#1207 was a performance issue, it doesn't affect the LazyInitializationException.

This further breaks when you return Entities in rest responses where the fetching does not occur and you get the error: Fetch the collection using 'Mutiny.fetch', 'Stage.fetch', or 'fetch join' in HQL

With Hibernate Reactive, there is no alternative to eagerly load associations or fetching them when needed.

Briefly, the problem is that associations are declared as non async types in an entity, for example @OneToMany List<Book> books. When we run a SQL query to load a lazy association, the vert.x client returns something like CompletionStage<ResultSet>. When the CompletionStage completes, Hibernate Reactive will convert it to CompletionStage<List<Book>>. But we cannot assign it to the field in the entity.

We could have some custom classes to map associations, but, a user will have to do something similar to what happens now with the .fetch method. That's because you will have access to the association only after the async type (CompletionStage<List> in this example) has completed.

Maybe in the future we will come up with something cleverer.

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

I'm going to close this issue:
the only way to avoid the LazyInitializationException is to make sure that the required association has been fetched, one way or another. There's no alternative at the moment.

Note that Hibernate Reactive has several different ways to load an association eagerly and which one is better depends on the use case. Loading an entity with a join fetch query or using an entity graph is usually the best approach because it will load everything using a single query.

@DavideD DavideD closed this as completed Nov 11, 2022
@StephenOTT
Copy link

StephenOTT commented Nov 11, 2022

I feels like the current default config of reactive handling in quarkus for lazy joins is problematic for default serialization: When you do something with panache such as: Uni<SomeItem> myItem = repository.findById(someId)..... and then return SomeItem, if SomeItem has a lazy join then it fails. It seems like lazy joins should not be auto-serialized ? (for example, adding jsonIgnore to all lazy fields works)

@StephenOTT
Copy link

@DavideD see above

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

Shouldn't one serialize the object only after the association has been fetched? Or, as you said, you can ignore the association for serialization.

What's the use case you have in mind?

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

Basically, one solution would be to have:

Uni<SomeItem> myItem = repository.findById(someId).call( someItem -> Mutiny.fetch(someItem.getAssociation())); 

but you can also use a projection (basically, you use a DTO that doesn't have the association):

Uni<SomeItemWithoutAssociation> myItem = repository.findById(someId).project(SomeItemWithoutAssociation.class); 

But yeah, before seirialization you need to make sure that all the data you need has been fetched.
I'm not sure if this is a Hibernate Reactive specific problem.

@StephenOTT
Copy link

StephenOTT commented Nov 11, 2022

If you create a simple entity with a lazy join and return it in a rest controller it fails by default. If the join was not fetched IMO it should not return in the serialization (equiv of doing JsonIgnore). It leads to having to add more complex configs such as JsonViews and other "tricks" to prevent the error.

having to explicitly add JSON ignore then becomes problematic because what happens when you do fetch it and want to return the value. Seems like you would then have to apply: @JsonInclude(Include.NON_NULL) to everything.

@StephenOTT
Copy link

also given that Joins are lazy by default it becomes additional configs that need to be "always added" or else you get the LazyInitializationException

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

I mean, if you know that you always need it, you can map it as EAGER.

I suppose we could assume that if the association is an uninitialize proxy, than it doesn't need to be serialized (not sure if it's doable though).
But I prefer developers to be mindful and explicit about what they want. If you see a LazyInitializationException you know that you have to make a choice:

  • load the association before serialization
  • ignore it

Much better than returning an empty result and than having to figure out why the association doesn't appear in the JSON.

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

I also think it's reasonable to have a separate class to map a JSON response if the entity doesn't fit your use case. I expect this to be the most common scenario actually.

@StephenOTT
Copy link

I agree those are the scenarios that complex impl end up creating as they need all of the various projections. But by default and from the examples: https://quarkus.io/guides/hibernate-orm-panache#writing-a-jax-rs-resource, the moment you use Reactive and add a default join, the rest request fails. :)

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

¯_(ツ)_/¯
I think all the alternatives I've heard so far would be worst :)

You are basically asking the library to pick for you:

  • we can load the lazy association: it might lead to performance issues or stackoverflows in case of bidirectional associations (if doable at all)
  • we don't load the association: you might expect the association to be loaded (hard to figure out what's going on, if you are not familiar with the libraries)

Throwing an error we tell the developer that there's something missing and they need to decide what they want to do.
I think it works.
This is just my opinion though. Maybe other people agree with you.

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

We should improve the guide and add this example if it's not included already, though

@DavideD
Copy link
Contributor

DavideD commented Nov 11, 2022

Thanks for the feedback.
I've created this issue to update the guides: #29217

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants