spring-data-relational icon indicating copy to clipboard operation
spring-data-relational copied to clipboard

possibility of data loss using CrudRepository in case of nested entities as java Record

Open ljpeters opened this issue 1 year ago • 4 comments

Hi,

I have encountered an issue that occurs in spring-boot-starter-parent 3.2.0 - 3.2.2. It worked fine in 3.1.8 and 2.7.18.

I have the following structure:

public record Car(@Id Long id,
                  String reference,
                  Engine engine) {
}
public record Engine(@Id Long id,
                     Set<Piston> pistons) {
}
public record Piston(@Id Long id,
                     String name) {
}
create table public.car
(
    id bigint generated by default as identity constraint pk_car primary key,
    reference varchar(36) not null constraint uk_car_reference unique,
    created_at timestamp with time zone default now()
);

create table public.engine
(
    id bigint generated by default as identity constraint pk_engine primary key,
    car bigint not null constraint fk_engine_car references public.car,
    created_at timestamp with time zone default now()
);

create table public.piston
(
    id bigint generated by default as identity constraint pk_piston primary key,
    engine bigint not null constraint fk_piston_engine references public.engine,
    name text not null,
    created_at timestamp with time zone default now()
);

Create a Car in the database without an engine to cause the id column in the engine table to always be different from the car id.

INSERT INTO public.car (id, reference) VALUES (1, 'no engine');

The Repository that extends CrudRepository:

@Repository
public interface EngineTroubleRepository extends CrudRepository<Car, Long> {
    Optional<Car> findByReference(String reference);
}

Creates a new Car in code;

        final var car = new Car(
                null,
                carReferenceProvider.get(),
                new Engine(
                        null,
                        Set.of(
                                new Piston(
                                        null,
                                        "piston1"
                                ),
                                new Piston(
                                        null,
                                        "piston2"
                                ),
                                new Piston(
                                        null,
                                        "piston3"
                                )
                        )
                ));
        engineTroubleRepository.save(car);

Next, I use a find to lookup the same Car. The save causes the pistons to be deleted.

        final var car = engineTroubleRepository.findByReference(carReference).orElseThrow(() -> Problem.builder()
                .withStatus(NOT_FOUND.value())
                .withTitle("car not found")
                .withDetail("No car with id [%s] could be found.", carReference)
                .buildException());

        engineTroubleRepository.save(car);

I think this happens: the findByReference does a SELECT on Car, and subsequently on Engine. When selecting pistons, it takes the Car.id, instead of the Engine.id. The resulting Engine object does not have Pistons, and when doing the save(), al the pistons in the database get deleted.

In the root pom:

root pom snippet
  <parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>3.2.2</version>
    <relativePath/>
  </parent>

These are the included spring dependencies, it does NOT use/include Hibernate:

pom snippet
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-web</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-data-jdbc</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-validation</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-actuator</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-test</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-aop</artifactId>
    </dependency>

I moved this issue from the spring-boot repository.

ljpeters avatar Feb 07 '24 13:02 ljpeters

Thank you for getting in touch - it would be great if you could take the time to provide the above code snippets as a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

christophstrobl avatar Feb 08 '24 07:02 christophstrobl

ofcourse, I already have a project, but I have to check if it needs to be cleaned a bit more before I can send it.

ljpeters avatar Feb 08 '24 08:02 ljpeters

Hi, I've created a repository for you: https://github.com/ljpeters/engine-trouble

The main code is in https://github.com/ljpeters/engine-trouble/blob/main/src/main/java/nl/topicus/enginetrouble/controllers/EngineTroubleController.java

I hope you can simply load it into intellij and make it work. A few run configurations are included as well, so it should be possible to start db docker container, build code, start application and run test...

The test should fail, and you should see an ERROR in the application log.

ljpeters avatar Feb 12 '24 21:02 ljpeters

Thank you @ljpeters for the reproducer - I moved the issue to spring-data-relational the home of the data jdbc project used in the sample.

christophstrobl avatar Feb 13 '24 07:02 christophstrobl

There is a snapshot available that fixes https://github.com/spring-projects/spring-data-relational/issues/1692 3.3.0-1692-collection-in-embedded-SNAPSHOT And a PR to go with it: https://github.com/spring-projects/spring-data-relational/pull/1773

I assume this issue will be fixed by it as well. If nobody objects I'll close this issue as a duplicate once the PR is merged.

schauder avatar Apr 17 '24 07:04 schauder

Duplicate of #1692

schauder avatar Apr 18 '24 08:04 schauder