Merge branch 'master' into msearch-782

folio-org · Jun 4, 2024 · 8bc3353 · 8bc3353
2 parents cdb21cb + 0578c3b
commit 8bc3353
Show file tree

Hide file tree

Showing 8 changed files with 196 additions and 5 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -24,6 +24,7 @@
 * Do additional search request on browse before getting backward succeeding in order to find preceding results ([MSEARCH-705](https://folio-org.atlassian.net/browse/MSEARCH-705))
 * Keep right context in resource-id thread ([MSEARCH-754](https://folio-org.atlassian.net/browse/MSEARCH-754))
 * Browse: Duplicate results in exact match with diacritics ([MSEARCH-751](https://folio-org.atlassian.net/browse/MSEARCH-751))
+* Classification browse: Fix instances count for Shared Facet ([MSEARCH-761](https://folio-org.atlassian.net/browse/MSEARCH-761))
 
 ### Tech Dept
 * Re-Index: delete all records from consortium_instance on full re-index ([MSEARCH-744](https://folio-org.atlassian.net/browse/MSEARCH-744))

diff --git a/...java/org/folio/search/repository/classification/InstanceClassificationJdbcRepository.java b/...java/org/folio/search/repository/classification/InstanceClassificationJdbcRepository.java
@@ -7,6 +7,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.sql.PreparedStatement;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import lombok.RequiredArgsConstructor;
@@ -59,7 +60,7 @@ ON CONFLICT (classification_type_id, classification_number, tenant_id, instance_
     WHERE classification_type_id = ? AND classification_number = ? AND tenant_id = ? AND instance_id = ?;
     """;
   private static final int BATCH_SIZE = 100;
-  private static final TypeReference<Set<InstanceSubResource>> VALUE_TYPE_REF = new TypeReference<>() { };
+  private static final TypeReference<LinkedHashSet<InstanceSubResource>> VALUE_TYPE_REF = new TypeReference<>() { };
 
   private final FolioExecutionContext context;
   private final JdbcTemplate jdbcTemplate;

diff --git a/src/main/java/org/folio/search/service/converter/preprocessor/InstanceEventPreProcessor.java b/src/main/java/org/folio/search/service/converter/preprocessor/InstanceEventPreProcessor.java
@@ -3,7 +3,9 @@
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptySet;
 import static org.apache.commons.collections4.MapUtils.getObject;
+import static org.apache.commons.lang3.StringUtils.removeStart;
 import static org.apache.commons.lang3.StringUtils.startsWith;
+import static org.folio.search.domain.dto.ResourceEventType.UPDATE;
 import static org.folio.search.utils.CollectionUtils.subtract;
 import static org.folio.search.utils.SearchConverterUtils.getNewAsMap;
 import static org.folio.search.utils.SearchConverterUtils.getOldAsMap;
@@ -19,6 +21,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
@@ -56,20 +59,55 @@ public List<ResourceEvent> preProcess(ResourceEvent event) {
     if (log.isDebugEnabled()) {
       log.debug("preProcess::Starting instance event pre-processing [{}]", event);
     }
-    if (startsWith(getResourceSource(event), SOURCE_CONSORTIUM_PREFIX)) {
+
+    List<ResourceEvent> events;
+
+    if (isUpdateForInstanceSharing(event)) {
+      events = prepareClassificationEventsOnInstanceSharing(event);
+    } else if (startsWith(getResourceSource(event), SOURCE_CONSORTIUM_PREFIX)) {
       log.info("preProcess::Finished instance event pre-processing. No additional events created for shadow instance.");
       return List.of(event);
+    } else {
+      events = prepareClassificationEvents(event);
     }
 
-    var events = prepareClassificationEvents(event);
-
     log.info("preProcess::Finished instance event pre-processing");
     if (log.isDebugEnabled()) {
       log.debug("preProcess::Finished instance event pre-processing. Events after: [{}], ", events);
     }
     return events;
   }
 
+  private boolean isUpdateForInstanceSharing(ResourceEvent event) {
+    var newSource = getResourceSource(getNewAsMap(event));
+    return event.getType() == UPDATE
+      && startsWith(newSource, SOURCE_CONSORTIUM_PREFIX)
+      && Objects.equals(getResourceSource(getOldAsMap(event)), removeStart(newSource, SOURCE_CONSORTIUM_PREFIX));
+  }
+
+  private List<ResourceEvent> prepareClassificationEventsOnInstanceSharing(ResourceEvent event) {
+    if (!featureConfigService.isEnabled(TenantConfiguredFeature.BROWSE_CLASSIFICATIONS)) {
+      return emptyList();
+    }
+
+    var classifications = getClassifications(getOldAsMap(event));
+
+    if (!classifications.equals(getClassifications(getNewAsMap(event)))) {
+      log.warn("Classifications are different on Update for instance sharing");
+      return emptyList();
+    }
+
+    var tenant = event.getTenant();
+    var instanceId = getResourceEventId(event);
+    var shared = isShared(tenant);
+
+    var entitiesForDelete = toEntities(classifications, instanceId, tenant, shared);
+    instanceClassificationRepository.deleteAll(entitiesForDelete);
+    var aggregatedEntities = instanceClassificationRepository.fetchAggregatedByClassifications(entitiesForDelete);
+
+    return getResourceEventsForUpdate(entitiesForDelete, aggregatedEntities, tenant);
+  }
+
   private List<ResourceEvent> prepareClassificationEvents(ResourceEvent event) {
     if (!featureConfigService.isEnabled(TenantConfiguredFeature.BROWSE_CLASSIFICATIONS)) {
       return emptyList();
@@ -128,6 +166,30 @@ private List<ResourceEvent> getResourceEventsForDeletion(List<InstanceClassifica
       .toList();
   }
 
+  private List<ResourceEvent> getResourceEventsForUpdate(List<InstanceClassificationEntity> entitiesForDelete,
+                                                         List<InstanceClassificationEntityAgg> aggregatedEntities,
+                                                         String tenant) {
+    for (var classification : entitiesForDelete) {
+      for (InstanceClassificationEntityAgg agg : aggregatedEntities) {
+        if (agg.number().equals(classification.number()) && Objects.equals(agg.typeId(), classification.typeId())) {
+          var subResource = InstanceSubResource.builder()
+            .instanceId(classification.instanceId())
+            .shared(classification.shared())
+            .tenantId(tenant)
+            .typeId(classification.typeId())
+            .build();
+          agg.instances().remove(subResource);
+          break;
+        }
+      }
+    }
+
+    return aggregatedEntities.stream()
+      .map(classification ->
+        getResourceEvent(tenant, classification.number(), classification.typeId(), classification.instances(), UPDATE))
+      .toList();
+  }
+
   private ResourceEvent toResourceDeleteEvent(InstanceClassificationEntity source, String tenant) {
     return getResourceEvent(tenant, source.number(), source.typeId(), null, ResourceEventType.DELETE);
   }

diff --git a/src/main/java/org/folio/search/utils/SearchUtils.java b/src/main/java/org/folio/search/utils/SearchUtils.java
@@ -65,6 +65,7 @@ public class SearchUtils {
   public static final String CLASSIFICATION_TYPE_FIELD = "classificationTypeId";
   public static final String SUBJECT_AGGREGATION_NAME = "subjects.value";
   public static final String SOURCE_CONSORTIUM_PREFIX = "CONSORTIUM-";
+  public static final String SOURCE_FOLIO = "FOLIO";
 
   public static final String CQL_META_FIELD_PREFIX = "cql.";
   public static final String MULTILANG_SOURCE_SUBFIELD = "src";
@@ -103,7 +104,6 @@ public class SearchUtils {
     """;
   //CHECKSTYLE.OFF: LineLength
 
-  private static final Pattern LCCN_NUMERIC_PART_REGEX = Pattern.compile("([1-9]\\d+)");
   private static final Pattern NON_ALPHA_NUMERIC_CHARS_PATTERN = Pattern.compile("[^a-zA-Z0-9]");
 
   /**

diff --git a/src/main/resources/swagger.api/parameters/batchIdsDto.yaml b/src/main/resources/swagger.api/parameters/batchIdsDto.yaml
@@ -4,6 +4,7 @@ properties:
   ids:
     description: Entity IDs
     type: array
+    maxItems: 1000
     items:
       type: string
       format: uuid

diff --git a/src/test/java/org/folio/search/controller/ConsortiumSearchHoldingsIT.java b/src/test/java/org/folio/search/controller/ConsortiumSearchHoldingsIT.java
@@ -1,6 +1,7 @@
 package org.folio.search.controller;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.folio.search.controller.ConsortiumSearchItemsIT.WRONG_SIZE_MSG;
 import static org.folio.search.controller.SearchConsortiumController.REQUEST_NOT_ALLOWED_MSG;
 import static org.folio.search.model.Pair.pair;
 import static org.folio.search.sample.SampleInstances.getSemanticWeb;
@@ -19,6 +20,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
+import java.util.stream.Stream;
 import org.folio.search.domain.dto.BatchIdsDto;
 import org.folio.search.domain.dto.ConsortiumHolding;
 import org.folio.search.domain.dto.ConsortiumHoldingCollection;
@@ -141,6 +143,23 @@ void tryGetConsortiumBatchHoldings_returns400_whenRequestedForNotCentralTenant()
       .andExpect(jsonPath("$.errors[0].parameters[0].value", is(MEMBER_TENANT_ID)));
   }
 
+  @Test
+  void tryGetConsortiumBatchHoldings_returns400_whenMoreIdsThanLimit() throws Exception {
+    var request = new BatchIdsDto()
+      .ids(
+        Stream.iterate(0, i -> i < 1001, i -> ++i)
+          .map(i -> UUID.randomUUID())
+          .toList()
+      );
+
+    tryPost(consortiumBatchHoldingsSearchPath(), request)
+      .andExpect(status().isBadRequest())
+      .andExpect(jsonPath("$.errors[0].message", is(WRONG_SIZE_MSG)))
+      .andExpect(jsonPath("$.errors[0].type", is("MethodArgumentNotValidException")))
+      .andExpect(jsonPath("$.errors[0].code", is("validation_error")))
+      .andExpect(jsonPath("$.errors[0].parameters[0].key", is("ids")));
+  }
+
   private ConsortiumHolding[] getExpectedHoldings() {
     var instance = getSemanticWeb();
     return instance.getHoldings().stream()

diff --git a/src/test/java/org/folio/search/controller/ConsortiumSearchItemsIT.java b/src/test/java/org/folio/search/controller/ConsortiumSearchItemsIT.java
@@ -19,6 +19,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
+import java.util.stream.Stream;
 import org.folio.search.domain.dto.BatchIdsDto;
 import org.folio.search.domain.dto.ConsortiumItem;
 import org.folio.search.domain.dto.ConsortiumItemCollection;
@@ -32,6 +33,9 @@
 @IntegrationTest
 class ConsortiumSearchItemsIT extends BaseConsortiumIntegrationTest {
 
+  static final String WRONG_SIZE_MSG =
+    "size must be between 0 and 1000";
+
   @BeforeAll
   static void prepare() {
     setUpTenant(CENTRAL_TENANT_ID);
@@ -145,6 +149,23 @@ void tryGetConsortiumBatchItems_returns400_whenRequestedForNotCentralTenant() th
       .andExpect(jsonPath("$.errors[0].parameters[0].value", is(MEMBER_TENANT_ID)));
   }
 
+  @Test
+  void tryGetConsortiumBatchItems_returns400_whenMoreIdsThanLimit() throws Exception {
+    var request = new BatchIdsDto()
+      .ids(
+        Stream.iterate(0, i -> i < 1001, i -> ++i)
+          .map(i -> UUID.randomUUID())
+          .toList()
+      );
+
+    tryPost(consortiumBatchItemsSearchPath(), request)
+      .andExpect(status().isBadRequest())
+      .andExpect(jsonPath("$.errors[0].message", is(WRONG_SIZE_MSG)))
+      .andExpect(jsonPath("$.errors[0].type", is("MethodArgumentNotValidException")))
+      .andExpect(jsonPath("$.errors[0].code", is("validation_error")))
+      .andExpect(jsonPath("$.errors[0].parameters[0].key", is("ids")));
+  }
+
   private ConsortiumItem[] getExpectedItems() {
     var instance = getSemanticWeb();
     return instance.getItems().stream()

diff --git a/...t/java/org/folio/search/service/converter/preprocessor/InstanceEventPreProcessorTest.java b/...t/java/org/folio/search/service/converter/preprocessor/InstanceEventPreProcessorTest.java
@@ -31,6 +31,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
+import org.apache.commons.collections4.SetUtils;
 import org.folio.search.domain.dto.ResourceEvent;
 import org.folio.search.domain.dto.ResourceEventType;
 import org.folio.search.domain.dto.TenantConfiguredFeature;
@@ -266,6 +267,91 @@ void preProcess_AnyEventInConsortium_ShouldProcessClassificationsAndSetShared(Re
       .containsExactlyInAnyOrder(tuple("t4", "n4", TENANT_ID, id, true));
   }
 
+  @Test
+  void preProcess_featureIsDisabledOnInstanceSharing_shouldNotProcessClassifications() {
+    // Arrange
+    var newData = instance(randomId(), SOURCE_CONSORTIUM_PREFIX + "FOLIO", null);
+    var oldData = instance(randomId(),  "FOLIO", null);
+    var resourceEvent = resourceEvent(randomId(), INSTANCE_RESOURCE, UPDATE, newData, oldData);
+    mockClassificationBrowseFeatureEnabled(Boolean.FALSE);
+
+    // Act
+    var resourceEvents = preProcessor.preProcess(resourceEvent);
+
+    // Assert
+    assertThat(resourceEvents)
+      .isEmpty();
+
+    verifyNoInteractions(instanceClassificationRepository);
+  }
+
+  @Test
+  void preProcess_differentClassificationsOnInstanceSharing_shouldNotProcessClassifications() {
+    // Arrange
+    var id = randomId();
+    var newData = instance(id, SOURCE_CONSORTIUM_PREFIX + "FOLIO", List.of(classification("n2", "t2")));
+    var oldData = instance(id,  "FOLIO", List.of(classification("n1", "t1")));
+    var resourceEvent = resourceEvent(randomId(), INSTANCE_RESOURCE, UPDATE, newData, oldData);
+    mockClassificationBrowseFeatureEnabled(Boolean.TRUE);
+
+    // Act
+    var resourceEvents = preProcessor.preProcess(resourceEvent);
+
+    // Assert
+    assertThat(resourceEvents)
+      .isEmpty();
+
+    verifyNoInteractions(instanceClassificationRepository);
+  }
+
+  @Test
+  void preProcess_DeleteEntityAndUpdateIndexOnInstanceSharing_shouldProcessClassifications() {
+    // Arrange
+    var id = randomId();
+    var typeId = "type";
+    var number = "num";
+    var newData = instance(id, SOURCE_CONSORTIUM_PREFIX + "FOLIO", List.of(classification(number, typeId)));
+    var oldData = instance(id,  "FOLIO", List.of(classification(number, typeId)));
+    var resourceEvent = resourceEvent(id, INSTANCE_RESOURCE, UPDATE, newData, oldData);
+    mockClassificationBrowseFeatureEnabled(Boolean.TRUE);
+    when(instanceClassificationRepository.fetchAggregatedByClassifications(anyList()))
+      .thenReturn(List.of(
+        new InstanceClassificationEntityAgg(typeId, number,
+          SetUtils.hashSet(
+            InstanceSubResource.builder().instanceId(id).tenantId(TENANT_ID).shared(false).build(),
+            InstanceSubResource.builder().instanceId(id).tenantId(TENANT_ID + "_central").shared(true).build()
+          )
+        )
+      ));
+    var classificationId = InstanceClassificationEntity.Id.builder()
+      .number(number)
+      .typeId(typeId)
+      .instanceId(id)
+      .tenantId(TENANT_ID)
+      .build();
+    var expectedDeletedClassificationEntity = new InstanceClassificationEntity(classificationId, false);
+
+    // Act
+    var resourceEvents = preProcessor.preProcess(resourceEvent);
+
+    // Assert
+    verify(instanceClassificationRepository).deleteAll(deleteCaptor.capture());
+    verify(instanceClassificationRepository).fetchAggregatedByClassifications(anyList());
+    var deletedClassifications = deleteCaptor.getValue();
+    assertThat(List.of(expectedDeletedClassificationEntity))
+      .isEqualTo(deletedClassifications);
+
+    assertThat(resourceEvents)
+      .hasSize(1)
+      .allSatisfy(event -> assertThat(event)
+        .extracting(ResourceEvent::getResourceName, ResourceEvent::getTenant, ResourceEvent::getType)
+        .containsExactly(INSTANCE_CLASSIFICATION_RESOURCE, TENANT_ID, UPDATE))
+      .extracting(ResourceEvent::getId)
+      .containsExactlyInAnyOrder("num|type");
+
+    verifyNoMoreInteractions(instanceClassificationRepository);
+  }
+
   private void mockClassificationBrowseFeatureEnabled(Boolean isEnabled) {
     when(featureConfigService.isEnabled(TenantConfiguredFeature.BROWSE_CLASSIFICATIONS)).thenReturn(isEnabled);
   }