Skip to content

Commit 72dd53a

Browse files
committed
Return deleted entity from derived deleteBy method.
We now return the deleted entity and check, guard the delete query against batch deletes if the delete yields more than done result. Closes #3995
1 parent 9025ca3 commit 72dd53a

File tree

4 files changed

+92
-11
lines changed

4 files changed

+92
-11
lines changed

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/JpaCodeBlocks.java

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.springframework.core.MethodParameter;
3232
import org.springframework.core.annotation.MergedAnnotation;
33+
import org.springframework.dao.IncorrectResultSizeDataAccessException;
3334
import org.springframework.data.domain.Pageable;
3435
import org.springframework.data.domain.Score;
3536
import org.springframework.data.domain.SliceImpl;
@@ -46,6 +47,7 @@
4647
import org.springframework.data.repository.aot.generate.AotQueryMethodGenerationContext;
4748
import org.springframework.data.repository.query.ReturnedType;
4849
import org.springframework.data.support.PageableExecutionUtils;
50+
import org.springframework.data.util.ReflectionUtils;
4951
import org.springframework.javapoet.CodeBlock;
5052
import org.springframework.javapoet.CodeBlock.Builder;
5153
import org.springframework.javapoet.TypeName;
@@ -662,13 +664,14 @@ public CodeBlock build() {
662664
: TypeName.get(context.getDomainType());
663665
builder.add("\n");
664666

667+
Class<?> methodReturnType = context.getMethod().getReturnType();
665668
if (modifying.isPresent()) {
666669

667670
if (modifying.getBoolean("flushAutomatically")) {
668671
builder.addStatement("this.$L.flush()", context.fieldNameOf(EntityManager.class));
669672
}
670673

671-
Class<?> returnType = context.getMethod().getReturnType();
674+
Class<?> returnType = methodReturnType;
672675

673676
if (returnsModifying(returnType)) {
674677
builder.addStatement("int $L = $L.executeUpdate()", context.localVariable("result"), queryVariableName);
@@ -697,19 +700,42 @@ public CodeBlock build() {
697700

698701
builder.addStatement("$T $L = $L.getResultList()", List.class,
699702
context.localVariable("resultList"), queryVariableName);
703+
704+
boolean returnCount = ClassUtils.isAssignable(Number.class, methodReturnType);
705+
boolean simpleBatch = returnCount || ReflectionUtils.isVoid(methodReturnType);
706+
boolean collectionQuery = queryMethod.isCollectionQuery();
707+
708+
if (!simpleBatch && !collectionQuery) {
709+
710+
builder.beginControlFlow("if ($L.size() > 1)", context.localVariable("resultList"));
711+
builder.addStatement("throw new $1T($2S + $3L.size(), 1, $3L.size())",
712+
IncorrectResultSizeDataAccessException.class,
713+
"Delete query returned more than one element: expected 1, actual ", context.localVariable("resultList"));
714+
builder.endControlFlow();
715+
716+
builder.addStatement("$L.forEach($L::remove)", context.localVariable("resultList"),
717+
context.fieldNameOf(EntityManager.class));
718+
}
719+
700720
builder.addStatement("$L.forEach($L::remove)", context.localVariable("resultList"),
701721
context.fieldNameOf(EntityManager.class));
702-
if (!Collection.class.isAssignableFrom(context.getReturnType().toClass())) {
703-
if (ClassUtils.isAssignable(Number.class, context.getMethod().getReturnType())) {
704-
builder.addStatement("return $T.valueOf($L.size())", context.getMethod().getReturnType(),
722+
723+
if (collectionQuery) {
724+
builder.addStatement("return ($T) $L", List.class, context.localVariable("resultList"));
725+
726+
} else if (returnCount) {
727+
builder.addStatement("return $T.valueOf($L.size())", methodReturnType,
705728
context.localVariable("resultList"));
706729
} else {
707-
builder.addStatement("return ($T) ($L.isEmpty() ? null : $L.iterator().next())", actualReturnType,
708-
context.localVariable("resultList"), context.localVariable("resultList"));
730+
731+
if (Optional.class.isAssignableFrom(methodReturnType)) {
732+
builder.addStatement("return ($1T) $1T.ofNullable($2L.isEmpty() ? null : $2L.iterator().next())",
733+
Optional.class, context.localVariable("resultList"));
734+
} else {
735+
builder.addStatement("return ($1T) ($2L.isEmpty() ? null : $2L.iterator().next())", actualReturnType,
736+
context.localVariable("resultList"));
737+
}
709738
}
710-
} else {
711-
builder.addStatement("return ($T) $L", List.class, context.localVariable("resultList"));
712-
}
713739
} else if (aotQuery != null && aotQuery.isExists()) {
714740
builder.addStatement("return !$L.getResultList().isEmpty()", queryVariableName);
715741
} else if (aotQuery != null) {

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.core.convert.ConversionService;
3333
import org.springframework.core.convert.support.ConfigurableConversionService;
3434
import org.springframework.core.convert.support.DefaultConversionService;
35+
import org.springframework.dao.IncorrectResultSizeDataAccessException;
3536
import org.springframework.dao.InvalidDataAccessApiUsageException;
3637
import org.springframework.data.domain.Pageable;
3738
import org.springframework.data.domain.Score;
@@ -51,6 +52,7 @@
5152
import org.springframework.data.util.StreamUtils;
5253
import org.springframework.util.Assert;
5354
import org.springframework.util.ClassUtils;
55+
import org.springframework.util.CollectionUtils;
5456
import org.springframework.util.ReflectionUtils;
5557

5658
/**
@@ -399,16 +401,33 @@ public DeleteExecution(EntityManager em) {
399401
}
400402

401403
@Override
402-
protected Object doExecute(AbstractJpaQuery jpaQuery, JpaParametersParameterAccessor accessor) {
404+
protected @Nullable Object doExecute(AbstractJpaQuery jpaQuery, JpaParametersParameterAccessor accessor) {
403405

404406
Query query = jpaQuery.createQuery(accessor);
405407
List<?> resultList = query.getResultList();
406408

409+
boolean simpleBatch = Number.class.isAssignableFrom(jpaQuery.getQueryMethod().getReturnType())
410+
|| org.springframework.data.util.ReflectionUtils.isVoid(jpaQuery.getQueryMethod().getReturnType());
411+
boolean collectionQuery = jpaQuery.getQueryMethod().isCollectionQuery();
412+
413+
if (!simpleBatch && !collectionQuery) {
414+
415+
if (resultList.size() > 1) {
416+
throw new IncorrectResultSizeDataAccessException(
417+
"Delete query returned more than one element: expected 1, actual " + resultList.size(), 1,
418+
resultList.size());
419+
}
420+
}
421+
407422
for (Object o : resultList) {
408423
em.remove(o);
409424
}
410425

411-
return jpaQuery.getQueryMethod().isCollectionQuery() ? resultList : resultList.size();
426+
if (simpleBatch) {
427+
return resultList.size();
428+
}
429+
430+
return collectionQuery ? resultList : CollectionUtils.firstElement(resultList);
412431
}
413432
}
414433

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,6 +1582,38 @@ void deleteByShouldReturnListOfDeletedElementsWhenRetunTypeIsCollectionLike() {
15821582
assertThat(result).containsOnly(firstUser);
15831583
}
15841584

1585+
@Test // GH-3995
1586+
void deleteOneByShouldReturnDeletedElement() {
1587+
1588+
assertThat(repository.deleteOneByLastname(firstUser.getLastname())).isNull();
1589+
1590+
flushTestUsers();
1591+
1592+
User result = repository.deleteOneByLastname(firstUser.getLastname());
1593+
assertThat(result).isEqualTo(firstUser);
1594+
}
1595+
1596+
@Test // GH-3995
1597+
void deleteOneOptionalByShouldReturnDeletedElement() {
1598+
1599+
flushTestUsers();
1600+
1601+
Optional<User> result = repository.deleteOneOptionalByLastname(firstUser.getLastname());
1602+
assertThat(result).contains(firstUser);
1603+
}
1604+
1605+
@Test // GH-3995
1606+
void deleteOneShouldFailWhenMatchingMultipleResults() {
1607+
1608+
firstUser.setLastname("foo");
1609+
secondUser.setLastname("foo");
1610+
firstUser = repository.save(firstUser);
1611+
secondUser = repository.save(secondUser);
1612+
1613+
assertThatExceptionOfType(IncorrectResultSizeDataAccessException.class)
1614+
.isThrownBy(() -> repository.deleteOneByLastname(firstUser.getLastname()));
1615+
}
1616+
15851617
@Test // DATAJPA-460
15861618
void deleteByShouldRemoveElementsMatchingDerivedQuery() {
15871619

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,10 @@ Window<User> findTop3ByFirstnameStartingWithOrderByFirstnameAscEmailAddressAsc(S
302302
// DATAJPA-460
303303
List<User> deleteByLastname(String lastname);
304304

305+
User deleteOneByLastname(String lastname);
306+
307+
Optional<User> deleteOneOptionalByLastname(String lastname);
308+
305309
/**
306310
* Explicitly mapped to a procedure with name "plus1inout" in database.
307311
*/

0 commit comments

Comments
 (0)