MODBULKOPS-334 - Preventing record update with values from different …

…tenants (#287) * MODBULKOPS-334 Squashed
folio-org · Oct 25, 2024 · 4504693 · 4504693
1 parent 736465d
commit 4504693
Show file tree

Hide file tree

Showing 17 changed files with 161 additions and 48 deletions.
diff --git a/src/main/java/org/folio/bulkops/domain/entity/BulkOperationRuleDetails.java b/src/main/java/org/folio/bulkops/domain/entity/BulkOperationRuleDetails.java
@@ -48,4 +48,5 @@ public class BulkOperationRuleDetails {
 
   private List<String> actionTenants;
   private List<String> ruleTenants;
+  private List<String> updatedTenants;
 }
diff --git a/src/main/java/org/folio/bulkops/processor/AbstractDataProcessor.java b/src/main/java/org/folio/bulkops/processor/AbstractDataProcessor.java
@@ -104,7 +104,7 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
 
   public String getRecordPropertyName(UpdateOptionType optionType) {
     return switch (optionType) {
-      case HOLDINGS_NOTE, ITEM_NOTE -> "note type";
+      case HOLDINGS_NOTE, ITEM_NOTE, ADMINISTRATIVE_NOTE, CHECK_IN_NOTE, CHECK_OUT_NOTE -> "note type";
       case PERMANENT_LOAN_TYPE -> "permanent loan type";
       case TEMPORARY_LOAN_TYPE -> "temporary loan type";
       case PERMANENT_LOCATION -> "permanent location";
@@ -113,12 +113,4 @@ public String getRecordPropertyName(UpdateOptionType optionType) {
       default -> optionType.getValue();
     };
   }
-
-  public String getTenantFromAction(Action action) {
-    var actionTenants = action.getTenants();
-    if (isNull(actionTenants) || actionTenants.isEmpty()) {
-      return folioExecutionContext.getTenantId();
-    }
-    return actionTenants.get(0);
-  }
 }
diff --git a/src/main/java/org/folio/bulkops/processor/ElectronicAccessUpdaterFactory.java b/src/main/java/org/folio/bulkops/processor/ElectronicAccessUpdaterFactory.java
@@ -5,21 +5,30 @@
 import static java.util.Optional.ofNullable;
 import static org.apache.commons.lang3.StringUtils.EMPTY;
 import static org.apache.commons.lang3.StringUtils.equalsIgnoreCase;
+import static org.folio.bulkops.util.Constants.ARRAY_DELIMITER;
 
-import org.folio.bulkops.domain.bean.ElectronicAccessEntity;
+import lombok.AllArgsConstructor;
+import lombok.extern.log4j.Log4j2;
+import org.folio.bulkops.domain.bean.ExtendedHoldingsRecord;
 import org.folio.bulkops.domain.dto.Action;
 import org.folio.bulkops.domain.dto.UpdateOptionType;
 import org.folio.bulkops.exception.BulkOperationException;
+import org.folio.bulkops.util.RuleUtils;
+import org.folio.spring.FolioExecutionContext;
 import org.springframework.stereotype.Component;
 
 import java.util.Objects;
 
 @Component
+@Log4j2
+@AllArgsConstructor
 public class ElectronicAccessUpdaterFactory {
 
-  public Updater<? extends ElectronicAccessEntity> updater(UpdateOptionType option, Action action) {
+  private final FolioExecutionContext folioExecutionContext;
+
+  public Updater<ExtendedHoldingsRecord> updater(UpdateOptionType option, Action action, boolean forPreview) {
     return switch (option) {
-      case ELECTRONIC_ACCESS_URL_RELATIONSHIP -> updateUrlRelationship(option, action);
+      case ELECTRONIC_ACCESS_URL_RELATIONSHIP -> updateUrlRelationship(option, action, forPreview);
       case ELECTRONIC_ACCESS_URI -> updateUri(option, action);
       case ELECTRONIC_ACCESS_LINK_TEXT -> updateLinkText(option, action);
       case ELECTRONIC_ACCESS_MATERIALS_SPECIFIED -> updateMaterialsSpecified(option, action);
@@ -28,7 +37,7 @@ public Updater<? extends ElectronicAccessEntity> updater(UpdateOptionType option
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> updateUrlRelationship(UpdateOptionType option, Action action) {
+  private Updater<ExtendedHoldingsRecord> updateUrlRelationship(UpdateOptionType option, Action action, boolean forPreview) {
     return switch (action.getType()) {
       case CLEAR_FIELD -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setRelationshipId(null)));
@@ -39,14 +48,22 @@ private Updater<? extends ElectronicAccessEntity> updateUrlRelationship(UpdateOp
       case FIND_AND_REPLACE -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.stream()
           .filter(electronicAccess -> equalsIgnoreCase(electronicAccess.getRelationshipId(), action.getInitial()))
-          .forEach(electronicAccess -> electronicAccess.setRelationshipId(action.getUpdated())));
+          .forEach(electronicAccess -> electronicAccess.setRelationshipId(getRelationshipId(action, forPreview))));
       case REPLACE_WITH -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
-        .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setRelationshipId(action.getUpdated())));
+        .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setRelationshipId(getRelationshipId(action, forPreview))));
       default -> notSupported(option, action);
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> updateUri(UpdateOptionType option, Action action) {
+  private String getRelationshipId(Action action, boolean forPreview) {
+    var id = action.getUpdated();
+    if (forPreview) {
+      id += ARRAY_DELIMITER + RuleUtils.getTenantFromAction(action, folioExecutionContext);
+    }
+    return id;
+  }
+
+  private Updater<ExtendedHoldingsRecord> updateUri(UpdateOptionType option, Action action) {
     return switch (action.getType()) {
       case CLEAR_FIELD -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setUri(EMPTY)));
@@ -64,7 +81,7 @@ private Updater<? extends ElectronicAccessEntity> updateUri(UpdateOptionType opt
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> updateLinkText(UpdateOptionType option, Action action) {
+  private Updater<ExtendedHoldingsRecord> updateLinkText(UpdateOptionType option, Action action) {
     return switch (action.getType()) {
       case CLEAR_FIELD -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setLinkText(null)));
@@ -82,7 +99,7 @@ private Updater<? extends ElectronicAccessEntity> updateLinkText(UpdateOptionTyp
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> updateMaterialsSpecified(UpdateOptionType option, Action action) {
+  private Updater<ExtendedHoldingsRecord> updateMaterialsSpecified(UpdateOptionType option, Action action) {
     return switch (action.getType()) {
       case CLEAR_FIELD -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setMaterialsSpecification(null)));
@@ -100,7 +117,7 @@ private Updater<? extends ElectronicAccessEntity> updateMaterialsSpecified(Updat
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> updatePublicNote(UpdateOptionType option, Action action) {
+  private Updater<ExtendedHoldingsRecord> updatePublicNote(UpdateOptionType option, Action action) {
     return switch (action.getType()) {
       case CLEAR_FIELD -> electronicAccessEntity -> ofNullable(electronicAccessEntity.getElectronicAccess())
         .ifPresent(list -> list.forEach(electronicAccess -> electronicAccess.setPublicNote(null)));
@@ -118,7 +135,7 @@ private Updater<? extends ElectronicAccessEntity> updatePublicNote(UpdateOptionT
     };
   }
 
-  private Updater<? extends ElectronicAccessEntity> notSupported(UpdateOptionType option, Action action) {
+  private Updater<ExtendedHoldingsRecord> notSupported(UpdateOptionType option, Action action) {
     return electronicAccessEntity -> {
       throw new BulkOperationException(format("Combination %s and %s isn't supported yet", option, action.getType()));
     };

diff --git a/src/main/java/org/folio/bulkops/processor/HoldingsDataProcessor.java b/src/main/java/org/folio/bulkops/processor/HoldingsDataProcessor.java
@@ -1,6 +1,7 @@
 package org.folio.bulkops.processor;
 
 import static java.lang.String.format;
+import static java.util.Objects.isNull;
 import static java.util.Objects.nonNull;
 import static org.apache.commons.lang3.ObjectUtils.isEmpty;
 import static org.folio.bulkops.domain.dto.UpdateActionType.CLEAR_FIELD;
@@ -41,6 +42,7 @@
 import org.folio.bulkops.service.HoldingsReferenceService;
 import org.folio.bulkops.service.ItemReferenceService;
 import org.folio.bulkops.service.ElectronicAccessReferenceService;
+import org.folio.bulkops.util.RuleUtils;
 import org.folio.spring.scope.FolioExecutionContextSetter;
 import org.springframework.stereotype.Component;
 
@@ -88,15 +90,12 @@ public Validator<UpdateOptionType, Action, BulkOperationRule> validator(Extended
   public Updater<ExtendedHoldingsRecord> updater(UpdateOptionType option, Action action, ExtendedHoldingsRecord entity,
                                                  boolean forPreview) throws RuleValidationTenantsException {
     if (isElectronicAccessUpdate(option)) {
-      if (nonNull(entity.getEntity().getElectronicAccess())) {
-        entity.getEntity().getElectronicAccess().forEach(el -> el.setTenantId(getTenantFromAction(action)));
-      }
-      return (Updater<ExtendedHoldingsRecord>) electronicAccessUpdaterFactory.updater(option, action);
+      return electronicAccessUpdaterFactory.updater(option, action, forPreview);
     } else if (REPLACE_WITH == action.getType()) {
       return extendedHoldingsRecord -> {
         var locationId = action.getUpdated();
         if (forPreview) {
-        var tenant = getTenantFromAction(action);
+          var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
           locationId += ARRAY_DELIMITER + tenant;
         }
         if (PERMANENT_LOCATION == option) {
@@ -118,10 +117,9 @@ public Updater<ExtendedHoldingsRecord> updater(UpdateOptionType option, Action a
       return extendedHoldingsRecord -> extendedHoldingsRecord.getEntity().setDiscoverySuppress(false);
     }
     var notesUpdaterOptional = holdingsNotesUpdater.updateNotes(action, option);
-    if (notesUpdaterOptional.isPresent()) return notesUpdaterOptional.get();
-    return holding -> {
+    return notesUpdaterOptional.orElseGet(() -> holding -> {
       throw new BulkOperationException(format("Combination %s and %s isn't supported yet", option, action.getType()));
-    };
+    });
   }
 
   private void validateReplacement(UpdateOptionType option, Action action) throws RuleValidationException {
@@ -134,7 +132,7 @@ private void validateReplacement(UpdateOptionType option, Action action) throws
         throw new RuleValidationException("UUID has invalid format: %s" + newId);
       }
 
-      var tenant = getTenantFromAction(action);
+      var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
       if (Set.of(PERMANENT_LOCATION, TEMPORARY_LOCATION).contains(option)) {
         try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
           itemReferenceService.getLocationById(newId, tenant);
@@ -143,9 +141,10 @@ private void validateReplacement(UpdateOptionType option, Action action) throws
         }
       } else if (ELECTRONIC_ACCESS_URL_RELATIONSHIP.equals(option)) {
         try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
+          log.info("ELECTRONIC_ACCESS_URL_RELATIONSHIP.equals(option), tenant: {}", tenant);
           electronicAccessReferenceService.getRelationshipNameById(newId, tenant);
         } catch (Exception e) {
-          throw new RuleValidationException(format("URL relationship %s doesn't exist", newId));
+          throw new RuleValidationException(format("URL relationship %s doesn't exist in tenant %s", newId, tenant));
         }
       }
     }
@@ -185,6 +184,10 @@ public ExtendedHoldingsRecord clone(ExtendedHoldingsRecord extendedEntity) {
       var holdingsNotes = entity.getNotes().stream().map(note -> note.toBuilder().build()).toList();
       clone.setNotes(new ArrayList<>(holdingsNotes));
     }
+    if (entity.getElectronicAccess() != null) {
+      var elAcc = entity.getElectronicAccess().stream().map(el -> el.toBuilder().build()).toList();
+      clone.setElectronicAccess(new ArrayList<>(elAcc));
+    }
 
     return ExtendedHoldingsRecord.builder().tenantId(extendedEntity.getTenantId()).entity(clone).build();
   }
@@ -210,7 +213,8 @@ private boolean ruleTenantsAreNotValid(BulkOperationRule rule, Action action, Up
     }
     if (nonNull(ruleTenants) && nonNull(actionTenants) && ruleTenants.isEmpty() && actionTenants.isEmpty() &&
       option == ELECTRONIC_ACCESS_URL_RELATIONSHIP && action.getType() == UpdateActionType.FIND_AND_REPLACE) {
-      return true;
+      return isNull(extendedHolding.getElectronicAccess()) ||
+        extendedHolding.getElectronicAccess().stream().noneMatch(el -> el.getRelationshipId().equals(action.getUpdated()));
     }
     return nonNull(ruleTenants) && !ruleTenants.isEmpty() && !ruleTenants.contains(extendedHolding.getTenant()) ||
       nonNull(actionTenants) && !actionTenants.isEmpty() && !actionTenants.contains(extendedHolding.getTenant());

diff --git a/src/main/java/org/folio/bulkops/processor/ItemDataProcessor.java b/src/main/java/org/folio/bulkops/processor/ItemDataProcessor.java
@@ -32,6 +32,7 @@
 import org.folio.bulkops.exception.RuleValidationTenantsException;
 import org.folio.bulkops.service.HoldingsReferenceService;
 import org.folio.bulkops.service.ItemReferenceService;
+import org.folio.bulkops.util.RuleUtils;
 import org.folio.spring.scope.FolioExecutionContextSetter;
 import org.springframework.stereotype.Component;
 
@@ -100,11 +101,11 @@ public Updater<ExtendedItem> updater(UpdateOptionType option, Action action, Ext
       return switch (option) {
         case PERMANENT_LOCATION -> extendedItem -> {
           extendedItem.getEntity().setPermanentLocation(null);
-          extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), getTenantFromAction(action)));
+          extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), RuleUtils.getTenantFromAction(action, folioExecutionContext)));
         };
         case TEMPORARY_LOCATION -> extendedItem -> {
           extendedItem.getEntity().setTemporaryLocation(null);
-          extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), getTenantFromAction(action)));
+          extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), RuleUtils.getTenantFromAction(action, folioExecutionContext)));
         };
         case TEMPORARY_LOAN_TYPE -> extendedItem -> extendedItem.getEntity().setTemporaryLoanType(null);
         default -> item -> {
@@ -179,29 +180,29 @@ private boolean ruleTenantsAreNotValid(BulkOperationRule rule, Action action, Up
   }
 
   private void replacePermanentLoanType(Action action, ExtendedItem extendedItem) {
-    var tenant = getTenantFromAction(action);
+    var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
     try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
       extendedItem.getEntity().setPermanentLoanType(itemReferenceService.getLoanTypeById(action.getUpdated(), tenant));
     }
   }
 
   private void replaceTemporaryLoanType(Action action, ExtendedItem extendedItem) {
-    var tenant = getTenantFromAction(action);
+    var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
     try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
       extendedItem.getEntity().setTemporaryLoanType(itemReferenceService.getLoanTypeById(action.getUpdated(), tenant));
     }
   }
 
   private void replacePermanentLocation(Action action, ExtendedItem extendedItem) {
-    var tenant = getTenantFromAction(action);
+    var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
     try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
       extendedItem.getEntity().setPermanentLocation(itemReferenceService.getLocationById(action.getUpdated(), tenant));
       extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), tenant));
     }
   }
 
   private void replaceTemporaryLocation(Action action, ExtendedItem extendedItem) {
-    var tenant = getTenantFromAction(action);
+    var tenant = RuleUtils.getTenantFromAction(action, folioExecutionContext);
     try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenant, folioModuleMetadata, folioExecutionContext))) {
       extendedItem.getEntity().setTemporaryLocation(itemReferenceService.getLocationById(action.getUpdated(), tenant));
       extendedItem.getEntity().setEffectiveLocation(getEffectiveLocation(extendedItem.getEntity(), tenant));

diff --git a/src/main/java/org/folio/bulkops/service/ElectronicAccessReferenceService.java b/src/main/java/org/folio/bulkops/service/ElectronicAccessReferenceService.java
@@ -24,7 +24,11 @@ public class ElectronicAccessReferenceService {
 
   @Cacheable(cacheNames = "electronicAccessRelationshipNames")
   public String getRelationshipNameById(String id, String tenantId) {
-    try {
+    log.info("getRelationshipNameById: {}, {}", id, tenantId);
+    if (isNull(tenantId)) {
+      tenantId = folioExecutionContext.getTenantId();
+    }
+    try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(tenantId, folioModuleMetadata, folioExecutionContext))) {
       return relationshipClient.getById(id).getName();
     } catch (NotFoundException e) {
       log.error("Electronic access relationship was not found by id={}", id);

diff --git a/src/main/java/org/folio/bulkops/service/ElectronicAccessService.java b/src/main/java/org/folio/bulkops/service/ElectronicAccessService.java
@@ -48,9 +48,9 @@ public ElectronicAccess restoreItemElectronicAccessItem(String access) {
   }
 
   private String electronicAccessToString(ElectronicAccess access, String delimiter) {
-    var tenantId = isNull(access.getTenantId()) ? folioExecutionContext.getTenantId() : access.getTenantId();
+    log.debug("electronicAccessToString: {}, {}", access.getRelationshipId(), folioExecutionContext.getTenantId());
     return String.join(delimiter,
-      isEmpty(access.getRelationshipId()) ? EMPTY : electronicAccessReferenceService.getRelationshipNameById(access.getRelationshipId(), tenantId),
+      isEmpty(access.getRelationshipId()) ? EMPTY : getRelationshipName(access),
       isNull(access.getUri()) ? EMPTY : access.getUri(),
       isEmpty(access.getLinkText()) ? EMPTY : access.getLinkText(),
       isEmpty(access.getMaterialsSpecification()) ? EMPTY : access.getMaterialsSpecification(),
@@ -74,4 +74,10 @@ private ElectronicAccess restoreElectronicAccessItem(String access, String delim
     }
     return null;
   }
+
+  private String getRelationshipName(ElectronicAccess access) {
+    log.debug("getRelationshipName {}, {}, {}", access.getRelationshipId(), access.getTenantId(), folioExecutionContext.getTenantId());
+    var idTenant = access.getRelationshipId().split(ARRAY_DELIMITER);
+    return electronicAccessReferenceService.getRelationshipNameById(idTenant[0], idTenant.length > 1 ? idTenant[1] : null);
+  }
 }
diff --git a/src/main/java/org/folio/bulkops/service/RuleService.java b/src/main/java/org/folio/bulkops/service/RuleService.java
@@ -49,6 +49,7 @@ public BulkOperationRuleCollection saveRules(BulkOperationRuleCollection ruleCol
           .parameters(action.getParameters())
           .ruleTenants(bulkOperationRule.getRuleDetails().getTenants())
           .actionTenants(action.getTenants())
+          .updatedTenants(action.getUpdatedTenants())
           .build()));
     });
     return ruleCollection;
@@ -75,7 +76,8 @@ private BulkOperationRule mapBulkOperationRuleToDto(org.folio.bulkops.domain.ent
             .initial(details.getInitialValue())
             .updated(details.getUpdatedValue())
             .parameters(details.getParameters())
-            .tenants(details.getActionTenants()))
+            .tenants(details.getActionTenants())
+            .updatedTenants(details.getUpdatedTenants()))
           .toList())
         .tenants(entity.getRuleDetails().get(0).getRuleTenants()));
   }