From 6c79fbc0a42bdd5a85e9766063e7edce86f888f7 Mon Sep 17 00:00:00 2001 From: Andrei Bordak Date: Wed, 20 Nov 2024 16:59:18 +0400 Subject: [PATCH] MODLD-582: LCCN Validation | Ensure uniquely assigned MODLD-582: Change lccn value in test MODLD-582: Additional verification for searchClient invocation MODLD-582: Review/Sonar fixes MODLD-582: Review/Sonar fixes MODLD-582: Handle failed dependency MODLD-582: Review/Sonar fixes MODLD-582: Review/Sonar fixes MODLD-582: Rework exception handling in search service MODLD-582 Review fixes MODLD-582 Review fixes MODLD-582: Fix sonar/review MODLD-582: Fix sonar/review --- descriptors/ModuleDescriptor-template.json | 6 + .../linked/data/client/SearchClient.java | 14 ++ .../configuration/ErrorResponseConfig.java | 1 + .../data/controller/ResourceController.java | 23 +++ .../advice/ApiExceptionHandler.java | 13 ++ .../RequestProcessingExceptionBuilder.java | 4 + .../data/repo/FolioMetadataRepository.java | 6 + .../service/search/InstanceSearchService.java | 11 ++ .../search/InstanceSearchServiceImpl.java | 48 +++++ .../org/folio/linked/data/util/LccnUtils.java | 17 ++ .../validation/LccnPatternConstraint.java | 1 + .../data/validation/LccnUniqueConstraint.java | 24 +++ .../validation/dto/LccnPatternValidator.java | 9 +- .../dto/LccnUniquenessValidator.java | 87 +++++++++ .../resources/ValidationMessages.properties | 1 + src/main/resources/errors.yml | 5 + .../resources/swagger.api/folio-modules.yaml | 2 + .../search/searchResponseTotalOnly.json | 12 ++ .../schema/resourceRequestDto.json | 6 + .../resource/ResourceControllerITBase.java | 109 +++++++++++ .../search/InstanceSearchServiceImplTest.java | 86 +++++++++ .../dto/LccnUniquenessValidatorTest.java | 175 ++++++++++++++++++ 22 files changed, 652 insertions(+), 8 deletions(-) create mode 100644 src/main/java/org/folio/linked/data/client/SearchClient.java create mode 100644 src/main/java/org/folio/linked/data/service/search/InstanceSearchService.java create mode 100644 src/main/java/org/folio/linked/data/service/search/InstanceSearchServiceImpl.java create mode 100644 src/main/java/org/folio/linked/data/util/LccnUtils.java create mode 100644 src/main/java/org/folio/linked/data/validation/LccnUniqueConstraint.java create mode 100644 src/main/java/org/folio/linked/data/validation/dto/LccnUniquenessValidator.java create mode 100644 src/main/resources/swagger.api/folio-modules/search/searchResponseTotalOnly.json create mode 100644 src/test/java/org/folio/linked/data/service/search/InstanceSearchServiceImplTest.java create mode 100644 src/test/java/org/folio/linked/data/validation/dto/LccnUniquenessValidatorTest.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index c0e982d0..211ec832 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -22,6 +22,7 @@ "inventory-storage.preceding-succeeding-titles.item.post", "inventory-storage.preceding-succeeding-titles.item.put", "inventory-storage.preceding-succeeding-titles.item.delete", + "search.instances.collection.get", "source-storage.snapshots.post", "source-storage.records.formatted.item.get", "source-storage.records.post", @@ -64,6 +65,7 @@ "inventory-storage.preceding-succeeding-titles.item.post", "inventory-storage.preceding-succeeding-titles.item.put", "inventory-storage.preceding-succeeding-titles.item.delete", + "search.instances.collection.get", "source-storage.snapshots.post", "source-storage.records.formatted.item.get", "source-storage.records.post", @@ -180,6 +182,10 @@ { "id": "specification-storage", "version": "1.0" + }, + { + "id": "search", + "version": "1.3" } ], "permissionSets": [ diff --git a/src/main/java/org/folio/linked/data/client/SearchClient.java b/src/main/java/org/folio/linked/data/client/SearchClient.java new file mode 100644 index 00000000..1e33fb0b --- /dev/null +++ b/src/main/java/org/folio/linked/data/client/SearchClient.java @@ -0,0 +1,14 @@ +package org.folio.linked.data.client; + +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; +import org.springframework.cloud.openfeign.FeignClient; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestParam; + +@FeignClient(name = "search") +public interface SearchClient { + + @GetMapping("/instances") + ResponseEntity searchInstances(@RequestParam("query") String query); +} diff --git a/src/main/java/org/folio/linked/data/configuration/ErrorResponseConfig.java b/src/main/java/org/folio/linked/data/configuration/ErrorResponseConfig.java index a33716c1..edca7d62 100644 --- a/src/main/java/org/folio/linked/data/configuration/ErrorResponseConfig.java +++ b/src/main/java/org/folio/linked/data/configuration/ErrorResponseConfig.java @@ -21,6 +21,7 @@ public class ErrorResponseConfig { private ErrorResponseConfig.Error genericBadRequest; private ErrorResponseConfig.Error validation; private ErrorResponseConfig.Error genericServer; + private ErrorResponseConfig.Error failedDependency; public record Error(int status, String code, List parameters, String message) { } diff --git a/src/main/java/org/folio/linked/data/controller/ResourceController.java b/src/main/java/org/folio/linked/data/controller/ResourceController.java index 7a6c26db..59d49406 100644 --- a/src/main/java/org/folio/linked/data/controller/ResourceController.java +++ b/src/main/java/org/folio/linked/data/controller/ResourceController.java @@ -1,7 +1,10 @@ package org.folio.linked.data.controller; +import static java.util.Optional.ofNullable; +import static org.springframework.http.HttpMethod.PUT; import static org.springframework.http.HttpStatus.CREATED; +import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.folio.linked.data.domain.dto.ResourceIdDto; @@ -12,6 +15,8 @@ import org.folio.linked.data.service.resource.ResourceMarcBibService; import org.folio.linked.data.service.resource.ResourceService; import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.WebDataBinder; +import org.springframework.web.bind.annotation.InitBinder; import org.springframework.web.bind.annotation.RestController; @RestController @@ -70,4 +75,22 @@ public ResponseEntity getResourceMarcViewById(Long id, Stri return ResponseEntity.ok(resourceMarcService.getResourceMarcView(id)); } + @InitBinder + public void bindIdForResourcePutRequest(WebDataBinder binder, HttpServletRequest request) { + var target = binder.getTarget(); + if (target instanceof ResourceRequestDto dto && isPutRequest(request)) { + dto.setId(idFromUri(request.getRequestURI())); + } + } + + private boolean isPutRequest(HttpServletRequest request) { + return ofNullable(request) + .map(r -> PUT.name().equalsIgnoreCase(r.getMethod())) + .orElse(false); + } + + private Long idFromUri(String path) { + String[] parts = path.split("/"); + return Long.parseLong(parts[parts.length - 1]); + } } diff --git a/src/main/java/org/folio/linked/data/controller/advice/ApiExceptionHandler.java b/src/main/java/org/folio/linked/data/controller/advice/ApiExceptionHandler.java index 4306f391..9e33ddab 100644 --- a/src/main/java/org/folio/linked/data/controller/advice/ApiExceptionHandler.java +++ b/src/main/java/org/folio/linked/data/controller/advice/ApiExceptionHandler.java @@ -2,6 +2,7 @@ import jakarta.persistence.EntityNotFoundException; import jakarta.validation.ConstraintViolationException; +import jakarta.validation.ValidationException; import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; import org.apache.logging.log4j.Level; @@ -88,6 +89,18 @@ public ResponseEntity handleMissingServletRequestParameterExcepti return genericBadRequestMapper.errorResponseEntity(e); } + @ExceptionHandler(ValidationException.class) + public ResponseEntity handleValidationException(ValidationException exception) { + return handleValidationExceptionCause(exception); + } + + private ResponseEntity handleValidationExceptionCause(ValidationException exception) { + if (exception.getCause() instanceof RequestProcessingException e) { + return handleRequestProcessingException(e); + } + return handleAllOtherExceptions(exception); + } + @ExceptionHandler(Exception.class) public ResponseEntity handleAllOtherExceptions(Exception exception) { logException(exception); diff --git a/src/main/java/org/folio/linked/data/exception/RequestProcessingExceptionBuilder.java b/src/main/java/org/folio/linked/data/exception/RequestProcessingExceptionBuilder.java index 6774c1cd..15d6510c 100644 --- a/src/main/java/org/folio/linked/data/exception/RequestProcessingExceptionBuilder.java +++ b/src/main/java/org/folio/linked/data/exception/RequestProcessingExceptionBuilder.java @@ -49,6 +49,10 @@ public RequestProcessingException notFoundSourceRecordException(String idType, S return requestProcessingException(notFoundError, "Source Record", idType, idValue, "Source Record storage"); } + public RequestProcessingException failedDependencyException(String message, String reason) { + return requestProcessingException(errorResponseConfig.getFailedDependency(), message, reason); + } + private RequestProcessingException requestProcessingException(ErrorResponseConfig.Error error, String... arguments) { var parameters = new HashMap(); for (int i = 0; i < arguments.length; i++) { diff --git a/src/main/java/org/folio/linked/data/repo/FolioMetadataRepository.java b/src/main/java/org/folio/linked/data/repo/FolioMetadataRepository.java index 3efca71c..4f8103af 100644 --- a/src/main/java/org/folio/linked/data/repo/FolioMetadataRepository.java +++ b/src/main/java/org/folio/linked/data/repo/FolioMetadataRepository.java @@ -14,7 +14,13 @@ public interface FolioMetadataRepository extends JpaRepository findByInventoryId(String inventoryId); + Optional findInventoryIdById(Long id); + interface IdOnly { Long getId(); } + + interface InventoryIdOnly { + String getInventoryId(); + } } diff --git a/src/main/java/org/folio/linked/data/service/search/InstanceSearchService.java b/src/main/java/org/folio/linked/data/service/search/InstanceSearchService.java new file mode 100644 index 00000000..212e0393 --- /dev/null +++ b/src/main/java/org/folio/linked/data/service/search/InstanceSearchService.java @@ -0,0 +1,11 @@ +package org.folio.linked.data.service.search; + +import java.util.Collection; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; + +public interface InstanceSearchService { + + SearchResponseTotalOnly searchByLccn(Collection lccn); + + SearchResponseTotalOnly searchByLccnExcludingId(Collection lccn, String instanceId); +} diff --git a/src/main/java/org/folio/linked/data/service/search/InstanceSearchServiceImpl.java b/src/main/java/org/folio/linked/data/service/search/InstanceSearchServiceImpl.java new file mode 100644 index 00000000..18253e19 --- /dev/null +++ b/src/main/java/org/folio/linked/data/service/search/InstanceSearchServiceImpl.java @@ -0,0 +1,48 @@ +package org.folio.linked.data.service.search; + +import static java.lang.String.format; +import static java.util.Objects.isNull; +import static java.util.stream.Collectors.joining; + +import java.util.Collection; +import java.util.Objects; +import lombok.RequiredArgsConstructor; +import org.folio.linked.data.client.SearchClient; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; +import org.springframework.stereotype.Service; + +@Service +@RequiredArgsConstructor +public class InstanceSearchServiceImpl implements InstanceSearchService { + + private static final String EXCLUDE_SUPPRESSED = "staffSuppress <> \"true\" and discoverySuppress <> \"true\""; + private static final String ID_NOT_EQUALS = "id <> \"%s\""; + private static final String LCCN_EQUALS = "lccn==\"%s\""; + + private final SearchClient searchClient; + + @Override + public SearchResponseTotalOnly searchByLccn(Collection lccn) { + return search(getLccnQuery(lccn)); + } + + @Override + public SearchResponseTotalOnly searchByLccnExcludingId(Collection lccn, String instanceId) { + if (isNull(instanceId)) { + return searchByLccn(lccn); + } + return search(format("%s and %s", getLccnQuery(lccn), format(ID_NOT_EQUALS, instanceId))); + } + + private SearchResponseTotalOnly search(String query) { + return searchClient.searchInstances(query).getBody(); + } + + private String getLccnQuery(Collection lccnCol) { + var orLccnQuery = lccnCol.stream() + .filter(Objects::nonNull) + .map(lccn -> format(LCCN_EQUALS, lccn)) + .collect(joining(" or ")); + return format("(%s) and (%s)", orLccnQuery, EXCLUDE_SUPPRESSED); + } +} diff --git a/src/main/java/org/folio/linked/data/util/LccnUtils.java b/src/main/java/org/folio/linked/data/util/LccnUtils.java new file mode 100644 index 00000000..8217839c --- /dev/null +++ b/src/main/java/org/folio/linked/data/util/LccnUtils.java @@ -0,0 +1,17 @@ +package org.folio.linked.data.util; + +import static org.apache.commons.collections4.CollectionUtils.isEmpty; + +import lombok.experimental.UtilityClass; +import org.folio.linked.data.domain.dto.LccnRequest; + +@UtilityClass +public class LccnUtils { + + public static boolean isCurrent(LccnRequest lccnRequest) { + return isEmpty(lccnRequest.getStatus()) || lccnRequest.getStatus() + .stream() + .flatMap(status -> status.getLink().stream()) + .anyMatch(link -> link.endsWith("current")); + } +} diff --git a/src/main/java/org/folio/linked/data/validation/LccnPatternConstraint.java b/src/main/java/org/folio/linked/data/validation/LccnPatternConstraint.java index f02c185a..9d59a857 100644 --- a/src/main/java/org/folio/linked/data/validation/LccnPatternConstraint.java +++ b/src/main/java/org/folio/linked/data/validation/LccnPatternConstraint.java @@ -10,6 +10,7 @@ import org.folio.linked.data.validation.dto.LccnPatternValidator; @Documented +@SuppressWarnings("javaarchitecture:S7091") @Constraint(validatedBy = LccnPatternValidator.class) @Target({ElementType.FIELD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) diff --git a/src/main/java/org/folio/linked/data/validation/LccnUniqueConstraint.java b/src/main/java/org/folio/linked/data/validation/LccnUniqueConstraint.java new file mode 100644 index 00000000..61e8f989 --- /dev/null +++ b/src/main/java/org/folio/linked/data/validation/LccnUniqueConstraint.java @@ -0,0 +1,24 @@ +package org.folio.linked.data.validation; + +import jakarta.validation.Constraint; +import jakarta.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.folio.linked.data.validation.dto.LccnUniquenessValidator; + +@Documented +@SuppressWarnings("javaarchitecture:S7091") +@Constraint(validatedBy = LccnUniquenessValidator.class) +@Target({ElementType.FIELD, ElementType.TYPE, ElementType.PARAMETER}) +@Retention(RetentionPolicy.RUNTIME) +public @interface LccnUniqueConstraint { + + String message() default "{lccnUniqueConstraint.message}"; + + Class[] groups() default {}; + + Class[] payload() default {}; +} diff --git a/src/main/java/org/folio/linked/data/validation/dto/LccnPatternValidator.java b/src/main/java/org/folio/linked/data/validation/dto/LccnPatternValidator.java index e994b1a0..8027fd06 100644 --- a/src/main/java/org/folio/linked/data/validation/dto/LccnPatternValidator.java +++ b/src/main/java/org/folio/linked/data/validation/dto/LccnPatternValidator.java @@ -1,6 +1,6 @@ package org.folio.linked.data.validation.dto; -import static org.apache.commons.collections4.CollectionUtils.isEmpty; +import static org.folio.linked.data.util.LccnUtils.isCurrent; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; @@ -32,13 +32,6 @@ public boolean isValid(LccnRequest lccnRequest, ConstraintValidatorContext const } } - private boolean isCurrent(LccnRequest lccnRequest) { - return isEmpty(lccnRequest.getStatus()) || lccnRequest.getStatus() - .stream() - .flatMap(status -> status.getLink().stream()) - .anyMatch(link -> link.endsWith("current")); - } - private boolean isLccnFormatValidationEnabled() { return specProvider.getSpecRules() .stream() diff --git a/src/main/java/org/folio/linked/data/validation/dto/LccnUniquenessValidator.java b/src/main/java/org/folio/linked/data/validation/dto/LccnUniquenessValidator.java new file mode 100644 index 00000000..07ea691a --- /dev/null +++ b/src/main/java/org/folio/linked/data/validation/dto/LccnUniquenessValidator.java @@ -0,0 +1,87 @@ +package org.folio.linked.data.validation.dto; + +import static java.util.Optional.ofNullable; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import java.util.Collection; +import java.util.List; +import java.util.stream.Stream; +import lombok.RequiredArgsConstructor; +import lombok.extern.log4j.Log4j2; +import org.folio.linked.data.domain.dto.InstanceField; +import org.folio.linked.data.domain.dto.LccnField; +import org.folio.linked.data.domain.dto.LccnRequest; +import org.folio.linked.data.domain.dto.ResourceRequestDto; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; +import org.folio.linked.data.exception.RequestProcessingExceptionBuilder; +import org.folio.linked.data.repo.FolioMetadataRepository; +import org.folio.linked.data.service.search.InstanceSearchService; +import org.folio.linked.data.util.LccnUtils; +import org.folio.linked.data.validation.LccnUniqueConstraint; +import org.springframework.stereotype.Component; + +@Component +@RequiredArgsConstructor +@Log4j2 +public class LccnUniquenessValidator implements ConstraintValidator { + + private final InstanceSearchService instanceSearchService; + private final FolioMetadataRepository folioMetadataRepository; + private final RequestProcessingExceptionBuilder exceptionBuilder; + + @Override + public boolean isValid(ResourceRequestDto resourceRequestDto, ConstraintValidatorContext constraintValidatorContext) { + var resource = resourceRequestDto.getResource(); + if (resource instanceof InstanceField instance && hasCurrentLccn(instance)) { + var inventoryId = findInventoryId(resourceRequestDto); + return isUnique(getLccnValues(instance), inventoryId); + } + return true; + } + + private String findInventoryId(ResourceRequestDto resourceRequestDto) { + return ofNullable(resourceRequestDto.getId()) + .flatMap(folioMetadataRepository::findInventoryIdById) + .map(FolioMetadataRepository.InventoryIdOnly::getInventoryId) + .orElse(null); + } + + private boolean isUnique(List lccn, String inventoryId) { + return ofNullable(findInstanceWithLccn(lccn, inventoryId)) + .map(SearchResponseTotalOnly::getTotalRecords) + .map(count -> count == 0) + .orElse(false); + } + + private SearchResponseTotalOnly findInstanceWithLccn(List lccn, String inventoryId) { + try { + return instanceSearchService.searchByLccnExcludingId(lccn, inventoryId); + } catch (Exception e) { + log.error(e); + throw exceptionBuilder.failedDependencyException( + "Could not validate LCCN for duplicate", "Unable to reach search service"); + } + } + + private boolean hasCurrentLccn(InstanceField instance) { + return getLccnRequest(instance).anyMatch(LccnUtils::isCurrent); + } + + private List getLccnValues(InstanceField instance) { + return getLccnRequest(instance) + .filter(LccnUtils::isCurrent) + .map(LccnRequest::getValue) + .flatMap(Collection::stream) + .toList(); + } + + private Stream getLccnRequest(InstanceField instance) { + return instance.getInstance() + .getMap() + .stream() + .filter(LccnField.class::isInstance) + .map(i -> (LccnField) i) + .map(LccnField::getLccn); + } +} diff --git a/src/main/resources/ValidationMessages.properties b/src/main/resources/ValidationMessages.properties index b99896a0..9a2b8efc 100644 --- a/src/main/resources/ValidationMessages.properties +++ b/src/main/resources/ValidationMessages.properties @@ -1,2 +1,3 @@ primaryTitleConstraint.message=required_primary_main_title lccnPatternConstraint.message=lccn_does_not_match_pattern +lccnUniqueConstraint.message=lccn_not_unique diff --git a/src/main/resources/errors.yml b/src/main/resources/errors.yml index a349de83..cc7dc4c1 100644 --- a/src/main/resources/errors.yml +++ b/src/main/resources/errors.yml @@ -37,3 +37,8 @@ genericServer: code: server parameters: exception, message message: 'Server error: [%s] with message [%s]' +failedDependency: + status: 424 + code: failed_dependency + parameters: message, reason + message: '[%s] - reason: [%s]. Please try later.' diff --git a/src/main/resources/swagger.api/folio-modules.yaml b/src/main/resources/swagger.api/folio-modules.yaml index 7893fcf8..c08c0ada 100644 --- a/src/main/resources/swagger.api/folio-modules.yaml +++ b/src/main/resources/swagger.api/folio-modules.yaml @@ -12,6 +12,8 @@ components: $ref: folio-modules/search/resourceIndexEvent.json resourceIndexEventType: $ref: folio-modules/search/resourceIndexEventType.json + searchResponseTotalOnly: + $ref: folio-modules/search/searchResponseTotalOnly.json linkedDataInstance: $ref: folio-modules/search/linkedDataInstance.json linkedDataWork: diff --git a/src/main/resources/swagger.api/folio-modules/search/searchResponseTotalOnly.json b/src/main/resources/swagger.api/folio-modules/search/searchResponseTotalOnly.json new file mode 100644 index 00000000..45367fd5 --- /dev/null +++ b/src/main/resources/swagger.api/folio-modules/search/searchResponseTotalOnly.json @@ -0,0 +1,12 @@ +{ + "description": "Wrapper for search response", + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "properties": { + "totalRecords": { + "description": "Total number of records", + "type": "integer", + "format": "int64" + } + } +} diff --git a/src/main/resources/swagger.api/schema/resourceRequestDto.json b/src/main/resources/swagger.api/schema/resourceRequestDto.json index 96ae316f..bfbb32de 100644 --- a/src/main/resources/swagger.api/schema/resourceRequestDto.json +++ b/src/main/resources/swagger.api/schema/resourceRequestDto.json @@ -1,8 +1,14 @@ { "$schema": "http://json-schema.org/draft-04/schema#", "description": "Resource request DTO", + "x-class-extra-annotation": "@org.folio.linked.data.validation.LccnUniqueConstraint", "type": "object", "properties": { + "id":{ + "type": "integer", + "format": "int64", + "description": "ID of the resource" + }, "resource": { "type": "object", "title": "resourceRequestField", diff --git a/src/test/java/org/folio/linked/data/e2e/resource/ResourceControllerITBase.java b/src/test/java/org/folio/linked/data/e2e/resource/ResourceControllerITBase.java index 34294b7b..cc35f291 100644 --- a/src/test/java/org/folio/linked/data/e2e/resource/ResourceControllerITBase.java +++ b/src/test/java/org/folio/linked/data/e2e/resource/ResourceControllerITBase.java @@ -135,6 +135,8 @@ import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -155,11 +157,13 @@ import org.folio.ld.dictionary.PredicateDictionary; import org.folio.ld.dictionary.PropertyDictionary; import org.folio.ld.dictionary.ResourceTypeDictionary; +import org.folio.linked.data.client.SearchClient; import org.folio.linked.data.client.SpecClient; import org.folio.linked.data.client.SrsClient; import org.folio.linked.data.domain.dto.InstanceResponseField; import org.folio.linked.data.domain.dto.ResourceIndexEventType; import org.folio.linked.data.domain.dto.ResourceResponseDto; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; import org.folio.linked.data.domain.dto.WorkResponseField; import org.folio.linked.data.model.entity.FolioMetadata; import org.folio.linked.data.model.entity.PredicateEntity; @@ -183,6 +187,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.core.env.Environment; +import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; import org.springframework.jdbc.core.JdbcTemplate; @@ -191,6 +196,8 @@ public abstract class ResourceControllerITBase { + public static final String LCCN_VALIDATION_NOT_AVAILABLE = + "[Could not validate LCCN for duplicate] - reason: [Unable to reach search service]. Please try later."; public static final String RESOURCE_URL = "/resource"; private static final String ROLES_PROPERTY = "roles"; private static final String NOTES_PROPERTY = "_notes"; @@ -223,6 +230,8 @@ public abstract class ResourceControllerITBase { private SrsClient srsClient; @MockBean private SpecClient specClient; + @MockBean + private SearchClient searchClient; @BeforeEach public void beforeEach() { @@ -279,6 +288,8 @@ void createInstanceWithWorkRef_shouldReturn400_ifLccnIsInvalid() throws Exceptio .replaceAll(WORK_ID_PLACEHOLDER, work.getId().toString()) .replace("lccn status link", "http://id.loc.gov/vocabulary/mstatus/current") ); + when(searchClient.searchInstances(any())) + .thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(0L), HttpStatus.OK)); // when var resultActions = mockMvc.perform(requestBuilder); @@ -292,6 +303,102 @@ void createInstanceWithWorkRef_shouldReturn400_ifLccnIsInvalid() throws Exceptio .andExpect(jsonPath("total_records", equalTo(1))); } + @Test + void createInstanceWithWorkRef_shouldReturn424_ifSearchServiceThrownException() throws Exception { + // given + var work = getSampleWork(null); + setExistingResourcesIds(work); + resourceTestService.saveGraph(work); + var requestBuilder = post(RESOURCE_URL) + .contentType(APPLICATION_JSON) + .headers(defaultHeaders(env)) + .content(INSTANCE_WITH_WORK_REF_SAMPLE + .replaceAll(WORK_ID_PLACEHOLDER, work.getId().toString()) + .replace("lccn status link", "http://id.loc.gov/vocabulary/mstatus/current") + .replace("lccn value", "nn0123456789") + ); + var query = "(lccn==\"nn0123456789\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")"; + when(searchClient.searchInstances(any())).thenThrow(new RuntimeException()); + + // when + var resultActions = mockMvc.perform(requestBuilder); + + // then + resultActions + .andExpect(status().isFailedDependency()) + .andExpect(content().contentType(APPLICATION_JSON)) + .andExpect(jsonPath("errors[0].code", equalTo("failed_dependency"))) + .andExpect(jsonPath("errors[0].message", equalTo(LCCN_VALIDATION_NOT_AVAILABLE))) + .andExpect(jsonPath("total_records", equalTo(1))); + verify(searchClient).searchInstances(query); + } + + @Test + void createInstanceWithWorkRef_shouldReturn400_ifLccnIsNotUnique() throws Exception { + // given + var work = getSampleWork(null); + setExistingResourcesIds(work); + resourceTestService.saveGraph(work); + var requestBuilder = post(RESOURCE_URL) + .contentType(APPLICATION_JSON) + .headers(defaultHeaders(env)) + .content(INSTANCE_WITH_WORK_REF_SAMPLE + .replaceAll(WORK_ID_PLACEHOLDER, work.getId().toString()) + .replace("lccn status link", "http://id.loc.gov/vocabulary/mstatus/current") + .replace("lccn value", "nn0123456789") + ); + var query = "(lccn==\"nn0123456789\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")"; + when(searchClient.searchInstances(query)) + .thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK)); + + // when + var resultActions = mockMvc.perform(requestBuilder); + + // then + resultActions + .andExpect(status().isBadRequest()) + .andExpect(content().contentType(APPLICATION_JSON)) + .andExpect(jsonPath("errors[0].code", equalTo("lccn_not_unique"))) + .andExpect(jsonPath("total_records", equalTo(1))); + verify(searchClient).searchInstances(query); + } + + @Test + void update_shouldReturn400_ifLccnIsNotUnique() throws Exception { + // given + var updateDto = getSampleInstanceDtoMap(); + var instance = (LinkedHashMap) ((LinkedHashMap) updateDto.get("resource")).get(INSTANCE.getUri()); + instance.remove("inventoryId"); + instance.remove("srsId"); + var status = getStatus(instance); + ((LinkedHashMap) status.get(0)).put(LINK.getValue(), List.of("http://id.loc.gov/vocabulary/mstatus/current")); + var work = getSampleWork(null); + var originalInstance = resourceTestService.saveGraph(getSampleInstanceResource(null, work)); + + var updateRequest = put(RESOURCE_URL + "/" + originalInstance.getId()) + .contentType(APPLICATION_JSON) + .headers(defaultHeaders(env)) + .content( + OBJECT_MAPPER.writeValueAsString(updateDto).replaceAll(WORK_ID_PLACEHOLDER, work.getId().toString()) + .replace("lccn value", "nn0123456789") + ); + var query = "(lccn==\"nn0123456789\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")" + + " and id <> \"2165ef4b-001f-46b3-a60e-52bcdeb3d5a1\""; + when(searchClient.searchInstances(query)) + .thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK)); + + // when + var resultActions = mockMvc.perform(updateRequest); + + // then + resultActions + .andExpect(status().isBadRequest()) + .andExpect(content().contentType(APPLICATION_JSON)) + .andExpect(jsonPath("errors[0].code", equalTo("lccn_not_unique"))) + .andExpect(jsonPath("total_records", equalTo(1))); + verify(searchClient).searchInstances(query); + } + @Test void createWorkWithInstanceRef_shouldCreateAuthorityFromSrs() throws Exception { // given @@ -466,6 +573,8 @@ void update_shouldReturn400_ifLccnIsInvalid() throws Exception { .content( OBJECT_MAPPER.writeValueAsString(updateDto).replaceAll(WORK_ID_PLACEHOLDER, work.getId().toString()) ); + when(searchClient.searchInstances(any())) + .thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(0L), HttpStatus.OK)); // when var resultActions = mockMvc.perform(updateRequest); diff --git a/src/test/java/org/folio/linked/data/service/search/InstanceSearchServiceImplTest.java b/src/test/java/org/folio/linked/data/service/search/InstanceSearchServiceImplTest.java new file mode 100644 index 00000000..aa5eb3fd --- /dev/null +++ b/src/test/java/org/folio/linked/data/service/search/InstanceSearchServiceImplTest.java @@ -0,0 +1,86 @@ +package org.folio.linked.data.service.search; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.params.provider.Arguments.arguments; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import feign.FeignException; +import java.util.List; +import java.util.stream.Stream; +import org.folio.linked.data.client.SearchClient; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; +import org.folio.spring.testing.type.UnitTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; + +@UnitTest +@ExtendWith(MockitoExtension.class) +class InstanceSearchServiceImplTest { + + private static final String EXPECTED_QUERY = + "(lccn==\"12\" or lccn==\"34\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")"; + private static final String EXPECTED_QUERY_ID_EXCLUDED = EXPECTED_QUERY + " and id <> \"1234\""; + + @InjectMocks + private InstanceSearchServiceImpl searchService; + + @Mock + private SearchClient searchClient; + + @Test + void shouldReturnZeroTotalRecords_ifSearchClientThrowsNotFound() { + // given + when(searchClient.searchInstances(any())).thenThrow(FeignException.NotFound.class); + var lccn = List.of(""); + + // expect + assertThrows(FeignException.NotFound.class, () -> searchService.searchByLccn(lccn)); + } + + @Test + void searchByLccn_shouldReturnResponseWithTotalRecords() { + // given + var lccns = List.of("12", "34"); + var expectedResponse = new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK); + when(searchClient.searchInstances(EXPECTED_QUERY)).thenReturn(expectedResponse); + + // when + var actualResponse = searchService.searchByLccn(lccns); + + // then + assertEquals(expectedResponse.getBody(), actualResponse); + } + + + @ParameterizedTest + @MethodSource("queryArgumentsProvider") + void searchByLccnExcludingId_shouldReturnResponseWithTotalRecords(String inventoryId, String expectedQuery) { + // given + var lccns = List.of("12", "34"); + var expectedResponse = new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK); + when(searchClient.searchInstances(expectedQuery)).thenReturn(expectedResponse); + + // when + var actualResponse = searchService.searchByLccnExcludingId(lccns, inventoryId); + + // then + assertEquals(expectedResponse.getBody(), actualResponse); + } + + public static Stream queryArgumentsProvider() { + return Stream.of( + arguments("1234", EXPECTED_QUERY_ID_EXCLUDED), + arguments(null, EXPECTED_QUERY) + ); + } +} diff --git a/src/test/java/org/folio/linked/data/validation/dto/LccnUniquenessValidatorTest.java b/src/test/java/org/folio/linked/data/validation/dto/LccnUniquenessValidatorTest.java new file mode 100644 index 00000000..b1eb5b63 --- /dev/null +++ b/src/test/java/org/folio/linked/data/validation/dto/LccnUniquenessValidatorTest.java @@ -0,0 +1,175 @@ +package org.folio.linked.data.validation.dto; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Optional; +import org.folio.linked.data.domain.dto.InstanceField; +import org.folio.linked.data.domain.dto.InstanceRequest; +import org.folio.linked.data.domain.dto.LccnField; +import org.folio.linked.data.domain.dto.LccnRequest; +import org.folio.linked.data.domain.dto.ResourceRequestDto; +import org.folio.linked.data.domain.dto.SearchResponseTotalOnly; +import org.folio.linked.data.domain.dto.Status; +import org.folio.linked.data.domain.dto.WorkField; +import org.folio.linked.data.exception.RequestProcessingException; +import org.folio.linked.data.exception.RequestProcessingExceptionBuilder; +import org.folio.linked.data.repo.FolioMetadataRepository; +import org.folio.linked.data.service.search.InstanceSearchService; +import org.folio.spring.testing.type.UnitTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; + +@UnitTest +@ExtendWith(MockitoExtension.class) +class LccnUniquenessValidatorTest { + + @InjectMocks + private LccnUniquenessValidator validator; + + @Mock + private InstanceSearchService instanceSearchService; + + @Mock + private FolioMetadataRepository folioMetadataRepository; + + @Mock + private RequestProcessingExceptionBuilder exceptionBuilder; + + @Test + void shouldThrowFailedDependency_ifSearchServiceThrownException() { + // given + var requestDto = new ResourceRequestDto() + .id(123L) + .resource(new InstanceField().instance(createInstanceRequest(createLccnRequest()))); + when(instanceSearchService.searchByLccnExcludingId(any(), any())) + .thenThrow(new RuntimeException("msg")); + when(exceptionBuilder.failedDependencyException(any(), any())) + .thenReturn(new RequestProcessingException(HttpStatus.FAILED_DEPENDENCY.value(), "failed", null, "msg")); + + // expect + assertThrows(RequestProcessingException.class, + () -> validator.isValid(requestDto, null), "msg"); + } + + @Test + void shouldReturnTrue_ifResourceIsNotInstanceField() { + // given + var requestDto = new ResourceRequestDto().resource(new WorkField()); + + // expect + assertTrue(validator.isValid(requestDto, null)); + } + + @Test + void shouldReturnTrue_ifResourceDoesNotHaveCurrentLccn() { + // given + var requestDto = new ResourceRequestDto().resource( + new InstanceField().instance(createInstanceRequest())); + + // expect + assertTrue(validator.isValid(requestDto, null)); + } + + @Test + void shouldReturnTrue_ifResourceNotExistsInRepoAndInSearch() { + // given + var requestDto = new ResourceRequestDto() + .id(123L) + .resource(new InstanceField().instance(createInstanceRequest(createLccnRequest()))); + when(folioMetadataRepository.findInventoryIdById(123L)).thenReturn(Optional.empty()); + when(instanceSearchService.searchByLccnExcludingId(any(), any())) + .thenReturn(new SearchResponseTotalOnly().totalRecords(0L)); + + // when + boolean isValid = validator.isValid(requestDto, null); + + // then + assertTrue(isValid); + verify(folioMetadataRepository).findInventoryIdById(123L); + verify(instanceSearchService).searchByLccnExcludingId(any(), any()); + } + + @Test + void shouldReturnTrue_ifResourceExistsInRepoButNotInSearch() { + // given + var requestDto = new ResourceRequestDto() + .id(123L) + .resource(new InstanceField().instance(createInstanceRequest(createLccnRequest()))); + when(folioMetadataRepository.findInventoryIdById(123L)) + .thenReturn(Optional.of(() -> "6dcb9a08-9884-4a15-b990-89c879a8e999")); + when(instanceSearchService.searchByLccnExcludingId(any(), any())) + .thenReturn(new SearchResponseTotalOnly().totalRecords(0L)); + + // when + boolean isValid = validator.isValid(requestDto, null); + + // then + assertTrue(isValid); + verify(folioMetadataRepository).findInventoryIdById(123L); + verify(instanceSearchService).searchByLccnExcludingId(any(), any()); + } + + @Test + void shouldReturnFalse_ifResourceExistsInRepoAndSearch() { + // given + var requestDto = new ResourceRequestDto() + .id(123L) + .resource(new InstanceField().instance(createInstanceRequest(createLccnRequest()))); + when(folioMetadataRepository.findInventoryIdById(123L)) + .thenReturn(Optional.of(() -> "6dcb9a08-9884-4a15-b990-89c879a8e999")); + when(instanceSearchService.searchByLccnExcludingId(any(), any())) + .thenReturn(new SearchResponseTotalOnly().totalRecords(1L)); + + // when + boolean isValid = validator.isValid(requestDto, null); + + // then + assertFalse(isValid); + verify(folioMetadataRepository).findInventoryIdById(123L); + verify(instanceSearchService).searchByLccnExcludingId(any(), any()); + } + + @Test + void shouldReturnFalse_ifResourceNotExistsInReposButExistsInSearch() { + // given + var requestDto = new ResourceRequestDto() + .id(123L) + .resource(new InstanceField().instance(createInstanceRequest(createLccnRequest()))); + when(folioMetadataRepository.findInventoryIdById(123L)).thenReturn(Optional.empty()); + when(instanceSearchService.searchByLccnExcludingId(any(), any())) + .thenReturn(new SearchResponseTotalOnly().totalRecords(1L)); + + // when + boolean isValid = validator.isValid(requestDto, null); + + // then + assertFalse(isValid); + verify(folioMetadataRepository).findInventoryIdById(123L); + verify(instanceSearchService).searchByLccnExcludingId(any(), any()); + } + + private InstanceRequest createInstanceRequest(LccnRequest lccnRequest) { + return new InstanceRequest(List.of()) + .addMapItem(new LccnField().lccn(lccnRequest)); + } + + private InstanceRequest createInstanceRequest() { + return new InstanceRequest(List.of()); + } + + private LccnRequest createLccnRequest() { + return new LccnRequest() + .value(List.of("n-0123456789")) + .status(List.of(new Status().link(List.of("http://id.loc.gov/vocabulary/mstatus/current")))); + } +}