From 82687437bae6ce6030b29b289fdc3736a3eb59e6 Mon Sep 17 00:00:00 2001 From: Viacheslav Poliakov <159778173+viacheslavpoliakov@users.noreply.github.com> Date: Thu, 11 Apr 2024 19:37:44 +0300 Subject: [PATCH] fix(authority-source-files): restrict deletion referenced authority source file in member tenants (#284) Closes: MODELINKS-227 (cherry picked from commit 4e9a8b154cd13e31d66305ea38f4f0ab2347cb62) --- NEWS.md | 1 + docker/docker-compose.yml | 2 +- .../test/resources => }/mappings/authn.json | 0 .../test/resources => }/mappings/users.json | 0 .../entlinks/controller/ApiErrorHandler.java | 4 +- .../AuthoritySourceFileServiceDelegate.java | 19 +++- .../AuthorityArchiveConstraintException.java | 16 +++ .../authority/AuthoritySourceFileService.java | 6 +- .../ConsortiumAuthoritySourceFilesIT.java | 106 ++++++++++++++++++ ...uthoritySourceFileServiceDelegateTest.java | 4 +- .../AuthoritySourceFileServiceTest.java | 2 +- 11 files changed, 150 insertions(+), 10 deletions(-) rename docker/{src/test/resources => }/mappings/authn.json (100%) rename docker/{src/test/resources => }/mappings/users.json (100%) create mode 100644 src/main/java/org/folio/entlinks/exception/AuthorityArchiveConstraintException.java create mode 100644 src/test/java/org/folio/entlinks/controller/ConsortiumAuthoritySourceFilesIT.java diff --git a/NEWS.md b/NEWS.md index cb287cf7..ffc911ed 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ ## v3.0.1 2024-04-11 ### Bug fixes * Do not delete kafka topics if tenant collection topic feature is enabled ([MODELINKS-233](https://folio-org.atlassian.net/browse/MODELINKS-233)) +* Add checking for Authority source file references for member tenant in ECS ([MODELINKS-227](https://issues.folio.org/browse/MODELINKS-227)) ## v3.0.0 2024-03-19 ### Breaking changes diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index c81be631..ea23fd64 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -39,7 +39,7 @@ services: ports: - "9130:8080" volumes: - - ../src/test/resources/mappings:/home/wiremock/mappings + - ./mappings:/home/wiremock/mappings zookeeper: container_name: zookeeper_mod-entities-links diff --git a/docker/src/test/resources/mappings/authn.json b/docker/mappings/authn.json similarity index 100% rename from docker/src/test/resources/mappings/authn.json rename to docker/mappings/authn.json diff --git a/docker/src/test/resources/mappings/users.json b/docker/mappings/users.json similarity index 100% rename from docker/src/test/resources/mappings/users.json rename to docker/mappings/users.json diff --git a/src/main/java/org/folio/entlinks/controller/ApiErrorHandler.java b/src/main/java/org/folio/entlinks/controller/ApiErrorHandler.java index 467005d2..a85da55e 100644 --- a/src/main/java/org/folio/entlinks/controller/ApiErrorHandler.java +++ b/src/main/java/org/folio/entlinks/controller/ApiErrorHandler.java @@ -20,6 +20,7 @@ import org.folio.entlinks.config.constants.ErrorCode; import org.folio.entlinks.domain.entity.AuthoritySourceFile; import org.folio.entlinks.exception.AuthoritiesRequestNotSupportedMediaTypeException; +import org.folio.entlinks.exception.AuthorityArchiveConstraintException; import org.folio.entlinks.exception.AuthoritySourceFileHridException; import org.folio.entlinks.exception.OptimisticLockingException; import org.folio.entlinks.exception.RequestBodyValidationException; @@ -129,7 +130,8 @@ public ResponseEntity handlerHttpMessageNotReadableException(HttpMessage @ExceptionHandler({ DataIntegrityViolationException.class, - InvalidDataAccessApiUsageException.class + InvalidDataAccessApiUsageException.class, + AuthorityArchiveConstraintException.class }) public ResponseEntity conflict(Exception e) { var cause = e.getCause(); diff --git a/src/main/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegate.java b/src/main/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegate.java index 6b92d0a4..7bf3ecd9 100644 --- a/src/main/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegate.java +++ b/src/main/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegate.java @@ -19,6 +19,7 @@ import org.folio.entlinks.domain.dto.AuthoritySourceFilePatchDto; import org.folio.entlinks.domain.dto.AuthoritySourceFilePostDto; import org.folio.entlinks.domain.entity.AuthoritySourceFile; +import org.folio.entlinks.exception.AuthorityArchiveConstraintException; import org.folio.entlinks.exception.RequestBodyValidationException; import org.folio.entlinks.integration.dto.event.DomainEventType; import org.folio.entlinks.service.authority.AuthoritySourceFileService; @@ -36,6 +37,8 @@ public class AuthoritySourceFileServiceDelegate { private static final String URL_PROTOCOL_PATTERN = "^(https?://www\\.|https?://|www\\.)"; + private static final String AUTHORITY_TABLE_NAME = "authority"; + private static final String AUTHORITY_ARCHIVE_TABLE_NAME = "authority_archive"; private final AuthoritySourceFileService service; private final AuthoritySourceFileMapper mapper; @@ -91,9 +94,15 @@ public void deleteAuthoritySourceFileById(UUID id) { "Unable to delete. Authority source file has referenced authorities", Collections.emptyList()); } + if (anyReferenceForSourceFile(entity)) { + throw new AuthorityArchiveConstraintException( + "Unable to delete. Authority source file has referenced authority archives"); + } + if (entity.getSequenceName() != null) { service.deleteSequence(entity.getSequenceName()); } + service.deleteById(id); propagationService.propagate(entity, DELETE, context.getTenantId()); } @@ -152,19 +161,25 @@ private void validatePatchRequest(AuthoritySourceFilePatchDto patchDto, Authorit } } + public boolean anyReferenceForSourceFile(AuthoritySourceFile sourceFile) { + return anyDataExistForSourceFile(sourceFile.getId(), AUTHORITY_ARCHIVE_TABLE_NAME); + } + public boolean anyAuthoritiesExistForSourceFile(AuthoritySourceFile sourceFile) { var sourceFileId = sourceFile.getId(); if (service.authoritiesExistForSourceFile(sourceFileId)) { return true; } + return anyDataExistForSourceFile(sourceFileId, AUTHORITY_TABLE_NAME); + } + private boolean anyDataExistForSourceFile(UUID sourceFileId, String tableName) { var consortiumTenants = consortiumTenantsService.getConsortiumTenants(context.getTenantId()); for (String memberTenant : consortiumTenants) { - if (service.authoritiesExistForSourceFile(sourceFileId, memberTenant)) { + if (service.authoritiesExistForSourceFile(sourceFileId, memberTenant, tableName)) { return true; } } - return false; } } diff --git a/src/main/java/org/folio/entlinks/exception/AuthorityArchiveConstraintException.java b/src/main/java/org/folio/entlinks/exception/AuthorityArchiveConstraintException.java new file mode 100644 index 00000000..ba0f55bd --- /dev/null +++ b/src/main/java/org/folio/entlinks/exception/AuthorityArchiveConstraintException.java @@ -0,0 +1,16 @@ +package org.folio.entlinks.exception; + +import static org.folio.entlinks.config.constants.ErrorCode.VIOLATION_OF_RELATION_BETWEEN_AUTHORITY_ARCHIVE_AND_SOURCE_FILE; + +import java.sql.SQLException; +import org.hibernate.exception.ConstraintViolationException; + +public class AuthorityArchiveConstraintException extends RuntimeException { + + private static final String MESSAGE = VIOLATION_OF_RELATION_BETWEEN_AUTHORITY_ARCHIVE_AND_SOURCE_FILE.getMessage(); + private static final String CONSTRAINT = "authority_archive_source_file_id_foreign_key"; + + public AuthorityArchiveConstraintException(String message) { + super(message, new ConstraintViolationException(MESSAGE, new SQLException(), CONSTRAINT)); + } +} diff --git a/src/main/java/org/folio/entlinks/service/authority/AuthoritySourceFileService.java b/src/main/java/org/folio/entlinks/service/authority/AuthoritySourceFileService.java index 4cca5891..879095be 100644 --- a/src/main/java/org/folio/entlinks/service/authority/AuthoritySourceFileService.java +++ b/src/main/java/org/folio/entlinks/service/authority/AuthoritySourceFileService.java @@ -168,13 +168,13 @@ public boolean authoritiesExistForSourceFile(UUID sourceFileId) { return authorityRepository.existsAuthorityByAuthoritySourceFileId(sourceFileId); } - public boolean authoritiesExistForSourceFile(UUID sourceFileId, String tenantId) { + public boolean authoritiesExistForSourceFile(UUID sourceFileId, String tenantId, String tableName) { if (sourceFileId == null || tenantId == null) { return false; } - var command = String.format("select exists (select true from %s.authority a where a.source_file_id='%s' limit 1)", - moduleMetadata.getDBSchemaName(tenantId), sourceFileId); + var command = String.format("select exists (select true from %s.%s a where a.source_file_id='%s' limit 1)", + moduleMetadata.getDBSchemaName(tenantId), tableName, sourceFileId); return Boolean.TRUE.equals(jdbcTemplate.queryForObject(command, Boolean.class)); } diff --git a/src/test/java/org/folio/entlinks/controller/ConsortiumAuthoritySourceFilesIT.java b/src/test/java/org/folio/entlinks/controller/ConsortiumAuthoritySourceFilesIT.java new file mode 100644 index 00000000..afcdb9b9 --- /dev/null +++ b/src/test/java/org/folio/entlinks/controller/ConsortiumAuthoritySourceFilesIT.java @@ -0,0 +1,106 @@ +package org.folio.entlinks.controller; + +import static org.folio.entlinks.controller.ConsortiumLinksSuggestionsIT.COLLEGE_TENANT_ID; +import static org.folio.support.DatabaseHelper.AUTHORITY_ARCHIVE_TABLE; +import static org.folio.support.DatabaseHelper.AUTHORITY_SOURCE_FILE_CODE_TABLE; +import static org.folio.support.DatabaseHelper.AUTHORITY_SOURCE_FILE_TABLE; +import static org.folio.support.DatabaseHelper.AUTHORITY_TABLE; +import static org.folio.support.base.TestConstants.CENTRAL_TENANT_ID; +import static org.folio.support.base.TestConstants.authorityEndpoint; +import static org.folio.support.base.TestConstants.authoritySourceFilesEndpoint; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.util.Collections; +import java.util.List; +import java.util.UUID; +import lombok.SneakyThrows; +import org.folio.entlinks.domain.dto.AuthorityDto; +import org.folio.entlinks.domain.dto.AuthoritySourceFilePostDto; +import org.folio.entlinks.exception.AuthorityArchiveConstraintException; +import org.folio.spring.integration.XOkapiHeaders; +import org.folio.spring.testing.extension.DatabaseCleanup; +import org.folio.spring.testing.type.IntegrationTest; +import org.folio.support.base.IntegrationTestBase; +import org.hamcrest.Matcher; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.test.web.servlet.ResultMatcher; + +@IntegrationTest +@DatabaseCleanup( + tables = { + AUTHORITY_TABLE, + AUTHORITY_ARCHIVE_TABLE, + AUTHORITY_SOURCE_FILE_CODE_TABLE, + AUTHORITY_SOURCE_FILE_TABLE}, + tenants = {CENTRAL_TENANT_ID, COLLEGE_TENANT_ID}) +class ConsortiumAuthoritySourceFilesIT extends IntegrationTestBase { + + public static final String COLLEGE_TENANT_ID = "college"; + private static final String AUTHORITY_ID = "417f3355-081c-4aae-9209-ccb305f25f7e"; + + @BeforeAll + static void prepare() { + setUpConsortium(CENTRAL_TENANT_ID, List.of(COLLEGE_TENANT_ID), false); + } + + @Test + @SneakyThrows + @DisplayName("DELETE: Should not delete authority source file with referenced authority archive in member tenant") + void deleteAsfWithMemberTenantAuthorityArchiveReference_negative_failDeletingAsf() { + var sourceFileId = UUID.randomUUID(); + var dto = new AuthoritySourceFilePostDto() + .id(sourceFileId).name("authority source file").code("sly").type("type"); + + // create source file + doPost(authoritySourceFilesEndpoint(), dto, tenantHeaders(CENTRAL_TENANT_ID)); + awaitUntilAsserted(() -> + assertEquals(1, databaseHelper.countRows(AUTHORITY_SOURCE_FILE_TABLE, COLLEGE_TENANT_ID))); + awaitUntilAsserted(() -> + assertEquals(1, databaseHelper.countRows(AUTHORITY_SOURCE_FILE_TABLE, CENTRAL_TENANT_ID))); + + var authorityDto = new AuthorityDto() + .id(UUID.fromString(AUTHORITY_ID)) + .sourceFileId(sourceFileId) + .naturalId("n12345") + .source("MARC") + .personalName("Sylvester Stallone") + .subjectHeadings("a"); + + // create authority in member tenant + doPost(authorityEndpoint(), authorityDto, tenantHeaders(COLLEGE_TENANT_ID)); + awaitUntilAsserted(() -> + assertEquals(1, databaseHelper.countRows(AUTHORITY_TABLE, COLLEGE_TENANT_ID))); + + // delete authority in member tenant + doDelete(authorityEndpoint(authorityDto.getId()), tenantHeaders(COLLEGE_TENANT_ID)); + awaitUntilAsserted(() -> + assertEquals(0, databaseHelper.countRows(AUTHORITY_TABLE, COLLEGE_TENANT_ID))); + awaitUntilAsserted(() -> + assertEquals(1, databaseHelper.countRows(AUTHORITY_ARCHIVE_TABLE, COLLEGE_TENANT_ID))); + + // try ti delete in central tenant the authority source file with reference in member tenant + tryDelete(authoritySourceFilesEndpoint(sourceFileId), tenantHeaders(CENTRAL_TENANT_ID)) + .andExpect(status().isUnprocessableEntity()) + .andExpect(exceptionMatch(AuthorityArchiveConstraintException.class)) + .andExpect(errorMessageMatch(is("Cannot complete operation on the entity due to it's relation with" + + " Authority Archive/Authority."))); + + assertEquals(1, databaseHelper.countRows(AUTHORITY_SOURCE_FILE_TABLE, CENTRAL_TENANT_ID)); + } + + private ResultMatcher errorMessageMatch(Matcher errorMessageMatcher) { + return jsonPath("$.errors.[0].message", errorMessageMatcher); + } + + private HttpHeaders tenantHeaders(String tenant) { + var httpHeaders = defaultHeaders(); + httpHeaders.put(XOkapiHeaders.TENANT, Collections.singletonList(tenant)); + return httpHeaders; + } +} diff --git a/src/test/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegateTest.java b/src/test/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegateTest.java index 0a430d2e..19dd3b89 100644 --- a/src/test/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegateTest.java +++ b/src/test/java/org/folio/entlinks/controller/delegate/AuthoritySourceFileServiceDelegateTest.java @@ -201,7 +201,7 @@ void shouldNotPatchAuthoritySourceFile_whenSourceFolioOrAuthoritiesReferenced(Au if (!authoritiesReferencedAtCentral) { when(context.getTenantId()).thenReturn("central"); when(consortiumTenantsService.getConsortiumTenants("central")).thenReturn(List.of("member")); - when(service.authoritiesExistForSourceFile(id, "member")).thenReturn(authoritiesReferencedAtMember); + when(service.authoritiesExistForSourceFile(id, "member", "authority")).thenReturn(authoritiesReferencedAtMember); } var dto = new AuthoritySourceFilePatchDto() .name("name").type("type").selectable(true).version(1).baseUrl("baseUrl").code("a") @@ -267,7 +267,7 @@ void shouldNotDeleteIfReferencedAuthorityExistForSourceFile() { when(context.getTenantId()).thenReturn(TENANT_ID); when(service.authoritiesExistForSourceFile(id)).thenReturn(false); when(consortiumTenantsService.getConsortiumTenants(anyString())).thenReturn(List.of("college")); - when(service.authoritiesExistForSourceFile(id, "college")).thenReturn(true); + when(service.authoritiesExistForSourceFile(id, "college", "authority")).thenReturn(true); var exc = assertThrows(RequestBodyValidationException.class, () -> delegate.deleteAuthoritySourceFileById(id)); diff --git a/src/test/java/org/folio/entlinks/service/authority/AuthoritySourceFileServiceTest.java b/src/test/java/org/folio/entlinks/service/authority/AuthoritySourceFileServiceTest.java index 90df72b8..46f7adf3 100644 --- a/src/test/java/org/folio/entlinks/service/authority/AuthoritySourceFileServiceTest.java +++ b/src/test/java/org/folio/entlinks/service/authority/AuthoritySourceFileServiceTest.java @@ -469,7 +469,7 @@ void shouldCheckAuthoritiesExistForSourceFileAndTenant() { when(jdbcTemplate.queryForObject(anyString(), eq(Boolean.class))).thenReturn(true); var captor = ArgumentCaptor.forClass(String.class); - var actual = service.authoritiesExistForSourceFile(id, tenant); + var actual = service.authoritiesExistForSourceFile(id, tenant, "authority"); assertThat(actual).isEqualTo(expected); verify(jdbcTemplate).queryForObject(captor.capture(), eq(Boolean.class));