Problem
JpaCrudService.save() and update() both delegate to the same CrudRepository.save() without checking the entity's ID state, so neither enforces its own semantic contract:
save(entity) is documented as creating a new entity (CreationService contract: "saves a new entity to the database") and runs CreationValidator. However, if an entity with an existing ID is passed, it silently performs an update while CreationValidator was already executed — bypassing the intended validator semantics.
update(entity) is documented as updating an existing entity (UpdateService contract: "updates the given entity") and runs UpdateValidator. However, if an entity with a null ID is passed, it silently performs an insert while UpdateValidator was already executed — again bypassing the intended validator semantics.
Expected Behavior
save() should throw IllegalArgumentException if the entity already has an ID set.
update() should throw IllegalArgumentException if the entity has a null ID.
Precedent
The DAO layer already enforces the update() contract. ConversionJpaDaoSupport.update() (line 80) does:
Objects.requireNonNull(persistentEntity.getId(), "id");
JpaCrudService should apply the same enforcement at the service layer for both methods.
Suggested Fix
@Override
public K save(T entity) {
K id = getId(entity);
if (id != null) {
throw new IllegalArgumentException("Cannot save entity that already has an id: " + id);
}
validate(CreationValidator.class, entity, CreationValidationException::new);
return getId(getCrudRepository().save(entity));
}
@Override
public void update(T entity) {
K id = getId(entity);
if (id == null) {
throw new IllegalArgumentException("Cannot update entity without an id");
}
validate(UpdateValidator.class, entity, UpdateValidationException::new);
getCrudRepository().save(entity);
}
No new imports are needed. The existing getId() reflection helper (line 45) is reused as-is, and the check is placed before validate() since it is a structural pre-condition rather than a business rule.
Problem
JpaCrudService.save()andupdate()both delegate to the sameCrudRepository.save()without checking the entity's ID state, so neither enforces its own semantic contract:save(entity)is documented as creating a new entity (CreationServicecontract: "saves a new entity to the database") and runsCreationValidator. However, if an entity with an existing ID is passed, it silently performs an update while CreationValidator was already executed — bypassing the intended validator semantics.update(entity)is documented as updating an existing entity (UpdateServicecontract: "updates the given entity") and runsUpdateValidator. However, if an entity with a null ID is passed, it silently performs an insert while UpdateValidator was already executed — again bypassing the intended validator semantics.Expected Behavior
save()should throwIllegalArgumentExceptionif the entity already has an ID set.update()should throwIllegalArgumentExceptionif the entity has a null ID.Precedent
The DAO layer already enforces the
update()contract.ConversionJpaDaoSupport.update()(line 80) does:JpaCrudServiceshould apply the same enforcement at the service layer for both methods.Suggested Fix
No new imports are needed. The existing
getId()reflection helper (line 45) is reused as-is, and the check is placed beforevalidate()since it is a structural pre-condition rather than a business rule.