spring-projects / spring-data-relational

Spring Data Relational. Home of Spring Data JDBC and Spring Data R2DBC.
https://spring.io/projects/spring-data-jdbc
Apache License 2.0
740 stars 342 forks source link

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

Closed ljpeters closed 3 months ago

ljpeters commented 5 months ago

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 ```xml org.springframework.boot spring-boot-starter-parent 3.2.2 ```

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

pom snippet ```xml org.springframework.boot spring-boot-starter-web org.springframework.boot spring-boot-starter-data-jdbc org.springframework.boot spring-boot-starter-validation org.springframework.boot spring-boot-starter-actuator org.springframework.boot spring-boot-starter-test org.springframework.boot spring-boot-starter-aop ```

I moved this issue from the spring-boot repository.

christophstrobl commented 5 months ago

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.

ljpeters commented 5 months ago

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 commented 5 months ago

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.

christophstrobl commented 5 months ago

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.

schauder commented 3 months ago

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 commented 3 months ago

Duplicate of #1692