Skip to content

Commit

Permalink
MODLD-601: LCCN deduplication configuration setting
Browse files Browse the repository at this point in the history
MODLD-601: Review fixes

MODLD-601: Review fixes

MODLD-601: Change log level for settings service outage
  • Loading branch information
AndreiBordak committed Nov 26, 2024
1 parent 440424c commit 4d1c0c7
Show file tree
Hide file tree
Showing 15 changed files with 281 additions and 10 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ To run mod-linked-data in standalone mode, set the value of the environment vari
| KAFKA_WORK_SEARCH_INDEX_TOPIC_PARTITIONS`*` | 1 | Custom Work Search Index topic partitions number |
| KAFKA_WORK_SEARCH_INDEX_TOPIC_REPLICATION_FACTOR`*` | - | Custom Work Search Index topic replication factor |
| KAFKA_INVENTORY_INSTANCE_INGRESS_EVENT_TOPIC`*` | inventory.instance_ingress | Custom Inventory Instance Ingress Event topic name |
| CACHE_TTL_SPEC_RULES_MS | 18000000 | Specifies time to live for `spec-rules` cache |
| CACHE_TTL_SPEC_RULES_MS | 18000000 | Specifies time to live for `spec-rules` cache |
| CACHE_TTL_SETTINGS_ENTRIES_MS | 18000000 | Specifies time to live for `settings-entries` cache |
* Applicable only in FOLIO mode
## REST API
Full list of APIs are documented in [src/main/resources/swagger.api/mod-linked-data.yaml](https://github.com/folio-org/mod-linked-data/blob/master/src/main/resources/swagger.api/mod-linked-data.yaml).
Expand Down
9 changes: 6 additions & 3 deletions descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
"source-storage.records.post",
"source-storage.records.put",
"specification-storage.specifications.collection.get",
"specification-storage.specification.rules.collection.get"
"specification-storage.specification.rules.collection.get",
"ui-quick-marc.settings.lccn-duplicate-check.edit"
]
},
{
Expand Down Expand Up @@ -71,7 +72,8 @@
"source-storage.records.post",
"source-storage.records.put",
"specification-storage.specifications.collection.get",
"specification-storage.specification.rules.collection.get"
"specification-storage.specification.rules.collection.get",
"ui-quick-marc.settings.lccn-duplicate-check.edit"
]
},
{
Expand Down Expand Up @@ -292,7 +294,8 @@
{ "name": "KAFKA_WORK_SEARCH_INDEX_TOPIC_PARTITIONS", "value": "1" },
{ "name": "KAFKA_WORK_SEARCH_INDEX_TOPIC_REPLICATION_FACTOR", "value": "" },
{ "name": "KAFKA_INVENTORY_INSTANCE_INGRESS_EVENT_TOPIC", "value": "" },
{ "name": "CACHE_TTL_SPEC_RULES_MS", "value": "18000000" }
{ "name": "CACHE_TTL_SPEC_RULES_MS", "value": "18000000" },
{ "name": "CACHE_TTL_SETTINGS_ENTRIES_MS", "value": "18000000" }
]
}
}
18 changes: 18 additions & 0 deletions src/main/java/org/folio/linked/data/client/SettingsClient.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.folio.linked.data.client;

import static org.folio.linked.data.util.Constants.Cache.SETTINGS_ENTRIES;

import org.folio.linked.data.domain.dto.SettingsSearchResponse;
import org.springframework.cache.annotation.Cacheable;
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("settings")
public interface SettingsClient {

@Cacheable(cacheNames = SETTINGS_ENTRIES, key = "#query")
@GetMapping("/entries")
ResponseEntity<SettingsSearchResponse> getEntries(@RequestParam("query") String query);
}
7 changes: 7 additions & 0 deletions src/main/java/org/folio/linked/data/job/CacheCleaningJob.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.folio.linked.data.job;

import static org.folio.linked.data.util.Constants.Cache.SETTINGS_ENTRIES;
import static org.folio.linked.data.util.Constants.Cache.SPEC_RULES;

import lombok.extern.slf4j.Slf4j;
Expand All @@ -16,4 +17,10 @@ public class CacheCleaningJob {
public void emptySpecRules() {
log.info("Emptying {} cache", SPEC_RULES);
}

@CacheEvict(value = SETTINGS_ENTRIES, allEntries = true)
@Scheduled(fixedRateString = "${mod-linked-data.cache.ttl.settings-entries}")
public void emptySettingsEntries() {
log.info("Emptying {} cache", SETTINGS_ENTRIES);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.folio.linked.data.service;

public interface SettingsService {

boolean isSettingEnabled(String scope, String key, String property);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.folio.linked.data.service;

import static java.lang.String.format;
import static java.util.Optional.ofNullable;

import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.folio.linked.data.client.SettingsClient;
import org.folio.linked.data.domain.dto.SettingsItem;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;

@Service
@RequiredArgsConstructor
public class SettingsServiceImpl implements SettingsService {

private static final String SCOPE_AND_KEY_PATTERN = "(scope==%s and key==%s)";

private final SettingsClient settingsClient;

@Override
public boolean isSettingEnabled(String scope, String key, String property) {
return getSetting(scope, key)
.map(item -> isPropertyEnabled(item, property))
.orElse(false);
}

private Optional<SettingsItem> getSetting(String scope, String key) {
return ofNullable(settingsClient.getEntries(buildQuery(scope, key)))
.map(ResponseEntity::getBody)
.flatMap(body -> body.getItems().stream().findFirst());
}

private boolean isPropertyEnabled(SettingsItem item, String property) {
var propertyValue = item.getValue().getOrDefault(property, false);
return propertyValue instanceof Boolean value && value;
}

private String buildQuery(String scope, String key) {
return format(SCOPE_AND_KEY_PATTERN, scope, key);
}
}
1 change: 1 addition & 0 deletions src/main/java/org/folio/linked/data/util/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ public class Constants {
@UtilityClass
public static class Cache {
public static final String SPEC_RULES = "spec-rules";
public static final String SETTINGS_ENTRIES = "settings-entries";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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.SettingsService;
import org.folio.linked.data.service.search.InstanceSearchService;
import org.folio.linked.data.util.LccnUtils;
import org.folio.linked.data.validation.LccnUniqueConstraint;
Expand All @@ -27,12 +28,24 @@
@SuppressWarnings("javaarchitecture:S7091")
public class LccnUniquenessValidator implements ConstraintValidator<LccnUniqueConstraint, ResourceRequestDto> {

private static final String DUPLICATE_CHECK_SCOPE = "ui-quick-marc.lccn-duplicate-check.manage";
private static final String DUPLICATE_CHECK_KEY = "lccn-duplicate-check";
private static final String DUPLICATE_CHECK_PROPERTY = "duplicateLccnCheckingEnabled";

private final InstanceSearchService instanceSearchService;
private final FolioMetadataRepository folioMetadataRepository;
private final RequestProcessingExceptionBuilder exceptionBuilder;
private final SettingsService settingsService;

@Override
public boolean isValid(ResourceRequestDto resourceRequestDto, ConstraintValidatorContext constraintValidatorContext) {
if (isValidationDisabled()) {
return true;
}
return isValid(resourceRequestDto);
}

private boolean isValid(ResourceRequestDto resourceRequestDto) {
var resource = resourceRequestDto.getResource();
if (resource instanceof InstanceField instance && hasCurrentLccn(instance)) {
var inventoryId = findInventoryId(resourceRequestDto);
Expand All @@ -41,6 +54,15 @@ public boolean isValid(ResourceRequestDto resourceRequestDto, ConstraintValidato
return true;
}

private boolean isValidationDisabled() {
try {
return !settingsService.isSettingEnabled(DUPLICATE_CHECK_SCOPE, DUPLICATE_CHECK_KEY, DUPLICATE_CHECK_PROPERTY);
} catch (Exception e) {
log.error("An exception occurred during settings service call", e);
return true;
}
}

private String findInventoryId(ResourceRequestDto resourceRequestDto) {
return ofNullable(resourceRequestDto.getId())
.flatMap(folioMetadataRepository::findInventoryIdById)
Expand Down
3 changes: 2 additions & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ mod-linked-data:
instance-ingress: ${KAFKA_INVENTORY_INSTANCE_INGRESS_EVENT_TOPIC:inventory.instance_ingress}
cache:
ttl:
spec-rules: ${CACHE_TTL_SPEC_RULES:18000000}
spec-rules: ${CACHE_TTL_SPEC_RULES_MS:18000000}
settings-entries: ${CACHE_TTL_SETTINGS_ENTRIES_MS:18000000}

management:
endpoints:
Expand Down
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 @@ -14,6 +14,8 @@ components:
$ref: folio-modules/search/resourceIndexEventType.json
searchResponseTotalOnly:
$ref: folio-modules/search/searchResponseTotalOnly.json
settingsSearchResponse:
$ref: folio-modules/search/settingsSearchResponse.json
linkedDataInstance:
$ref: folio-modules/search/linkedDataInstance.json
linkedDataWork:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"description": "Settings item containing map of properties",
"$schema": "http://json-schema.org/draft-04/schema#",
"type": "object",
"properties": {
"value": {
"type": "object",
"additionalProperties": {
"type": "object"
},
"description": "Map of setting properties"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"description": "Wrapper for settings search response",
"$schema": "http://json-schema.org/draft-04/schema#",
"type": "object",
"properties": {
"items": {
"description": "List of settings items",
"type": "array",
"items": {
"$ref": "settingsItem.json"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@
import org.folio.linked.data.model.entity.Resource;
import org.folio.linked.data.model.entity.ResourceEdge;
import org.folio.linked.data.model.entity.ResourceTypeEntity;
import org.folio.linked.data.service.SettingsService;
import org.folio.linked.data.service.resource.hash.HashService;
import org.folio.linked.data.test.ResourceTestService;
import org.folio.linked.data.test.TestUtil;
Expand Down Expand Up @@ -232,6 +233,8 @@ public abstract class ResourceControllerITBase {
private SpecClient specClient;
@MockBean
private SearchClient searchClient;
@MockBean
private SettingsService settingsService;

@BeforeEach
public void beforeEach() {
Expand Down Expand Up @@ -319,6 +322,7 @@ void createInstanceWithWorkRef_shouldReturn424_ifSearchServiceThrownException()
);
var query = "(lccn==\"nn0123456789\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")";
when(searchClient.searchInstances(any())).thenThrow(new RuntimeException());
when(settingsService.isSettingEnabled(any(), any(), any())).thenReturn(true);

// when
var resultActions = mockMvc.perform(requestBuilder);
Expand Down Expand Up @@ -350,6 +354,7 @@ void createInstanceWithWorkRef_shouldReturn400_ifLccnIsNotUnique() throws Except
var query = "(lccn==\"nn0123456789\") and (staffSuppress <> \"true\" and discoverySuppress <> \"true\")";
when(searchClient.searchInstances(query))
.thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK));
when(settingsService.isSettingEnabled(any(), any(), any())).thenReturn(true);

// when
var resultActions = mockMvc.perform(requestBuilder);
Expand All @@ -363,6 +368,31 @@ void createInstanceWithWorkRef_shouldReturn400_ifLccnIsNotUnique() throws Except
verify(searchClient).searchInstances(query);
}

@Test
void createInstanceWithWorkRef_shouldSuccess_ifLccnDeduplicationDisabled() 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")
);
when(settingsService.isSettingEnabled(any(), any(), any())).thenReturn(false);

// when
var resultActions = mockMvc.perform(requestBuilder);

// then
resultActions
.andExpect(status().isOk())
.andExpect(content().contentType(APPLICATION_JSON));
}

@Test
void update_shouldReturn400_ifLccnIsNotUnique() throws Exception {
// given
Expand All @@ -386,6 +416,7 @@ void update_shouldReturn400_ifLccnIsNotUnique() throws Exception {
+ " and id <> \"2165ef4b-001f-46b3-a60e-52bcdeb3d5a1\"";
when(searchClient.searchInstances(query))
.thenReturn(new ResponseEntity<>(new SearchResponseTotalOnly().totalRecords(1L), HttpStatus.OK));
when(settingsService.isSettingEnabled(any(), any(), any())).thenReturn(true);

// when
var resultActions = mockMvc.perform(updateRequest);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.folio.linked.data.service;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

import java.util.stream.Stream;
import org.folio.linked.data.client.SettingsClient;
import org.folio.linked.data.domain.dto.SettingsItem;
import org.folio.linked.data.domain.dto.SettingsSearchResponse;
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 SettingsServiceImplTest {

@InjectMocks
private SettingsServiceImpl settingsService;

@Mock
private SettingsClient settingsClient;

@Test
void isSettingEnabled_shouldReturnFalse_ifSettingsAreEmpty() {
// given
when(settingsClient.getEntries(any()))
.thenReturn(new ResponseEntity<>(new SettingsSearchResponse(), HttpStatus.OK));

// when
boolean isEnabled = settingsService.isSettingEnabled("scope", "key", "property");

// then
assertFalse(isEnabled);
}

@ParameterizedTest
@MethodSource("settingsProvider")
void isSettingEnabled_shouldBaseOnSettingPropertyValue(SettingsItem item, boolean expected) {
// given
when(settingsClient.getEntries(any()))
.thenReturn(new ResponseEntity<>(
new SettingsSearchResponse().addItemsItem(item), HttpStatus.OK));

// when
boolean isEnabled = settingsService.isSettingEnabled("scope", "key", "property");

// then
assertEquals(expected, isEnabled);
}

private static Stream<Arguments> settingsProvider() {
return Stream.of(
arguments(new SettingsItem().putValueItem("property", true), true),
arguments(new SettingsItem().putValueItem("property", false), false),
arguments(new SettingsItem().putValueItem("property", new Object()), false),
arguments(new SettingsItem(), false)
);
}
}
Loading

0 comments on commit 4d1c0c7

Please sign in to comment.