Skip to content

Commit

Permalink
MODLD-582: LCCN Validation | Ensure uniquely assigned
Browse files Browse the repository at this point in the history
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
  • Loading branch information
AndreiBordak committed Nov 25, 2024
1 parent 1eb09ea commit f543e6f
Show file tree
Hide file tree
Showing 22 changed files with 660 additions and 8 deletions.
6 changes: 6 additions & 0 deletions descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -62,6 +63,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",
Expand Down Expand Up @@ -172,6 +174,10 @@
{
"id": "source-storage-records",
"version": "3.2 3.3"
},
{
"id": "search",
"version": "1.3"
}
],
"permissionSets": [
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/org/folio/linked/data/client/SearchClient.java
Original file line number Diff line number Diff line change
@@ -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<SearchResponseTotalOnly> searchInstances(@RequestParam("query") String query);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> parameters, String message) {
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -70,4 +75,22 @@ public ResponseEntity<ResourceMarcViewDto> 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]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -88,6 +89,18 @@ public ResponseEntity<ErrorResponse> handleMissingServletRequestParameterExcepti
return genericBadRequestMapper.errorResponseEntity(e);
}

@ExceptionHandler(ValidationException.class)
public ResponseEntity<ErrorResponse> handleValidationException(ValidationException exception) {
return handleValidationExceptionCause(exception);
}

private ResponseEntity<ErrorResponse> handleValidationExceptionCause(ValidationException exception) {
if (exception.getCause() instanceof RequestProcessingException e) {
return handleRequestProcessingException(e);
}
return handleAllOtherExceptions(exception);
}

@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorResponse> handleAllOtherExceptions(Exception exception) {
logException(exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String>();
for (int i = 0; i < arguments.length; i++) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.folio.linked.data.exception;

public class SearchException extends RuntimeException {

public SearchException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ public interface FolioMetadataRepository extends JpaRepository<FolioMetadata, Lo

Optional<FolioMetadata> findByInventoryId(String inventoryId);

Optional<InventoryIdOnly> findInventoryIdById(Long id);

interface IdOnly {
Long getId();
}

interface InventoryIdOnly {
String getInventoryId();
}
}
Original file line number Diff line number Diff line change
@@ -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<String> lccn);

SearchResponseTotalOnly searchByLccnExcludingId(Collection<String> lccn, String instanceId);
}
Original file line number Diff line number Diff line change
@@ -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<String> lccn) {
return search(getLccnQuery(lccn));
}

@Override
public SearchResponseTotalOnly searchByLccnExcludingId(Collection<String> 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<String> 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);
}
}
17 changes: 17 additions & 0 deletions src/main/java/org/folio/linked/data/util/LccnUtils.java
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.folio.linked.data.validation;

import jakarta.validation.Constraint;
import jakarta.validation.Payload;
import jakarta.validation.Valid;
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
@Constraint(validatedBy = LccnUniquenessValidator.class)
@Target({ElementType.FIELD, ElementType.TYPE, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
@Valid
public @interface LccnUniqueConstraint {

String message() default "{lccnUniqueConstraint.message}";

Class<?>[] groups() default {};

Class<? extends Payload>[] payload() default {};
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -24,13 +24,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 hasValidPattern(String lccnValue) {
return LCCN_STRUCTURE_A_PATTERN.matcher(lccnValue).matches()
|| LCCN_STRUCTURE_B_PATTERN.matcher(lccnValue).matches();
Expand Down
Original file line number Diff line number Diff line change
@@ -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<LccnUniqueConstraint, ResourceRequestDto> {

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<String> lccn, String inventoryId) {
return ofNullable(findInstanceWithLccn(lccn, inventoryId))
.map(SearchResponseTotalOnly::getTotalRecords)
.map(count -> count == 0)
.orElse(false);
}

private SearchResponseTotalOnly findInstanceWithLccn(List<String> 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<String> getLccnValues(InstanceField instance) {
return getLccnRequest(instance)
.filter(LccnUtils::isCurrent)
.map(LccnRequest::getValue)
.flatMap(Collection::stream)
.toList();
}

private Stream<LccnRequest> getLccnRequest(InstanceField instance) {
return instance.getInstance()
.getMap()
.stream()
.filter(LccnField.class::isInstance)
.map(i -> (LccnField) i)
.map(LccnField::getLccn);
}
}
1 change: 1 addition & 0 deletions src/main/resources/ValidationMessages.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
primaryTitleConstraint.message=required_primary_main_title
lccnPatternConstraint.message=lccn_does_not_match_pattern
lccnUniqueConstraint.message=lccn_not_unique
5 changes: 5 additions & 0 deletions src/main/resources/errors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
2 changes: 2 additions & 0 deletions src/main/resources/swagger.api/folio-modules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit f543e6f

Please sign in to comment.