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

…tenants (#278) * MODBULKOPS-334 Added validation
folio-org · Oct 17, 2024 · 04d462f · 04d462f
1 parent 9367b4e
commit 04d462f
Show file tree

Hide file tree

Showing 41 changed files with 657 additions and 188 deletions.
diff --git a/src/main/java/org/folio/bulkops/domain/bean/BulkOperationsEntity.java b/src/main/java/org/folio/bulkops/domain/bean/BulkOperationsEntity.java
@@ -3,6 +3,9 @@
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import org.apache.commons.lang3.StringUtils;
 import org.folio.bulkops.domain.dto.IdentifierType;
+import org.folio.bulkops.domain.dto.TenantNotePair;
+
+import java.util.List;
 
 /**
  * Marker interface for entities for which bulk operations is applicable.
@@ -19,7 +22,7 @@ default String getTenant() {
     return StringUtils.EMPTY;
   }
   @JsonIgnore
-  default void setTenantToNotes() {
+  default void setTenantToNotes(List<TenantNotePair> tenantNotePairs) {
   }
   @JsonIgnore
   default void setTenant(String tenantId) {

diff --git a/src/main/java/org/folio/bulkops/domain/bean/ElectronicAccess.java b/src/main/java/org/folio/bulkops/domain/bean/ElectronicAccess.java
@@ -1,5 +1,6 @@
 package org.folio.bulkops.domain.bean;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import lombok.AllArgsConstructor;
@@ -29,5 +30,8 @@ public class ElectronicAccess {
 
   @JsonProperty("relationshipId")
   private String relationshipId;
+
+  @JsonIgnore
+  private String tenantId;
 }
 
diff --git a/src/main/java/org/folio/bulkops/domain/bean/ExtendedHoldingsRecord.java b/src/main/java/org/folio/bulkops/domain/bean/ExtendedHoldingsRecord.java
@@ -8,8 +8,10 @@
 import lombok.NoArgsConstructor;
 import lombok.With;
 import org.folio.bulkops.domain.dto.IdentifierType;
+import org.folio.bulkops.domain.dto.TenantNotePair;
 
 import java.util.List;
+import java.util.Optional;
 
 @Data
 @With
@@ -50,7 +52,17 @@ public List<ElectronicAccess> getElectronicAccess() {
   }
 
   @Override
-  public void setTenantToNotes() {
-    entity.getNotes().forEach(note -> note.setTenantId(tenantId));
+  public void setTenantToNotes(List<TenantNotePair> tenantNotePairs) {
+    entity.getNotes().forEach(note -> {
+      var tenantNotePair = tenantNotePairs.stream()
+        .filter(pair -> pair.getNoteTypeId().equals(note.getHoldingsNoteTypeId()))
+        .findFirst();
+      if (tenantNotePair.isPresent()) {
+        note.setTenantId(tenantNotePair.get().getTenantId());
+        note.setHoldingsNoteTypeName(tenantNotePair.get().getNoteTypeName());
+      } else {
+        note.setTenantId(tenantId);
+      }
+    });
   }
 }
diff --git a/src/main/java/org/folio/bulkops/domain/bean/ExtendedItem.java b/src/main/java/org/folio/bulkops/domain/bean/ExtendedItem.java
@@ -7,6 +7,9 @@
 import lombok.NoArgsConstructor;
 import lombok.With;
 import org.folio.bulkops.domain.dto.IdentifierType;
+import org.folio.bulkops.domain.dto.TenantNotePair;
+
+import java.util.List;
 
 @Data
 @With
@@ -42,7 +45,17 @@ public String getTenant() {
   }
 
   @Override
-  public void setTenantToNotes() {
-    entity.getNotes().forEach(note -> note.setTenantId(tenantId));
+  public void setTenantToNotes(List<TenantNotePair> tenantNotePairs) {
+    entity.getNotes().forEach(note -> {
+      var tenantNotePair = tenantNotePairs.stream()
+        .filter(pair -> pair.getNoteTypeId().equals(note.getItemNoteTypeId()))
+        .findFirst();
+      if (tenantNotePair.isPresent()) {
+        note.setTenantId(tenantNotePair.get().getTenantId());
+        note.setItemNoteTypeName(tenantNotePair.get().getNoteTypeName());
+      } else {
+        note.setTenantId(tenantId);
+      }
+    });
   }
 }
diff --git a/src/main/java/org/folio/bulkops/domain/bean/HoldingsNote.java b/src/main/java/org/folio/bulkops/domain/bean/HoldingsNote.java
@@ -26,5 +26,6 @@ public class HoldingsNote {
   @JsonProperty("staffOnly")
   private Boolean staffOnly;
   private String tenantId;
+  private String holdingsNoteTypeName;
 }
 
diff --git a/src/main/java/org/folio/bulkops/domain/bean/HoldingsRecord.java b/src/main/java/org/folio/bulkops/domain/bean/HoldingsRecord.java
@@ -32,14 +32,15 @@
 import lombok.EqualsAndHashCode;
 import lombok.NoArgsConstructor;
 import lombok.With;
+import org.folio.bulkops.domain.dto.TenantNotePair;
 
 @Data
 @With
 @Builder(toBuilder = true)
 @NoArgsConstructor
 @AllArgsConstructor
 @JsonTypeName("holdingsRecord")
-@EqualsAndHashCode(exclude = {"metadata", "instanceId", "permanentLocation", "effectiveLocationId", "illPolicy", "instanceHrid", "itemBarcode"})
+@EqualsAndHashCode(exclude = {"metadata", "instanceId", "permanentLocation", "effectiveLocationId", "illPolicy", "instanceHrid", "itemBarcode", "tenantId"})
 public class HoldingsRecord implements BulkOperationsEntity {
 
   @JsonProperty("id")
@@ -278,8 +279,12 @@ public Boolean getDiscoverySuppress() {
   }
 
   @Override
-  public void setTenantToNotes() {
-    getNotes().forEach(note -> note.setTenantId(tenantId));
+  public void setTenantToNotes(List<TenantNotePair> tenantNotePairs) {
+    getNotes().forEach(note -> note.setTenantId(
+      tenantNotePairs.stream()
+        .filter(pair -> pair.getNoteTypeId().equals(note.getHoldingsNoteTypeId()))
+        .map(pair -> pair.getTenantId()).findFirst().orElseGet(() -> tenantId)
+    ));
   }
 
   @Override

diff --git a/src/main/java/org/folio/bulkops/domain/bean/Item.java b/src/main/java/org/folio/bulkops/domain/bean/Item.java
@@ -31,6 +31,7 @@
 import org.folio.bulkops.domain.converter.TagsConverter;
 import org.folio.bulkops.domain.dto.DataType;
 import org.folio.bulkops.domain.dto.IdentifierType;
+import org.folio.bulkops.domain.dto.TenantNotePair;
 
 import java.util.Date;
 import java.util.List;
@@ -46,7 +47,7 @@
 @NoArgsConstructor
 @AllArgsConstructor
 @JsonTypeName("item")
-@EqualsAndHashCode(exclude = {"effectiveCallNumberComponents", "effectiveLocation", "boundWithTitles", "holdingsData"})
+@EqualsAndHashCode(exclude = {"effectiveCallNumberComponents", "effectiveLocation", "boundWithTitles", "holdingsData", "tenantId"})
 public class Item implements BulkOperationsEntity, ElectronicAccessEntity {
 
   @JsonProperty("id")
@@ -354,8 +355,12 @@ public Boolean getDiscoverySuppress() {
   }
 
   @Override
-  public void setTenantToNotes() {
-    getNotes().forEach(note -> note.setTenantId(tenantId));
+  public void setTenantToNotes(List<TenantNotePair> tenantNotePairs) {
+    getNotes().forEach(note -> note.setTenantId(
+      tenantNotePairs.stream()
+        .filter(pair -> pair.getNoteTypeId().equals(note.getItemNoteTypeId()))
+        .map(pair -> pair.getTenantId()).findFirst().orElseGet(() -> tenantId)
+    ));
   }
 
   @Override

diff --git a/src/main/java/org/folio/bulkops/domain/bean/ItemNote.java b/src/main/java/org/folio/bulkops/domain/bean/ItemNote.java
@@ -23,5 +23,6 @@ public class ItemNote {
   @JsonProperty("staffOnly")
   private Boolean staffOnly = false;
   private String tenantId;
+  private String itemNoteTypeName;
 }
 
diff --git a/src/main/java/org/folio/bulkops/domain/converter/HoldingsLocationConverter.java b/src/main/java/org/folio/bulkops/domain/converter/HoldingsLocationConverter.java
@@ -2,6 +2,8 @@
 
 import org.folio.bulkops.service.HoldingsReferenceHelper;
 
+import static org.folio.bulkops.util.Constants.ARRAY_DELIMITER;
+
 public class HoldingsLocationConverter extends BaseConverter<String> {
 
   @Override
@@ -11,6 +13,7 @@ public String convertToObject(String value) {
 
   @Override
   public String convertToString(String object) {
-    return HoldingsReferenceHelper.service().getLocationById(object).getName();
+    var objTenant = object.split(ARRAY_DELIMITER);
+    return HoldingsReferenceHelper.service().getLocationById(objTenant[0], objTenant.length > 1 ? objTenant[1] : null).getName();
   }
 }
diff --git a/src/main/java/org/folio/bulkops/domain/converter/HoldingsNoteListConverter.java b/src/main/java/org/folio/bulkops/domain/converter/HoldingsNoteListConverter.java
@@ -1,22 +1,34 @@
 package org.folio.bulkops.domain.converter;
 
+import static java.util.Objects.isNull;
 import static org.folio.bulkops.domain.format.SpecialCharacterEscaper.escape;
 import static org.folio.bulkops.domain.format.SpecialCharacterEscaper.restore;
 import static org.folio.bulkops.util.Constants.ARRAY_DELIMITER;
 import static org.folio.bulkops.util.Constants.ITEM_DELIMITER;
 import static org.folio.bulkops.util.Constants.ITEM_DELIMITER_PATTERN;
+import static org.folio.bulkops.util.FolioExecutionContextUtil.prepareContextForTenant;
 import static org.folio.bulkops.util.Utils.booleanToStringNullSafe;
 
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
+import lombok.RequiredArgsConstructor;
+import lombok.extern.log4j.Log4j2;
 import org.apache.commons.lang3.ObjectUtils;
 import org.folio.bulkops.domain.bean.HoldingsNote;
+import org.folio.bulkops.exception.ConverterException;
 import org.folio.bulkops.exception.EntityFormatException;
+import org.folio.bulkops.exception.NotFoundException;
 import org.folio.bulkops.service.HoldingsReferenceHelper;
+import org.folio.spring.FolioExecutionContext;
+import org.folio.spring.FolioModuleMetadata;
+import org.folio.spring.scope.FolioExecutionContextSetter;
+import org.springframework.stereotype.Component;
 
+@Log4j2
 public class HoldingsNoteListConverter extends BaseConverter<List<HoldingsNote>> {
   private static final int NUMBER_OF_HOLDINGS_NOTE_ELEMENTS = 5;
   private static final int HOLDINGS_NOTE_NOTE_TYPE_INDEX = 0;
@@ -36,13 +48,23 @@ public List<HoldingsNote> convertToObject(String value) {
   public String convertToString(List<HoldingsNote> object) {
     return object.stream()
       .filter(Objects::nonNull)
-      .map(note -> String.join(ARRAY_DELIMITER,
-        escape(HoldingsReferenceHelper.service().getNoteTypeNameById(note.getHoldingsNoteTypeId())),
-        escape(note.getNote()),
-        booleanToStringNullSafe(note.getStaffOnly()),
-        note.getTenantId(),
-        note.getHoldingsNoteTypeId()))
-      .collect(Collectors.joining(ITEM_DELIMITER));
+      .map(note -> {
+        var noteTypeName = note.getHoldingsNoteTypeName();
+        if (isNull(noteTypeName)) {
+          noteTypeName = "";
+          try {
+            noteTypeName = HoldingsReferenceHelper.service().getNoteTypeNameById(note.getHoldingsNoteTypeId());
+          } catch (NotFoundException e) {
+            log.error("Holding note type with id = {} not found : {}", note.getHoldingsNoteTypeId(), e.getMessage());
+          }
+        }
+        return String.join(ARRAY_DELIMITER,
+          escape(noteTypeName),
+          escape(note.getNote()),
+          booleanToStringNullSafe(note.getStaffOnly()),
+          note.getTenantId(),
+          note.getHoldingsNoteTypeId());
+      }).collect(Collectors.joining(ITEM_DELIMITER));
   }
 
   private HoldingsNote restoreHoldingsNote(String s) {

diff --git a/src/main/java/org/folio/bulkops/domain/converter/ItemNoteListConverter.java b/src/main/java/org/folio/bulkops/domain/converter/ItemNoteListConverter.java
@@ -1,5 +1,6 @@
 package org.folio.bulkops.domain.converter;
 
+import static java.util.Objects.isNull;
 import static org.apache.commons.lang3.StringUtils.isNotEmpty;
 import static org.folio.bulkops.domain.format.SpecialCharacterEscaper.escape;
 import static org.folio.bulkops.domain.format.SpecialCharacterEscaper.restore;
@@ -13,11 +14,15 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+import lombok.extern.log4j.Log4j2;
 import org.folio.bulkops.domain.bean.ItemNote;
 import org.folio.bulkops.domain.format.SpecialCharacterEscaper;
 import org.folio.bulkops.exception.EntityFormatException;
+import org.folio.bulkops.exception.NotFoundException;
+import org.folio.bulkops.service.HoldingsReferenceHelper;
 import org.folio.bulkops.service.ItemReferenceHelper;
 
+@Log4j2
 public class ItemNoteListConverter extends BaseConverter<List<ItemNote>> {
   private static final int NUMBER_OF_ITEM_NOTE_COMPONENTS = 5;
   private static final int NOTE_TYPE_NAME_INDEX = 0;
@@ -36,12 +41,23 @@ public List<ItemNote> convertToObject(String value) {
   public String convertToString(List<ItemNote> object) {
     return  object.stream()
       .filter(Objects::nonNull)
-      .map(itemNote -> String.join(ARRAY_DELIMITER,
-        escape(ItemReferenceHelper.service().getNoteTypeNameById(itemNote.getItemNoteTypeId(), itemNote.getTenantId())),
-        escape(itemNote.getNote()),
-        escape(booleanToStringNullSafe(itemNote.getStaffOnly())),
-        itemNote.getTenantId(),
-        itemNote.getItemNoteTypeId()))
+      .map(itemNote -> {
+        var noteTypeName = itemNote.getItemNoteTypeName();
+        if (isNull(noteTypeName)) {
+          noteTypeName = "";
+          try {
+            noteTypeName = ItemReferenceHelper.service().getNoteTypeNameById(itemNote.getItemNoteTypeId(), itemNote.getTenantId());
+          } catch (NotFoundException e) {
+            log.error("Item note type with id = {} not found : {}", itemNote.getItemNoteTypeId(), e.getMessage());
+          }
+        }
+          return String.join(ARRAY_DELIMITER,
+            escape(noteTypeName),
+            escape(itemNote.getNote()),
+            escape(booleanToStringNullSafe(itemNote.getStaffOnly())),
+            itemNote.getTenantId(),
+            itemNote.getItemNoteTypeId());
+      })
       .collect(Collectors.joining(ITEM_DELIMITER));
   }
 

diff --git a/src/main/java/org/folio/bulkops/domain/entity/BulkOperationRuleDetails.java b/src/main/java/org/folio/bulkops/domain/entity/BulkOperationRuleDetails.java
@@ -46,5 +46,6 @@ public class BulkOperationRuleDetails {
   @Column(columnDefinition = "jsonb")
   private List<Parameter> parameters;
 
-  private List<String> tenants;
+  private List<String> actionTenants;
+  private List<String> ruleTenants;
 }
diff --git a/src/main/java/org/folio/bulkops/processor/AbstractDataProcessor.java b/src/main/java/org/folio/bulkops/processor/AbstractDataProcessor.java
@@ -19,6 +19,7 @@
 
 import lombok.extern.log4j.Log4j2;
 
+import static java.util.Objects.isNull;
 import static org.folio.bulkops.util.FolioExecutionContextUtil.prepareContextForTenant;
 
 @Log4j2
@@ -27,7 +28,7 @@ public abstract class AbstractDataProcessor<T extends BulkOperationsEntity> impl
   @Autowired
   private ErrorService errorService;
   @Autowired
-  private FolioModuleMetadata folioModuleMetadata;
+  protected FolioModuleMetadata folioModuleMetadata;
   @Autowired
   private ConsortiaService consortiaService;
   @Autowired
@@ -43,9 +44,9 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
       var option = details.getOption();
       for (Action action : details.getActions()) {
         try {
-          updater(option, action, entity, rule).apply(preview);
+          updater(option, action, entity, true).apply(preview);
           validator(entity).validate(option, action, rule);
-          updater(option, action, entity, rule).apply(updated);
+          updater(option, action, entity, false).apply(updated);
         } catch (RuleValidationException e) {
           errorService.saveError(rule.getBulkOperationId(), identifier, e.getMessage());
         } catch (RuleValidationTenantsException e) {
@@ -79,10 +80,10 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
    * @param option {@link UpdateOptionType} for update
    * @param action {@link Action} for update
    * @param action {@link T} for update
-   * @param action {@link BulkOperationRule} for update
+   * @param forPreview {@link Boolean} true if for preview, otherwise false
    * @return updater
    */
-  public abstract Updater<T> updater(UpdateOptionType option, Action action, T entity, BulkOperationRule rule) throws RuleValidationTenantsException;
+  public abstract Updater<T> updater(UpdateOptionType option, Action action, T entity, boolean forPreview) throws RuleValidationTenantsException;
 
   /**
    * Clones object of type {@link T}
@@ -100,4 +101,24 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
    * @return true if objects are equal, otherwise - false
    */
   public abstract boolean compare(T first, T second);
+
+  public String getRecordPropertyName(UpdateOptionType optionType) {
+    return switch (optionType) {
+      case HOLDINGS_NOTE, ITEM_NOTE -> "note type";
+      case PERMANENT_LOAN_TYPE -> "permanent loan type";
+      case TEMPORARY_LOAN_TYPE -> "temporary loan type";
+      case PERMANENT_LOCATION -> "permanent location";
+      case TEMPORARY_LOCATION -> "temporary location";
+      case ELECTRONIC_ACCESS_URL_RELATIONSHIP -> "URL relationship";
+      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
@@ -16,6 +16,7 @@
 
 @Component
 public class ElectronicAccessUpdaterFactory {
+
   public Updater<? extends ElectronicAccessEntity> updater(UpdateOptionType option, Action action) {
     return switch (option) {
       case ELECTRONIC_ACCESS_URL_RELATIONSHIP -> updateUrlRelationship(option, action);