Skip to content

Commit

Permalink
MODBULKOPS-334 - Preventing record update with values from different …
Browse files Browse the repository at this point in the history
…tenants (#264)

* MODBULKOPS-334 Added validation
  • Loading branch information
obozhko-folio authored Sep 26, 2024
1 parent 5ecb891 commit 8f75807
Show file tree
Hide file tree
Showing 20 changed files with 419 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,6 @@ public class BulkOperationRuleDetails {
@Type(JsonBinaryType.class)
@Column(columnDefinition = "jsonb")
private List<Parameter> parameters;

private List<String> tenants;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.folio.bulkops.exception;

public class RuleValidationTenantsException extends Exception {
public RuleValidationTenantsException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,30 @@
import org.folio.bulkops.domain.dto.BulkOperationRuleCollection;
import org.folio.bulkops.domain.dto.UpdateOptionType;
import org.folio.bulkops.exception.RuleValidationException;
import org.folio.bulkops.exception.RuleValidationTenantsException;
import org.folio.bulkops.service.ConsortiaService;
import org.folio.bulkops.service.ErrorService;
import org.folio.spring.FolioExecutionContext;
import org.folio.spring.FolioModuleMetadata;
import org.folio.spring.scope.FolioExecutionContextSetter;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import lombok.extern.log4j.Log4j2;

import static org.folio.bulkops.util.FolioExecutionContextUtil.prepareContextForTenant;

@Log4j2
@Component
public abstract class AbstractDataProcessor<T extends BulkOperationsEntity> implements DataProcessor<T> {
@Autowired
private ErrorService errorService;
@Autowired
private FolioModuleMetadata folioModuleMetadata;
@Autowired
private ConsortiaService consortiaService;
@Autowired
protected FolioExecutionContext folioExecutionContext;

@Override
public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRuleCollection rules) {
Expand All @@ -30,11 +43,17 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
var option = details.getOption();
for (Action action : details.getActions()) {
try {
updater(option, action).apply(preview);
validator(entity).validate(option, action);
updater(option, action).apply(updated);
updater(option, action, entity, rule).apply(preview);
validator(entity).validate(option, action, rule);
updater(option, action, entity, rule).apply(updated);
} catch (RuleValidationException e) {
errorService.saveError(rule.getBulkOperationId(), identifier, e.getMessage());
} catch (RuleValidationTenantsException e) {
try (var ignored = new FolioExecutionContextSetter(prepareContextForTenant(consortiaService.getCentralTenantId(folioExecutionContext.getTenantId()), folioModuleMetadata, folioExecutionContext))) {
log.info("current tenant: {}", folioExecutionContext.getTenantId());
errorService.saveError(rule.getBulkOperationId(), identifier, e.getMessage());
}
log.error(e.getMessage());
} catch (Exception e) {
log.error(String.format("%s id=%s, error: %s", updated.getRecordBulkOperationEntity().getClass().getSimpleName(), "id", e.getMessage()));
errorService.saveError(rule.getBulkOperationId(), identifier, e.getMessage());
Expand All @@ -50,18 +69,20 @@ public UpdatedEntityHolder process(String identifier, T entity, BulkOperationRul
* Returns validator
*
* @param entity entity of type {@link T} to validate
* @return true if {@link UpdateOptionType} and {@link Action} can be applied to entity
* @return true if {@link UpdateOptionType} and {@link Action}, and {@link BulkOperationRule} can be applied to entity
*/
public abstract Validator<UpdateOptionType, Action> validator(T entity);
public abstract Validator<UpdateOptionType, Action, BulkOperationRule> validator(T entity);

/**
* Returns {@link Consumer<T>} for applying changes for entity of type {@link T}
*
* @param option {@link UpdateOptionType} for update
* @param action {@link Action} for update
* @param action {@link T} for update
* @param action {@link BulkOperationRule} for update
* @return updater
*/
public abstract Updater<T> updater(UpdateOptionType option, Action action);
public abstract Updater<T> updater(UpdateOptionType option, Action action, T entity, BulkOperationRule rule) throws RuleValidationTenantsException;

/**
* Clones object of type {@link T}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.folio.bulkops.domain.bean.ExtendedInstance;
import org.folio.bulkops.domain.dto.Action;
import org.folio.bulkops.domain.dto.UpdateOptionType;
import org.folio.bulkops.domain.dto.BulkOperationRule;
import org.folio.bulkops.exception.BulkOperationException;
import org.folio.bulkops.exception.RuleValidationException;
import org.springframework.stereotype.Component;
Expand All @@ -32,8 +33,8 @@ public class FolioInstanceDataProcessor extends AbstractDataProcessor<ExtendedIn
private final InstanceNotesUpdaterFactory instanceNotesUpdaterFactory;

@Override
public Validator<UpdateOptionType, Action> validator(ExtendedInstance extendedInstance) {
return (option, action) -> {
public Validator<UpdateOptionType, Action, BulkOperationRule> validator(ExtendedInstance extendedInstance) {
return (option, action, rule) -> {
if (CLEAR_FIELD.equals(action.getType()) && Set.of(STAFF_SUPPRESS, SUPPRESS_FROM_DISCOVERY).contains(option)) {
throw new RuleValidationException("Suppress flag cannot be cleared.");
} else if (INSTANCE_NOTE.equals(option) && !"FOLIO".equals(extendedInstance.getEntity().getSource())) {
Expand All @@ -45,7 +46,7 @@ public Validator<UpdateOptionType, Action> validator(ExtendedInstance extendedIn
}

@Override
public Updater<ExtendedInstance> updater(UpdateOptionType option, Action action) {
public Updater<ExtendedInstance> updater(UpdateOptionType option, Action action, ExtendedInstance entity, BulkOperationRule rule) {
if (STAFF_SUPPRESS.equals(option)) {
if (SET_TO_TRUE.equals(action.getType())) {
return extendedInstance -> extendedInstance.getEntity().setStaffSuppress(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.folio.bulkops.processor;

import static java.lang.String.format;
import static java.util.Objects.nonNull;
import static org.apache.commons.lang3.ObjectUtils.isEmpty;
import static org.folio.bulkops.domain.dto.UpdateActionType.CLEAR_FIELD;
import static org.folio.bulkops.domain.dto.UpdateActionType.REPLACE_WITH;
Expand All @@ -16,6 +17,7 @@
import static org.folio.bulkops.domain.dto.UpdateOptionType.PERMANENT_LOCATION;
import static org.folio.bulkops.domain.dto.UpdateOptionType.SUPPRESS_FROM_DISCOVERY;
import static org.folio.bulkops.domain.dto.UpdateOptionType.TEMPORARY_LOCATION;
import static org.folio.bulkops.util.Constants.RECORD_CANNOT_BE_UPDATED_ERROR_TEMPLATE;

import java.util.ArrayList;
import java.util.Objects;
Expand All @@ -25,15 +27,16 @@
import org.apache.commons.lang3.StringUtils;
import org.folio.bulkops.domain.bean.ExtendedHoldingsRecord;
import org.folio.bulkops.domain.dto.Action;
import org.folio.bulkops.domain.dto.BulkOperationRule;
import org.folio.bulkops.domain.dto.UpdateActionType;
import org.folio.bulkops.domain.dto.UpdateOptionType;
import org.folio.bulkops.exception.BulkOperationException;
import org.folio.bulkops.exception.NotFoundException;
import org.folio.bulkops.exception.RuleValidationException;
import org.folio.bulkops.exception.RuleValidationTenantsException;
import org.folio.bulkops.service.HoldingsReferenceService;
import org.folio.bulkops.service.ItemReferenceService;
import org.folio.bulkops.service.ElectronicAccessReferenceService;
import org.folio.spring.FolioExecutionContext;
import org.springframework.stereotype.Component;

import lombok.AllArgsConstructor;
Expand All @@ -50,14 +53,13 @@ public class HoldingsDataProcessor extends AbstractDataProcessor<ExtendedHolding
private final HoldingsNotesUpdater holdingsNotesUpdater;
private final ElectronicAccessUpdaterFactory electronicAccessUpdaterFactory;
private final ElectronicAccessReferenceService electronicAccessReferenceService;
private final FolioExecutionContext folioExecutionContext;

private static final Pattern UUID_REGEX =
Pattern.compile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$");

@Override
public Validator<UpdateOptionType, Action> validator(ExtendedHoldingsRecord extendedHoldingsRecord) {
return (option, action) -> {
public Validator<UpdateOptionType, Action, BulkOperationRule> validator(ExtendedHoldingsRecord extendedHoldingsRecord) {
return (option, action, rule) -> {
try {
if ("MARC".equals(holdingsReferenceService.getSourceById(extendedHoldingsRecord.getEntity().getSourceId(), folioExecutionContext.getTenantId()).getName())) {
throw new RuleValidationException("Holdings records that have source \"MARC\" cannot be changed");
Expand All @@ -74,7 +76,10 @@ public Validator<UpdateOptionType, Action> validator(ExtendedHoldingsRecord exte
};
}

public Updater<ExtendedHoldingsRecord> updater(UpdateOptionType option, Action action) {
public Updater<ExtendedHoldingsRecord> updater(UpdateOptionType option, Action action, ExtendedHoldingsRecord entity, BulkOperationRule rule) throws RuleValidationTenantsException {
if (ruleTenantsAreNotValid(rule, action, entity)) {
throw new RuleValidationTenantsException(String.format(RECORD_CANNOT_BE_UPDATED_ERROR_TEMPLATE, entity.getIdentifier(org.folio.bulkops.domain.dto.IdentifierType.ID), entity.getTenant(), option.getValue()));
}
if (isElectronicAccessUpdate(option)) {
return (Updater<ExtendedHoldingsRecord>) electronicAccessUpdaterFactory.updater(option, action);
} else if (REPLACE_WITH == action.getType()) {
Expand Down Expand Up @@ -181,4 +186,11 @@ public boolean compare(ExtendedHoldingsRecord first, ExtendedHoldingsRecord seco
public Class<ExtendedHoldingsRecord> getProcessedType() {
return ExtendedHoldingsRecord.class;
}

private boolean ruleTenantsAreNotValid(BulkOperationRule rule, Action action, ExtendedHoldingsRecord extendedHolding) {
var ruleTenants = rule.getRuleDetails().getTenants();
var actionTenants = action.getTenants();
return nonNull(ruleTenants) && !ruleTenants.isEmpty() && !ruleTenants.contains(extendedHolding.getTenant()) ||
nonNull(actionTenants) && !actionTenants.isEmpty() && !actionTenants.contains(extendedHolding.getTenant());
}
}
22 changes: 17 additions & 5 deletions src/main/java/org/folio/bulkops/processor/ItemDataProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static java.lang.String.format;
import static java.util.Objects.isNull;
import static java.util.Objects.nonNull;
import static org.apache.commons.lang3.StringUtils.isEmpty;
import static org.folio.bulkops.domain.dto.UpdateActionType.CLEAR_FIELD;
import static org.folio.bulkops.domain.dto.UpdateActionType.REPLACE_WITH;
Expand All @@ -10,6 +11,7 @@
import static org.folio.bulkops.domain.dto.UpdateOptionType.PERMANENT_LOAN_TYPE;
import static org.folio.bulkops.domain.dto.UpdateOptionType.STATUS;
import static org.folio.bulkops.domain.dto.UpdateOptionType.SUPPRESS_FROM_DISCOVERY;
import static org.folio.bulkops.util.Constants.RECORD_CANNOT_BE_UPDATED_ERROR_TEMPLATE;

import java.util.ArrayList;
import java.util.Date;
Expand All @@ -22,11 +24,12 @@
import org.folio.bulkops.domain.bean.ItemLocation;
import org.folio.bulkops.domain.dto.Action;
import org.folio.bulkops.domain.dto.UpdateOptionType;
import org.folio.bulkops.domain.dto.BulkOperationRule;
import org.folio.bulkops.exception.BulkOperationException;
import org.folio.bulkops.exception.RuleValidationException;
import org.folio.bulkops.exception.RuleValidationTenantsException;
import org.folio.bulkops.service.HoldingsReferenceService;
import org.folio.bulkops.service.ItemReferenceService;
import org.folio.spring.FolioExecutionContext;
import org.springframework.stereotype.Component;

import lombok.AllArgsConstructor;
Expand All @@ -39,11 +42,10 @@ public class ItemDataProcessor extends AbstractDataProcessor<ExtendedItem> {
private final HoldingsReferenceService holdingsReferenceService;
private final ItemReferenceService itemReferenceService;
private final ItemsNotesUpdater itemsNotesUpdater;
private final FolioExecutionContext folioExecutionContext;

@Override
public Validator<UpdateOptionType, Action> validator(ExtendedItem extendedItem) {
return (option, action) -> {
public Validator<UpdateOptionType, Action, BulkOperationRule> validator(ExtendedItem extendedItem) {
return (option, action, rule) -> {
if (CLEAR_FIELD == action.getType() && STATUS == option) {
throw new RuleValidationException("Status field can not be cleared");
} else if (CLEAR_FIELD == action.getType() && PERMANENT_LOAN_TYPE == option) {
Expand All @@ -66,7 +68,10 @@ public Validator<UpdateOptionType, Action> validator(ExtendedItem extendedItem)
}

@Override
public Updater<ExtendedItem> updater(UpdateOptionType option, Action action) {
public Updater<ExtendedItem> updater(UpdateOptionType option, Action action, ExtendedItem entity, BulkOperationRule rule) throws RuleValidationTenantsException {
if (ruleTenantsAreNotValid(rule, action, entity)) {
throw new RuleValidationTenantsException(String.format(RECORD_CANNOT_BE_UPDATED_ERROR_TEMPLATE, entity.getIdentifier(org.folio.bulkops.domain.dto.IdentifierType.ID), entity.getTenant(), option.getValue()));
}
if (REPLACE_WITH == action.getType()) {
return switch (option) {
case PERMANENT_LOAN_TYPE ->
Expand Down Expand Up @@ -156,5 +161,12 @@ private ItemLocation getEffectiveLocation(Item item) {
return isNull(item.getTemporaryLocation()) ? item.getPermanentLocation() : item.getTemporaryLocation();
}
}

private boolean ruleTenantsAreNotValid(BulkOperationRule rule, Action action, ExtendedItem extendedItem) {
var ruleTenants = rule.getRuleDetails().getTenants();
var actionTenants = action.getTenants();
return nonNull(ruleTenants) && !ruleTenants.isEmpty() && !ruleTenants.contains(extendedItem.getTenant()) ||
nonNull(actionTenants) && !actionTenants.isEmpty() && !actionTenants.contains(extendedItem.getTenant());
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.folio.bulkops.domain.bean.User;
import org.folio.bulkops.domain.dto.Action;
import org.folio.bulkops.domain.dto.UpdateOptionType;
import org.folio.bulkops.domain.dto.BulkOperationRule;
import org.folio.bulkops.exception.BulkOperationException;
import org.folio.bulkops.exception.RuleValidationException;
import org.folio.bulkops.service.UserReferenceService;
Expand All @@ -33,8 +34,8 @@ public class UserDataProcessor extends AbstractDataProcessor<User> {
private final UserReferenceService userReferenceService;

@Override
public Validator<UpdateOptionType, Action> validator(User entity) {
return (option, action) -> {
public Validator<UpdateOptionType, Action, BulkOperationRule> validator(User entity) {
return (option, action, rule) -> {
if (EXPIRATION_DATE == option) {
if (action.getType() != REPLACE_WITH) {
throw new RuleValidationException(
Expand Down Expand Up @@ -63,7 +64,7 @@ public Validator<UpdateOptionType, Action> validator(User entity) {
}

@Override
public Updater<User> updater(UpdateOptionType option, Action action) {
public Updater<User> updater(UpdateOptionType option, Action action, User entity, BulkOperationRule rule) {
return switch (option) {
case PATRON_GROUP -> user -> user.setPatronGroup(action.getUpdated());
case EXPIRATION_DATE -> user -> {
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/folio/bulkops/processor/Validator.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package org.folio.bulkops.processor;

import org.folio.bulkops.exception.RuleValidationException;
import org.folio.bulkops.exception.RuleValidationTenantsException;

@FunctionalInterface
public interface Validator<T, U> {
void validate(T t, U u) throws RuleValidationException;
public interface Validator<T, U, V> {
void validate(T t, U u, V v) throws RuleValidationException, RuleValidationTenantsException;
}
4 changes: 2 additions & 2 deletions src/main/java/org/folio/bulkops/service/ConsortiaService.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ public String getCentralTenantId(String currentTenantId) {
var userTenantCollection = consortiaClient.getUserTenantCollection();
var userTenants = userTenantCollection.getUserTenants();
if (!userTenants.isEmpty()) {
log.debug("userTenants: {}", userTenants);
log.info("userTenants: {}", userTenants);
return userTenants.get(0).getCentralTenantId();
}
log.debug("No central tenant found for {}", currentTenantId);
log.info("No central tenant found for {}", currentTenantId);
return StringUtils.EMPTY;
}

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/folio/bulkops/service/RuleService.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public BulkOperationRuleCollection saveRules(BulkOperationRuleCollection ruleCol
.initialValue(action.getInitial())
.updatedValue(action.getUpdated())
.parameters(action.getParameters())
.tenants(action.getTenants())
.build()));
});
return ruleCollection;
Expand All @@ -72,7 +73,8 @@ private BulkOperationRule mapBulkOperationRuleToDto(org.folio.bulkops.domain.ent
.type(details.getUpdateAction())
.initial(details.getInitialValue())
.updated(details.getUpdatedValue())
.parameters(details.getParameters()))
.parameters(details.getParameters())
.tenants(details.getTenants()))
.toList()));
}

Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/folio/bulkops/util/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public class Constants {
public static final String MSG_ERROR_OPTIMISTIC_LOCKING_DEFAULT = "The record cannot be saved because it is not the most recent version.";

public static final String CSV_MSG_ERROR_TEMPLATE_OPTIMISTIC_LOCKING = "The record cannot be saved because it is not the most recent version. Stored version is %s, bulk edit version is %s.";
public static final String RECORD_CANNOT_BE_UPDATED_ERROR_TEMPLATE = "%s cannot be updated because the record is associated with %s and %s is not associated with this tenant.";
public static final String ITEM_TYPE = "ITEM";
public static final String HOLDING_TYPE = "HOLDINGS_RECORD";
public static final Set<String> SPLIT_NOTE_ENTITIES = Set.of(ITEM_TYPE, HOLDING_TYPE);
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/db/changelog/changelog-master.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@
<include file="changes/14-06-2024_add_marc_links_to_bulk_operation_table.xml" relativeToChangelogFile="true"/>
<include file="changes/18-06-2024_updates_for_editing_marc.xml" relativeToChangelogFile="true"/>
<include file="changes/29-08-2024_add_used_tenants.xml" relativeToChangelogFile="true"/>
<include file="changes/20-09-2024_add_tenants_to_rules.xml" relativeToChangelogFile="true"/>
</databaseChangeLog>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE bulk_operation_rule_details
ADD COLUMN IF NOT EXISTS tenants TEXT[];
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.8.xsd">


<changeSet id="20-09-2024_add_tenants_to_rules.sql" author="firebird">
<sqlFile path="20-09-2024_add_tenants_to_rules.sql" relativeToChangelogFile="true" />
</changeSet>

</databaseChangeLog>
6 changes: 6 additions & 0 deletions src/main/resources/swagger.api/schemas/action.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
"items": {
"$ref": "action_parameter.json#/Parameter"
}
},
"tenants": {
"type": "array",
"items": {
"type": "string"
}
}
},
"required": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
"description": "Option to change",
"$ref": "update_option_type.json#/UpdateOptionType"
},
"tenants": {
"type": "array",
"items": {
"type": "string"
}
},
"actions": {
"type": "array",
"items": {
Expand Down
Loading

0 comments on commit 8f75807

Please sign in to comment.