quarkusio/quarkus

REST Data Panache: JPA Entity with multiple associations not consistently updated

Closed this issue · 17 comments

mrizzi commented

Describe the bug

When an Entity has multiple JPA associations (i.e. ManyToOne, ManyToMany and OneToMany), a removal to the ManyToOne field is not persisted when doing a PUT call.

Expected behavior

Get all the fields updated.

Actual behavior

The ManyToOne field is not part of the update.

To Reproduce

The reproducer is available at multiple-associations/hibernate-orm-panache-quickstart (based on from hibernate-orm-panache-quickstart from development branch).

To reproduce execute the test $ ./mvnw test -Dtest=PersonEndpointTest

Reproducer

The reproducer application has the Person entity with the following associations:

  • M:1 with Country
  • M:N with Fruits
  • 1:M with Car

The bean is deliberately missing the accessor methods for managing changes to associations with Fruits and Cars.

The test() method, basically, posts a Person with a valid Country and then it sends a PUT with a null country but the country is not updated consistently due to a "wrong" update query:

Hibernate: 
    update
        Person 
    set
        birth=?,
        name=? 
    where
        id=?

Commenting one of @ManyToMany or @OneToMany associations (@Transient annotation ready to be decommented) makes the test to work thanks to the execution of the expected update query.

I've also provided a testDb() method that works directly with the DB and it's not affected by the same issue.

Configuration

Screenshots

Environment (please complete the following information):

Output of uname -a or ver

Linux 5.10.16-200.fc33.x86_64 #1 SMP Sun Feb 14 03:02:32 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment 18.9 (build 11.0.10+9)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.10+9, mixed mode, sharing)

GraalVM version (if different from Java)

Quarkus version or git rev

999-SNAPSHOT (build 20210323.071124-626)

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

Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)

Additional context

(Add any other context about the problem here.)

gytis commented

Thanks, @mrizzi, will look into this.

gytis commented

The difference is caused by the EntityManager.merge being used in the update as opposed to the PanacheEntityBase.persist which is used in your testDb test scenario.
We use merge there, because the PUT operation updates as well as creates a new entity with a predefined ID (as long as it is supported). In such case a regular persist does not work. I'll try to think of some workaround to make the relations update work.

mrizzi commented

@gytis thanks for the info 👍

I've tried to have a look at the code before and after having opened this issue and the best idea I had to analyzed this was to use the quarkus.debug.transformed-classes-dir and quarkus.debug.generated-classes-dir options to read the code but I found no way to debug the test execution.

Have you got a better approach to share for debugging methods created for an interface that extends PanacheEntityResource during a test?

gytis commented

Good question but unfortunately I don't have a good answer to that. Maybe @geoand has some suggestions on debugging the generated/transformed code?

But specifically for this case, I've changed the generated code to use persist and the update worked as you'd expect. However, the creation stopped working.

geoand commented

I usually do mvn package -Dquarkus.package.fernflower.enabled=true -DskipTests.
That results in target/decompiled being created which contains all generated and transformed code.
Then I try to understand what went wrong by looking at that :)

mrizzi commented

Thanks a lot guys for the answers, both helpful 👍

  • 👍1
mrizzi commented

Hi guys,
upgrading to Quarkus 1.13.1, we've spotted another issue very close to this one.
Now it seems like also the POST method is affected by the issue in managing associations.
The issue appeared from Quarkus 1.13.0 so I've updated the reproducer adding:

  • PersonEndpointTest#testInsertWithReferencedCollection as a reproducer
  • quarkus-12 profile to run the tests with Quarkus 1.12.2
  • missing %test DB properties
  • @PostPersist method to manage the @ManyToMany association from the non-owning side

Tests with Quarkus 1.12.2 executing $ ./mvnw test -Dtest=PersonEndpointTest#testInsertWithReferencedCollection -Pquarkus-12 works.
Tests with Quarkus 999-SNAPSHOT executing $ ./mvnw test -Dtest=PersonEndpointTest#testInsertWithReferencedCollection fails.

gytis commented

I'm not sure why did it stop working on POST, but I can fix that by replacing persist with persistAndFlush (will raise a PR today).
However, this solution still does not fix the PUT operation. The other side of a many-to-many relation stays detached, and only the main object is updated.

FroMage commented

This sounds like a questions for @Sanne about ORM's behaviour wrt merge and detached entities.

Sanne commented

Honestly I'm not a fan of re-attaching detached objects out of their original transaction scope.

Before I look at the reproducer, you all know ORM doesn't cascade automatically on all relations right? You have to specify the cascading rules of your operations explicitly when attaching partially managed graphs (and that's another reason for which I tend to avoid such approaches).

MartinX3 commented

Still an issue with Quarkus 2.1
Create works.
Update doesn't save the change in the database.

Sgitario commented

I've been digging into this issue and I reproduce the issue with the reproducer (concretely, using the test `PersonEndpointTest#testInsertWithReferencedCollection'. However, I think this is not an Quarkus issue, but a configuration problem of the reproducer:

First of all, what the test is doing is to create a person with a couple of fruits at the same time.

Therefore, the owning entity is the person, so the @ManyToMany(mappedBy=... should be in the Fruit (see the line https://github.com/mrizzi/quarkus-quickstarts/blob/8c011043973c8850f73fccf8158fee2c067dd808/hibernate-orm-panache-quickstart/src/main/java/org/acme/hibernate/orm/panache/Person.java#L26) not in the Person as it's in the reproducer.

The second configuration issue is that we want to insert a bidirectional relationship, but we're only adding the person -> fruit entity, not the fruit -> person one. This is what I did to add also the fruit -> person automatically:

@PrePersist
public void prePersist() {
    System.out.println("prePresist: " + fruits);
    for (Fruit fruit : fruits) {
        System.out.println("prePresist: fruit: " + fruit.id);
        fruit.persons.add(this);
    }
}

Also, the annotation @JsonBackReference was problematic because in prod mode, the list of fruits that was sent was empty (it worked fine in test mode...). To solve this, I needed to remove this annotation and add @JsonIgnore in the field "persons" from Fruit.java.

Finally, I didn't need the postPersist method: https://github.com/mrizzi/quarkus-quickstarts/blob/8c011043973c8850f73fccf8158fee2c067dd808/hibernate-orm-panache-quickstart/src/main/java/org/acme/hibernate/orm/panache/Person.java#L36. But I'm not sure if this was necessary for other tests.

Can you confirm my findings? You can see all my changes in this commit: Sgitario/quarkus-quickstarts@b91c80c

I added a couple of scenarios: (1) using H2 and (2) using Postgresql. Also, I extended the test case to cover the creation of the person + fruits at once, and another test case to create the person and then update the list of fruits separately. All the tests are passing now for me.

cc @mrizzi @gytis @geoand @Sanne

geoand commented

Thanks for looking into it @Sgitario.

I haven't followed this, so I'll let the others comment

Sgitario commented

Closing as per my comment. Please, reopen it if you think there is an issue with Quarkus here.