From 8e0e1c61567681d303f1d184d28d58cd51785bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 26 Jun 2024 17:40:13 +0200 Subject: [PATCH] Do not (incorrectly) set the value of @GeneratedValue IDs in tests It causes failures on Hibernate ORM 6.6, because it's forbidden, because it can cause ambiguity as to whether the entity is expected to already exist in DB. See https://docs.jboss.org/hibernate/orm/6.6/migration-guide/migration-guide.html#merge-versioned-deleted Current behavior in Hibernate ORM 6.5 is to ignore the explicitly set ID, so arguably even worse. Some tests were succeeding by sheer chance, because the explicitly set ID happened to match the next value of the sequence. Other tests were actually ignoring the explicitly set ID, and retrieving the generated one. See also https://hibernate.zulipchat.com/#narrow/stream/132094-hibernate-orm-dev/topic/EntityManager.2Emerge --- .../quarkus/hibernate/orm/quoting_strategies/Group.java | 3 --- .../spring/data/rest/CrudAndPagedResourceTest.java | 8 ++++---- .../java/io/quarkus/spring/data/rest/JpaResourceTest.java | 8 ++++---- .../spring/data/rest/crud/DefaultCrudResourceTest.java | 8 ++++---- .../data/panache/HibernateOrmRestDataPanacheTest.java | 2 +- .../io/quarkus/it/spring/data/jpa/complex/Parent.java | 3 --- .../io/quarkus/it/spring/data/jpa/complex/Parent2.java | 3 --- .../quarkus/it/spring/data/rest/SpringDataRestTest.java | 2 +- 8 files changed, 14 insertions(+), 23 deletions(-) diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java index ea871f59232ee..875abe97b070c 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/quoting_strategies/Group.java @@ -2,8 +2,6 @@ import jakarta.persistence.Column; import jakarta.persistence.Entity; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.GenerationType; import jakarta.persistence.Id; import jakarta.persistence.Table; @@ -24,7 +22,6 @@ public class Group { private String value; @Id - @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "groupSeq") public Long getId() { return id; } diff --git a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/CrudAndPagedResourceTest.java b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/CrudAndPagedResourceTest.java index b0e69ad9c3128..998cf692b0bd7 100644 --- a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/CrudAndPagedResourceTest.java +++ b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/CrudAndPagedResourceTest.java @@ -342,8 +342,8 @@ void shouldCreateHal() { void shouldCreateAndUpdate() { Response createResponse = given().accept("application/json") .and().contentType("application/json") - .and().body("{\"id\": \"101\", \"name\": \"test-update-create\"}") - .when().put("/crud-and-paged-records/101") + .and().body("{\"name\": \"test-update-create\"}") + .when().post("/crud-and-paged-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); @@ -370,8 +370,8 @@ void shouldCreateAndUpdate() { void shouldCreateAndUpdateHal() { Response createResponse = given().accept("application/hal+json") .and().contentType("application/json") - .and().body("{\"id\": \"102\", \"name\": \"test-update-create-hal\"}") - .when().put("/crud-and-paged-records/102") + .and().body("{\"name\": \"test-update-create-hal\"}") + .when().post("/crud-and-paged-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); diff --git a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/JpaResourceTest.java b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/JpaResourceTest.java index c5caf26481a44..c689def8e7d38 100644 --- a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/JpaResourceTest.java +++ b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/JpaResourceTest.java @@ -342,8 +342,8 @@ void shouldCreateHal() { void shouldCreateAndUpdate() { Response createResponse = given().accept("application/json") .and().contentType("application/json") - .and().body("{\"id\": \"101\", \"name\": \"test-update-create\"}") - .when().put("/jpa-records/101") + .and().body("{\"name\": \"test-update-create\"}") + .when().post("/jpa-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); @@ -370,8 +370,8 @@ void shouldCreateAndUpdate() { void shouldCreateAndUpdateHal() { Response createResponse = given().accept("application/hal+json") .and().contentType("application/json") - .and().body("{\"id\": \"102\", \"name\": \"test-update-create-hal\"}") - .when().put("/jpa-records/102") + .and().body("{\"name\": \"test-update-create-hal\"}") + .when().post("/jpa-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); diff --git a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/crud/DefaultCrudResourceTest.java b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/crud/DefaultCrudResourceTest.java index a716dcceaed74..e73fa217a95b0 100644 --- a/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/crud/DefaultCrudResourceTest.java +++ b/extensions/spring-data-rest/deployment/src/test/java/io/quarkus/spring/data/rest/crud/DefaultCrudResourceTest.java @@ -149,8 +149,8 @@ void shouldCreateHal() { void shouldCreateAndUpdate() { Response createResponse = given().accept("application/json") .and().contentType("application/json") - .and().body("{\"id\": \"101\", \"name\": \"test-update-create\"}") - .when().put("/default-records/101") + .and().body("{\"name\": \"test-update-create\"}") + .when().post("/default-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); @@ -177,8 +177,8 @@ void shouldCreateAndUpdate() { void shouldCreateAndUpdateHal() { Response createResponse = given().accept("application/hal+json") .and().contentType("application/json") - .and().body("{\"id\": \"102\", \"name\": \"test-update-create-hal\"}") - .when().put("/default-records/102") + .and().body("{\"name\": \"test-update-create-hal\"}") + .when().post("/default-records/") .thenReturn(); assertThat(createResponse.statusCode()).isEqualTo(201); diff --git a/integration-tests/hibernate-orm-rest-data-panache/src/test/java/io/quarkus/it/hibernate/orm/rest/data/panache/HibernateOrmRestDataPanacheTest.java b/integration-tests/hibernate-orm-rest-data-panache/src/test/java/io/quarkus/it/hibernate/orm/rest/data/panache/HibernateOrmRestDataPanacheTest.java index 4e926a5cf2d6a..73a33ea36ef1a 100644 --- a/integration-tests/hibernate-orm-rest-data-panache/src/test/java/io/quarkus/it/hibernate/orm/rest/data/panache/HibernateOrmRestDataPanacheTest.java +++ b/integration-tests/hibernate-orm-rest-data-panache/src/test/java/io/quarkus/it/hibernate/orm/rest/data/panache/HibernateOrmRestDataPanacheTest.java @@ -268,7 +268,7 @@ void shouldCreateUpdateAndDeleteBook() { Response response = given().accept("application/json") .and().contentType("application/json") .and().body(book.toString()) - .when().put("/books/100") + .when().post("/books/") .thenReturn(); assertThat(response.statusCode()).isEqualTo(201); assertThat(response.header("Location")).isNotEmpty(); diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent.java index f51c6dfcbc3fd..367206a859bc4 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent.java +++ b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent.java @@ -1,14 +1,11 @@ package io.quarkus.it.spring.data.jpa.complex; import jakarta.persistence.Entity; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.GenerationType; import jakarta.persistence.Id; @Entity public class Parent extends ParentBase { @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; public Parent(Long id, String name, String detail, int age, float test, TestEnum testEnum) { diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent2.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent2.java index 338e89d1d7810..96b37abf80b5d 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent2.java +++ b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/complex/Parent2.java @@ -1,15 +1,12 @@ package io.quarkus.it.spring.data.jpa.complex; import jakarta.persistence.Entity; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.GenerationType; import jakarta.persistence.Id; @Entity public class Parent2 extends ParentBase { @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; public Parent2(String name, String detail, int age, float test, TestEnum testEnum, Long id) { diff --git a/integration-tests/spring-data-rest/src/test/java/io/quarkus/it/spring/data/rest/SpringDataRestTest.java b/integration-tests/spring-data-rest/src/test/java/io/quarkus/it/spring/data/rest/SpringDataRestTest.java index caa599b4af964..d8e71d707d4a6 100644 --- a/integration-tests/spring-data-rest/src/test/java/io/quarkus/it/spring/data/rest/SpringDataRestTest.java +++ b/integration-tests/spring-data-rest/src/test/java/io/quarkus/it/spring/data/rest/SpringDataRestTest.java @@ -218,7 +218,7 @@ void shouldCreateUpdateAndDeleteBook() { Response response = given().accept("application/json") .and().contentType("application/json") .and().body(book.toString()) - .when().put("/books/100") + .when().post("/books/") .thenReturn(); assertThat(response.statusCode()).isEqualTo(201); assertThat(response.header("Location")).isNotEmpty();