Panache deleteById fails silently

[rmanibus]

trafficstars

Describe the bug

It is currently impossible to delete an entity owning a ManyToOne relation.

deleteById return true, but the entity is still in database.

    MyEntity entity;

    @BeforeEach
    @Transactional
    public void setup() {
        entity = new MyEntity();
        entity.setDependency(new MyEntity());
        entity.persist();
    }

    @Test
    @Transactional
    public void testHelloEndpoint() {
        assertEquals(2, MyEntity.count());
        boolean deleted = MyEntity.deleteById(entity.getId());
        assertTrue(deleted);
        MyEntity.getEntityManager().flush();
        assertEquals(1, MyEntity.count()); // This is failing, expected: 1, actual: 2
    }

Expected behavior

the entity is deleted and the related entity stays in db.

Actual behavior

the entity is not deleted. setting cascade delete effectively allow to remove the entity but also remove the related entity.

How to Reproduce?

Reproducer: https://github.com/rmanibus/quarkus_27640

Output of uname -a or ver

No response

Output of java -version

openjdk 17.0.3 2022-04-19

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.11.3.Final

Build tool (ie. output of mvnw --version or gradlew --version)

mvn

Additional information

No response

[quarkus-bot[bot]]

/cc @FroMage, @loicmathieu

[rmanibus]

cc @Sanne I think this is more related to hibernate than panache ! entityManager.remove(entity) produce the same result

[Sanne]

@Transactional
public void testHelloEndpoint() {
    assertEquals(2, MyEntity.count());
    boolean deleted = MyEntity.deleteById(entity.getId());
    assertTrue(deleted);
    assertEquals(1, MyEntity.count()); // This is failing, expected: 1, actual: 2
}

The transaction only gets committed after the method is completed - which is after your count. All ORM's modifications are postponed to commit time, so that an optimal execution plan can be executed.

[Sanne]

In other words, you'd need to verify the new count in a new transaction. Alternatively you can force a flush() on the EntityManager to apply the changes immediately.

[rmanibus]

hello @Sanne , I can still see the entity in the db after the transaction. the count was just a way to demonstrate this.

Both executing QueryEntity.getEntityManager().flush() and running the delete opperation in another transaction produce the same result.

Please see the updated reproducer.

[Sanne]

Ok, thanks.

[rmanibus]

Hi @Sanne, do you have an idea about what can cause that ?

[Dutch-0]

Is there any update on this? I noticed the removal of the bug label while closing this issue, but it hasn't been re-added.

To add to his:

val e = repo.findById(id).awaitSuspending()
e.delete()
repo.flush()
val e2 = repo.findById(id).awaitSuspending()
repo.delete(e2)
repo.flush()
val e3 = repo.findById(id).awaitSuspending()

Every delete returns true, but the entity is still in the database. I even removed @ManyToOne and only got @OneToOne and one @Transient

[Sanne]

sorry, indeed should have re-added the bug label.

@Dutch-0 I don't understand your new example - using Hibernate Reactive for the find operations, but not doing reactive for delete and flush operations?

[Dutch-0]

Just to show that the problem is not only with Hibernate Reactive. In my project I try to delete with: repo.deleteById(id), which also does not work.

Interestingly, this works:

val session = repo.session.awaitSuspending()
val entity = session.find(User::class.java, id).awaitSuspending()
val result = session.remove(entity).awaitSuspending()
session.flush().awaitSuspending() 

[Dutch-0]

I managed to create a workaround, based on my previous findings. It could probably use a lot of clean-up and optimization, since its my first quarkus/mutiny project.

Note; I use a second trip to the database to verify that the entity has been removed.

return repo.session.chain { session ->
      // Get the entity from the db
      repo.findById(id)
          .onItem()
              // If entity found
              .ifNotNull().transformToUni { entity ->
                  //Remove the entity
                  session.remove(entity).chain { _ ->
                      // save changes to the db
                      session.flush().chain { _ ->
                          // Retry to get the entity from the db
                          repo.findById(id)
                              // If not null -> error
                              .onItem().ifNotNull().transform { Response.status(Response.Status.INTERNAL_SERVER_ERROR).build() }
                              // OK
                              .onItem().ifNull().continueWith(Response.ok().status(Response.Status.NO_CONTENT).build())
                      }
                  }
              }
              .onItem().ifNull().continueWith(Response.status(Response.Status.NOT_FOUND).build())
        }

[gsmet]

/cc @FroMage, @loicmathieu

[Sanne]

@Dutch-0 what you call "workaround" is actually the correct way of coding with the reactive stack.

To explain, this code makes no sense as the delete and flush operations aren't being executed:

val e = repo.findById(id).awaitSuspending()
e.delete()
repo.flush()
val e2 = repo.findById(id).awaitSuspending()
repo.delete(e2)
repo.flush()
val e3 = repo.findById(id).awaitSuspending()

This is extremely inefficient and won't scale in terms of performance, but should work (and you confirmed it worked indeed):

val session = repo.session.awaitSuspending()
val entity = session.find(User::class.java, id).awaitSuspending()
val result = session.remove(entity).awaitSuspending()
session.flush().awaitSuspending() 

So I'm not clear on what problem you're expressing; someone should have a look at @rmanibus 's revised reproducer though, sorry I've not been able. @loicmathieu perhaps?

[Dutch-0]

@Sanne You're obviously right. I simply overlooked the flush operation in the entry post and in my project.

[saiello]

@rmanibus @Sanne I went through the same problem but I eventually figured out I misconfigured the Cascade attributes.

Also in the @rmanibus's reproducer the entity deletion is un-scheduled due to the cascade configuration and the recursive relationship:

    @JoinColumn(name = "dependency_id")
    @ManyToOne(cascade = CascadeType.PERSIST)
    QueryEntity dependency;

    @OneToMany(mappedBy = "dependency", cascade = CascadeType.ALL)
    List<QueryEntity> requiredBy;

---

I increased the verbosity of org.hibernate and saw this:

2024-08-13 11:07:05,642 TRACE [org.hib.eve.int.DefaultDeleteEventListener] (main) Deleting a persistent instance
2024-08-13 11:07:05,642 TRACE [org.hib.eve.int.DefaultDeleteEventListener] (main) Deleting [org.acme.QueryEntity#c0a80076-914a-1daf-8191-4afdb0eb0000]
2024-08-13 11:07:05,642 TRACE [org.hib.eng.int.Cascade] (main) Processing cascade ACTION_DELETE for: org.acme.QueryEntity
2024-08-13 11:07:05,642 TRACE [org.hib.eng.int.Cascade] (main) Cascade ACTION_DELETE for collection: org.acme.QueryEntity.requiredBy

... 

2024-08-13 11:07:05,645 TRACE [org.hib.eng.int.Cascade] (main) Done cascade ACTION_DELETE for collection: org.acme.QueryEntity.requiredBy
2024-08-13 11:07:05,645 TRACE [org.hib.eng.int.Cascade] (main) Done processing cascade ACTION_DELETE for: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eng.int.Cascade] (main) Processing cascade ACTION_DELETE for: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eng.int.Cascade] (main) Done processing cascade ACTION_DELETE for: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eve.int.AbstractFlushingEventListener] (main) Flushing session
2024-08-13 11:07:05,646 DEBUG [org.hib.eve.int.AbstractFlushingEventListener] (main) Processing flush-time cascades
2024-08-13 11:07:05,646 TRACE [org.hib.eng.int.Cascade] (main) Processing cascade ACTION_PERSIST_ON_FLUSH for: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eng.int.Cascade] (main) Cascade ACTION_PERSIST_ON_FLUSH for collection: org.acme.QueryEntity.requiredBy
2024-08-13 11:07:05,646 TRACE [org.hib.eng.spi.CascadingAction] (main) Cascading to persist on flush: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eve.int.EntityState] (main) Deleted instance of: org.acme.QueryEntity
2024-08-13 11:07:05,646 TRACE [org.hib.eve.int.DefaultPersistEventListener] (main) un-scheduling entity deletion [[org.acme.QueryEntity#c0a80076-914a-1daf-8191-4afdb0eb0000]]

As far as I understand, removing the first entity triggers an ACTION_DELETE, resulting in a cascade delete on the second entity. The deletion of the second entity then triggers another ACTION_DELETE, which causes the first entity to be persisted.

---

To make the test pass, you can set cascade = CascadeType.PERSIST on both relationships.

[FroMage]

I'm not entirely sure what this is about then. Why would an entity delete operation be cancelled, because persist and delete occur in the same transaction?

Sounds like an ORM question, and not a Panache issue, WDYT @yrodiere ?

[yrodiere]

Sounds like an ORM question, and not a Panache issue, WDYT @yrodiere ?

Agreed.

I'm not entirely sure what this is about then. Why would an entity delete operation be cancelled, because persist and delete occur in the same transaction?

It looks odd, but I must admit I'm not sure what's going on either. I've never had to dig into cascade cycle handling, and was quite happy about that.

I, or someone else who knows better about this topic, will need to have a closer look when they have time. As a reminder for that good soul, here's the reproducer:

Reproducer: https://github.com/rmanibus/quarkus_27640

That being said, If there is a bug, this doesn't really look like a Quarkus bug, but more like a Hibernate ORM bug, and so should probably be reported at https://hibernate.atlassian.net/browse/HHH

[FroMage]

Thanks.

[DavideD]

I updated the test case to the latest Quarkus here: https://github.com/DavideD/quarkus_27640/commit/1112d1c766ca58a4c41154f9311aba9e217a2f8f

and I created a test with only Hibernate ORM (without Quarkus and Bytecode enhancements) here: https://github.com/DavideD/hibernate-test-case-templates/commit/a629441b60da1769172cbd72608d8a58f5ac6369

The test with vanilla Hibernate works as expected. I don't have the time to check what's going on this week, but I will keep it assigned to me for the time being and get back to it as soon as I can.

In the meanwhile, if somebody else wants to have a look, just let me know. Thanks

[DavideD]

I gave this issue another look and I don't know why we ignored @saiello comment containing the correct solution (sorry):

@OneToMany(mappedBy = "dependency", cascade = CascadeType.PERSIST)
List<QueryEntity> requiredBy;

will fix the issue.

The deletion of the entity is skipped because of the cascade mapping on the bidirectional association. It doesn't seem a bug to me, more like a complex mapping that could be simplified.

If we think that this is an issue, we should discuss it with the Hibernate ORM team on zulip or create an issue in the Hibernate ORM issue tracker.

I'm going to close this issue.