From 578ecf378b0a924ade64e439f182f2d01d505516 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 28 Jan 2025 11:12:30 +0000 Subject: [PATCH 1/5] Updated queries to remove order by improving performance --- .../ExternalObjectDirectoryRepository.java | 99 ++++++++++--------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java index 564e186f6f..39d40f5ecd 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java @@ -373,36 +373,38 @@ default List findEodsForTransfer(ObjectRecordStat } @Query( - """ - SELECT eod FROM ExternalObjectDirectoryEntity eod - WHERE eod.status = :status - AND eod.externalLocationType = :type - AND eod.media is not null - AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 - where (eod2.status = :notExistsStatus or eod2.transferAttempts >= :maxTransferAttempts) - AND eod2.externalLocationType = :notExistsType - and (eod.media is not null and eod.media = eod2.media)) - order by eod.lastModifiedDateTime - """ + value = """ + select eode1_0.elt_id from darts.external_object_directory eode1_0 + where eode1_0.ors_id=:status + and eode1_0.elt_id=:type + and eode1_0.med_id is not null + and not exists(select 1 from darts.external_object_directory eode2_0 + where (eode2_0.elt_id=:notExistsStatus or eode2_0.transfer_attempts >= :maxTransferAttempts) + and eode1_0.elt_id=:notExistsType + and eode1_0.med_id is not null and eode1_0.med_id=eode2_0.med_id) + fetch first :limitRecords rows only + """, + nativeQuery = true ) List findEodsForTransferOnlyMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ObjectRecordStatusEntity notExistsStatus, ExternalLocationTypeEntity notExistsType, Integer maxTransferAttempts, Limit limit); @Query( - """ - SELECT eod FROM ExternalObjectDirectoryEntity eod - WHERE eod.status = :status - AND eod.externalLocationType = :type - AND eod.media is null - AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 - where (eod2.status = :notExistsStatus or eod2.transferAttempts >= :maxTransferAttempts) - AND eod2.externalLocationType = :notExistsType - and ((eod.transcriptionDocumentEntity is not null and eod.transcriptionDocumentEntity = eod2.transcriptionDocumentEntity) - OR (eod.annotationDocumentEntity is not null and eod.annotationDocumentEntity = eod2.annotationDocumentEntity) - OR (eod.caseDocument is not null and eod.caseDocument = eod2.caseDocument ))) - order by eod.lastModifiedDateTime - """ + value = """ + select eode1_0.elt_id from darts.external_object_directory eode1_0 + where eode1_0.ors_id=:status + and eode1_0.elt_id=:type + and eode1_0.med_id is null + and not exists(select 1 from darts.external_object_directory eode2_0 + where (eode2_0.elt_id=:notExistsStatus or eode2_0.transfer_attempts >= :maxTransferAttempts) + and eode1_0.elt_id=:notExistsType + and ((eode1_0.trd_id is not null and eode1_0.trd_id = eode2_0.trd_id) + or (eode1_0.ado_id is not null and eode1_0.ado_id = eode2_0.ado_id) + or (eode1_0.cad_id is not null and eode1_0.cad_id = eode2_0.cad_id))) + fetch first :limitRecords rows only + """, + nativeQuery = true ) List findEodsForTransferExcludingMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ObjectRecordStatusEntity notExistsStatus, ExternalLocationTypeEntity notExistsType, @@ -420,36 +422,35 @@ default List findEodsNotInOtherStorage(ObjectRecordStatusEntity status, } @Query( - """ - SELECT eod.id FROM ExternalObjectDirectoryEntity eod - WHERE eod.status = :status - AND eod.externalLocationType = :type - AND eod.media is not null - AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 - WHERE eod2.externalLocationType = :notExistsLocation - AND eod.media = eod2.media) - ORDER BY eod.id - LIMIT :limitRecords - """ + value = """ + select eode1_0.elt_id from darts.external_object_directory eode1_0 + where eode1_0.ors_id=:status + and eode1_0.elt_id=:type + and eode1_0.med_id is not null + and not exists(select 1 from darts.external_object_directory eode2_0 + where eode2_0.elt_id=:notExistsLocation + and eode1_0.med_id=eode2_0.med_id) + fetch first :limitRecords rows only + """, + nativeQuery = true ) List findEodsNotInOtherStorageOnlyMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ExternalLocationTypeEntity notExistsLocation, Integer limitRecords); - @Query( - """ - SELECT eod.id FROM ExternalObjectDirectoryEntity eod - WHERE eod.status = :status - AND eod.externalLocationType = :type - AND eod.media is null - AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 - where eod2.externalLocationType = :notExistsLocation - and ((eod.transcriptionDocumentEntity is not null and eod.transcriptionDocumentEntity = eod2.transcriptionDocumentEntity) - OR (eod.annotationDocumentEntity is not null and eod.annotationDocumentEntity = eod2.annotationDocumentEntity) - OR (eod.caseDocument is not null and eod.caseDocument = eod2.caseDocument ))) - order by eod.id - LIMIT :limitRecords - """ + value = """ + select eode1_0.elt_id from darts.external_object_directory eode1_0 + where eode1_0.ors_id=:status + and eode1_0.elt_id=:type + and eode1_0.med_id is null + and not exists(select 1 from darts.external_object_directory eode2_0 + where eode2_0.elt_id=:notExistsLocation + and ((eode1_0.trd_id is not null and eode1_0.trd_id = eode2_0.trd_id) + or (eode1_0.ado_id is not null and eode1_0.ado_id = eode2_0.ado_id) + or (eode1_0.cad_id is not null and eode1_0.cad_id = eode2_0.cad_id))) + fetch first :limitRecords rows only + """, + nativeQuery = true ) List findEodsNotInOtherStorageExcludingMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ExternalLocationTypeEntity notExistsLocation, Integer limitRecords); From 41a53bd7014c8d56613567f62c8d514e89488456 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 28 Jan 2025 11:59:53 +0000 Subject: [PATCH 2/5] Reverted one change --- .../ExternalObjectDirectoryRepository.java | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java index 39d40f5ecd..7f35e73084 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java @@ -373,38 +373,36 @@ default List findEodsForTransfer(ObjectRecordStat } @Query( - value = """ - select eode1_0.elt_id from darts.external_object_directory eode1_0 - where eode1_0.ors_id=:status - and eode1_0.elt_id=:type - and eode1_0.med_id is not null - and not exists(select 1 from darts.external_object_directory eode2_0 - where (eode2_0.elt_id=:notExistsStatus or eode2_0.transfer_attempts >= :maxTransferAttempts) - and eode1_0.elt_id=:notExistsType - and eode1_0.med_id is not null and eode1_0.med_id=eode2_0.med_id) - fetch first :limitRecords rows only - """, - nativeQuery = true + """ + SELECT eod FROM ExternalObjectDirectoryEntity eod + WHERE eod.status = :status + AND eod.externalLocationType = :type + AND eod.media is not null + AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 + where (eod2.status = :notExistsStatus or eod2.transferAttempts >= :maxTransferAttempts) + AND eod2.externalLocationType = :notExistsType + and (eod.media is not null and eod.media = eod2.media)) + order by eod.lastModifiedDateTime + """ ) List findEodsForTransferOnlyMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ObjectRecordStatusEntity notExistsStatus, ExternalLocationTypeEntity notExistsType, Integer maxTransferAttempts, Limit limit); @Query( - value = """ - select eode1_0.elt_id from darts.external_object_directory eode1_0 - where eode1_0.ors_id=:status - and eode1_0.elt_id=:type - and eode1_0.med_id is null - and not exists(select 1 from darts.external_object_directory eode2_0 - where (eode2_0.elt_id=:notExistsStatus or eode2_0.transfer_attempts >= :maxTransferAttempts) - and eode1_0.elt_id=:notExistsType - and ((eode1_0.trd_id is not null and eode1_0.trd_id = eode2_0.trd_id) - or (eode1_0.ado_id is not null and eode1_0.ado_id = eode2_0.ado_id) - or (eode1_0.cad_id is not null and eode1_0.cad_id = eode2_0.cad_id))) - fetch first :limitRecords rows only - """, - nativeQuery = true + """ + SELECT eod FROM ExternalObjectDirectoryEntity eod + WHERE eod.status = :status + AND eod.externalLocationType = :type + AND eod.media is null + AND NOT EXISTS (select 1 from ExternalObjectDirectoryEntity eod2 + where (eod2.status = :notExistsStatus or eod2.transferAttempts >= :maxTransferAttempts) + AND eod2.externalLocationType = :notExistsType + and ((eod.transcriptionDocumentEntity is not null and eod.transcriptionDocumentEntity = eod2.transcriptionDocumentEntity) + OR (eod.annotationDocumentEntity is not null and eod.annotationDocumentEntity = eod2.annotationDocumentEntity) + OR (eod.caseDocument is not null and eod.caseDocument = eod2.caseDocument ))) + order by eod.lastModifiedDateTime + """ ) List findEodsForTransferExcludingMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ObjectRecordStatusEntity notExistsStatus, ExternalLocationTypeEntity notExistsType, From 65a0f5ae805bd234508f8aee9712090c27d8e757 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 28 Jan 2025 12:47:01 +0000 Subject: [PATCH 3/5] Fixed tests --- .../ExternalObjectDirectoryRepository.java | 12 ++++++------ .../ExternalObjectDirectoryRepositoryTest.java | 13 ++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java index 7f35e73084..e89e3be234 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java @@ -412,9 +412,9 @@ List findEodsForTransferExcludingMedia(ObjectReco default List findEodsNotInOtherStorage(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, ExternalLocationTypeEntity notExistsLocation, Integer limitRecords) { Set results = new HashSet<>();//Ensures no duplicates - results.addAll(findEodsNotInOtherStorageOnlyMedia(status, type, notExistsLocation, limitRecords)); + results.addAll(findEodsNotInOtherStorageOnlyMedia(status.getId(), type.getId(), notExistsLocation.getId(), limitRecords)); if (results.size() < limitRecords) { - results.addAll(findEodsNotInOtherStorageExcludingMedia(status, type, notExistsLocation, limitRecords - results.size())); + results.addAll(findEodsNotInOtherStorageExcludingMedia(status.getId(), type.getId(), notExistsLocation.getId(), limitRecords - results.size())); } return new ArrayList<>(results); } @@ -432,8 +432,8 @@ and not exists(select 1 from darts.external_object_directory eode2_0 """, nativeQuery = true ) - List findEodsNotInOtherStorageOnlyMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, - ExternalLocationTypeEntity notExistsLocation, Integer limitRecords); + List findEodsNotInOtherStorageOnlyMedia(Integer status, Integer type, + Integer notExistsLocation, Integer limitRecords); @Query( value = """ @@ -450,8 +450,8 @@ and not exists(select 1 from darts.external_object_directory eode2_0 """, nativeQuery = true ) - List findEodsNotInOtherStorageExcludingMedia(ObjectRecordStatusEntity status, ExternalLocationTypeEntity type, - ExternalLocationTypeEntity notExistsLocation, Integer limitRecords); + List findEodsNotInOtherStorageExcludingMedia(Integer status, Integer type, + Integer notExistsLocation, Integer limitRecords); @Query( """ diff --git a/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java b/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java index 71df5925e2..50df4d9073 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class ExternalObjectDirectoryRepositoryTest { @@ -93,8 +94,11 @@ void findEodsNotInOtherStorage_mediaItemsArelessThanLimit_shouldReturnBothMediaA .findEodsNotInOtherStorageExcludingMedia(any(), any(), any(), any()); ObjectRecordStatusEntity status = mock(ObjectRecordStatusEntity.class); + when(status.getId()).thenReturn(1); ExternalLocationTypeEntity type = mock(ExternalLocationTypeEntity.class); + when(type.getId()).thenReturn(2); ExternalLocationTypeEntity notExistsType = mock(ExternalLocationTypeEntity.class); + when(notExistsType.getId()).thenReturn(3); List eods = externalObjectDirectoryRepository.findEodsNotInOtherStorage( status, type, notExistsType, 5); @@ -104,9 +108,9 @@ void findEodsNotInOtherStorage_mediaItemsArelessThanLimit_shouldReturnBothMediaA .containsExactlyInAnyOrder(mediaEod1Id, mediaEod2Id, nonMediaEod1Id); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageOnlyMedia(status, type, notExistsType, 5); + .findEodsNotInOtherStorageOnlyMedia(1, 2, 3, 5); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageExcludingMedia(status, type, notExistsType, 3); + .findEodsNotInOtherStorageExcludingMedia(1, 2, 3, 3); } @Test @@ -118,8 +122,11 @@ void findEodsNotInOtherStorage_mediaItemsAreEqualToLimit_shouldReturnOnlyMedia() .when(externalObjectDirectoryRepository) .findEodsNotInOtherStorageOnlyMedia(any(), any(), any(), any()); ObjectRecordStatusEntity status = mock(ObjectRecordStatusEntity.class); + when(status.getId()).thenReturn(1); ExternalLocationTypeEntity type = mock(ExternalLocationTypeEntity.class); + when(type.getId()).thenReturn(2); ExternalLocationTypeEntity notExistsType = mock(ExternalLocationTypeEntity.class); + when(notExistsType.getId()).thenReturn(3); List eods = externalObjectDirectoryRepository.findEodsNotInOtherStorage( status, type, notExistsType, 2); @@ -129,7 +136,7 @@ void findEodsNotInOtherStorage_mediaItemsAreEqualToLimit_shouldReturnOnlyMedia() .containsExactlyInAnyOrder(mediaEod1Id, mediaEod2Id); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageOnlyMedia(status, type, notExistsType, 2); + .findEodsNotInOtherStorageOnlyMedia(1, 2, 3, 2); verify(externalObjectDirectoryRepository, never()) .findEodsNotInOtherStorageExcludingMedia(any(), any(), any(), any()); } From 8765197f833b7b4ac813ae2cc39856804107fdd3 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 28 Jan 2025 16:04:40 +0000 Subject: [PATCH 4/5] Updated sql --- .../common/repository/ExternalObjectDirectoryRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java index e89e3be234..455ab7fcc5 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepository.java @@ -421,7 +421,7 @@ default List findEodsNotInOtherStorage(ObjectRecordStatusEntity status, @Query( value = """ - select eode1_0.elt_id from darts.external_object_directory eode1_0 + select eode1_0.eod_id from darts.external_object_directory eode1_0 where eode1_0.ors_id=:status and eode1_0.elt_id=:type and eode1_0.med_id is not null @@ -437,7 +437,7 @@ List findEodsNotInOtherStorageOnlyMedia(Integer status, Integer type, @Query( value = """ - select eode1_0.elt_id from darts.external_object_directory eode1_0 + select eode1_0.eod_id from darts.external_object_directory eode1_0 where eode1_0.ors_id=:status and eode1_0.elt_id=:type and eode1_0.med_id is null From d17133403f86814c55cb8ae7e544f5bf56fbb2f7 Mon Sep 17 00:00:00 2001 From: benedwards Date: Fri, 31 Jan 2025 17:44:17 +0000 Subject: [PATCH 5/5] Updated test --- ...ExternalObjectDirectoryRepositoryTest.java | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java b/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java index 50df4d9073..76e88f3aba 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/repository/ExternalObjectDirectoryRepositoryTest.java @@ -82,10 +82,16 @@ void findEodsForTransfer_mediaItemsAreEqualToLimit_shouldReturnOnlyMedia() { @Test void findEodsNotInOtherStorage_mediaItemsArelessThanLimit_shouldReturnBothMediaAndNonMedia() { + final int mediaEod1Id = 1; + final int mediaEod2Id = 2; + final int nonMediaEod1Id = 3; + final int statusId = 4; + final int typeId = 5; + final int notExistsTypeId = 6; + final int limitRecords = 5; + final int numberOfMediaEods = 2; + ExternalObjectDirectoryRepository externalObjectDirectoryRepository = spy(ExternalObjectDirectoryRepository.class); - int mediaEod1Id = 1; - int mediaEod2Id = 2; - int nonMediaEod1Id = 3; doReturn(List.of(mediaEod1Id, mediaEod2Id)) .when(externalObjectDirectoryRepository) .findEodsNotInOtherStorageOnlyMedia(any(), any(), any(), any()); @@ -93,50 +99,55 @@ void findEodsNotInOtherStorage_mediaItemsArelessThanLimit_shouldReturnBothMediaA .when(externalObjectDirectoryRepository) .findEodsNotInOtherStorageExcludingMedia(any(), any(), any(), any()); + ObjectRecordStatusEntity status = mock(ObjectRecordStatusEntity.class); - when(status.getId()).thenReturn(1); + when(status.getId()).thenReturn(statusId); ExternalLocationTypeEntity type = mock(ExternalLocationTypeEntity.class); - when(type.getId()).thenReturn(2); + when(type.getId()).thenReturn(typeId); ExternalLocationTypeEntity notExistsType = mock(ExternalLocationTypeEntity.class); - when(notExistsType.getId()).thenReturn(3); + when(notExistsType.getId()).thenReturn(notExistsTypeId); List eods = externalObjectDirectoryRepository.findEodsNotInOtherStorage( - status, type, notExistsType, 5); + status, type, notExistsType, limitRecords); assertThat(eods) .hasSize(3) .containsExactlyInAnyOrder(mediaEod1Id, mediaEod2Id, nonMediaEod1Id); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageOnlyMedia(1, 2, 3, 5); + .findEodsNotInOtherStorageOnlyMedia(statusId, typeId, notExistsTypeId, limitRecords); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageExcludingMedia(1, 2, 3, 3); + .findEodsNotInOtherStorageExcludingMedia(statusId, typeId, notExistsTypeId, limitRecords - numberOfMediaEods); } @Test void findEodsNotInOtherStorage_mediaItemsAreEqualToLimit_shouldReturnOnlyMedia() { + final int mediaEod1Id = 1; + final int mediaEod2Id = 2; + final int statusId = 4; + final int typeId = 5; + final int notExistsTypeId = 6; + final int limitRecords = 2; ExternalObjectDirectoryRepository externalObjectDirectoryRepository = spy(ExternalObjectDirectoryRepository.class); - int mediaEod1Id = 1; - int mediaEod2Id = 2; doReturn(List.of(mediaEod1Id, mediaEod2Id)) .when(externalObjectDirectoryRepository) .findEodsNotInOtherStorageOnlyMedia(any(), any(), any(), any()); ObjectRecordStatusEntity status = mock(ObjectRecordStatusEntity.class); - when(status.getId()).thenReturn(1); + when(status.getId()).thenReturn(statusId); ExternalLocationTypeEntity type = mock(ExternalLocationTypeEntity.class); - when(type.getId()).thenReturn(2); + when(type.getId()).thenReturn(typeId); ExternalLocationTypeEntity notExistsType = mock(ExternalLocationTypeEntity.class); - when(notExistsType.getId()).thenReturn(3); + when(notExistsType.getId()).thenReturn(notExistsTypeId); List eods = externalObjectDirectoryRepository.findEodsNotInOtherStorage( - status, type, notExistsType, 2); + status, type, notExistsType, limitRecords); assertThat(eods) .hasSize(2) .containsExactlyInAnyOrder(mediaEod1Id, mediaEod2Id); verify(externalObjectDirectoryRepository) - .findEodsNotInOtherStorageOnlyMedia(1, 2, 3, 2); + .findEodsNotInOtherStorageOnlyMedia(statusId, typeId, notExistsTypeId, limitRecords); verify(externalObjectDirectoryRepository, never()) .findEodsNotInOtherStorageExcludingMedia(any(), any(), any(), any()); }