Current Situation

I noticed that the save() method in SimpleJpaRepository uses entityInformation.isNew() to determine whether to call persist() or merge():

public <S extends T> S save(S entity) {
    Assert.notNull(entity, ENTITY_MUST_NOT_BE_NULL);

    if (entityInformation.isNew(entity)) {
        entityManager.persist(entity);  // Path 1: New entity
        return entity;
    } else {
        return entityManager.merge(entity);  // Path 2: Existing entity
    }
}

Observation

In SimpleJpaRepositoryUnitTests, I found tests related to DATAJPA-931 and DATAJPA-1261:

• mergeGetsCalledWhenDetached() (DATAJPA-931)
• mergeGetsCalledWhenAttached() (DATAJPA-931, DATAJPA-1261)

These tests verify the merge() path using em.contains() stubbing. However, the actual implementation uses entityInformation.isNew() for branching, not em.contains().

I noticed that both tests do not stub entityInformation.isNew(), which means Mockito returns false by default.

As a result, both tests only verify the merge() path, and there appears to be no test that verifies entityManager.persist() is called when isNew() returns true.

Question

I wanted to ask if this is intentional, or if this might be a test coverage gap that should be addressed?

The persist() path seems to be untested in the current test suite.

If this is indeed a coverage gap, I'd be happy to contribute a test case to cover the persist() path.

Proposed Solution

Add a unit test that properly stubs entityInformation.isNew() to verify the persist path:

@Test
void persistGetsCalledWhenEntityIsNew() {

    User user = new User();

    when(information.isNew(user)).thenReturn(true);

    repo.save(user);

    verify(em).persist(user);
}

This would ensure both code paths in save() are properly tested and align the tests with the actual implementation logic.

Thank you for maintaining this excellent project, and I appreciate any feedback you might have!