Skip to content

Commit 7051ccf

Browse files
perf: User delete - deletion handler veto improvement [DHIS2-21725] (#24304)
* perf: User delete, exists query flush mode changed from auto to commit * perf: Improve user deletion handler veto checks [DHIS2-21725] * update tests, flush before checking exists * update tests, flush before checking exists
1 parent 745e119 commit 7051ccf

3 files changed

Lines changed: 18 additions & 2 deletions

File tree

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/hibernate/HibernateIdentifiableObjectStore.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static org.hisp.dhis.query.JpaQueryUtils.generateHqlQueryForSharingCheck;
3434

3535
import jakarta.persistence.EntityManager;
36+
import jakarta.persistence.FlushModeType;
3637
import jakarta.persistence.TypedQuery;
3738
import jakarta.persistence.criteria.CriteriaBuilder;
3839
import jakarta.persistence.criteria.CriteriaQuery;
@@ -919,7 +920,14 @@ public List<T> findByCreatedBy(@Nonnull UserDetails user) {
919920
}
920921

921922
/**
922-
* Look up objects which have property createdBy or lastUpdatedBy linked to given {@link User}
923+
* Look up objects which have property createdBy or lastUpdatedBy linked to given {@link User}.
924+
*
925+
* <p>Runs with {@link FlushModeType#COMMIT}, so the JPA auto-flush (a full-session dirty-check)
926+
* is skipped before this query. This is safe for the current caller (the read-only deletion veto
927+
* phase, which runs before any deletion handler writes), and avoids a per-query flush that
928+
* dominates the cost when a large object graph is in the session. If a future caller reads within
929+
* a transaction and must observe earlier un-flushed changes, change this method to accept a
930+
* flush-mode parameter rather than relaxing the mode for everyone.
923931
*
924932
* @param user the {@link User} for filtering
925933
* @return TRUE of objects found. FALSE otherwise.
@@ -939,7 +947,12 @@ public boolean existsByUser(@Nonnull User user, final Set<String> checkPropertie
939947
return false;
940948
}
941949
query.where(builder.or(predicates.toArray(new Predicate[0])));
942-
return !entityManager.createQuery(query).setMaxResults(1).getResultList().isEmpty();
950+
return !entityManager
951+
.createQuery(query)
952+
.setFlushMode(FlushModeType.COMMIT)
953+
.setMaxResults(1)
954+
.getResultList()
955+
.isEmpty();
943956
}
944957

945958
/**

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataelement/DataElementStoreTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ void testExistsByUser() {
347347
DataElement dataElementA = createDataElement('A');
348348
dataElementA.setCreatedBy(userA);
349349
dataElementStore.save(dataElementA);
350+
entityManager.flush();
350351
assertTrue(dataElementStore.existsByUser(userA, Set.of("createdBy")));
351352
assertFalse(dataElementStore.existsByUser(userB, Set.of("createdBy")));
352353

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/user/UserServiceTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ void testDeleteCreatedByUser() {
194194
DataElement dataElement = createDataElement('A');
195195
dataElement.setCreatedBy(userA);
196196
idObjectManager.save(dataElement);
197+
entityManager.flush();
197198

198199
assertThrows(DeleteNotAllowedException.class, () -> userService.deleteUser(userA));
199200
}
@@ -208,6 +209,7 @@ void testDeleteLastUpdatedByUser() {
208209

209210
dataElement.setDescription("Updated");
210211
idObjectManager.update(dataElement);
212+
entityManager.flush();
211213

212214
assertThrows(DeleteNotAllowedException.class, () -> userService.deleteUser(userA));
213215
}

0 commit comments

Comments
 (0)