From 1677f4890c29b29593b8886b20cac3d78ddc83d0 Mon Sep 17 00:00:00 2001 From: benedwards Date: Fri, 22 Nov 2024 14:02:56 +0000 Subject: [PATCH 01/16] Updated lastModifiedBy and createdBy to auto update based on entity lifecycle events --- .../config/AuthenticationConfiguration.java | 16 ++++++++++++++++ .../common/entity/base/CreatedBaseEntity.java | 2 ++ .../entity/base/CreatedModifiedBaseEntity.java | 2 ++ 3 files changed, 20 insertions(+) diff --git a/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java b/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java index 5fab67de3a..8ef64a2aeb 100644 --- a/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java +++ b/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java @@ -2,9 +2,14 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.data.domain.AuditorAware; import uk.gov.hmcts.darts.authentication.exception.AuthenticationError; +import uk.gov.hmcts.darts.authorisation.component.UserIdentity; +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import uk.gov.hmcts.darts.common.exception.DartsApiException; +import java.util.Optional; + @Configuration public class AuthenticationConfiguration { @@ -14,4 +19,15 @@ public AuthConfigFallback getNoAuthConfigurationFallback(DefaultAuthConfiguratio throw new DartsApiException(AuthenticationError.FAILED_TO_OBTAIN_AUTHENTICATION_CONFIG); }; } + + @Bean + public AuditorAware auditorAware(UserIdentity userIdentity) { + return () -> { + try { + return Optional.ofNullable(userIdentity.getUserAccount()); + } catch (Exception e) { + return Optional.empty(); + } + }; + } } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java index fd719bc7c2..81d6e1989e 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java @@ -9,6 +9,7 @@ import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.envers.NotAudited; +import org.springframework.data.annotation.CreatedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import java.time.OffsetDateTime; @@ -24,6 +25,7 @@ public class CreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "created_by") + @CreatedBy private UserAccountEntity createdBy; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java index 8eaff96393..1269f485f6 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java @@ -9,6 +9,7 @@ import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; import org.hibernate.envers.NotAudited; +import org.springframework.data.annotation.LastModifiedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import java.time.OffsetDateTime; @@ -25,5 +26,6 @@ public class CreatedModifiedBaseEntity extends CreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "last_modified_by") + @LastModifiedBy private UserAccountEntity lastModifiedBy; } From c9fa327111057b4ea028b39bf05d947566cee1c2 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 25 Nov 2024 10:04:47 +0000 Subject: [PATCH 02/16] Applied review comments --- .../TransientObjectDirectoryServiceTest.java | 8 ++++++-- .../audio/helper/TransformedMediaHelper.java | 8 ++++++-- .../base/MandatoryCreatedBaseEntity.java | 2 ++ .../MandatoryCreatedModifiedBaseEntity.java | 2 ++ .../repository/TransformedMediaRepository.java | 9 +++++++++ .../AudioTransformationServiceImplTest.java | 18 ++++++++++++++---- 6 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/common/service/TransientObjectDirectoryServiceTest.java b/src/integrationTest/java/uk/gov/hmcts/darts/common/service/TransientObjectDirectoryServiceTest.java index 946ddd132d..e1d69ab0da 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/common/service/TransientObjectDirectoryServiceTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/common/service/TransientObjectDirectoryServiceTest.java @@ -8,11 +8,13 @@ import uk.gov.hmcts.darts.audiorequests.model.AudioRequestType; import uk.gov.hmcts.darts.common.entity.TransformedMediaEntity; import uk.gov.hmcts.darts.common.entity.TransientObjectDirectoryEntity; +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import uk.gov.hmcts.darts.testutils.IntegrationBase; import java.time.OffsetDateTime; import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -37,7 +39,8 @@ void shouldSaveTransientDataLocation() { MediaRequestEntity mediaRequestEntity = mediaRequestService.getMediaRequestEntityById(mediaRequestEntity1.getId()); UUID blobId = UUID.fromString("f744a74f-83c0-47e4-8bb2-2fd4d2b68647"); - + UserAccountEntity userAccount = dartsDatabase.getUserAccountStub().createAuthorisedIntegrationTestUser(false, "NEWCASTLE"); + givenBearerTokenExists(userAccount.getEmailAddress()); TransformedMediaEntity transformedMediaEntity = transformedMediaHelper.createTransformedMediaEntity( mediaRequestEntity, "aFilename", @@ -45,6 +48,8 @@ void shouldSaveTransientDataLocation() { mediaRequestEntity.getEndTime(), 1000L ); + assertThat(transformedMediaEntity.getCreatedBy()).isEqualTo(mediaRequestEntity.getCreatedBy()); + assertThat(transformedMediaEntity.getLastModifiedBy()).isEqualTo(mediaRequestEntity.getCreatedBy()); TransientObjectDirectoryEntity transientObjectDirectoryEntity = transientObjectDirectoryService.saveTransientObjectDirectoryEntity( transformedMediaEntity, blobId @@ -62,5 +67,4 @@ void shouldSaveTransientDataLocation() { .isAfter(OffsetDateTime.parse("2023-07-06T16:05:00.000Z"))); assertNotNull(transientObjectDirectoryEntity.getLastModifiedBy()); } - } diff --git a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java index 5e979ab5c0..a43c465206 100644 --- a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java +++ b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java @@ -102,8 +102,12 @@ public TransformedMediaEntity createTransformedMediaEntity(MediaRequestEntity me if (nonNull(fileSize)) { entity.setOutputFilesize(fileSize.intValue()); } - transformedMediaRepository.save(entity); - return entity; + //Ensures createdBy / LastModified does not get overridden by the @CreatedBy / @LastModifiedBy annotaiton + TransformedMediaEntity savedTM = transformedMediaRepository.save(entity); + transformedMediaRepository.updateCreatedByLastModifiedBy(entity.getId(), + mediaRequest.getCreatedBy().getId(), + mediaRequest.getCreatedBy().getId()); + return savedTM; } @SuppressWarnings({"PMD.CognitiveComplexity"}) diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java index 83a4815041..20f2860efc 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java @@ -10,6 +10,7 @@ import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.envers.NotAudited; +import org.springframework.data.annotation.CreatedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import java.time.OffsetDateTime; @@ -29,6 +30,7 @@ public class MandatoryCreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "created_by", nullable = false) + @CreatedBy private UserAccountEntity createdBy; } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java index d4a5aba265..035036db50 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java @@ -10,6 +10,7 @@ import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; import org.hibernate.envers.NotAudited; +import org.springframework.data.annotation.LastModifiedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import java.time.OffsetDateTime; @@ -26,5 +27,6 @@ public class MandatoryCreatedModifiedBaseEntity extends MandatoryCreatedBaseEnti @NotAudited @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "last_modified_by", nullable = false) + @LastModifiedBy private UserAccountEntity lastModifiedBy; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java index c42a6883e1..4799da8677 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java @@ -3,6 +3,7 @@ import org.springframework.data.domain.Limit; import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.stereotype.Repository; import uk.gov.hmcts.darts.audio.model.TransformedMediaDetailsDto; @@ -93,4 +94,12 @@ List findTransformedMedia(Integer mediaId, OffsetDateTime requestedAtTo); + @Query(value = """ + UPDATE darts.transformed_media + set created_by = :id1, + last_modified_by = :id2 + where trm_id = :id + """, nativeQuery = true) + @Modifying + void updateCreatedByLastModifiedBy(Integer id, Integer id1, Integer id2); } \ No newline at end of file diff --git a/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java index 0e767c119a..4bb18dab00 100644 --- a/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java @@ -50,6 +50,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -180,6 +181,9 @@ void getMediaByHearingIdShouldReturnRepositoryResultsUnmodifiedWhenRepositoryRes void saveProcessedDataShouldSaveBlobAndSetStatus() { final MediaRequestEntity mediaRequestEntity = new MediaRequestEntity(); mediaRequestEntity.setRequestType(DOWNLOAD); + UserAccountEntity userAccount = new UserAccountEntity(); + userAccount.setId(1); + mediaRequestEntity.setCreatedBy(userAccount); BlobClientUploadResponse blobClientUploadResponse = mock(BlobClientUploadResponseImpl.class); UUID blobName = UUID.randomUUID(); @@ -198,9 +202,10 @@ void saveProcessedDataShouldSaveBlobAndSetStatus() { TransformedMediaEntity transformedMediaEntity = new TransformedMediaEntity(); transformedMediaEntity.setId(1); - when(transformedMediaRepository.save( - any() - )).thenReturn(transformedMediaEntity); + doAnswer(invocation -> invocation.getArgument(0)).when(transformedMediaRepository).save(any()); + + when(mockTransientObjectDirectoryEntity.getTransformedMedia()) + .thenReturn(transformedMediaEntity); when(mockTransientObjectDirectoryEntity.getTransformedMedia( )).thenReturn(transformedMediaEntity); @@ -543,7 +548,10 @@ void whenCreateTransformMediaEntityIsCalled_thenFilenameFormatSizeShouldBeSet() MediaRequestEntity mediaRequest = new MediaRequestEntity(); mediaRequest.setRequestType(AudioRequestType.PLAYBACK); - + UserAccountEntity userAccount = new UserAccountEntity(); + userAccount.setId(1); + mediaRequest.setCreatedBy(userAccount); + doAnswer(invocation -> invocation.getArgument(0)).when(transformedMediaRepository).save(any()); TransformedMediaEntity transformedMediaEntity = transformedMediaHelper.createTransformedMediaEntity( mediaRequest, "case1_23_Nov_2023.mp3", @@ -552,6 +560,8 @@ void whenCreateTransformMediaEntityIsCalled_thenFilenameFormatSizeShouldBeSet() BINARY_DATA.getLength() ); + assertEquals(userAccount, transformedMediaEntity.getCreatedBy()); + assertEquals(userAccount, transformedMediaEntity.getLastModifiedBy()); assertEquals(TEST_FILE_NAME, transformedMediaEntity.getOutputFilename()); assertEquals(TEST_EXTENSION, transformedMediaEntity.getOutputFormat().getExtension()); assertEquals(TEST_BINARY_STRING.length(), transformedMediaEntity.getOutputFilesize()); From 90c1ad2a688d180cc558f22c8918133b3dd7b9aa Mon Sep 17 00:00:00 2001 From: benedwards Date: Wed, 4 Dec 2024 11:28:51 +0000 Subject: [PATCH 03/16] Applied review comments --- .../gov/hmcts/darts/audio/helper/TransformedMediaHelper.java | 4 ++-- .../service/impl/AudioTransformationServiceImplTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java index a43c465206..e097d40e2f 100644 --- a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java +++ b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java @@ -96,14 +96,14 @@ public TransformedMediaEntity createTransformedMediaEntity(MediaRequestEntity me entity.setOutputFilename(filename); entity.setStartTime(startTime); entity.setEndTime(endTime); - entity.setCreatedBy(mediaRequest.getCreatedBy()); - entity.setLastModifiedBy(mediaRequest.getCreatedBy()); entity.setOutputFormat(audioRequestOutputFormat); if (nonNull(fileSize)) { entity.setOutputFilesize(fileSize.intValue()); } //Ensures createdBy / LastModified does not get overridden by the @CreatedBy / @LastModifiedBy annotaiton TransformedMediaEntity savedTM = transformedMediaRepository.save(entity); + savedTM.setLastModifiedBy(mediaRequest.getCreatedBy()); + savedTM.setCreatedBy(mediaRequest.getCreatedBy()); transformedMediaRepository.updateCreatedByLastModifiedBy(entity.getId(), mediaRequest.getCreatedBy().getId(), mediaRequest.getCreatedBy().getId()); diff --git a/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java index 4bb18dab00..220831dfb5 100644 --- a/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/audio/service/impl/AudioTransformationServiceImplTest.java @@ -178,7 +178,7 @@ void getMediaByHearingIdShouldReturnRepositoryResultsUnmodifiedWhenRepositoryRes } @Test - void saveProcessedDataShouldSaveBlobAndSetStatus() { + void saveProcessedData_shouldSaveBlobAndSetStatus() { final MediaRequestEntity mediaRequestEntity = new MediaRequestEntity(); mediaRequestEntity.setRequestType(DOWNLOAD); UserAccountEntity userAccount = new UserAccountEntity(); From 531a450117bc154fcaa42b3c0d17766d70973d07 Mon Sep 17 00:00:00 2001 From: benedwards Date: Thu, 5 Dec 2024 21:39:29 +0000 Subject: [PATCH 04/16] Fixed typo --- .../uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java index e097d40e2f..7486f950fc 100644 --- a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java +++ b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java @@ -100,7 +100,7 @@ public TransformedMediaEntity createTransformedMediaEntity(MediaRequestEntity me if (nonNull(fileSize)) { entity.setOutputFilesize(fileSize.intValue()); } - //Ensures createdBy / LastModified does not get overridden by the @CreatedBy / @LastModifiedBy annotaiton + //Ensures createdBy / LastModified does not get overridden by the @CreatedBy / @LastModifiedBy annotation TransformedMediaEntity savedTM = transformedMediaRepository.save(entity); savedTM.setLastModifiedBy(mediaRequest.getCreatedBy()); savedTM.setCreatedBy(mediaRequest.getCreatedBy()); From 4cf8620ff63362d0a15c7a2ec0e490b443d22931 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 9 Dec 2024 11:50:41 +0000 Subject: [PATCH 05/16] Added integration test --- .../TransformedMediaRepositoryTest.java | 95 ++++++++++++------- .../testutils/PostgresIntegrationBase.java | 3 + 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java b/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java index 3afe27b017..d8b0ffffff 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java @@ -13,6 +13,8 @@ import java.util.List; import java.util.Locale; +import static org.assertj.core.api.Assertions.assertThat; + class TransformedMediaRepositoryTest extends PostgresIntegrationBase { @Autowired @@ -41,8 +43,8 @@ void closeHibernateSession() { void testFindTransformedMediaWithoutAnyParameters() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, - null, null, null, null, null); + null, null, null, + null, null, null, null, null); Assertions.assertEquals(generatedMediaEntities.size(), transformedMediaEntityList.size()); Assertions.assertTrue(transformedMediaStub.getTransformedMediaIds(transformedMediaEntityList) .containsAll(transformedMediaStub.getTransformedMediaIds(generatedMediaEntities))); @@ -52,8 +54,8 @@ void testFindTransformedMediaWithoutAnyParameters() { void testFindTransformedMediaWithId() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - generatedMediaEntities.get(0).getMediaRequest().getId(), - null, null, null, null, null, null, null); + generatedMediaEntities.get(0).getMediaRequest().getId(), + null, null, null, null, null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(0).getId(), transformedMediaEntityList.size()); } @@ -62,7 +64,7 @@ void testFindTransformedMediaWithId() { void testFindTransformedMediaWithCaseNumber() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, generatedMediaEntities.get(3).getMediaRequest().getHearing().getCourtCase().getCaseNumber(), null, null, null, null, null, null); + null, generatedMediaEntities.get(3).getMediaRequest().getHearing().getCourtCase().getCaseNumber(), null, null, null, null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(transformedMediaEntityList.get(0).getId(), generatedMediaEntities.get(3).getId()); } @@ -72,8 +74,8 @@ void testFindTransformedMediaWithCourtDisplayNameSubstringPrefixMatchOne() { int nameMatchIndex = 13; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, - TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPrefix(Integer.toString(nameMatchIndex)), null, null, null, null, null); + null, null, + TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPrefix(Integer.toString(nameMatchIndex)), null, null, null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); } @@ -95,8 +97,8 @@ void testFindTransformedMediaWithCourtDisplayNameSubstringPostFixMatchOne() { int nameMatchIndex = 13; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, - TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPostfix(Integer.toString(nameMatchIndex)), null, null, null, null, null); + null, null, + TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPostfix(Integer.toString(nameMatchIndex)), null, null, null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); } @@ -117,8 +119,8 @@ void testFindTransformedMediaWithCourtDisplayNameCaseInsensitiveSubstringPostFix void testFindTransformedMediaWithCourtDisplayNameSubstringMatchAll() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, - TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPrefix(), null, null, null, null, null); + null, null, + TransformedMediaSubStringQueryEnum.COURT_HOUSE.getQueryStringPrefix(), null, null, null, null, null); Assertions.assertEquals(GENERATION_COUNT, transformedMediaEntityList.size()); Assertions.assertTrue(transformedMediaStub.getTransformedMediaIds(transformedMediaEntityList) .containsAll(transformedMediaStub.getTransformedMediaIds(generatedMediaEntities))); @@ -128,8 +130,8 @@ void testFindTransformedMediaWithCourtDisplayNameSubstringMatchAll() { void testFindTransformedMediaWithHearingDate() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, - generatedMediaEntities.get(3).getMediaRequest().getHearing().getHearingDate(), null, null, null, null); + null, null, null, + generatedMediaEntities.get(3).getMediaRequest().getHearing().getHearingDate(), null, null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(3).getId(), transformedMediaEntityList.get(0).getId()); } @@ -139,8 +141,8 @@ void testFindTransformedMediaWithOwnerExact() { int nameMatchIndex = 3; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, - TransformedMediaSubStringQueryEnum.OWNER.getQueryString(Integer.toString(nameMatchIndex)), null, null, null); + null, null, null, null, + TransformedMediaSubStringQueryEnum.OWNER.getQueryString(Integer.toString(nameMatchIndex)), null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); } @@ -150,8 +152,8 @@ void testFindTransformedMediaWithOwnerPrefix() { int nameMatchIndex = 13; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, - TransformedMediaSubStringQueryEnum.OWNER.getQueryStringPostfix(Integer.toString(nameMatchIndex)), null, null, null); + null, null, null, null, + TransformedMediaSubStringQueryEnum.OWNER.getQueryStringPostfix(Integer.toString(nameMatchIndex)), null, null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); @@ -196,8 +198,8 @@ void testFindTransformedMediaWithOwnerCaseInsensitivePostfix() { void testFindTransformedMediaWithOwnerSubstringMatchAll() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, - TransformedMediaSubStringQueryEnum.OWNER.getQueryStringPostfix(), null, null, null); + null, null, null, null, + TransformedMediaSubStringQueryEnum.OWNER.getQueryStringPostfix(), null, null, null); Assertions.assertEquals(GENERATION_COUNT, transformedMediaEntityList.size()); Assertions.assertTrue(transformedMediaStub .getTransformedMediaIds(transformedMediaEntityList) @@ -209,8 +211,8 @@ void testFindTransformedMediaWithRequestedByExact() { int nameMatchIndex = 13; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, null, - TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryString(Integer.toString(nameMatchIndex)), null, null); + null, null, null, null, null, + TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryString(Integer.toString(nameMatchIndex)), null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); } @@ -220,8 +222,8 @@ void testFindTransformedMediaWithRequestedByPrefix() { int nameMatchIndex = 13; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, null, - TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryStringPrefix(Integer.toString(13)), null, null); + null, null, null, null, null, + TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryStringPrefix(Integer.toString(13)), null, null); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities.get(nameMatchIndex).getId(), transformedMediaEntityList.get(0).getId()); } @@ -263,8 +265,8 @@ void testFindTransformedMediaWithRequestedByCaseInsensitivePostfix() { void testFindTransformedMediaWithRequestedBySubstringMatchAll() { List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, null, - TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryStringPostfix(), null, null); + null, null, null, null, null, + TransformedMediaSubStringQueryEnum.REQUESTED_BY.getQueryStringPostfix(), null, null); Assertions.assertEquals(GENERATION_COUNT, transformedMediaEntityList.size()); Assertions.assertTrue(transformedMediaStub .getTransformedMediaIds(transformedMediaEntityList) @@ -276,9 +278,9 @@ void testFindTransformedMediaWithRequestedAtFromAndRequestedAtToSameDay() { int fromAtPosition = 3; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, null, null, - generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime(), - generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime()); + null, null, null, null, null, null, + generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime(), + generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime()); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(generatedMediaEntities .get(fromAtPosition).getId(), transformedMediaEntityList.get(0).getId()); @@ -289,8 +291,8 @@ void testFindTransformedMediaWithRequestedAtFrom() { int fromAtPosition = 3; List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia( - null, null, null, null, null, - null, generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime(), null); + null, null, null, null, null, + null, generatedMediaEntities.get(fromAtPosition).getMediaRequest().getCreatedDateTime(), null); Assertions.assertEquals(GENERATION_COUNT - fromAtPosition, transformedMediaEntityList.size()); Assertions.assertTrue(transformedMediaStub .getTransformedMediaIds(transformedMediaEntityList) @@ -314,13 +316,36 @@ void testFindTransformedMediaWithAllQueryParameters() { TransformedMediaEntity transformedMediaEntityFind = generatedMediaEntities.get(1); List transformedMediaEntityList = transformedMediaRepository.findTransformedMedia(transformedMediaEntityFind.getMediaRequest().getId(), transformedMediaEntityFind.getMediaRequest() - .getHearing().getCourtCase().getCaseNumber(), transformedMediaEntityFind.getMediaRequest() - .getHearing().getCourtroom().getCourthouse().getDisplayName(), transformedMediaEntityFind.getMediaRequest() - .getHearing().getHearingDate(), transformedMediaEntityFind.getMediaRequest() - .getCurrentOwner().getUserFullName(), transformedMediaEntityFind.getCreatedBy().getUserFullName(), transformedMediaEntityFind.getMediaRequest() - .getCreatedBy().getCreatedDateTime(), generatedMediaEntities.get(1).getCreatedDateTime()); + .getHearing().getCourtCase().getCaseNumber(), transformedMediaEntityFind.getMediaRequest() + .getHearing().getCourtroom().getCourthouse().getDisplayName(), + transformedMediaEntityFind.getMediaRequest() + .getHearing().getHearingDate(), transformedMediaEntityFind.getMediaRequest() + .getCurrentOwner().getUserFullName(), + transformedMediaEntityFind.getCreatedBy().getUserFullName(), + transformedMediaEntityFind.getMediaRequest() + .getCreatedBy().getCreatedDateTime(), generatedMediaEntities.get(1).getCreatedDateTime()); Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(transformedMediaEntityFind.getId(), transformedMediaEntityList.get(0).getId()); } + + @Test + void updateCreatedByLastModifiedBy_shouldUpdateCreatedByAndLastModified() throws Exception { + TransformedMediaEntity transformedMediaEntityFind = generatedMediaEntities.get(1); + assertThat(transformedMediaEntityFind.getCreatedBy().getId()).isNotEqualTo(1); + assertThat(transformedMediaEntityFind.getLastModifiedBy().getId()).isNotEqualTo(2); + dartsDatabase.getTransactionalUtil().executeInTransaction( + () -> { + transformedMediaRepository.updateCreatedByLastModifiedBy(transformedMediaEntityFind.getId(), 1, 2); + dartsDatabase.getEntityManager().flush(); + dartsDatabase.getEntityManager().clear(); + }); + + dartsDatabase.getTransactionalUtil() + .executeInTransaction(() -> { + TransformedMediaEntity newTransformedMediaEntityFind = transformedMediaRepository.findById(transformedMediaEntityFind.getId()).orElseThrow(); + assertThat(newTransformedMediaEntityFind.getCreatedBy().getId()).isEqualTo(1); + assertThat(newTransformedMediaEntityFind.getLastModifiedBy().getId()).isEqualTo(2); + }); + } } \ No newline at end of file diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java index a06c80b0f8..e63566d24d 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java @@ -14,6 +14,8 @@ import uk.gov.hmcts.darts.testutils.stubs.DartsDatabaseStub; import uk.gov.hmcts.darts.testutils.stubs.DartsPersistence; +import java.util.List; + /** * Base class for integration tests running against a containerized Postgres with Testcontainers. */ @@ -50,6 +52,7 @@ public class PostgresIntegrationBase { ).withDatabaseName("darts") .withUsername("darts") .withPassword("darts"); + POSTGRES.setPortBindings(List.of("5433:5432")); } @DynamicPropertySource From 2a0110ed3378c8b552a6525503022b6724370c26 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 9 Dec 2024 14:28:25 +0000 Subject: [PATCH 06/16] Updated how we update createdBy and LastModifiedBy --- .../TransformedMediaRepositoryTest.java | 23 --- .../audio/helper/TransformedMediaHelper.java | 7 +- .../config/AuthenticationConfiguration.java | 16 -- .../common/entity/base/CreatedBaseEntity.java | 13 +- .../darts/common/entity/base/CreatedBy.java | 15 ++ .../common/entity/base/LastModifiedBy.java | 15 ++ .../base/MandatoryCreatedBaseEntity.java | 14 +- .../MandatoryCreatedModifiedBaseEntity.java | 14 +- .../entity/base/ModifiedBaseEntity.java | 17 +- .../entity/listener/UserAuditListener.java | 75 ++++++++ .../TransformedMediaRepository.java | 11 -- .../listener/UserAuditListenerTest.java | 168 ++++++++++++++++++ 12 files changed, 323 insertions(+), 65 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java create mode 100644 src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java create mode 100644 src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java create mode 100644 src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java b/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java index d8b0ffffff..e89648ef41 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/audio/repository/TransformedMediaRepositoryTest.java @@ -13,8 +13,6 @@ import java.util.List; import java.util.Locale; -import static org.assertj.core.api.Assertions.assertThat; - class TransformedMediaRepositoryTest extends PostgresIntegrationBase { @Autowired @@ -327,25 +325,4 @@ void testFindTransformedMediaWithAllQueryParameters() { Assertions.assertEquals(1, transformedMediaEntityList.size()); Assertions.assertEquals(transformedMediaEntityFind.getId(), transformedMediaEntityList.get(0).getId()); } - - - @Test - void updateCreatedByLastModifiedBy_shouldUpdateCreatedByAndLastModified() throws Exception { - TransformedMediaEntity transformedMediaEntityFind = generatedMediaEntities.get(1); - assertThat(transformedMediaEntityFind.getCreatedBy().getId()).isNotEqualTo(1); - assertThat(transformedMediaEntityFind.getLastModifiedBy().getId()).isNotEqualTo(2); - dartsDatabase.getTransactionalUtil().executeInTransaction( - () -> { - transformedMediaRepository.updateCreatedByLastModifiedBy(transformedMediaEntityFind.getId(), 1, 2); - dartsDatabase.getEntityManager().flush(); - dartsDatabase.getEntityManager().clear(); - }); - - dartsDatabase.getTransactionalUtil() - .executeInTransaction(() -> { - TransformedMediaEntity newTransformedMediaEntityFind = transformedMediaRepository.findById(transformedMediaEntityFind.getId()).orElseThrow(); - assertThat(newTransformedMediaEntityFind.getCreatedBy().getId()).isEqualTo(1); - assertThat(newTransformedMediaEntityFind.getLastModifiedBy().getId()).isEqualTo(2); - }); - } } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java index 7486f950fc..08c28d5fbf 100644 --- a/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java +++ b/src/main/java/uk/gov/hmcts/darts/audio/helper/TransformedMediaHelper.java @@ -96,17 +96,14 @@ public TransformedMediaEntity createTransformedMediaEntity(MediaRequestEntity me entity.setOutputFilename(filename); entity.setStartTime(startTime); entity.setEndTime(endTime); + entity.setLastModifiedBy(mediaRequest.getCreatedBy()); + entity.setCreatedBy(mediaRequest.getCreatedBy()); entity.setOutputFormat(audioRequestOutputFormat); if (nonNull(fileSize)) { entity.setOutputFilesize(fileSize.intValue()); } //Ensures createdBy / LastModified does not get overridden by the @CreatedBy / @LastModifiedBy annotation TransformedMediaEntity savedTM = transformedMediaRepository.save(entity); - savedTM.setLastModifiedBy(mediaRequest.getCreatedBy()); - savedTM.setCreatedBy(mediaRequest.getCreatedBy()); - transformedMediaRepository.updateCreatedByLastModifiedBy(entity.getId(), - mediaRequest.getCreatedBy().getId(), - mediaRequest.getCreatedBy().getId()); return savedTM; } diff --git a/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java b/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java index 8ef64a2aeb..5fab67de3a 100644 --- a/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java +++ b/src/main/java/uk/gov/hmcts/darts/authentication/config/AuthenticationConfiguration.java @@ -2,14 +2,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.domain.AuditorAware; import uk.gov.hmcts.darts.authentication.exception.AuthenticationError; -import uk.gov.hmcts.darts.authorisation.component.UserIdentity; -import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import uk.gov.hmcts.darts.common.exception.DartsApiException; -import java.util.Optional; - @Configuration public class AuthenticationConfiguration { @@ -19,15 +14,4 @@ public AuthConfigFallback getNoAuthConfigurationFallback(DefaultAuthConfiguratio throw new DartsApiException(AuthenticationError.FAILED_TO_OBTAIN_AUTHENTICATION_CONFIG); }; } - - @Bean - public AuditorAware auditorAware(UserIdentity userIdentity) { - return () -> { - try { - return Optional.ofNullable(userIdentity.getUserAccount()); - } catch (Exception e) { - return Optional.empty(); - } - }; - } } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java index 81d6e1989e..e00b8d878f 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.darts.common.entity.base; import jakarta.persistence.Column; +import jakarta.persistence.EntityListeners; import jakarta.persistence.FetchType; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; @@ -9,15 +10,16 @@ import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.envers.NotAudited; -import org.springframework.data.annotation.CreatedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.listener.UserAuditListener; import java.time.OffsetDateTime; @MappedSuperclass @Getter @Setter -public class CreatedBaseEntity { +@EntityListeners(UserAuditListener.class) +public class CreatedBaseEntity implements CreatedBy { @CreationTimestamp @Column(name = "created_ts") private OffsetDateTime createdDateTime; @@ -25,7 +27,12 @@ public class CreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "created_by") - @CreatedBy private UserAccountEntity createdBy; + public void setCreatedBy(UserAccountEntity userAccount) { + this.createdBy = userAccount; + this.skipUserAudit = true;//As this was manualy set we should not override it + } + + private transient boolean skipUserAudit; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java new file mode 100644 index 0000000000..7b52ddcd42 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java @@ -0,0 +1,15 @@ +package uk.gov.hmcts.darts.common.entity.base; + +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; + +import java.time.OffsetDateTime; + +public interface CreatedBy { + UserAccountEntity getCreatedBy(); + + void setCreatedBy(UserAccountEntity userAccount); + + void setCreatedDateTime(OffsetDateTime now); + + boolean isSkipUserAudit(); +} diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java new file mode 100644 index 0000000000..458519749d --- /dev/null +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java @@ -0,0 +1,15 @@ +package uk.gov.hmcts.darts.common.entity.base; + +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; + +import java.time.OffsetDateTime; + +public interface LastModifiedBy { + UserAccountEntity getLastModifiedBy(); + + void setLastModifiedBy(UserAccountEntity userAccount); + + void setLastModifiedDateTime(OffsetDateTime now); + + boolean isSkipUserAudit(); +} diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java index 20f2860efc..cc9ba89aa8 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java @@ -2,6 +2,7 @@ import jakarta.persistence.CascadeType; import jakarta.persistence.Column; +import jakarta.persistence.EntityListeners; import jakarta.persistence.FetchType; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; @@ -10,8 +11,8 @@ import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.envers.NotAudited; -import org.springframework.data.annotation.CreatedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.listener.UserAuditListener; import java.time.OffsetDateTime; import javax.validation.constraints.NotNull; @@ -19,7 +20,8 @@ @MappedSuperclass @Getter @Setter -public class MandatoryCreatedBaseEntity { +@EntityListeners(UserAuditListener.class) +public class MandatoryCreatedBaseEntity implements CreatedBy { @NotNull @CreationTimestamp @@ -30,7 +32,13 @@ public class MandatoryCreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "created_by", nullable = false) - @CreatedBy private UserAccountEntity createdBy; + public void setCreatedBy(UserAccountEntity userAccount) { + this.createdBy = userAccount; + this.skipUserAudit = true;//As this was manualy set we should not override it + } + + protected transient boolean skipUserAudit; + } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java index 035036db50..1b4e68cec7 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java @@ -2,6 +2,7 @@ import jakarta.persistence.CascadeType; import jakarta.persistence.Column; +import jakarta.persistence.EntityListeners; import jakarta.persistence.FetchType; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; @@ -10,15 +11,16 @@ import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; import org.hibernate.envers.NotAudited; -import org.springframework.data.annotation.LastModifiedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.listener.UserAuditListener; import java.time.OffsetDateTime; @MappedSuperclass @Getter @Setter -public class MandatoryCreatedModifiedBaseEntity extends MandatoryCreatedBaseEntity { +@EntityListeners(UserAuditListener.class) +public class MandatoryCreatedModifiedBaseEntity extends MandatoryCreatedBaseEntity implements LastModifiedBy { @UpdateTimestamp @Column(name = "last_modified_ts", nullable = false) @@ -27,6 +29,12 @@ public class MandatoryCreatedModifiedBaseEntity extends MandatoryCreatedBaseEnti @NotAudited @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "last_modified_by", nullable = false) - @LastModifiedBy + @org.springframework.data.annotation.LastModifiedBy private UserAccountEntity lastModifiedBy; + + public void setLastModifiedBy(UserAccountEntity userAccount) { + this.lastModifiedBy = userAccount; + this.skipUserAudit = true;//As this was manualy set we should not override it + } + } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java index 9d97b92ea9..5ab4313303 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.darts.common.entity.base; import jakarta.persistence.Column; +import jakarta.persistence.EntityListeners; import jakarta.persistence.FetchType; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; @@ -9,13 +10,15 @@ import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.listener.UserAuditListener; import java.time.OffsetDateTime; @MappedSuperclass @Getter @Setter -public class ModifiedBaseEntity { +@EntityListeners(UserAuditListener.class) +public class ModifiedBaseEntity implements LastModifiedBy { @UpdateTimestamp @Column(name = "last_modified_ts") @@ -24,4 +27,16 @@ public class ModifiedBaseEntity { @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "last_modified_by") private UserAccountEntity lastModifiedBy; + + @Override + public void setLastModifiedDateTime(OffsetDateTime lastModifiedTimestamp) { + this.lastModifiedTimestamp = lastModifiedTimestamp; + } + + public void setLastModifiedBy(UserAccountEntity userAccount) { + this.lastModifiedBy = userAccount; + this.skipUserAudit = true;//As this was manualy set we should not override it + } + + private transient boolean skipUserAudit; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java new file mode 100644 index 0000000000..798145ecb4 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java @@ -0,0 +1,75 @@ +package uk.gov.hmcts.darts.common.entity.listener; + +import jakarta.persistence.PrePersist; +import jakarta.persistence.PreUpdate; +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import uk.gov.hmcts.darts.authorisation.component.UserIdentity; +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.base.CreatedBy; +import uk.gov.hmcts.darts.common.entity.base.LastModifiedBy; + +import java.time.Clock; +import java.time.OffsetDateTime; +import java.util.Optional; + +@Slf4j +@AllArgsConstructor +public class UserAuditListener { + + private final UserIdentity userIdentity; + private final Clock clock; + + + @PrePersist + void beforeSave(Object object) { + log.debug("Before save: {}", object.getClass().getSimpleName()); + Optional userAccountOpt = getUserAccount(); + if (userAccountOpt.isEmpty()) { + log.debug("Skipping audit as user account not found"); + return; + } + UserAccountEntity userAccount = userAccountOpt.get(); + updateCreatedBy(object, userAccount); + updateModifiedBy(object, userAccount); + } + + @PreUpdate + void beforeUpdate(Object object) { + log.debug("Before update: {}", object.getClass().getSimpleName()); + Optional userAccountOpt = getUserAccount(); + if (userAccountOpt.isEmpty()) { + log.debug("Skipping audit as user account not found"); + return; + } + UserAccountEntity userAccount = userAccountOpt.get(); + updateModifiedBy(object, userAccount); + } + + Optional getUserAccount() { + return Optional.ofNullable(userIdentity.getUserAccount()); + } + + + void updateCreatedBy(Object object, UserAccountEntity userAccount) { + if (object instanceof CreatedBy entity) { + if (entity.isSkipUserAudit() || entity.getCreatedBy() != null) { + log.debug("Skipping audit as isSkipUserAudit is set or createdBy is already set"); + return; + } + entity.setCreatedBy(userAccount); + entity.setCreatedDateTime(OffsetDateTime.now(clock)); + } + } + + void updateModifiedBy(Object object, UserAccountEntity userAccount) { + if (object instanceof LastModifiedBy entity) { + if (entity.isSkipUserAudit()) { + log.debug("Skipping audit as isSkipUserAudit is set"); + return; + } + entity.setLastModifiedBy(userAccount); + entity.setLastModifiedDateTime(OffsetDateTime.now(clock)); + } + } +} diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java index 4799da8677..7a00e38b6e 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/TransformedMediaRepository.java @@ -3,7 +3,6 @@ import org.springframework.data.domain.Limit; import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.stereotype.Repository; import uk.gov.hmcts.darts.audio.model.TransformedMediaDetailsDto; @@ -92,14 +91,4 @@ List findTransformedMedia(Integer mediaId, String requestedBy, OffsetDateTime requestedAtFrom, OffsetDateTime requestedAtTo); - - - @Query(value = """ - UPDATE darts.transformed_media - set created_by = :id1, - last_modified_by = :id2 - where trm_id = :id - """, nativeQuery = true) - @Modifying - void updateCreatedByLastModifiedBy(Integer id, Integer id1, Integer id2); } \ No newline at end of file diff --git a/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java new file mode 100644 index 0000000000..b1de77b980 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java @@ -0,0 +1,168 @@ +package uk.gov.hmcts.darts.common.entity.listener; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import uk.gov.hmcts.darts.authorisation.component.UserIdentity; +import uk.gov.hmcts.darts.common.entity.UserAccountEntity; +import uk.gov.hmcts.darts.common.entity.base.CreatedBy; +import uk.gov.hmcts.darts.common.entity.base.LastModifiedBy; + +import java.time.Clock; +import java.time.Instant; +import java.time.OffsetDateTime; +import java.time.ZoneId; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class UserAuditListenerTest { + + private final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); + + @Mock + private UserIdentity userIdentity; + + private UserAuditListener userAuditListener; + + + @BeforeEach + void beforeEach() { + userAuditListener = spy(new UserAuditListener(userIdentity, clock)); + } + + private UserAccountEntity mockUserAccount() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + when(userIdentity.getUserAccount()).thenReturn(userAccount); + return userAccount; + } + + @Test + void beforeSave_shouldUpdateCreatedByAndModifiedBy_whenUserAccountIsPresent() { + UserAccountEntity userAccount = mockUserAccount(); + doNothing().when(userAuditListener).updateCreatedBy(any(), any()); + doNothing().when(userAuditListener).updateModifiedBy(any(), any()); + + CreatedBy createdBy = mock(CreatedBy.class); + userAuditListener.beforeSave(createdBy); + + verify(userAuditListener).updateCreatedBy(createdBy, userAccount); + verify(userAuditListener).updateModifiedBy(createdBy, userAccount); + verify(userIdentity).getUserAccount(); + } + + @Test + void beforeSave_shouldSkipAudit_whenUserAccountIsNotPresent() { + CreatedBy createdBy = mock(CreatedBy.class); + userAuditListener.beforeSave(createdBy); + verify(userAuditListener, never()).updateCreatedBy(any(), any()); + verify(userAuditListener, never()).updateModifiedBy(any(), any()); + verify(userIdentity).getUserAccount(); + } + + @Test + void beforeUpdate_shouldUpdateModifiedBy_whenUserAccountIsPresent() { + UserAccountEntity userAccount = mockUserAccount(); + doNothing().when(userAuditListener).updateModifiedBy(any(), any()); + + CreatedBy createdBy = mock(CreatedBy.class); + userAuditListener.beforeUpdate(createdBy); + + verify(userAuditListener, never()).updateCreatedBy(any(), any()); + verify(userAuditListener).updateModifiedBy(createdBy, userAccount); + verify(userIdentity).getUserAccount(); + } + + @Test + void beforeUpdate_shouldSkipAudit_whenUserAccountIsNotPresent() { + CreatedBy createdBy = mock(CreatedBy.class); + userAuditListener.beforeUpdate(createdBy); + verify(userAuditListener, never()).updateCreatedBy(any(), any()); + verify(userAuditListener, never()).updateModifiedBy(any(), any()); + verify(userIdentity).getUserAccount(); + } + + @Test + void getUserAccount_shouldReturnUserAccount_whenUserAccountIsPresent() { + UserAccountEntity mockUserAccount = mockUserAccount(); + + Optional userAccount = userAuditListener.getUserAccount(); + assertThat(userAccount.isPresent()).isTrue(); + assertThat(userAccount.get()).isEqualTo(mockUserAccount); + verify(userIdentity).getUserAccount(); + } + + @Test + void getUserAccount_shouldReturnEmpty_whenUserAccountIsNotPresent() { + Optional userAccount = userAuditListener.getUserAccount(); + assertThat(userAccount.isPresent()).isFalse(); + verify(userIdentity).getUserAccount(); + } + + @Test + void updateCreatedBy_shouldUpdateCreatedBy_whenEntityIsCreatedByAndSkipUserAuditIsFalseAndCreatedByIsNull() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + CreatedBy createdBy = mock(CreatedBy.class); + when(createdBy.isSkipUserAudit()).thenReturn(false); + when(createdBy.getCreatedBy()).thenReturn(null); + + userAuditListener.updateCreatedBy(createdBy, userAccount); + verify(createdBy).setCreatedBy(userAccount); + verify(createdBy).setCreatedDateTime(OffsetDateTime.now(clock)); + } + + @Test + void updateCreatedBy_shouldSkipAudit_whenEntityIsCreatedByAndSkipUserAuditIsTrue() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + CreatedBy createdBy = mock(CreatedBy.class); + when(createdBy.isSkipUserAudit()).thenReturn(true); + + userAuditListener.updateCreatedBy(createdBy, userAccount); + verify(createdBy, never()).setCreatedBy(any()); + verify(createdBy, never()).setCreatedDateTime(any()); + } + + @Test + void updateCreatedBy_shouldSkipAudit_whenEntityIsCreatedByAndCreatedByIsNotNull() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + CreatedBy createdBy = mock(CreatedBy.class); + when(createdBy.isSkipUserAudit()).thenReturn(false); + when(createdBy.getCreatedBy()).thenReturn(mock(UserAccountEntity.class)); + + userAuditListener.updateCreatedBy(createdBy, userAccount); + verify(createdBy, never()).setCreatedBy(any()); + verify(createdBy, never()).setCreatedDateTime(any()); + } + + @Test + void updateModifiedBy_shouldUpdateModifiedBy_whenEntityIsLastModifiedByAndSkipUserAuditIsFalse() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + LastModifiedBy lastModifiedBy = mock(LastModifiedBy.class); + when(lastModifiedBy.isSkipUserAudit()).thenReturn(false); + + userAuditListener.updateModifiedBy(lastModifiedBy, userAccount); + verify(lastModifiedBy).setLastModifiedBy(userAccount); + verify(lastModifiedBy).setLastModifiedDateTime(OffsetDateTime.now(clock)); + } + + @Test + void updateModifiedBy_shouldSkipAudit_whenEntityIsLastModifiedByAndSkipUserAuditIsTrue() { + UserAccountEntity userAccount = mock(UserAccountEntity.class); + LastModifiedBy lastModifiedBy = mock(LastModifiedBy.class); + when(lastModifiedBy.isSkipUserAudit()).thenReturn(true); + + userAuditListener.updateModifiedBy(lastModifiedBy, userAccount); + verify(lastModifiedBy, never()).setLastModifiedBy(any()); + verify(lastModifiedBy, never()).setLastModifiedDateTime(any()); + } +} From 7914ce0b636188e38eaa1a9abbefc7c33f638304 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 9 Dec 2024 15:48:30 +0000 Subject: [PATCH 07/16] Fixed circular reference --- .../entity/listener/UserAuditListener.java | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java index 798145ecb4..3916df3bc7 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java @@ -3,7 +3,10 @@ import jakarta.persistence.PrePersist; import jakarta.persistence.PreUpdate; import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; import uk.gov.hmcts.darts.authorisation.component.UserIdentity; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import uk.gov.hmcts.darts.common.entity.base.CreatedBy; @@ -14,11 +17,16 @@ import java.util.Optional; @Slf4j +@NoArgsConstructor @AllArgsConstructor public class UserAuditListener { - private final UserIdentity userIdentity; - private final Clock clock; + @Lazy + @Autowired + private UserIdentity userIdentity; + + @Autowired + private Clock clock; @PrePersist @@ -47,7 +55,12 @@ void beforeUpdate(Object object) { } Optional getUserAccount() { - return Optional.ofNullable(userIdentity.getUserAccount()); + try { + return Optional.ofNullable(userIdentity.getUserAccount()); + } catch (Exception e) { + log.error("Error getting user account", e); + return Optional.empty(); + } } From 02606b74d0d90cba9e4ec62bc23358d83d39c1c7 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 9 Dec 2024 18:17:37 +0000 Subject: [PATCH 08/16] Fixed test --- .../hmcts/darts/testutils/stubs/DartsDatabaseStub.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java index 72c1572cd6..76cbce4f24 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java @@ -668,9 +668,7 @@ public T saveWithTransientEntities(T entity) { Method getIdMethod = fieldValue.getClass().getMethod("getId"); Object id = getIdMethod.invoke(fieldValue); if (id == null || (id instanceof Integer && (Integer) id == 0)) { - // Save the transient entity - entityManager.persist(fieldValue); - entityManager.flush(); + save(fieldValue); } } } @@ -892,8 +890,9 @@ public List findAnnotationsFor(Integer hearingId) { public void addUserToGroup(UserAccountEntity userAccount, SecurityGroupEntity securityGroup) { securityGroup.getUsers().add(userAccount); userAccount.getSecurityGroupEntities().add(securityGroup); - dartsDatabaseSaveStub.save(securityGroup); - dartsDatabaseSaveStub.save(userAccount); + + saveWithTransientEntities(securityGroup); + saveWithTransientEntities(userAccount); } @Transactional From 7f37da877e4d18093b54800eb73b8017291d0a1a Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 14:44:31 +0000 Subject: [PATCH 09/16] Fixed issue with integration tests --- .../stubs/DartsDatabaseSaveStub.java | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java index c3e39dc90c..216242748c 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java @@ -4,6 +4,8 @@ import lombok.AllArgsConstructor; import org.hibernate.proxy.HibernateProxy; import org.junit.platform.commons.JUnitException; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import uk.gov.hmcts.darts.common.entity.CaseManagementRetentionEntity; @@ -26,26 +28,43 @@ public class DartsDatabaseSaveStub { private final EntityManager entityManager; private final TransactionalUtil transactionalUtil; - @Transactional public T save(T entity) { - if (entity == null) { - return null; + if (entity == null + || (entity instanceof HibernateProxy proxy + && proxy.getHibernateLazyInitializer().isUninitialized())) { + return entity; } return transactionalUtil.executeInTransaction(() -> { + Authentication authentication = null; + //Remove the authentication from the context to bypass UserAuditListener. + //This will be added back again at the end of the method. + if (SecurityContextHolder.getContext().getAuthentication() != null) { + authentication = SecurityContextHolder.getContext().getAuthentication(); + SecurityContextHolder.getContext().setAuthentication(null); + } + if (entity instanceof CreatedModifiedBaseEntity createdModifiedBaseEntity) { updateCreatedByLastModifiedBy(createdModifiedBaseEntity); } Method getIdInstanceMethod; try { + this.entityManager.clear(); getIdInstanceMethod = getIdMethod(entity.getClass()); Integer id = (Integer) getIdInstanceMethod.invoke(entity); + T toReturn; if (id == null) { this.entityManager.persist(entity); - return entity; + this.entityManager.flush(); + toReturn = entity; } else { - return this.entityManager.merge(entity); + toReturn = this.entityManager.merge(entity); + this.entityManager.flush(); + } + if (authentication != null) { + SecurityContextHolder.getContext().setAuthentication(authentication); } + return toReturn; } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { throw new JUnitException("Failed to save entity", e); } From c94111ece76ccf63a17bc3f4155446d4cdbeb934 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 14:53:07 +0000 Subject: [PATCH 10/16] Fixed issue with integration tests --- .../common/entity/listener/UserAuditListener.java | 15 +++++++-------- .../entity/listener/UserAuditListenerTest.java | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java index 3916df3bc7..70c8f351c2 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java @@ -2,8 +2,6 @@ import jakarta.persistence.PrePersist; import jakarta.persistence.PreUpdate; -import lombok.AllArgsConstructor; -import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; @@ -17,16 +15,17 @@ import java.util.Optional; @Slf4j -@NoArgsConstructor -@AllArgsConstructor public class UserAuditListener { - @Lazy - @Autowired - private UserIdentity userIdentity; + private final Clock clock; + private final UserIdentity userIdentity; + @Autowired - private Clock clock; + public UserAuditListener(Clock clock, @Lazy UserIdentity userIdentity) { + this.clock = clock; + this.userIdentity = userIdentity; + } @PrePersist diff --git a/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java index b1de77b980..9cfb05cc8a 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java @@ -38,7 +38,7 @@ class UserAuditListenerTest { @BeforeEach void beforeEach() { - userAuditListener = spy(new UserAuditListener(userIdentity, clock)); + userAuditListener = spy(new UserAuditListener(clock, userIdentity)); } private UserAccountEntity mockUserAccount() { From 80aa299023aa1f29b656001ddd671989c1e850f6 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 15:23:38 +0000 Subject: [PATCH 11/16] Test fixes --- .../testutils/stubs/DartsPersistence.java | 81 ++++++++++++------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java index 6d4938e845..932ebd2d0a 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java @@ -6,6 +6,9 @@ import lombok.AllArgsConstructor; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import uk.gov.hmcts.darts.audio.entity.MediaRequestEntity; import uk.gov.hmcts.darts.common.entity.AnnotationDocumentEntity; @@ -154,6 +157,7 @@ public class DartsPersistence { private final EntityManager entityManager; private final CurrentTimeHelper currentTimeHelper; private final TransactionalUtil transactionalUtil; + private final DartsDatabaseSaveStub dartsDatabaseSaveStub; @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") @@ -165,7 +169,7 @@ public HearingEntity save(HearingEntity hearing) { hearing.setCreatedBy(save(hearing.getCreatedBy())); hearing.setLastModifiedBy(save(hearing.getLastModifiedBy())); hearing.setJudges(saveJudgeList(hearing.getJudges())); - hearing = hearingRepository.save(hearing); + hearing = save(hearingRepository, hearing); saveMediaList(hearing.getMediaList()); return hearing; @@ -183,12 +187,27 @@ public AnnotationEntity save(AnnotationEntity annotationEntity) { if (annotationEntity.getHearingList() != null) { annotationEntity.setHearingList(saveHearingEntity(annotationEntity.getHearingList())); } - return annotationRepository.save(annotationEntity); + return save(annotationRepository, annotationEntity); } else { return entityManager.merge(annotationEntity); } } + private T save(JpaRepository repository, T annotationEntity) { + Authentication authentication = null; + //Remove the authentication from the context to bypass UserAuditListener. + //This will be added back again at the end of the method. + if (SecurityContextHolder.getContext().getAuthentication() != null) { + authentication = SecurityContextHolder.getContext().getAuthentication(); + SecurityContextHolder.getContext().setAuthentication(null); + } + T value = repository.saveAndFlush(annotationEntity); + if (authentication != null) { + SecurityContextHolder.getContext().setAuthentication(authentication); + } + return value; + } + @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") public CourthouseEntity save(CourthouseEntity courthouse) { @@ -197,7 +216,7 @@ public CourthouseEntity save(CourthouseEntity courthouse) { if (courthouse.getId() == null) { courthouse.setCreatedBy(save(courthouse.getCreatedBy())); courthouse.setLastModifiedBy(save(courthouse.getLastModifiedBy())); - courthouseRepository.save(courthouse); + save(courthouseRepository, courthouse); } else { courthouse = entityManager.merge(courthouse); } @@ -215,7 +234,7 @@ public CourtroomEntity save(CourtroomEntity courtroom) { courtroom.setCourthouse(save(courtroom.getCourthouse())); } courtroom.setCreatedBy(save(courtroom.getCreatedBy())); - return courtroomRepository.save(courtroom); + return save(courtroomRepository, courtroom); } else { return entityManager.merge(courtroom); } @@ -233,7 +252,7 @@ public MediaRequestEntity save(MediaRequestEntity mediaRequest) { mediaRequest.setCreatedBy(save(mediaRequest.getCreatedBy())); mediaRequest.setLastModifiedBy(save(mediaRequest.getLastModifiedBy())); - return mediaRequestRepository.save(mediaRequest); + return save(mediaRequestRepository, mediaRequest); } else { return entityManager.merge(mediaRequest); } @@ -246,7 +265,7 @@ public MediaLinkedCaseEntity save(MediaLinkedCaseEntity mediaLinkedCaseEntity) { if (mediaLinkedCaseEntity.getId() == null) { mediaLinkedCaseEntity.setCourtCase(save(mediaLinkedCaseEntity.getCourtCase())); - return mediaLinkedCaseRepository.save(mediaLinkedCaseEntity); + return save(mediaLinkedCaseRepository, mediaLinkedCaseEntity); } else { return entityManager.merge(mediaLinkedCaseEntity); } @@ -259,7 +278,7 @@ public EventHandlerEntity save(EventHandlerEntity eventHandlerEntity) { if (eventHandlerEntity.getId() == null) { eventHandlerEntity.setCreatedBy(save(eventHandlerEntity.getCreatedBy())); - return eventHandlerRepository.save(eventHandlerEntity); + return save(eventHandlerRepository, eventHandlerEntity); } else { return entityManager.merge(eventHandlerEntity); } @@ -295,7 +314,7 @@ public CourtCaseEntity save(CourtCaseEntity courtCase) { courtCase.setProsecutorList(saveProsecutorList(courtCase.getProsecutorList())); } - courtCase = caseRepository.save(courtCase); + courtCase = save(caseRepository, courtCase); } else { courtCase = entityManager.merge(courtCase); @@ -317,7 +336,7 @@ public TranscriptionDocumentEntity save(TranscriptionDocumentEntity transcriptio transcriptionDocumentEntity.getAdminActions().forEach(this::save); } - return transcriptionDocumentRepository.save(transcriptionDocumentEntity); + return save(transcriptionDocumentRepository, transcriptionDocumentEntity); } else { return entityManager.merge(transcriptionDocumentEntity); } @@ -326,7 +345,7 @@ public TranscriptionDocumentEntity save(TranscriptionDocumentEntity transcriptio @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") public ObjectAdminActionEntity save(ObjectAdminActionEntity adminAction) { - return objectAdminActionRepository.save(adminAction); + return save(objectAdminActionRepository, adminAction); } @@ -355,7 +374,7 @@ public ExternalObjectDirectoryEntity save(ExternalObjectDirectoryEntity eod) { eod.setCreatedBy(save(eod.getCreatedBy())); eod.setLastModifiedBy(save(eod.getLastModifiedBy())); - return externalObjectDirectoryRepository.save(eod); + return save(externalObjectDirectoryRepository, eod); } else { return entityManager.merge(eod); } @@ -370,7 +389,7 @@ public DefenceEntity save(DefenceEntity defence) { defence.setCreatedBy(save(defence.getCreatedBy())); defence.setLastModifiedBy(save(defence.getLastModifiedBy())); defence.setCourtCase(save(defence.getCourtCase())); - return defenceRepository.save(defence); + return save(defenceRepository, defence); } else { return entityManager.merge(defence); } @@ -385,7 +404,7 @@ public DefendantEntity save(DefendantEntity defendant) { defendant.setCreatedBy(save(defendant.getCreatedBy())); defendant.setLastModifiedBy(save(defendant.getLastModifiedBy())); defendant.setCourtCase(save(defendant.getCourtCase())); - return defendantRepository.save(defendant); + return save(defendantRepository, defendant); } else { return entityManager.merge(defendant); } @@ -400,7 +419,7 @@ public ProsecutorEntity save(ProsecutorEntity prosecutor) { prosecutor.setCreatedBy(save(prosecutor.getCreatedBy())); prosecutor.setLastModifiedBy(save(prosecutor.getCreatedBy())); prosecutor.setCourtCase(save(prosecutor.getCourtCase())); - return prosecutorRepository.save(prosecutor); + return save(prosecutorRepository, prosecutor); } else { return entityManager.merge(prosecutor); } @@ -430,7 +449,7 @@ public TranscriptionEntity save(TranscriptionEntity transcription) { transcription.setCreatedBy(save(transcription.getCreatedBy())); transcription.setLastModifiedBy(save(transcription.getLastModifiedBy())); - transcription = transcriptionRepository.save(transcription); + transcription = save(transcriptionRepository, transcription); } else { transcription = entityManager.merge(transcription); @@ -456,7 +475,7 @@ public TranscriptionWorkflowEntity save(TranscriptionWorkflowEntity workflowEnti workflowEntity.setTranscription(save(workflowEntity.getTranscription())); workflowEntity.setWorkflowActor(save(workflowEntity.getWorkflowActor())); - return transcriptionWorkflowRepository.save(workflowEntity); + return save(transcriptionWorkflowRepository, workflowEntity); } else { return entityManager.merge(workflowEntity); } @@ -471,7 +490,7 @@ public EventEntity save(EventEntity event) { event.setCourtroom(save(event.getCourtroom())); event.setCreatedBy(save(event.getCreatedBy())); event.setLastModifiedBy(save(event.getLastModifiedBy())); - return eventRepository.save(event); + return save(eventRepository, event); } else { return entityManager.merge(event); } @@ -485,7 +504,7 @@ public RetentionPolicyTypeEntity save(RetentionPolicyTypeEntity retentionPolicyT if (retentionPolicyType.getId() == null) { retentionPolicyType.setLastModifiedBy(save(retentionPolicyType.getLastModifiedBy())); retentionPolicyType.setCreatedBy(save(retentionPolicyType.getCreatedBy())); - return retentionPolicyTypeRepository.save(retentionPolicyType); + return save(retentionPolicyTypeRepository, retentionPolicyType); } else { return entityManager.merge(retentionPolicyType); } @@ -500,7 +519,7 @@ public CaseManagementRetentionEntity save(CaseManagementRetentionEntity caseMana caseManagementRetention.setCourtCase(save(caseManagementRetention.getCourtCase())); caseManagementRetention.setEventEntity(save(caseManagementRetention.getEventEntity())); caseManagementRetention.setRetentionPolicyTypeEntity(save(caseManagementRetention.getRetentionPolicyTypeEntity())); - return caseManagementRetentionRepository.save(caseManagementRetention); + return save(caseManagementRetentionRepository, caseManagementRetention); } else { return entityManager.merge(caseManagementRetention); } @@ -515,7 +534,7 @@ public UserAccountEntity save(UserAccountEntity userAccount) { UserAccountEntity systemUser = userAccountRepository.getReferenceById(0); userAccount.setCreatedBy(systemUser); userAccount.setLastModifiedBy(systemUser); - return userAccountRepository.save(userAccount); + return save(userAccountRepository, userAccount); } else { return entityManager.merge(userAccount); } @@ -531,7 +550,7 @@ public AnnotationDocumentEntity save(AnnotationDocumentEntity annotationDocument annotationDocument.setLastModifiedBy(save(annotationDocument.getLastModifiedBy())); annotationDocument.setUploadedBy(save(annotationDocument.getUploadedBy())); save(annotationDocument.getAnnotation()); - return annotationDocumentRepository.save(annotationDocument); + return save(annotationDocumentRepository, annotationDocument); } else { return entityManager.merge(annotationDocument); } @@ -546,7 +565,7 @@ public JudgeEntity save(JudgeEntity judge) { judge.setCreatedBy(save(judge.getCreatedBy())); judge.setLastModifiedBy(save(judge.getLastModifiedBy())); - return judgeRepository.save(judge); + return save(judgeRepository, judge); } else { return entityManager.merge(judge); } @@ -569,7 +588,7 @@ public CaseDocumentEntity save(CaseDocumentEntity caseDocumentEntity) { caseDocumentEntity.setCreatedBy(save(caseDocumentEntity.getCreatedBy())); caseDocumentEntity.setLastModifiedBy(save(caseDocumentEntity.getLastModifiedBy())); caseDocumentEntity.setCourtCase(save(caseDocumentEntity.getCourtCase())); - return caseDocumentRepository.save(caseDocumentEntity); + return save(caseDocumentRepository, caseDocumentEntity); } else { return entityManager.merge(caseDocumentEntity); } @@ -589,9 +608,9 @@ public MediaEntity save(MediaEntity media, boolean processhHearing) { media.setMediaLinkedCaseList(saveLinkedCaseList(media.getMediaLinkedCaseList())); } - media = mediaRepository.save(media); + media = save(mediaRepository, media); } else { - media = mediaRepository.save(media); + media = save(mediaRepository, media); } return media; } @@ -605,7 +624,7 @@ public TransformedMediaEntity save(TransformedMediaEntity transformedMedia) { transformedMedia.setCreatedBy(save(transformedMedia.getCreatedBy())); transformedMedia.setLastModifiedBy(save(transformedMedia.getLastModifiedBy())); transformedMedia.setMediaRequest(save(transformedMedia.getMediaRequest())); - return transformedMediaRepository.save(transformedMedia); + return save(transformedMediaRepository, transformedMedia); } else { entityManager.merge(transformedMedia); } @@ -622,7 +641,7 @@ public TranscriptionCommentEntity save(TranscriptionCommentEntity commentEntity) commentEntity.setCreatedBy(save(commentEntity.getCreatedBy())); commentEntity.setLastModifiedBy(save(commentEntity.getLastModifiedBy())); commentEntity.setTranscription(save(commentEntity.getTranscription())); - return transcriptionCommentRepository.save(commentEntity); + return save(transcriptionCommentRepository, commentEntity); } else { entityManager.merge(commentEntity); } @@ -638,7 +657,7 @@ public ArmRpoExecutionDetailEntity save(ArmRpoExecutionDetailEntity armRpoExecut if (armRpoExecutionDetailEntity.getId() == null) { armRpoExecutionDetailEntity.setCreatedBy(save(armRpoExecutionDetailEntity.getCreatedBy())); armRpoExecutionDetailEntity.setLastModifiedBy(save(armRpoExecutionDetailEntity.getLastModifiedBy())); - return armRpoExecutionDetailRepository.save(armRpoExecutionDetailEntity); + return save(armRpoExecutionDetailRepository, armRpoExecutionDetailEntity); } else { entityManager.merge(armRpoExecutionDetailEntity); } @@ -652,7 +671,7 @@ public ArmRpoStateEntity save(ArmRpoStateEntity armRpoStateEntity) { armRpoStateEntity = (ArmRpoStateEntity) preCheckPersist(armRpoStateEntity); if (armRpoStateEntity.getId() == null) { - return armRpoStateRepository.save(armRpoStateEntity); + return save(armRpoStateRepository, armRpoStateEntity); } else { return entityManager.merge(armRpoStateEntity); } @@ -664,7 +683,7 @@ public ArmRpoStatusEntity save(ArmRpoStatusEntity armRpoStatusEntity) { armRpoStatusEntity = (ArmRpoStatusEntity) preCheckPersist(armRpoStatusEntity); if (armRpoStatusEntity.getId() == null) { - return armRpoStatusRepository.save(armRpoStatusEntity); + return save(armRpoStatusRepository, armRpoStatusEntity); } else { return entityManager.merge(armRpoStatusEntity); } @@ -679,8 +698,8 @@ public void saveAll(UserAccountEntity... userAccounts) { UserAccountEntity systemUser = userAccountRepository.getReferenceById(0); user.setCreatedBy(systemUser); user.setLastModifiedBy(systemUser); + save(userAccountRepository, user); }); - userAccountRepository.saveAll(asList(userAccounts)); } @Transactional From 6aed3901ac83ec437549cd0a5f84e0d96e2dacfa Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 15:38:58 +0000 Subject: [PATCH 12/16] Fixed style --- .../uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java index 932ebd2d0a..a9a7f97f55 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java @@ -93,7 +93,6 @@ import java.util.ArrayList; import java.util.List; -import static java.util.Arrays.asList; import static java.util.Arrays.stream; import static java.util.Objects.isNull; From ff6d5a630c246f8171c99b10f49c6e98e77af68b Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 18:03:08 +0000 Subject: [PATCH 13/16] Disabled beforeUpdate to check tests --- .../component/impl/UserIdentityImpl.java | 33 ++++++++------ .../common/entity/base/CreatedBaseEntity.java | 4 +- .../darts/common/entity/base/CreatedBy.java | 2 + .../base/CreatedModifiedBaseEntity.java | 2 - .../common/entity/base/LastModifiedBy.java | 2 + .../base/MandatoryCreatedBaseEntity.java | 4 +- .../MandatoryCreatedModifiedBaseEntity.java | 1 - .../entity/base/ModifiedBaseEntity.java | 4 +- .../entity/listener/UserAuditListener.java | 44 +++++++++++++------ 9 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java b/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java index e4bbe5e511..013eb5500c 100644 --- a/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java @@ -31,7 +31,7 @@ public class UserIdentityImpl implements UserIdentity { private final UserAccountRepository userAccountRepository; private final UserRolesCourthousesRepository userRolesCourthousesRepository; - private String getGuidFromToken(Jwt token) { + public String getGuidFromToken(Jwt token) { if (token != null) { Object oid = token.getClaims().get(OID); if (nonNull(oid) && oid instanceof String guid && StringUtils.isNotBlank(guid)) { @@ -47,22 +47,27 @@ public UserAccountEntity getUserAccount() { @Override public UserAccountEntity getUserAccount(Jwt jwt) { - UserAccountEntity userAccount = null; - String guid = getGuidFromToken(jwt); - if (nonNull(guid)) { - // System users will use GUID not email address - userAccount = userAccountRepository.findByAccountGuidAndActive(guid, true).orElse(null); - } - if (isNull(userAccount)) { - String emailAddressFromToken = EmailAddressFromTokenUtil.getEmailAddressFromToken(jwt); - userAccount = userAccountRepository.findByEmailAddressIgnoreCaseAndActive(emailAddressFromToken, true).stream() - .findFirst() - .orElseThrow(() -> new DartsApiException(USER_DETAILS_INVALID)); + try { + UserAccountEntity userAccount = null; + String guid = getGuidFromToken(jwt); + if (nonNull(guid)) { + // System users will use GUID not email address + userAccount = userAccountRepository.findByAccountGuidAndActive(guid, true).orElse(null); + } + if (isNull(userAccount)) { + String emailAddressFromToken = EmailAddressFromTokenUtil.getEmailAddressFromToken(jwt); + userAccount = userAccountRepository.findByEmailAddressIgnoreCaseAndActive(emailAddressFromToken, true).stream() + .findFirst() + .orElseThrow(() -> new DartsApiException(USER_DETAILS_INVALID)); + } + return userAccount; + } catch (Throwable t) { + log.error("Error in getUserAccount", t); } - return userAccount; + return null; } - private Jwt getJwt() { + public Jwt getJwt() { if (SecurityContextHolder.getContext().getAuthentication() != null) { if (SecurityContextHolder.getContext().getAuthentication().getPrincipal() instanceof Jwt jwt) { return jwt; diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java index e00b8d878f..f9e05325f6 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBaseEntity.java @@ -6,6 +6,7 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.Transient; import lombok.Getter; import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; @@ -34,5 +35,6 @@ public void setCreatedBy(UserAccountEntity userAccount) { this.skipUserAudit = true;//As this was manualy set we should not override it } - private transient boolean skipUserAudit; + @Transient + private transient boolean skipUserAudit = true; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java index 7b52ddcd42..bd5970eca9 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedBy.java @@ -12,4 +12,6 @@ public interface CreatedBy { void setCreatedDateTime(OffsetDateTime now); boolean isSkipUserAudit(); + + void setSkipUserAudit(boolean value); } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java index 1269f485f6..8eaff96393 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/CreatedModifiedBaseEntity.java @@ -9,7 +9,6 @@ import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; import org.hibernate.envers.NotAudited; -import org.springframework.data.annotation.LastModifiedBy; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import java.time.OffsetDateTime; @@ -26,6 +25,5 @@ public class CreatedModifiedBaseEntity extends CreatedBaseEntity { @NotAudited @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "last_modified_by") - @LastModifiedBy private UserAccountEntity lastModifiedBy; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java index 458519749d..0ca625ccfe 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/LastModifiedBy.java @@ -12,4 +12,6 @@ public interface LastModifiedBy { void setLastModifiedDateTime(OffsetDateTime now); boolean isSkipUserAudit(); + + void setSkipUserAudit(boolean value); } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java index cc9ba89aa8..61918027d0 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedBaseEntity.java @@ -7,6 +7,7 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.Transient; import lombok.Getter; import lombok.Setter; import org.hibernate.annotations.CreationTimestamp; @@ -39,6 +40,7 @@ public void setCreatedBy(UserAccountEntity userAccount) { this.skipUserAudit = true;//As this was manualy set we should not override it } - protected transient boolean skipUserAudit; + @Transient + protected transient boolean skipUserAudit = true; } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java index 1b4e68cec7..cc61f201c5 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/MandatoryCreatedModifiedBaseEntity.java @@ -29,7 +29,6 @@ public class MandatoryCreatedModifiedBaseEntity extends MandatoryCreatedBaseEnti @NotAudited @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) @JoinColumn(name = "last_modified_by", nullable = false) - @org.springframework.data.annotation.LastModifiedBy private UserAccountEntity lastModifiedBy; public void setLastModifiedBy(UserAccountEntity userAccount) { diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java b/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java index 5ab4313303..d4194d8bad 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/base/ModifiedBaseEntity.java @@ -6,6 +6,7 @@ import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.MappedSuperclass; +import jakarta.persistence.Transient; import lombok.Getter; import lombok.Setter; import org.hibernate.annotations.UpdateTimestamp; @@ -38,5 +39,6 @@ public void setLastModifiedBy(UserAccountEntity userAccount) { this.skipUserAudit = true;//As this was manualy set we should not override it } - private transient boolean skipUserAudit; + @Transient + private transient boolean skipUserAudit = true; } diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java index 70c8f351c2..b081a0b68e 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java @@ -1,10 +1,11 @@ package uk.gov.hmcts.darts.common.entity.listener; +import jakarta.persistence.PostLoad; import jakarta.persistence.PrePersist; -import jakarta.persistence.PreUpdate; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; +import org.springframework.security.core.context.SecurityContextHolder; import uk.gov.hmcts.darts.authorisation.component.UserIdentity; import uk.gov.hmcts.darts.common.entity.UserAccountEntity; import uk.gov.hmcts.darts.common.entity.base.CreatedBy; @@ -33,7 +34,7 @@ void beforeSave(Object object) { log.debug("Before save: {}", object.getClass().getSimpleName()); Optional userAccountOpt = getUserAccount(); if (userAccountOpt.isEmpty()) { - log.debug("Skipping audit as user account not found"); + log.debug("Before save: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); return; } UserAccountEntity userAccount = userAccountOpt.get(); @@ -41,19 +42,34 @@ void beforeSave(Object object) { updateModifiedBy(object, userAccount); } - @PreUpdate - void beforeUpdate(Object object) { - log.debug("Before update: {}", object.getClass().getSimpleName()); - Optional userAccountOpt = getUserAccount(); - if (userAccountOpt.isEmpty()) { - log.debug("Skipping audit as user account not found"); - return; + @PostLoad + void postLoad(Object object) { + if (object instanceof CreatedBy entity) { + entity.setSkipUserAudit(false); + } + if (object instanceof LastModifiedBy entity) { + entity.setSkipUserAudit(false); } - UserAccountEntity userAccount = userAccountOpt.get(); - updateModifiedBy(object, userAccount); } +// @PreUpdate +// void beforeUpdate(Object object) { +// log.debug("Before update: {}", object.getClass().getSimpleName()); +// Optional userAccountOpt = getUserAccount(); +// if (userAccountOpt.isEmpty()) { +// log.debug("Before update: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); +// return; +// } +// UserAccountEntity userAccount = userAccountOpt.get(); +// +// updateModifiedBy(object, userAccount); +// } + Optional getUserAccount() { + if (SecurityContextHolder.getContext().getAuthentication() == null + || SecurityContextHolder.getContext().getAuthentication().getPrincipal() == null) { + return Optional.empty(); + } try { return Optional.ofNullable(userIdentity.getUserAccount()); } catch (Exception e) { @@ -62,7 +78,6 @@ Optional getUserAccount() { } } - void updateCreatedBy(Object object, UserAccountEntity userAccount) { if (object instanceof CreatedBy entity) { if (entity.isSkipUserAudit() || entity.getCreatedBy() != null) { @@ -74,14 +89,17 @@ void updateCreatedBy(Object object, UserAccountEntity userAccount) { } } + void updateModifiedBy(Object object, UserAccountEntity userAccount) { if (object instanceof LastModifiedBy entity) { if (entity.isSkipUserAudit()) { log.debug("Skipping audit as isSkipUserAudit is set"); return; } - entity.setLastModifiedBy(userAccount); entity.setLastModifiedDateTime(OffsetDateTime.now(clock)); + if (!userAccount.getId().equals(entity.getLastModifiedBy().getId())) { + entity.setLastModifiedBy(userAccount); + } } } } From 66589bdb19a01dc677f2ca636780e75c20c70b83 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 18:16:46 +0000 Subject: [PATCH 14/16] Tmp changes to verify integration tests --- .../authorisation/component/UserIdentity.java | 2 + .../entity/listener/UserAuditListener.java | 59 +++++++++---------- .../listener/UserAuditListenerTest.java | 57 ++++++------------ 3 files changed, 48 insertions(+), 70 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/darts/authorisation/component/UserIdentity.java b/src/main/java/uk/gov/hmcts/darts/authorisation/component/UserIdentity.java index 9b8903cabe..ed37bd9c28 100644 --- a/src/main/java/uk/gov/hmcts/darts/authorisation/component/UserIdentity.java +++ b/src/main/java/uk/gov/hmcts/darts/authorisation/component/UserIdentity.java @@ -16,4 +16,6 @@ public interface UserIdentity { boolean userHasGlobalAccess(Set globalAccessRolest); List getListOfCourthouseIdsUserHasAccessTo(); + + Jwt getJwt(); } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java index b081a0b68e..e8873d1508 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java +++ b/src/main/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListener.java @@ -2,6 +2,7 @@ import jakarta.persistence.PostLoad; import jakarta.persistence.PrePersist; +import jakarta.persistence.PreUpdate; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; @@ -32,14 +33,14 @@ public UserAuditListener(Clock clock, @Lazy UserIdentity userIdentity) { @PrePersist void beforeSave(Object object) { log.debug("Before save: {}", object.getClass().getSimpleName()); - Optional userAccountOpt = getUserAccount(); - if (userAccountOpt.isEmpty()) { - log.debug("Before save: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); - return; - } - UserAccountEntity userAccount = userAccountOpt.get(); - updateCreatedBy(object, userAccount); - updateModifiedBy(object, userAccount); + updateCreatedBy(object); + updateModifiedBy(object); + } + + @PreUpdate + void beforeUpdate(Object object) { + log.debug("Before update: {}", object.getClass().getSimpleName()); + updateModifiedBy(object); } @PostLoad @@ -52,24 +53,7 @@ void postLoad(Object object) { } } -// @PreUpdate -// void beforeUpdate(Object object) { -// log.debug("Before update: {}", object.getClass().getSimpleName()); -// Optional userAccountOpt = getUserAccount(); -// if (userAccountOpt.isEmpty()) { -// log.debug("Before update: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); -// return; -// } -// UserAccountEntity userAccount = userAccountOpt.get(); -// -// updateModifiedBy(object, userAccount); -// } - Optional getUserAccount() { - if (SecurityContextHolder.getContext().getAuthentication() == null - || SecurityContextHolder.getContext().getAuthentication().getPrincipal() == null) { - return Optional.empty(); - } try { return Optional.ofNullable(userIdentity.getUserAccount()); } catch (Exception e) { @@ -78,28 +62,43 @@ Optional getUserAccount() { } } - void updateCreatedBy(Object object, UserAccountEntity userAccount) { + boolean isUserAccountPresent() { + return SecurityContextHolder.getContext().getAuthentication() != null; + } + + void updateCreatedBy(Object object) { if (object instanceof CreatedBy entity) { if (entity.isSkipUserAudit() || entity.getCreatedBy() != null) { log.debug("Skipping audit as isSkipUserAudit is set or createdBy is already set"); return; } + Optional userAccountOpt = getUserAccount(); + if (userAccountOpt.isEmpty()) { + log.debug("Before save: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); + return; + } + UserAccountEntity userAccount = userAccountOpt.get(); entity.setCreatedBy(userAccount); entity.setCreatedDateTime(OffsetDateTime.now(clock)); } } - void updateModifiedBy(Object object, UserAccountEntity userAccount) { + void updateModifiedBy(Object object) { if (object instanceof LastModifiedBy entity) { if (entity.isSkipUserAudit()) { log.debug("Skipping audit as isSkipUserAudit is set"); return; } - entity.setLastModifiedDateTime(OffsetDateTime.now(clock)); - if (!userAccount.getId().equals(entity.getLastModifiedBy().getId())) { - entity.setLastModifiedBy(userAccount); + + Optional userAccountOpt = getUserAccount(); + if (userAccountOpt.isEmpty()) { + log.debug("Before update: {} - Skipping audit as user account not found", object.getClass().getSimpleName()); + return; } + UserAccountEntity userAccount = userAccountOpt.get(); + entity.setLastModifiedDateTime(OffsetDateTime.now(clock)); + entity.setLastModifiedBy(userAccount); } } } diff --git a/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java index 9cfb05cc8a..4164de9773 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/entity/listener/UserAuditListenerTest.java @@ -28,6 +28,7 @@ @ExtendWith(MockitoExtension.class) class UserAuditListenerTest { + //TODO finish testing private final Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); @Mock @@ -48,49 +49,28 @@ private UserAccountEntity mockUserAccount() { } @Test - void beforeSave_shouldUpdateCreatedByAndModifiedBy_whenUserAccountIsPresent() { - UserAccountEntity userAccount = mockUserAccount(); - doNothing().when(userAuditListener).updateCreatedBy(any(), any()); - doNothing().when(userAuditListener).updateModifiedBy(any(), any()); + void beforeSave_shouldUpdateCreatedByAndModifiedBy() { + doNothing().when(userAuditListener).updateCreatedBy(any()); + doNothing().when(userAuditListener).updateModifiedBy(any()); CreatedBy createdBy = mock(CreatedBy.class); userAuditListener.beforeSave(createdBy); - verify(userAuditListener).updateCreatedBy(createdBy, userAccount); - verify(userAuditListener).updateModifiedBy(createdBy, userAccount); - verify(userIdentity).getUserAccount(); + verify(userAuditListener).updateCreatedBy(createdBy); + verify(userAuditListener).updateModifiedBy(createdBy); } @Test - void beforeSave_shouldSkipAudit_whenUserAccountIsNotPresent() { - CreatedBy createdBy = mock(CreatedBy.class); - userAuditListener.beforeSave(createdBy); - verify(userAuditListener, never()).updateCreatedBy(any(), any()); - verify(userAuditListener, never()).updateModifiedBy(any(), any()); - verify(userIdentity).getUserAccount(); - } - - @Test - void beforeUpdate_shouldUpdateModifiedBy_whenUserAccountIsPresent() { - UserAccountEntity userAccount = mockUserAccount(); - doNothing().when(userAuditListener).updateModifiedBy(any(), any()); + void beforeUpdate_shouldUpdateModifiedBy() { + doNothing().when(userAuditListener).updateModifiedBy(any()); CreatedBy createdBy = mock(CreatedBy.class); userAuditListener.beforeUpdate(createdBy); - verify(userAuditListener, never()).updateCreatedBy(any(), any()); - verify(userAuditListener).updateModifiedBy(createdBy, userAccount); - verify(userIdentity).getUserAccount(); + verify(userAuditListener, never()).updateCreatedBy(any()); + verify(userAuditListener).updateModifiedBy(createdBy); } - @Test - void beforeUpdate_shouldSkipAudit_whenUserAccountIsNotPresent() { - CreatedBy createdBy = mock(CreatedBy.class); - userAuditListener.beforeUpdate(createdBy); - verify(userAuditListener, never()).updateCreatedBy(any(), any()); - verify(userAuditListener, never()).updateModifiedBy(any(), any()); - verify(userIdentity).getUserAccount(); - } @Test void getUserAccount_shouldReturnUserAccount_whenUserAccountIsPresent() { @@ -111,57 +91,54 @@ void getUserAccount_shouldReturnEmpty_whenUserAccountIsNotPresent() { @Test void updateCreatedBy_shouldUpdateCreatedBy_whenEntityIsCreatedByAndSkipUserAuditIsFalseAndCreatedByIsNull() { - UserAccountEntity userAccount = mock(UserAccountEntity.class); + UserAccountEntity userAccount = mockUserAccount(); CreatedBy createdBy = mock(CreatedBy.class); when(createdBy.isSkipUserAudit()).thenReturn(false); when(createdBy.getCreatedBy()).thenReturn(null); - userAuditListener.updateCreatedBy(createdBy, userAccount); + userAuditListener.updateCreatedBy(createdBy); verify(createdBy).setCreatedBy(userAccount); verify(createdBy).setCreatedDateTime(OffsetDateTime.now(clock)); } @Test void updateCreatedBy_shouldSkipAudit_whenEntityIsCreatedByAndSkipUserAuditIsTrue() { - UserAccountEntity userAccount = mock(UserAccountEntity.class); CreatedBy createdBy = mock(CreatedBy.class); when(createdBy.isSkipUserAudit()).thenReturn(true); - userAuditListener.updateCreatedBy(createdBy, userAccount); + userAuditListener.updateCreatedBy(createdBy); verify(createdBy, never()).setCreatedBy(any()); verify(createdBy, never()).setCreatedDateTime(any()); } @Test void updateCreatedBy_shouldSkipAudit_whenEntityIsCreatedByAndCreatedByIsNotNull() { - UserAccountEntity userAccount = mock(UserAccountEntity.class); CreatedBy createdBy = mock(CreatedBy.class); when(createdBy.isSkipUserAudit()).thenReturn(false); when(createdBy.getCreatedBy()).thenReturn(mock(UserAccountEntity.class)); - userAuditListener.updateCreatedBy(createdBy, userAccount); + userAuditListener.updateCreatedBy(createdBy); verify(createdBy, never()).setCreatedBy(any()); verify(createdBy, never()).setCreatedDateTime(any()); } @Test void updateModifiedBy_shouldUpdateModifiedBy_whenEntityIsLastModifiedByAndSkipUserAuditIsFalse() { - UserAccountEntity userAccount = mock(UserAccountEntity.class); + UserAccountEntity userAccount = mockUserAccount(); LastModifiedBy lastModifiedBy = mock(LastModifiedBy.class); when(lastModifiedBy.isSkipUserAudit()).thenReturn(false); - userAuditListener.updateModifiedBy(lastModifiedBy, userAccount); + userAuditListener.updateModifiedBy(lastModifiedBy); verify(lastModifiedBy).setLastModifiedBy(userAccount); verify(lastModifiedBy).setLastModifiedDateTime(OffsetDateTime.now(clock)); } @Test void updateModifiedBy_shouldSkipAudit_whenEntityIsLastModifiedByAndSkipUserAuditIsTrue() { - UserAccountEntity userAccount = mock(UserAccountEntity.class); LastModifiedBy lastModifiedBy = mock(LastModifiedBy.class); when(lastModifiedBy.isSkipUserAudit()).thenReturn(true); - userAuditListener.updateModifiedBy(lastModifiedBy, userAccount); + userAuditListener.updateModifiedBy(lastModifiedBy); verify(lastModifiedBy, never()).setLastModifiedBy(any()); verify(lastModifiedBy, never()).setLastModifiedDateTime(any()); } From 17bd66144325a44bf9238f7aa10f8fae2e99651e Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 19:04:14 +0000 Subject: [PATCH 15/16] Tmp changes to verify integration tests --- .../stubs/DartsDatabaseSaveStub.java | 1 - .../component/impl/UserIdentityImpl.java | 31 ++++++++----------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java index 216242748c..a82584d80c 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java @@ -49,7 +49,6 @@ public T save(T entity) { } Method getIdInstanceMethod; try { - this.entityManager.clear(); getIdInstanceMethod = getIdMethod(entity.getClass()); Integer id = (Integer) getIdInstanceMethod.invoke(entity); T toReturn; diff --git a/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java b/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java index 013eb5500c..d60a3e0aa2 100644 --- a/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/authorisation/component/impl/UserIdentityImpl.java @@ -31,7 +31,7 @@ public class UserIdentityImpl implements UserIdentity { private final UserAccountRepository userAccountRepository; private final UserRolesCourthousesRepository userRolesCourthousesRepository; - public String getGuidFromToken(Jwt token) { + private String getGuidFromToken(Jwt token) { if (token != null) { Object oid = token.getClaims().get(OID); if (nonNull(oid) && oid instanceof String guid && StringUtils.isNotBlank(guid)) { @@ -47,24 +47,19 @@ public UserAccountEntity getUserAccount() { @Override public UserAccountEntity getUserAccount(Jwt jwt) { - try { - UserAccountEntity userAccount = null; - String guid = getGuidFromToken(jwt); - if (nonNull(guid)) { - // System users will use GUID not email address - userAccount = userAccountRepository.findByAccountGuidAndActive(guid, true).orElse(null); - } - if (isNull(userAccount)) { - String emailAddressFromToken = EmailAddressFromTokenUtil.getEmailAddressFromToken(jwt); - userAccount = userAccountRepository.findByEmailAddressIgnoreCaseAndActive(emailAddressFromToken, true).stream() - .findFirst() - .orElseThrow(() -> new DartsApiException(USER_DETAILS_INVALID)); - } - return userAccount; - } catch (Throwable t) { - log.error("Error in getUserAccount", t); + UserAccountEntity userAccount = null; + String guid = getGuidFromToken(jwt); + if (nonNull(guid)) { + // System users will use GUID not email address + userAccount = userAccountRepository.findByAccountGuidAndActive(guid, true).orElse(null); } - return null; + if (isNull(userAccount)) { + String emailAddressFromToken = EmailAddressFromTokenUtil.getEmailAddressFromToken(jwt); + userAccount = userAccountRepository.findByEmailAddressIgnoreCaseAndActive(emailAddressFromToken, true).stream() + .findFirst() + .orElseThrow(() -> new DartsApiException(USER_DETAILS_INVALID)); + } + return userAccount; } public Jwt getJwt() { From 895e7106f28dcc2803c3e6a77625344917e77d94 Mon Sep 17 00:00:00 2001 From: benedwards Date: Tue, 10 Dec 2024 21:36:09 +0000 Subject: [PATCH 16/16] Reverted a few tmp changes --- .../stubs/DartsDatabaseSaveStub.java | 30 ++----- .../testutils/stubs/DartsDatabaseStub.java | 9 +- .../testutils/stubs/DartsPersistence.java | 82 ++++++++----------- 3 files changed, 43 insertions(+), 78 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java index a82584d80c..839f2710d9 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseSaveStub.java @@ -4,8 +4,6 @@ import lombok.AllArgsConstructor; import org.hibernate.proxy.HibernateProxy; import org.junit.platform.commons.JUnitException; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import uk.gov.hmcts.darts.common.entity.CaseManagementRetentionEntity; @@ -28,22 +26,13 @@ public class DartsDatabaseSaveStub { private final EntityManager entityManager; private final TransactionalUtil transactionalUtil; + @Transactional public T save(T entity) { - if (entity == null - || (entity instanceof HibernateProxy proxy - && proxy.getHibernateLazyInitializer().isUninitialized())) { - return entity; + if (entity == null) { + return null; } return transactionalUtil.executeInTransaction(() -> { - Authentication authentication = null; - //Remove the authentication from the context to bypass UserAuditListener. - //This will be added back again at the end of the method. - if (SecurityContextHolder.getContext().getAuthentication() != null) { - authentication = SecurityContextHolder.getContext().getAuthentication(); - SecurityContextHolder.getContext().setAuthentication(null); - } - if (entity instanceof CreatedModifiedBaseEntity createdModifiedBaseEntity) { updateCreatedByLastModifiedBy(createdModifiedBaseEntity); } @@ -51,19 +40,12 @@ public T save(T entity) { try { getIdInstanceMethod = getIdMethod(entity.getClass()); Integer id = (Integer) getIdInstanceMethod.invoke(entity); - T toReturn; if (id == null) { this.entityManager.persist(entity); - this.entityManager.flush(); - toReturn = entity; + return entity; } else { - toReturn = this.entityManager.merge(entity); - this.entityManager.flush(); - } - if (authentication != null) { - SecurityContextHolder.getContext().setAuthentication(authentication); + return this.entityManager.merge(entity); } - return toReturn; } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { throw new JUnitException("Failed to save entity", e); } @@ -119,4 +101,4 @@ public void updateCreatedByLastModifiedBy(CreatedModifiedBaseEntity createdModif public void saveAll(List caseManagementRetentionsWithEvents) { caseManagementRetentionsWithEvents.forEach(this::save); } -} +} \ No newline at end of file diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java index 76cbce4f24..72c1572cd6 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsDatabaseStub.java @@ -668,7 +668,9 @@ public T saveWithTransientEntities(T entity) { Method getIdMethod = fieldValue.getClass().getMethod("getId"); Object id = getIdMethod.invoke(fieldValue); if (id == null || (id instanceof Integer && (Integer) id == 0)) { - save(fieldValue); + // Save the transient entity + entityManager.persist(fieldValue); + entityManager.flush(); } } } @@ -890,9 +892,8 @@ public List findAnnotationsFor(Integer hearingId) { public void addUserToGroup(UserAccountEntity userAccount, SecurityGroupEntity securityGroup) { securityGroup.getUsers().add(userAccount); userAccount.getSecurityGroupEntities().add(securityGroup); - - saveWithTransientEntities(securityGroup); - saveWithTransientEntities(userAccount); + dartsDatabaseSaveStub.save(securityGroup); + dartsDatabaseSaveStub.save(userAccount); } @Transactional diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java index a9a7f97f55..6d4938e845 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/stubs/DartsPersistence.java @@ -6,9 +6,6 @@ import lombok.AllArgsConstructor; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import uk.gov.hmcts.darts.audio.entity.MediaRequestEntity; import uk.gov.hmcts.darts.common.entity.AnnotationDocumentEntity; @@ -93,6 +90,7 @@ import java.util.ArrayList; import java.util.List; +import static java.util.Arrays.asList; import static java.util.Arrays.stream; import static java.util.Objects.isNull; @@ -156,7 +154,6 @@ public class DartsPersistence { private final EntityManager entityManager; private final CurrentTimeHelper currentTimeHelper; private final TransactionalUtil transactionalUtil; - private final DartsDatabaseSaveStub dartsDatabaseSaveStub; @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") @@ -168,7 +165,7 @@ public HearingEntity save(HearingEntity hearing) { hearing.setCreatedBy(save(hearing.getCreatedBy())); hearing.setLastModifiedBy(save(hearing.getLastModifiedBy())); hearing.setJudges(saveJudgeList(hearing.getJudges())); - hearing = save(hearingRepository, hearing); + hearing = hearingRepository.save(hearing); saveMediaList(hearing.getMediaList()); return hearing; @@ -186,27 +183,12 @@ public AnnotationEntity save(AnnotationEntity annotationEntity) { if (annotationEntity.getHearingList() != null) { annotationEntity.setHearingList(saveHearingEntity(annotationEntity.getHearingList())); } - return save(annotationRepository, annotationEntity); + return annotationRepository.save(annotationEntity); } else { return entityManager.merge(annotationEntity); } } - private T save(JpaRepository repository, T annotationEntity) { - Authentication authentication = null; - //Remove the authentication from the context to bypass UserAuditListener. - //This will be added back again at the end of the method. - if (SecurityContextHolder.getContext().getAuthentication() != null) { - authentication = SecurityContextHolder.getContext().getAuthentication(); - SecurityContextHolder.getContext().setAuthentication(null); - } - T value = repository.saveAndFlush(annotationEntity); - if (authentication != null) { - SecurityContextHolder.getContext().setAuthentication(authentication); - } - return value; - } - @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") public CourthouseEntity save(CourthouseEntity courthouse) { @@ -215,7 +197,7 @@ public CourthouseEntity save(CourthouseEntity courthouse) { if (courthouse.getId() == null) { courthouse.setCreatedBy(save(courthouse.getCreatedBy())); courthouse.setLastModifiedBy(save(courthouse.getLastModifiedBy())); - save(courthouseRepository, courthouse); + courthouseRepository.save(courthouse); } else { courthouse = entityManager.merge(courthouse); } @@ -233,7 +215,7 @@ public CourtroomEntity save(CourtroomEntity courtroom) { courtroom.setCourthouse(save(courtroom.getCourthouse())); } courtroom.setCreatedBy(save(courtroom.getCreatedBy())); - return save(courtroomRepository, courtroom); + return courtroomRepository.save(courtroom); } else { return entityManager.merge(courtroom); } @@ -251,7 +233,7 @@ public MediaRequestEntity save(MediaRequestEntity mediaRequest) { mediaRequest.setCreatedBy(save(mediaRequest.getCreatedBy())); mediaRequest.setLastModifiedBy(save(mediaRequest.getLastModifiedBy())); - return save(mediaRequestRepository, mediaRequest); + return mediaRequestRepository.save(mediaRequest); } else { return entityManager.merge(mediaRequest); } @@ -264,7 +246,7 @@ public MediaLinkedCaseEntity save(MediaLinkedCaseEntity mediaLinkedCaseEntity) { if (mediaLinkedCaseEntity.getId() == null) { mediaLinkedCaseEntity.setCourtCase(save(mediaLinkedCaseEntity.getCourtCase())); - return save(mediaLinkedCaseRepository, mediaLinkedCaseEntity); + return mediaLinkedCaseRepository.save(mediaLinkedCaseEntity); } else { return entityManager.merge(mediaLinkedCaseEntity); } @@ -277,7 +259,7 @@ public EventHandlerEntity save(EventHandlerEntity eventHandlerEntity) { if (eventHandlerEntity.getId() == null) { eventHandlerEntity.setCreatedBy(save(eventHandlerEntity.getCreatedBy())); - return save(eventHandlerRepository, eventHandlerEntity); + return eventHandlerRepository.save(eventHandlerEntity); } else { return entityManager.merge(eventHandlerEntity); } @@ -313,7 +295,7 @@ public CourtCaseEntity save(CourtCaseEntity courtCase) { courtCase.setProsecutorList(saveProsecutorList(courtCase.getProsecutorList())); } - courtCase = save(caseRepository, courtCase); + courtCase = caseRepository.save(courtCase); } else { courtCase = entityManager.merge(courtCase); @@ -335,7 +317,7 @@ public TranscriptionDocumentEntity save(TranscriptionDocumentEntity transcriptio transcriptionDocumentEntity.getAdminActions().forEach(this::save); } - return save(transcriptionDocumentRepository, transcriptionDocumentEntity); + return transcriptionDocumentRepository.save(transcriptionDocumentEntity); } else { return entityManager.merge(transcriptionDocumentEntity); } @@ -344,7 +326,7 @@ public TranscriptionDocumentEntity save(TranscriptionDocumentEntity transcriptio @Transactional @SuppressWarnings("PMD.AvoidReassigningParameters") public ObjectAdminActionEntity save(ObjectAdminActionEntity adminAction) { - return save(objectAdminActionRepository, adminAction); + return objectAdminActionRepository.save(adminAction); } @@ -373,7 +355,7 @@ public ExternalObjectDirectoryEntity save(ExternalObjectDirectoryEntity eod) { eod.setCreatedBy(save(eod.getCreatedBy())); eod.setLastModifiedBy(save(eod.getLastModifiedBy())); - return save(externalObjectDirectoryRepository, eod); + return externalObjectDirectoryRepository.save(eod); } else { return entityManager.merge(eod); } @@ -388,7 +370,7 @@ public DefenceEntity save(DefenceEntity defence) { defence.setCreatedBy(save(defence.getCreatedBy())); defence.setLastModifiedBy(save(defence.getLastModifiedBy())); defence.setCourtCase(save(defence.getCourtCase())); - return save(defenceRepository, defence); + return defenceRepository.save(defence); } else { return entityManager.merge(defence); } @@ -403,7 +385,7 @@ public DefendantEntity save(DefendantEntity defendant) { defendant.setCreatedBy(save(defendant.getCreatedBy())); defendant.setLastModifiedBy(save(defendant.getLastModifiedBy())); defendant.setCourtCase(save(defendant.getCourtCase())); - return save(defendantRepository, defendant); + return defendantRepository.save(defendant); } else { return entityManager.merge(defendant); } @@ -418,7 +400,7 @@ public ProsecutorEntity save(ProsecutorEntity prosecutor) { prosecutor.setCreatedBy(save(prosecutor.getCreatedBy())); prosecutor.setLastModifiedBy(save(prosecutor.getCreatedBy())); prosecutor.setCourtCase(save(prosecutor.getCourtCase())); - return save(prosecutorRepository, prosecutor); + return prosecutorRepository.save(prosecutor); } else { return entityManager.merge(prosecutor); } @@ -448,7 +430,7 @@ public TranscriptionEntity save(TranscriptionEntity transcription) { transcription.setCreatedBy(save(transcription.getCreatedBy())); transcription.setLastModifiedBy(save(transcription.getLastModifiedBy())); - transcription = save(transcriptionRepository, transcription); + transcription = transcriptionRepository.save(transcription); } else { transcription = entityManager.merge(transcription); @@ -474,7 +456,7 @@ public TranscriptionWorkflowEntity save(TranscriptionWorkflowEntity workflowEnti workflowEntity.setTranscription(save(workflowEntity.getTranscription())); workflowEntity.setWorkflowActor(save(workflowEntity.getWorkflowActor())); - return save(transcriptionWorkflowRepository, workflowEntity); + return transcriptionWorkflowRepository.save(workflowEntity); } else { return entityManager.merge(workflowEntity); } @@ -489,7 +471,7 @@ public EventEntity save(EventEntity event) { event.setCourtroom(save(event.getCourtroom())); event.setCreatedBy(save(event.getCreatedBy())); event.setLastModifiedBy(save(event.getLastModifiedBy())); - return save(eventRepository, event); + return eventRepository.save(event); } else { return entityManager.merge(event); } @@ -503,7 +485,7 @@ public RetentionPolicyTypeEntity save(RetentionPolicyTypeEntity retentionPolicyT if (retentionPolicyType.getId() == null) { retentionPolicyType.setLastModifiedBy(save(retentionPolicyType.getLastModifiedBy())); retentionPolicyType.setCreatedBy(save(retentionPolicyType.getCreatedBy())); - return save(retentionPolicyTypeRepository, retentionPolicyType); + return retentionPolicyTypeRepository.save(retentionPolicyType); } else { return entityManager.merge(retentionPolicyType); } @@ -518,7 +500,7 @@ public CaseManagementRetentionEntity save(CaseManagementRetentionEntity caseMana caseManagementRetention.setCourtCase(save(caseManagementRetention.getCourtCase())); caseManagementRetention.setEventEntity(save(caseManagementRetention.getEventEntity())); caseManagementRetention.setRetentionPolicyTypeEntity(save(caseManagementRetention.getRetentionPolicyTypeEntity())); - return save(caseManagementRetentionRepository, caseManagementRetention); + return caseManagementRetentionRepository.save(caseManagementRetention); } else { return entityManager.merge(caseManagementRetention); } @@ -533,7 +515,7 @@ public UserAccountEntity save(UserAccountEntity userAccount) { UserAccountEntity systemUser = userAccountRepository.getReferenceById(0); userAccount.setCreatedBy(systemUser); userAccount.setLastModifiedBy(systemUser); - return save(userAccountRepository, userAccount); + return userAccountRepository.save(userAccount); } else { return entityManager.merge(userAccount); } @@ -549,7 +531,7 @@ public AnnotationDocumentEntity save(AnnotationDocumentEntity annotationDocument annotationDocument.setLastModifiedBy(save(annotationDocument.getLastModifiedBy())); annotationDocument.setUploadedBy(save(annotationDocument.getUploadedBy())); save(annotationDocument.getAnnotation()); - return save(annotationDocumentRepository, annotationDocument); + return annotationDocumentRepository.save(annotationDocument); } else { return entityManager.merge(annotationDocument); } @@ -564,7 +546,7 @@ public JudgeEntity save(JudgeEntity judge) { judge.setCreatedBy(save(judge.getCreatedBy())); judge.setLastModifiedBy(save(judge.getLastModifiedBy())); - return save(judgeRepository, judge); + return judgeRepository.save(judge); } else { return entityManager.merge(judge); } @@ -587,7 +569,7 @@ public CaseDocumentEntity save(CaseDocumentEntity caseDocumentEntity) { caseDocumentEntity.setCreatedBy(save(caseDocumentEntity.getCreatedBy())); caseDocumentEntity.setLastModifiedBy(save(caseDocumentEntity.getLastModifiedBy())); caseDocumentEntity.setCourtCase(save(caseDocumentEntity.getCourtCase())); - return save(caseDocumentRepository, caseDocumentEntity); + return caseDocumentRepository.save(caseDocumentEntity); } else { return entityManager.merge(caseDocumentEntity); } @@ -607,9 +589,9 @@ public MediaEntity save(MediaEntity media, boolean processhHearing) { media.setMediaLinkedCaseList(saveLinkedCaseList(media.getMediaLinkedCaseList())); } - media = save(mediaRepository, media); + media = mediaRepository.save(media); } else { - media = save(mediaRepository, media); + media = mediaRepository.save(media); } return media; } @@ -623,7 +605,7 @@ public TransformedMediaEntity save(TransformedMediaEntity transformedMedia) { transformedMedia.setCreatedBy(save(transformedMedia.getCreatedBy())); transformedMedia.setLastModifiedBy(save(transformedMedia.getLastModifiedBy())); transformedMedia.setMediaRequest(save(transformedMedia.getMediaRequest())); - return save(transformedMediaRepository, transformedMedia); + return transformedMediaRepository.save(transformedMedia); } else { entityManager.merge(transformedMedia); } @@ -640,7 +622,7 @@ public TranscriptionCommentEntity save(TranscriptionCommentEntity commentEntity) commentEntity.setCreatedBy(save(commentEntity.getCreatedBy())); commentEntity.setLastModifiedBy(save(commentEntity.getLastModifiedBy())); commentEntity.setTranscription(save(commentEntity.getTranscription())); - return save(transcriptionCommentRepository, commentEntity); + return transcriptionCommentRepository.save(commentEntity); } else { entityManager.merge(commentEntity); } @@ -656,7 +638,7 @@ public ArmRpoExecutionDetailEntity save(ArmRpoExecutionDetailEntity armRpoExecut if (armRpoExecutionDetailEntity.getId() == null) { armRpoExecutionDetailEntity.setCreatedBy(save(armRpoExecutionDetailEntity.getCreatedBy())); armRpoExecutionDetailEntity.setLastModifiedBy(save(armRpoExecutionDetailEntity.getLastModifiedBy())); - return save(armRpoExecutionDetailRepository, armRpoExecutionDetailEntity); + return armRpoExecutionDetailRepository.save(armRpoExecutionDetailEntity); } else { entityManager.merge(armRpoExecutionDetailEntity); } @@ -670,7 +652,7 @@ public ArmRpoStateEntity save(ArmRpoStateEntity armRpoStateEntity) { armRpoStateEntity = (ArmRpoStateEntity) preCheckPersist(armRpoStateEntity); if (armRpoStateEntity.getId() == null) { - return save(armRpoStateRepository, armRpoStateEntity); + return armRpoStateRepository.save(armRpoStateEntity); } else { return entityManager.merge(armRpoStateEntity); } @@ -682,7 +664,7 @@ public ArmRpoStatusEntity save(ArmRpoStatusEntity armRpoStatusEntity) { armRpoStatusEntity = (ArmRpoStatusEntity) preCheckPersist(armRpoStatusEntity); if (armRpoStatusEntity.getId() == null) { - return save(armRpoStatusRepository, armRpoStatusEntity); + return armRpoStatusRepository.save(armRpoStatusEntity); } else { return entityManager.merge(armRpoStatusEntity); } @@ -697,8 +679,8 @@ public void saveAll(UserAccountEntity... userAccounts) { UserAccountEntity systemUser = userAccountRepository.getReferenceById(0); user.setCreatedBy(systemUser); user.setLastModifiedBy(systemUser); - save(userAccountRepository, user); }); + userAccountRepository.saveAll(asList(userAccounts)); } @Transactional