spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
3.01k stars 1.42k forks source link

Entity with embedded key is not deleted #3615

Closed javydreamercsw closed 1 month ago

javydreamercsw commented 1 month ago

I'm learning Spring Boot as I go, and everything has been fine so far using Spring JPA to manage the database until I tried deleting an entity with an embedded key. See the attached project. test-stability-api.zip

The relevant entity is as follows:

```@Setter
@Getter
@Data
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
@Cacheable
@ToString
@Builder
@Table(name = "test_case_execution_result")
@NamedQueries({
@NamedQuery(
name = "TestCaseExecutionResult.findAll",
query = "SELECT t FROM TestCaseExecutionResult t"),
@NamedQuery(
name = "TestCaseExecutionResult.findById",
query = "SELECT t FROM TestCaseExecutionResult t WHERE t.testCaseExecutionResultPK.id = :id"),
@NamedQuery(
name = "TestCaseExecutionResult.findByTestCaseResultId",
query =
"SELECT t FROM TestCaseExecutionResult t WHERE"
\+ " t.testCaseExecutionResultPK.testCaseResultId = :testCaseResultId"),
@NamedQuery(
name = "TestCaseExecutionResult.findByTestCaseId",
query =
"SELECT t FROM TestCaseExecutionResult t WHERE t.testCaseExecutionResultPK.testCaseId ="
\+ " :testCaseId"),
@NamedQuery(
name = "TestCaseExecutionResult.findByTestRegionId",
query =
"SELECT t FROM TestCaseExecutionResult t WHERE t.testCaseExecutionResultPK.testRegionId ="
\+ " :testRegionId"),
@NamedQuery(
name = "TestCaseExecutionResult.findByDate",
query = "SELECT t FROM TestCaseExecutionResult t WHERE t.date = :date"),
@NamedQuery(
name = "TestCaseExecutionResult.findByRelatedJob",
query = "SELECT t FROM TestCaseExecutionResult t WHERE t.relatedJob = :relatedJob")
})

public class TestCaseExecutionResult implements Serializable {

@Serial private static final long serialVersionUID = 1L;
@EmbeddedId protected TestCaseExecutionResultPK testCaseExecutionResultPK;

@Basic(optional = false)
@NotNull @Column(name = "date")
@Temporal(TemporalType.TIMESTAMP)
private Date date;

@Basic(optional = false)
@NotNull @Size(min = 1, max = 255)
@Column(name = "relatedJob")
private String relatedJob;

@JoinColumn(
name = "test_case_id",
referencedColumnName = "id",
insertable = false,
updatable = false)
@ManyToOne(optional = false)
private TestCase testCase;

@JoinColumn(
name = "test_case_result_id",
referencedColumnName = "id",
insertable = false,
updatable = false)
@ManyToOne(optional = false)
private TestCaseResult testCaseResult;

@JoinColumn(
name = "test_region_id",
referencedColumnName = "id",
insertable = false,
updatable = false)
@ManyToOne(optional = false)
private TestRegion testRegion;

public TestCaseExecutionResult(TestCaseExecutionResultPK testCaseExecutionResultPK) {
this.testCaseExecutionResultPK = testCaseExecutionResultPK;
}

public TestCaseExecutionResult(
TestCaseExecutionResultPK testCaseExecutionResultPK, Date date, String relatedJob) {
this.testCaseExecutionResultPK = testCaseExecutionResultPK;
this.date = date;
this.relatedJob = relatedJob;
}

public TestCaseExecutionResult(
int testCaseExecutionResultPK, int testCaseResultId, int testCaseId, int testRegionId) {
this.testCaseExecutionResultPK =
new TestCaseExecutionResultPK(
testCaseExecutionResultPK, testCaseResultId, testCaseId, testRegionId);
}

@Override
public int hashCode() {
int hash = 0;
hash += (testCaseExecutionResultPK != null ? testCaseExecutionResultPK.hashCode() : 0);
return hash;
}

@Override
public boolean equals(Object object) {
// TODO: Warning - this method won't work in the case the id fields are not set
if (!(object instanceof TestCaseExecutionResult other)) {
return false;
}
return (this.testCaseExecutionResultPK != null || other.testCaseExecutionResultPK == null)
&& (this.testCaseExecutionResultPK == null
|| this.testCaseExecutionResultPK.equals(other.testCaseExecutionResultPK));
}
}

This is the relevant controller:

    ```@RestController
    @SuppressWarnings("PMD.TestClassWithoutTestCases")
    public class TestCaseExecutionResultController {

    private final TestCaseExecutionResultRepository repository;
    private final TestCaseExecutionResultAssembler assembler;

    public TestCaseExecutionResultController(
    TestCaseExecutionResultRepository repository, TestCaseExecutionResultAssembler assembler) {
    this.repository = repository;
    this.assembler = assembler;
    }

    @PostMapping(Endpoints.EXECUTION_RESULT_REGION)
    @ResponseStatus(HttpStatus.CREATED)
    public ResponseEntity\<EntityModel\<TestCaseExecutionResult\>\> createTestCaseExecutionResult(
    @RequestBody TestCaseExecutionResult testCaseExecutionResult,
    @PathVariable Integer tcId,
    @PathVariable Integer regionId,
    @PathVariable Integer resultId) {
    if (testCaseExecutionResult.getTestCaseExecutionResultPK() == null) {
    testCaseExecutionResult.setTestCaseExecutionResultPK(
    TestCaseExecutionResultPK.builder()
    .testCaseResultId(resultId)
    .testCaseId(tcId)
    .testRegionId(regionId)
    .build());
    }
    testCaseExecutionResult.setDate(new Date());
    return ResponseEntity.ok(this.assembler.toModel(this.repository.save(testCaseExecutionResult)));
    }

    @GetMapping(Endpoints.EXECUTION_RESULTS)
    public ResponseEntity\<CollectionModel\<EntityModel\<TestCaseExecutionResult\>\>\> findAll() {
    return ResponseEntity.ok(this.assembler.toCollectionModel(this.repository.findAll()));
    }

    @GetMapping(Endpoints.EXECUTION_RESULTS_REGION)
    public ResponseEntity\<CollectionModel\<EntityModel\<TestCaseExecutionResult\>\>\>
    executionResultsForTestCaseInRegion(
    @PathVariable Integer tcId, @PathVariable Integer regionId) throws NotFoundException {
    return ResponseEntity.ok(
    this.assembler.toCollectionModel(
    this.repository.findByTestCaseIdAndTestRegionId(tcId, regionId)));
    }

    @DeleteMapping(Endpoints.EXECUTION_RESULT_REGION_EXECUTION)
    public ResponseEntity\<?\> delete(
    @PathVariable Integer tcId,
    @PathVariable Integer regionId,
    @PathVariable Integer resultId,
    @PathVariable Integer executionId)
    throws NotFoundException {
    Iterable\<TestCaseExecutionResult\> results =
    repository.findAllById(
    List.of(new TestCaseExecutionResultPK(executionId, resultId, tcId, regionId)));
    if (results.iterator().hasNext()) {
    TestCaseExecutionResult target = results.iterator().next();
    this.repository.deleteById(target.getTestCaseExecutionResultPK());
    return ResponseEntity.noContent().build();
    } else {
    throw new NotFoundException(
    "Test case execution result with id "
    \+ new TestCaseExecutionResultPK(executionId, resultId, tcId, regionId)
    \+ " not found");
    }
    }
    }

Have the following test where I basically create the necesary entities and then create a test case execution result which works fine. Then I proceed to delete it but fail when I expect to no longer being able to find it via the API. This is the approach that I use in all the other tests and seem to work fine in those cases. The only difference I can find so far is the key for this entity.

```@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@Slf4j
public class TestCaseExecutionTest {

@Autowired private TestCaseExecutionResultController controller;

@Autowired TestCaseExecutionResultRepository repository;

@Autowired private TestRegionRepository regionRepository;

@Autowired private TestCaseResultRepository resultRepository;

@Autowired private TestCaseRepository testCaseRepository;

@LocalServerPort private int port;

@Autowired private TestRestTemplate restTemplate;

@Test
void contextLoads() {
assertThat(controller).isNotNull();
}

@Test
@SneakyThrows
public void testCreateTestCaseExecutionResult() {
TestCase tc =
TestCase.builder()
.className("ClassName")
.testName("testName")
.testMethod("testMethod")
.description("description")
.build();
TestCase testCase = testCaseRepository.save(tc);

    Iterator<TestRegion> iterator = regionRepository.findAll().iterator();
    TestRegion region;
    if (iterator.hasNext()) {
      region = iterator.next();
    } else {
      region = TestRegion.builder().regionName(UUID.randomUUID().toString()).build();
      regionRepository.save(region);
    }

    TestCaseResult testCaseResult = resultRepository.findById(1);

    // Add an execution result
    TestCaseExecutionResult request =
        TestCaseExecutionResult.builder()
            .testRegion(TestRegion.builder().id(region.getId()).build())
            .testCase(TestCase.builder().id(testCase.getId()).build())
            .testCaseResult(TestCaseResult.builder().id(testCaseResult.getId()).build())
            .relatedJob("Dummy URL")
            .build();

    this.restTemplate.getForEntity(
        "http://localhost:" + port + Endpoints.EXECUTION_RESULTS_REGION,
        TestCaseExecutionResult.class,
        request.getTestCase().getId(),
        request.getTestRegion().getId());

    assertTrue(Objects.requireNonNull(controller.findAll().getBody()).getContent().isEmpty());

    TestCaseExecutionResult testCaseExecutionResult =
        this.restTemplate.postForObject(
            "http://localhost:" + port + Endpoints.EXECUTION_RESULT_REGION,
            request,
            TestCaseExecutionResult.class,
            request.getTestCase().getId(),
            request.getTestRegion().getId(),
            request.getTestCaseResult().getId());

    assertThat(testCaseExecutionResult).isNotNull();
    assertThat(testCaseExecutionResult)
        .usingRecursiveComparison()
        .ignoringFields(
            "testCaseExecutionResultPK", "date", "testCase", "testCaseResult", "testRegion")
        .isEqualTo(request);

    assertFalse(Objects.requireNonNull(controller.findAll().getBody()).getContent().isEmpty());

    assertFalse(
        Objects.requireNonNull(
                controller
                    .executionResultsForTestCaseInRegion(
                        testCaseExecutionResult.getTestCase().getId(),
                        testCaseExecutionResult.getTestRegion().getId())
                    .getBody())
            .getContent()
            .isEmpty());

    ResponseEntity<TestCaseExecutionResultWrapper> response =
        this.restTemplate.getForEntity(
            "http://localhost:" + port + Endpoints.EXECUTION_RESULTS_REGION,
            TestCaseExecutionResultWrapper.class,
            request.getTestCase().getId(),
            request.getTestRegion().getId());

    TestCaseExecutionResultWrapper wrapper = response.getBody();
    assertNotNull(wrapper);
    List<TestCaseExecutionResult> results = wrapper.getEmbedded().getTestCaseExecutionResults();
    assertNotNull(results);
    assertEquals(results.size(), 1);

    testCaseExecutionResult = results.get(0);

    // Delete it
    this.restTemplate.delete(
        "http://localhost:" + port + Endpoints.EXECUTION_RESULT_REGION_EXECUTION,
        testCaseExecutionResult.getTestCaseExecutionResultPK().getTestCaseId(),
        testCaseExecutionResult.getTestCaseExecutionResultPK().getTestRegionId(),
        testCaseExecutionResult.getTestCaseExecutionResultPK().getTestCaseResultId(),
        testCaseExecutionResult.getTestCaseExecutionResultPK().getId());

    response =
        this.restTemplate.getForEntity(
            "http://localhost:" + port + Endpoints.EXECUTION_RESULTS_REGION,
            TestCaseExecutionResultWrapper.class,
            request.getTestCase().getId(),
            request.getTestRegion().getId());

    wrapper = response.getBody();
    assertNotNull(wrapper);
    results = wrapper.getEmbedded().getTestCaseExecutionResults();
    assertNotNull(results);
    if (!results.isEmpty()) {
      ObjectMapper objectMapper = new ObjectMapper();
      repository
          .findAll()
          .iterator()
          .forEachRemaining(
              item -> {
                try {
                  String json =
                      objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(item);
                  log.info(json);
                } catch (JsonProcessingException e) {
                  log.error("Error converting item to JSON", e);
                }
              });
    }
    assertEquals(results.size(), 0);

}
}


I was expecting the deleted entity not to be found afterward. 
schauder commented 1 month ago

Please provide a Minimimal Reproducable Example, preferable as a Github repository. Make sure to include the database, either as an in memory database or if that is not possible using Testcontainers.

Especially any web stuff is unrelated and has no place in a minimal reproducer.

Just looking at the keywords this might be related to JPAs 1st level cache.

javydreamercsw commented 1 month ago

I attached a zip file with the sample project. Is that not acceptable?

schauder commented 1 month ago

Nope. The project contains over three dozen classes. Seemingly concerned with a Rest/Web interface, logging. Four different test classes.

You describe a problem with an entity with and embedded PK. I therefore expect a reproducer to contain:

javydreamercsw commented 1 month ago

I've been unable to develop a reproducer so far after trying for hours. I removed the not relevant tests to get it as close as possible for your requirements and re-attached the project.

schauder commented 1 month ago

I'm sorry, but without a proper reproducer, we can't work on this issue.

javydreamercsw commented 1 month ago

I reduced the issue as much as possible. I was unable to reproduce it on a simpler project.

javydreamercsw commented 1 month ago

I enabled the SQL in the settings and noticed that the delete query is never done:

Hibernate: select tcss1_0.id,tcss1_0.test_case_id,tcss1_0.test_region_id,tcss1_0.score,tcss1_0.stability,tc1_0.id,tc1_0.class_name,tc1_0.description,tc1_0.test_method,tc1_0.test_name,tcerl1_0.test_case_id,tcerl1_0.id,tcerl1_0.test_case_result_id,tcerl1_0.test_region_id,tcerl1_0.date,tcerl1_0.related_job,tcr1_0.id,tcr1_0.result,tr1_0.id,tr1_0.region_name,tr2_0.id,tr2_0.region_name from test_case_stability_status tcss1_0 join test_case tc1_0 on tc1_0.id=tcss1_0.test_case_id left join test_case_execution_result tcerl1_0 on tc1_0.id=tcerl1_0.test_case_id left join test_case_result tcr1_0 on tcr1_0.id=tcerl1_0.test_case_result_id left join test_region tr1_0 on tr1_0.id=tcerl1_0.test_region_id join test_region tr2_0 on tr2_0.id=tcss1_0.test_region_id where (tcss1_0.id,tcss1_0.test_case_id,tcss1_0.test_region_id) in ((?,?,?)) Hibernate: select tcssl1_0.test_case_id,tcssl1_0.id,tcssl1_0.test_region_id,tcssl1_0.score,tcssl1_0.stability,tr1_0.id,tr1_0.region_name from test_case_stability_status tcssl1_0 left join test_region tr1_0 on tr1_0.id=tcssl1_0.test_region_id where tcssl1_0.test_case_id=? Hibernate: select tchil1_0.test_case_id,tchil1_0.issue_id,i1_0.id,i1_0.issue_reference,i1_0.issue_type_id,iti1_0.id,iti1_0.type from test_case_has_issue tchil1_0 left join issue i1_0 on i1_0.id=tchil1_0.issue_id left join issue_type iti1_0 on iti1_0.id=i1_0.issue_type_id where tchil1_0.test_case_id=?