Skip to content

Commit

Permalink
Do not (incorrectly) set the value of @GeneratedValue IDs in tests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yrodiere committed Jul 1, 2024
1 parent 819095f commit 8e0e1c6
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -24,7 +22,6 @@ public class Group {
private String value;

@Id
@GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "groupSeq")
public Long getId() {
return id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 8e0e1c6

Please sign in to comment.