diff --git a/pom.xml b/pom.xml index b51dd50dd..17a131976 100644 --- a/pom.xml +++ b/pom.xml @@ -70,6 +70,7 @@ + org.marc4j marc4j @@ -185,11 +186,6 @@ vertx-unit test - - io.vertx - vertx-junit5 - test - org.apache.commons commons-lang3 diff --git a/ramls/holdingsrecord.json b/ramls/holdingsrecord.json index 8638bf087..e33795c7f 100644 --- a/ramls/holdingsrecord.json +++ b/ramls/holdingsrecord.json @@ -14,7 +14,7 @@ }, "hrid": { "type": "string", - "description": "the human readable ID, also called eye readable ID. Unique. On creation it can be provided, otherwise the system assigns the next sequential ID using the settings of the /hrid-settings-storage/hrid-settings API. An update with a different hrid is rejected. An update without hrid is populated with the holdings record's current hrid." + "description": "the human readable ID, also called eye readable ID. A system-assigned sequential ID which maps to the Instance ID" }, "holdingsTypeId": { "type": "string", diff --git a/src/main/java/org/folio/persist/HoldingsRepository.java b/src/main/java/org/folio/persist/HoldingsRepository.java index e3954ef3a..2845cf1d1 100644 --- a/src/main/java/org/folio/persist/HoldingsRepository.java +++ b/src/main/java/org/folio/persist/HoldingsRepository.java @@ -5,14 +5,10 @@ import io.vertx.core.Context; import io.vertx.core.Future; -import io.vertx.core.json.JsonObject; import io.vertx.sqlclient.Row; import io.vertx.sqlclient.RowSet; -import io.vertx.sqlclient.Tuple; -import java.util.List; import java.util.Map; import org.folio.cql2pgjson.CQL2PgJSON; -import org.folio.dbschema.ObjectMapperTool; import org.folio.rest.jaxrs.model.HoldingsRecord; import org.folio.rest.persist.cql.CQLWrapper; @@ -21,28 +17,6 @@ public HoldingsRepository(Context context, Map okapiHeaders) { super(postgresClient(context, okapiHeaders), HOLDINGS_RECORD_TABLE, HoldingsRecord.class); } - /** - * Upsert holdings records. - * - *

Returns - * { "holdingsRecords": [{"old": {...}, "new": {...}}, ...], - * "items": [{"old": {...}, "new": {...}}, ...] - * } - * providing old and new jsonb content of all updated holdings and items. For a newly - * inserted holding only "new" is provided. - * - * @param holdingsRecords records to insert or update (upsert) - */ - public Future upsert(List holdingsRecords) { - try { - var array = ObjectMapperTool.getMapper().writeValueAsString(holdingsRecords); - return postgresClient.selectSingle("SELECT upsert_holdings($1::text::jsonb)", Tuple.of(array)) - .map(row -> row.getJsonObject(0)); - } catch (Exception e) { - return Future.failedFuture(e); - } - } - /** * Delete by CQL. For each deleted record return a {@link Row} with the instance id String * and with the holdings' jsonb String. diff --git a/src/main/java/org/folio/rest/support/EndpointFailureHandler.java b/src/main/java/org/folio/rest/support/EndpointFailureHandler.java index ce86438c7..0020d216b 100644 --- a/src/main/java/org/folio/rest/support/EndpointFailureHandler.java +++ b/src/main/java/org/folio/rest/support/EndpointFailureHandler.java @@ -13,7 +13,6 @@ import org.folio.rest.exceptions.NotFoundException; import org.folio.rest.exceptions.ValidationException; import org.folio.rest.jaxrs.model.Errors; -import org.folio.rest.persist.PgExceptionFacade; import org.folio.rest.persist.PgExceptionUtil; import org.folio.rest.persist.cql.CQLQueryValidationException; import org.folio.rest.tools.client.exceptions.ResponseException; @@ -69,10 +68,6 @@ public static Response failureResponse(Throwable error) { } else if (error instanceof ResponseException) { return ((ResponseException) error).getResponse(); } - var sqlState = new PgExceptionFacade(error).getSqlState(); - if ("239HR".equals(sqlState)) { // Cannot change hrid of holdings record - return textPlainResponse(400, error); - } String message = PgExceptionUtil.badRequestMessage(error); if (message != null) { return textPlainResponse(400, message); diff --git a/src/main/java/org/folio/services/domainevent/AbstractDomainEventPublisher.java b/src/main/java/org/folio/services/domainevent/AbstractDomainEventPublisher.java index eb2a391a5..c9b0f17c1 100644 --- a/src/main/java/org/folio/services/domainevent/AbstractDomainEventPublisher.java +++ b/src/main/java/org/folio/services/domainevent/AbstractDomainEventPublisher.java @@ -11,7 +11,6 @@ import io.vertx.core.Future; import io.vertx.core.Handler; -import io.vertx.core.json.JsonObject; import java.util.Collection; import java.util.List; import javax.ws.rs.core.Response; @@ -93,13 +92,6 @@ public Future publishAllRemoved() { return domainEventService.publishAllRecordsRemoved(); } - public Future publishUpserted(String instanceId, JsonObject oldRecord, JsonObject newRecord) { - if (oldRecord == null) { - return domainEventService.publishRecordCreated(instanceId, newRecord.encode()); - } - return domainEventService.publishRecordUpdated(instanceId, oldRecord.encode(), newRecord.encode()); - } - public Handler publishUpdated(D oldRecord) { return response -> { if (!isUpdateSuccessResponse(response)) { diff --git a/src/main/java/org/folio/services/domainevent/CommonDomainEventPublisher.java b/src/main/java/org/folio/services/domainevent/CommonDomainEventPublisher.java index c8bed84f2..59c540832 100644 --- a/src/main/java/org/folio/services/domainevent/CommonDomainEventPublisher.java +++ b/src/main/java/org/folio/services/domainevent/CommonDomainEventPublisher.java @@ -117,11 +117,6 @@ Future publishRecordUpdated(String instanceId, T oldRecord, T newRecord) { return publish(instanceId, domainEvent); } - Future publishRecordUpdated(String instanceId, String oldRecord, String newRecord) { - var domainEvent = DomainEventRaw.updateEvent(oldRecord, newRecord, tenantId(okapiHeaders)); - return publish(instanceId, domainEvent); - } - Future publishRecordsUpdated(Collection> updatedRecords) { if (updatedRecords.isEmpty()) { return succeededFuture(); @@ -139,11 +134,6 @@ Future publishRecordCreated(String instanceId, T newRecord) { return publish(instanceId, domainEvent); } - Future publishRecordCreated(String id, String newRecord) { - var domainEvent = DomainEventRaw.createEvent(newRecord, tenantId(okapiHeaders)); - return publish(id, domainEvent); - } - Future publishRecordsCreated(List> records) { if (records.isEmpty()) { return succeededFuture(); diff --git a/src/main/java/org/folio/services/holding/HoldingsService.java b/src/main/java/org/folio/services/holding/HoldingsService.java index 1304e6b67..78dab137f 100644 --- a/src/main/java/org/folio/services/holding/HoldingsService.java +++ b/src/main/java/org/folio/services/holding/HoldingsService.java @@ -14,12 +14,14 @@ import static org.folio.rest.persist.PgUtil.postSync; import static org.folio.rest.persist.PgUtil.postgresClient; import static org.folio.services.batch.BatchOperationContextFactory.buildBatchOperationContext; +import static org.folio.validator.HridValidators.refuseWhenHridChanged; +import io.vertx.core.AsyncResult; import io.vertx.core.CompositeFuture; import io.vertx.core.Context; import io.vertx.core.Future; +import io.vertx.core.Handler; import io.vertx.core.Promise; -import io.vertx.core.json.JsonObject; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -30,11 +32,11 @@ import org.folio.persist.HoldingsRepository; import org.folio.persist.InstanceInternalRepository; import org.folio.rest.jaxrs.model.HoldingsRecord; +import org.folio.rest.jaxrs.model.Item; import org.folio.rest.persist.PostgresClient; +import org.folio.rest.persist.SQLConnection; import org.folio.rest.support.CqlQuery; -import org.folio.rest.support.EndpointFailureHandler; import org.folio.rest.support.HridManager; -import org.folio.rest.tools.utils.OptimisticLockingUtil; import org.folio.services.caches.ConsortiumData; import org.folio.services.caches.ConsortiumDataCache; import org.folio.services.consortium.ConsortiumService; @@ -42,16 +44,17 @@ import org.folio.services.consortium.entities.SharingInstance; import org.folio.services.domainevent.HoldingDomainEventPublisher; import org.folio.services.domainevent.ItemDomainEventPublisher; +import org.folio.services.item.ItemService; import org.folio.validator.CommonValidators; import org.folio.validator.NotesValidators; public class HoldingsService { private static final Logger log = getLogger(HoldingsService.class); - private static final String INSTANCE_ID = "instanceId"; private final Context vertxContext; private final Map okapiHeaders; private final PostgresClient postgresClient; private final HridManager hridManager; + private final ItemService itemService; private final HoldingsRepository holdingsRepository; private final ItemDomainEventPublisher itemEventService; private final HoldingDomainEventPublisher domainEventPublisher; @@ -62,6 +65,7 @@ public HoldingsService(Context context, Map okapiHeaders) { this.vertxContext = context; this.okapiHeaders = okapiHeaders; + itemService = new ItemService(context, okapiHeaders); postgresClient = postgresClient(context, okapiHeaders); hridManager = new HridManager(postgresClient); holdingsRepository = new HoldingsRepository(context, okapiHeaders); @@ -86,7 +90,7 @@ public Future updateHoldingRecord(String holdingId, HoldingsRecord hol return holdingsRepository.getById(holdingId) .compose(existingHoldingsRecord -> { if (holdingsRecordFound(existingHoldingsRecord)) { - return updateHolding(holdingsRecord); + return updateHolding(existingHoldingsRecord, holdingsRecord); } else { return createHolding(holdingsRecord); } @@ -159,75 +163,37 @@ public Future createHoldings(List holdings, boolean up } return Future.succeededFuture(); }) - .compose(ar -> { - if (upsert) { - return upsertHoldings(holdings, optimisticLocking); - } else { - return hridManager.populateHridForHoldings(holdings) - .compose(NotesValidators::refuseHoldingLongNotes) - .compose(result -> buildBatchOperationContext(upsert, holdings, - holdingsRepository, HoldingsRecord::getId)) - .compose(batchOperation -> postSync(HOLDINGS_RECORD_TABLE, holdings, MAX_ENTITIES, - upsert, optimisticLocking, okapiHeaders, vertxContext, PostHoldingsStorageBatchSynchronousResponse.class) - .onSuccess(domainEventPublisher.publishCreatedOrUpdated(batchOperation))); - } - }); - } - - public Future upsertHoldings(List holdings, boolean optimisticLocking) { - try { - if (optimisticLocking) { - OptimisticLockingUtil.unsetVersionIfMinusOne(holdings); - } else { - if (! OptimisticLockingUtil.isSuppressingOptimisticLockingAllowed()) { - return Future.succeededFuture(Response.status(413).entity( - "DB_ALLOW_SUPPRESS_OPTIMISTIC_LOCKING environment variable doesn't allow " - + "to disable optimistic locking").build()); - } - OptimisticLockingUtil.setVersionToMinusOne(holdings); - } - - return NotesValidators.refuseHoldingLongNotes(holdings) - .compose(x -> holdingsRepository.upsert(holdings)) - .onSuccess(this::publishEvents) - .map(Response.status(201).build()) - .otherwise(EndpointFailureHandler::failureResponse); - } catch (ReflectiveOperationException e) { - throw new SecurityException(e); - } + .compose(ar -> hridManager.populateHridForHoldings(holdings) + .compose(NotesValidators::refuseHoldingLongNotes) + .compose(result -> buildBatchOperationContext(upsert, holdings, + holdingsRepository, HoldingsRecord::getId)) + .compose(batchOperation -> postSync(HOLDINGS_RECORD_TABLE, holdings, MAX_ENTITIES, + upsert, optimisticLocking, okapiHeaders, vertxContext, PostHoldingsStorageBatchSynchronousResponse.class) + .onSuccess(domainEventPublisher.publishCreatedOrUpdated(batchOperation)))); } - private Future updateHolding(HoldingsRecord newHoldings) { + private Future updateHolding(HoldingsRecord oldHoldings, HoldingsRecord newHoldings) { newHoldings.setEffectiveLocationId(calculateEffectiveLocation(newHoldings)); if (Integer.valueOf(-1).equals(newHoldings.getVersion())) { newHoldings.setVersion(null); // enforce optimistic locking } - return NotesValidators.refuseLongNotes(newHoldings) - .compose(x -> holdingsRepository.upsert(List.of(newHoldings))) - .onSuccess(this::publishEvents) - .map(x -> PutHoldingsStorageHoldingsByHoldingsRecordIdResponse.respond204()); - } + return refuseWhenHridChanged(oldHoldings, newHoldings) + .compose(notUsed -> NotesValidators.refuseLongNotes(newHoldings)) + .compose(notUsed -> { + final Promise> overallResult = promise(); + + postgresClient.startTx( + connection -> holdingsRepository.update(connection, oldHoldings.getId(), newHoldings) + .compose(updateRes -> itemService.updateItemsOnHoldingChanged(connection, newHoldings)) + .onComplete(handleTransaction(connection, overallResult))); - private void publishEvents(JsonObject holdingsItems) { - Map instanceIdByHoldingsRecordId = new HashMap<>(); - holdingsItems.getJsonArray("holdingsRecords").forEach(o -> { - var oldHolding = ((JsonObject) o).getJsonObject("old"); - var newHolding = ((JsonObject) o).getJsonObject("new"); - var instanceId = newHolding.getString(INSTANCE_ID); - var holdingId = newHolding.getString("id"); - instanceIdByHoldingsRecordId.put(holdingId, instanceId); - domainEventPublisher.publishUpserted(instanceId, oldHolding, newHolding); - }); - holdingsItems.getJsonArray("items").forEach(o -> { - var oldItem = ((JsonObject) o).getJsonObject("old"); - var newItem = ((JsonObject) o).getJsonObject("new"); - var instanceId = instanceIdByHoldingsRecordId.get(newItem.getString("holdingsRecordId")); - oldItem.put(INSTANCE_ID, instanceId); - newItem.put(INSTANCE_ID, instanceId); - itemEventService.publishUpserted(instanceId, oldItem, newItem); - }); + return overallResult.future() + .compose(itemsBeforeUpdate -> itemEventService.publishUpdated(oldHoldings, newHoldings, itemsBeforeUpdate)) + .map(res -> PutHoldingsStorageHoldingsByHoldingsRecordIdResponse.respond204()) + .onSuccess(domainEventPublisher.publishUpdated(oldHoldings)); + }); } private String calculateEffectiveLocation(HoldingsRecord holdingsRecord) { @@ -241,6 +207,31 @@ private String calculateEffectiveLocation(HoldingsRecord holdingsRecord) { } } + private Handler> handleTransaction( + AsyncResult connection, Promise overallResult) { + + return transactionResult -> { + if (transactionResult.succeeded()) { + postgresClient.endTx(connection, commitResult -> { + if (commitResult.succeeded()) { + overallResult.complete(transactionResult.result()); + } else { + log.error("Unable to commit transaction", commitResult.cause()); + overallResult.fail(commitResult.cause()); + } + }); + } else { + log.error("Reverting transaction"); + postgresClient.rollbackTx(connection, revertResult -> { + if (revertResult.failed()) { + log.error("Unable to revert transaction", revertResult.cause()); + } + overallResult.fail(transactionResult.cause()); + }); + } + }; + } + private CompositeFuture createShadowInstancesIfNeeded(List holdingsRecords, ConsortiumData consortiumData) { Map> instanceFuturesMap = new HashMap<>(); diff --git a/src/main/resources/templates/db_scripts/instance-hr-item/hridTrigger.sql b/src/main/resources/templates/db_scripts/instance-hr-item/hridTrigger.sql deleted file mode 100644 index 1990ba1e3..000000000 --- a/src/main/resources/templates/db_scripts/instance-hr-item/hridTrigger.sql +++ /dev/null @@ -1,32 +0,0 @@ --- Fill in the next hrid from the sequence when the instance/holding/item --- doesn't have a hrid. -CREATE OR REPLACE FUNCTION hrid_trigger() RETURNS TRIGGER -AS $$ -DECLARE - name TEXT; - hrid TEXT; - prefix TEXT; - zeroes BOOLEAN; -BEGIN - IF NEW.jsonb->'hrid' IS NOT NULL THEN - RETURN NEW; - END IF; - name = CASE TG_TABLE_NAME - WHEN 'instance' THEN 'instances' - WHEN 'holdings_record' THEN 'holdings' - WHEN 'item' THEN 'items' - END; - SELECT nextval('hrid_' || name || '_seq'), jsonb->name->>'prefix', jsonb->>'commonRetainLeadingZeroes' - INTO STRICT hrid, prefix, zeroes FROM hrid_settings; - IF zeroes IS TRUE THEN - hrid = repeat('0', 11 - length(hrid)) || hrid; - END IF; - NEW.jsonb = jsonb_set(NEW.jsonb, '{hrid}', to_jsonb(concat(prefix, hrid))); - RETURN NEW; -END; -$$ language 'plpgsql'; - --- currently only the holding hrid has a trigger; instance and item hrid still use Java code - -DROP TRIGGER IF EXISTS hrid_holdings_record ON holdings_record CASCADE; -CREATE TRIGGER hrid_holdings_record BEFORE INSERT ON holdings_record FOR EACH ROW EXECUTE FUNCTION hrid_trigger(); diff --git a/src/main/resources/templates/db_scripts/instance-hr-item/write.sql b/src/main/resources/templates/db_scripts/instance-hr-item/write.sql deleted file mode 100644 index 7b463f5b7..000000000 --- a/src/main/resources/templates/db_scripts/instance-hr-item/write.sql +++ /dev/null @@ -1,379 +0,0 @@ --- Input is a JSON array of the holdings records to insert. --- Returns the JSON array of the jsonb column of the inserted holdings --- records after triggers (_version, metadata, etc.) have been applied. -CREATE OR REPLACE FUNCTION insert_holdings(input jsonb) - RETURNS jsonb AS $$ - DECLARE - holding jsonb; - holdings jsonb = '[]'; - BEGIN - FOR holding IN - INSERT INTO holdings_record (id, jsonb) - SELECT (jsonb_array_elements(input)->>'id')::uuid, jsonb_array_elements(input) - RETURNING jsonb - LOOP - holdings = holdings || holding; - END LOOP; - RETURN holdings; - END; -$$ LANGUAGE plpgsql; - --- Input is a JSON array of the holdings records to update or insert. --- Returns --- { "holdingsRecords": [{"old": {...}, "new": {...}}, ...], --- "items": [{"old": {...}, "new": {...}}, ...] --- } --- providing old and new jsonb content of all updated holdings and items. For a newly --- inserted holding only "new" is provided. --- "new" has the content after triggers (_version, metadata, etc.) have been applied. -CREATE OR REPLACE FUNCTION upsert_holdings(input jsonb) - RETURNS jsonb AS $$ - DECLARE - holding jsonb; - holdings jsonb = '[]'; - item jsonb; - items jsonb = '[]'; - BEGIN - FOR holding IN - UPDATE holdings_record new - SET jsonb = input.jsonb || jsonb_build_object('hrid', COALESCE(input.jsonb->'hrid', old.jsonb->'hrid')) - FROM (SELECT (jsonb_array_elements(input)->>'id')::uuid id, jsonb_array_elements(input) jsonb) input, - (SELECT id, jsonb FROM holdings_record FOR UPDATE) old - WHERE new.id = input.id - AND old.id = input.id - RETURNING jsonb_build_object('old', old.jsonb, 'new', new.jsonb) - LOOP - IF holding->'new'->'hrid' <> holding->'old'->'hrid' THEN - RAISE 'Cannot change hrid of holdings record id=%, old hrid=%, new hrid=%', - holding->'old'->>'id', holding->'old'->>'hrid', holding->'new'->>'hrid' - USING ERRCODE='239HR'; - END IF; - holdings := holdings || holding; - FOR item IN - UPDATE item new - SET jsonb = - set_effective_shelving_order(old.jsonb || effective_call_number_components(holding->'new', new.jsonb)) - || jsonb_build_object('effectiveLocationId', - COALESCE(temporarylocationid::text, permanentlocationid::text, holding->'new'->>'effectiveLocationId')) - || jsonb_build_object('metadata', set_metadata(old.jsonb->'metadata', holding->'new'->'metadata')) - FROM (SELECT id, jsonb FROM item FOR UPDATE) old - WHERE old.id = new.id - AND holdingsrecordid = (holding->'new'->>'id')::uuid - AND ( (holding->'new'->'effectiveLocationId' IS DISTINCT FROM holding->'old'->'effectiveLocationId' - AND permanentlocationid IS NULL - AND temporarylocationid IS NULL) - OR (holding->'new'->'callNumber' IS DISTINCT FROM holding->'old'->'callNumber' - AND length(trim(coalesce(old.jsonb->>'itemLevelCallNumber', ''))) = 0) - OR (holding->'new'->'callNumberPrefix' IS DISTINCT FROM holding->'old'->'callNumberPrefix' - AND length(trim(coalesce(old.jsonb->>'itemLevelCallNumberPrefix', ''))) = 0) - OR (holding->'new'->'callNumberSuffix' IS DISTINCT FROM holding->'old'->'callNumberSuffix' - AND length(trim(coalesce(old.jsonb->>'itemLevelCallNumberSuffix', ''))) = 0) - OR (holding->'new'->'callNumberTypeId' IS DISTINCT FROM holding->'old'->'callNumberTypeId' - AND length(trim(coalesce(old.jsonb->>'itemLevelCallNumberTypeId', ''))) = 0) - ) - RETURNING jsonb_build_object('old', old.jsonb, 'new', new.jsonb) - LOOP - items := items || item; - END LOOP; - END LOOP; - - FOR holding IN - INSERT INTO holdings_record (id, jsonb) - SELECT (jsonb_array_elements(input)->>'id')::uuid, jsonb_array_elements(input) - ON CONFLICT DO NOTHING - RETURNING jsonb_build_object('new', jsonb) - LOOP - holdings = holdings || holding; - END LOOP; - - RETURN jsonb_build_object('holdingsRecords', holdings, 'items', items); - END; -$$ LANGUAGE plpgsql; - - -CREATE OR REPLACE FUNCTION set_metadata(item_metadata jsonb, holding_metadata jsonb) - RETURNS jsonb AS $$ - BEGIN - IF holding_metadata->'updatedDate' IS NULL THEN - item_metadata = item_metadata - 'updatedDate'; - ELSE - item_metadata = jsonb_set(item_metadata, '{updatedDate}', holding_metadata->'updatedDate'); - END IF; - IF holding_metadata->'updatedByUserId' IS NULL THEN - item_metadata = item_metadata - 'updatedByUserId'; - ELSE - item_metadata = jsonb_set(item_metadata, '{updatedByUserId}', holding_metadata->'updatedByUserId'); - END IF; - RETURN item_metadata; - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION set_item_effective_values(holding jsonb, item jsonb) - RETURNS jsonb AS $$ - BEGIN - RETURN set_effective_shelving_order( - item - || jsonb_build_object('effectiveLocationId', - COALESCE(item->>'temporaryLocationId', item->>'permanentLocationId', holding->>'effectiveLocationId')) - || effective_call_number_components(holding->'new', new.jsonb)); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION effective_call_number_components(holding jsonb, item jsonb) - RETURNS jsonb AS $$ - BEGIN - RETURN jsonb_build_object('effectiveCallNumberComponents', - effective_call_number_component('callNumber', item->>'itemLevelCallNumber', holding->>'callNumber') - || effective_call_number_component('prefix', item->>'itemLevelCallNumberPrefix', holding->>'callNumberPrefix') - || effective_call_number_component('suffix', item->>'itemLevelCallNumberSuffix', holding->>'callNumberSuffix') - || effective_call_number_component('typeId', item->>'itemLevelCallNumberTypeId', holding->>'callNumberTypeId')); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION effective_call_number_component(key text, value1 text, value2 text) - RETURNS jsonb AS $$ - BEGIN - IF length(trim(value1)) > 0 THEN - RETURN jsonb_build_object(key, value1); - END IF; - IF length(trim(value2)) > 0 THEN - RETURN jsonb_build_object(key, value2); - END IF; - RETURN jsonb_build_object(); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE; - - -CREATE OR REPLACE FUNCTION set_effective_shelving_order(item jsonb) - RETURNS jsonb AS $$ - DECLARE - call_number text; - BEGIN - call_number = trim(item->'effectiveCallNumberComponents'->>'callNumber'); - IF call_number IS NULL OR call_number = '' THEN - RETURN item - 'effectiveShelvingOrder'; - END IF; - call_number = concat_ws(' ', call_number, - trim2null(item->>'volume'), - trim2null(item->>'enumeration'), - trim2null(item->>'chronology'), - trim2null(item->>'copyNumber')); - CASE item->'effectiveCallNumberComponents'->>'typeId' - WHEN '03dd64d0-5626-4ecd-8ece-4531e0069f35' THEN call_number = dewey_call_number(call_number); - WHEN '95467209-6d7b-468b-94df-0f5d7ad2747d' THEN call_number = lc_nlm_call_number(call_number); -- LC - WHEN '054d460d-d6b9-4469-9e37-7a78a2266655' THEN call_number = lc_nlm_call_number(call_number); -- NLM - WHEN 'fc388041-6cd0-4806-8a74-ebe3b9ab4c6e' THEN call_number = su_doc_call_number(call_number); - ELSE NULL; - END CASE; - RETURN item || jsonb_build_object('effectiveShelvingOrder', - concat_ws(' ', call_number, trim2null(item->'effectiveCallNumberComponents'->>'suffix'))); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION dewey_call_number(call_number text) - RETURNS text AS $$ - DECLARE - matches text[]; - class_digits text; - class_decimal text; - cutter text; - other text; - BEGIN - matches = regexp_match(call_number, '^(\d+)(\.\d+)? *\.?(?:([A-Z]\d{1,3}(?:[A-Z]+)?) *(.*)|(.*))$'); - IF matches IS NULL THEN - RETURN NULL; - END IF; - class_digits = matches[1]; - class_decimal = matches[2]; - cutter = matches[3]; - other = numerically_sortable(trim2null(concat(trim2null(matches[4]), trim2null(matches[5])))); - RETURN concat_ws(' ', concat(sortable_number(class_digits), class_decimal), - cutter, - other - ); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION lc_nlm_call_number(call_number text) - RETURNS text AS $$ - DECLARE - matches text[]; - classification text; - classLetters text; - classDigits text; - classDecimal text; - everythingElse text; - classSuffix text; - cutter text; - BEGIN - call_number = upper(call_number); - matches = regexp_match(call_number, '^(([A-Z]+) *(?:(\d+)(\.\d+)?)?)(.*)$'); - IF matches IS NULL THEN - RETURN NULL; - END IF; - classification = trim(matches[1]); - classLetters = trim(matches[2]); - classDigits = trim(matches[3]); - classDecimal = trim(matches[4]); - everythingElse = matches[5]; - CASE substr(classLetters, 1, 1) - -- LC call numbers can't begin with I, O, W, X, or Y - -- NLM call numbers begin with W or S - WHEN 'I', 'O', 'X', 'Y' THEN - RETURN NULL; - ELSE - NULL; - END CASE; - IF classDigits IS NULL THEN - RETURN NULL; - END IF; - IF length(everythingElse) > 0 THEN - -- combining greedy and non-greedy: - -- https://www.postgresql.org/docs/current/functions-matching.html#POSIX-MATCHING-RULES - matches = regexp_match(everythingElse, '(?:(.*?)(\.?[A-Z]\d+|^\.[A-Z]| \.[A-Z])(.*)){1,1}'); - IF matches IS NULL THEN - classSuffix = trim2null(everythingElse); - ELSE - classSuffix = trim2null(matches[1]); - cutter = trim(matches[2] || matches[3]); - END IF; - END IF; - classSuffix = numerically_sortable(classSuffix); - IF substr(classSuffix, 1, 1) BETWEEN 'A' AND 'Z' THEN - classSuffix = '_' || classSuffix; - END IF; - cutter = cutter_shelf_key(cutter); - return trim(concat_ws(' ', classLetters, - concat(length(classDigits), classDigits, classDecimal), - trim(classSuffix), - trim(cutter) - )); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION su_doc_call_number(call_number text) - RETURNS text AS $$ - DECLARE - matches text[]; - BEGIN - matches = regexp_match(upper(call_number), - '^([A-Z]+)\s*(\d+)(\.(?:[A-Z]+\d*|\d+))(/(?:[A-Z]+(?:\d+(?:-\d+)?)?|\d+(?:-\d+)?))?:?(.*)$'); - IF matches IS NULL THEN - RETURN NULL; - END IF; - RETURN concat_ws(' ', matches[1], su_doc_part(matches[2]), su_doc_part(matches[3]), su_doc_part(matches[4]), su_doc_part(matches[5])); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - -CREATE OR REPLACE FUNCTION su_doc_part(part text) - RETURNS text AS $$ - DECLARE - chunk text; - key text = ''; - BEGIN - IF trim(part) = '' THEN - RETURN NULL; - END IF; - IF starts_with(part, '.') OR starts_with(part, '/') OR starts_with(part, '-') OR starts_with(part, ':') THEN - part = substr(part, 2); - END IF; - FOREACH chunk IN ARRAY regexp_split_to_array(part, '[./ -]') LOOP - IF length(key) > 0 THEN - key = concat(key, ' '); - END IF; - chunk = trim(chunk); - CONTINUE WHEN chunk = ''; - IF substring(chunk, 1, 1) BETWEEN 'A' AND 'Z' THEN - key = concat(key, ' !'); - ELSIF length(chunk) >= 3 THEN - key = concat(key, '!'); - END IF; - key = concat(key, numerically_sortable(chunk)); - END LOOP; - RETURN key; - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - --- return null if trim(v) is empty, otherwise trim(v) -CREATE OR REPLACE FUNCTION trim2null(v text) - RETURNS text AS $$ - BEGIN - IF trim(v) = '' THEN - RETURN null; - END IF; - RETURN trim(v); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - --- return n with all leading 0 removed and the resulting string prepended with its length --- 6.78 yiels 46.78, 5 yields 15, 0 yields 0 -CREATE OR REPLACE FUNCTION sortable_number(n text) - RETURNS text AS $$ - BEGIN - n = regexp_replace(n, '^0+', ''); - RETURN concat(length(n), n); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - --- A number is a sequence of digits and may contain a decimal, but not at the first position; --- all leading zeros of a number are removed, and the length is prepended: 00123.45 becomes 6123.45 --- A-Z is kept. --- A sequence of one or more other characters is replaced by a single space. -CREATE OR REPLACE FUNCTION numerically_sortable(s text) - RETURNS text AS $$ - DECLARE - match text; - result text = ''; - BEGIN - FOR match IN SELECT (regexp_matches(upper(s), '[A-Z]+|[0-9][.0-9]*|[^A-Z0-9]+', 'g'))[1] LOOP - CASE - WHEN substring(match, 1, 1) BETWEEN 'A' AND 'Z' THEN - result = result || match || ' '; - WHEN substring(match, 1, 1) BETWEEN '0' AND '9' THEN - result = result || sortable_number(match); - ELSE - result = trim(result) || ' '; - END CASE; - END LOOP; - RETURN trim(result); - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; - - --- A number is a sequence of digits and may contain a decimal, but not at the first position; --- all leading zeros of a number are removed, and the length is prepended: 00123.45 becomes 6123.45 --- A-Z is kept. --- A sequence of one or more other characters is replaced by a single space. -CREATE OR REPLACE FUNCTION cutter_shelf_key(s text) - RETURNS text AS $$ - DECLARE - chunk text; - matches text[]; - cutter text; - suffix text; - result text; - BEGIN - FOREACH chunk IN ARRAY regexp_split_to_array(s, '(?=[A-Z][0-9])') LOOP - matches = regexp_match(chunk, '([A-Z][0-9]+)(.*)'); - IF matches IS NULL THEN - -- before the first cutter - result = trim2null(numerically_sortable(chunk)); - ELSE - cutter = matches[1]; - suffix = trim2null(numerically_sortable(matches[2])); - result = concat_ws(' ', result, cutter, suffix); - END IF; - END LOOP; - RETURN result; - END; -$$ LANGUAGE plpgsql IMMUTABLE PARALLEL SAFE STRICT; diff --git a/src/main/resources/templates/db_scripts/schema.json b/src/main/resources/templates/db_scripts/schema.json index d9f554326..51a6c3f56 100644 --- a/src/main/resources/templates/db_scripts/schema.json +++ b/src/main/resources/templates/db_scripts/schema.json @@ -1126,16 +1126,6 @@ "snippetPath": "populateCirculationNoteIds.sql", "fromModuleVersion": "26.1.0" }, - { - "run": "after", - "snippetPath": "instance-hr-item/write.sql", - "fromModuleVersion": "26.1.0" - }, - { - "run": "after", - "snippetPath": "instance-hr-item/hridTrigger.sql", - "fromModuleVersion": "26.1.0" - }, { "run": "after", "snippetPath": "oaipmh/addCompleteUpdatedDate.sql", diff --git a/src/test/java/org/folio/persist/HoldingsRepositoryTest.java b/src/test/java/org/folio/persist/HoldingsRepositoryTest.java deleted file mode 100644 index 3c667ed77..000000000 --- a/src/test/java/org/folio/persist/HoldingsRepositoryTest.java +++ /dev/null @@ -1,45 +0,0 @@ -package org.folio.persist; - -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; - -import io.vertx.core.Vertx; -import io.vertx.junit5.VertxExtension; -import io.vertx.junit5.VertxTestContext; -import java.util.List; -import java.util.Map; -import org.folio.cql2pgjson.exception.QueryValidationException; -import org.folio.rest.jaxrs.model.HoldingsRecord; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; - -@ExtendWith(VertxExtension.class) -class HoldingsRepositoryTest { - - private static HoldingsRepository holdingsRepository = - new HoldingsRepository(Vertx.vertx().getOrCreateContext(), Map.of()); - private static HoldingsRecord holdingThrowingFoo = new HoldingsRecord() { - public String getId() { - throw new IllegalCallerException("foo"); - } - }; - - @Test - void upsertException(VertxTestContext vtc) { - holdingsRepository.upsert(List.of(holdingThrowingFoo)) - .onComplete(vtc.failing(e -> { - assertThat(e.getCause().getMessage(), is("foo")); - vtc.completeNow(); - })); - } - - @Test - void deleteException(VertxTestContext vtc) { - holdingsRepository.delete(")") - .onComplete(vtc.failing(e -> { - assertThat(e.getCause(), instanceOf(QueryValidationException.class)); - vtc.completeNow(); - })); - } -} diff --git a/src/test/java/org/folio/rest/api/HoldingsStorageTest.java b/src/test/java/org/folio/rest/api/HoldingsStorageTest.java index 72ff4d1a9..ec42f2a90 100644 --- a/src/test/java/org/folio/rest/api/HoldingsStorageTest.java +++ b/src/test/java/org/folio/rest/api/HoldingsStorageTest.java @@ -19,7 +19,6 @@ import static org.folio.rest.support.http.InterfaceUrls.holdingsStorageSyncUnsafeUrl; import static org.folio.rest.support.http.InterfaceUrls.holdingsStorageSyncUrl; import static org.folio.rest.support.http.InterfaceUrls.holdingsStorageUrl; -import static org.folio.rest.support.http.InterfaceUrls.itemsStorageSyncUrl; import static org.folio.rest.support.http.InterfaceUrls.itemsStorageUrl; import static org.folio.rest.support.matchers.PostgresErrorMessageMatchers.isMaximumSequenceValueError; import static org.folio.utility.ModuleUtility.getClient; @@ -55,7 +54,6 @@ import io.vertx.core.json.JsonObject; import java.net.HttpURLConnection; import java.net.URL; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -132,6 +130,7 @@ public static void beforeClass() { prepareTenant(TENANT_WITHOUT_USER_TENANTS_PERMISSIONS, false); } + @SneakyThrows @AfterClass public static void afterClass() { @@ -163,6 +162,22 @@ public void beforeEach() { mockUserTenantsForTenantWithoutPermissions(); } + private void canPostSynchronousBatchUnsafeAndCreateShadowInstance(String subpath) { + OptimisticLockingUtil.configureAllowSuppressOptimisticLocking( + Map.of(OptimisticLockingUtil.DB_ALLOW_SUPPRESS_OPTIMISTIC_LOCKING, "9999-12-31T23:59:59Z")); + mockSharingInstance(); + + JsonArray holdingsArray = threeHoldingsWithoutInstance(); + assertThat(postSynchronousBatchUnsafe(subpath, holdingsArray, CONSORTIUM_MEMBER_TENANT), + statusCodeIs(HTTP_CREATED)); + for (Object hrObj : holdingsArray) { + final JsonObject holding = (JsonObject) hrObj; + assertExists(holding, CONSORTIUM_MEMBER_TENANT); + holdingsMessageChecks.createdMessagePublished(getById(holding.getString("id"), + CONSORTIUM_MEMBER_TENANT).getJson(), CONSORTIUM_MEMBER_TENANT, mockServer.baseUrl()); + } + } + @SneakyThrows @After public void afterEach() { @@ -190,7 +205,7 @@ public void canCreateHolding() { holdingToCreate.put("administrativeNotes", new JsonArray().add(adminNote)); - JsonObject holding = postHoldingsRecord(holdingToCreate).getJson(); + JsonObject holding = holdingsClient.create(holdingToCreate).getJson(); assertThat(holding.getString("id"), is(holdingId.toString())); assertThat(holding.getString("instanceId"), is(instanceId.toString())); @@ -215,8 +230,6 @@ public void canCreateHolding() { assertThat(tags.size(), is(1)); assertThat(tags, hasItem(TAG_VALUE)); - - holdingsMessageChecks.createdMessagePublished(holding, TENANT_ID, mockServer.baseUrl()); } @Test @@ -230,7 +243,7 @@ public void canCreateHoldingWithoutProvidingAnId() { .forInstance(instanceId) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withTags(new JsonObject().put("tagList", new JsonArray().add(TAG_VALUE))); - IndividualResource holdingResponse = postHoldingsRecord(holdingBuilder.create()); + IndividualResource holdingResponse = holdingsClient.create(holdingBuilder.create()); JsonObject holding = holdingResponse.getJson(); @@ -411,7 +424,7 @@ public void canMoveHoldingsToNewInstance() throws ExecutionException, Interrupte .forInstance(instanceId) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .create(); - IndividualResource holdingResource = postHoldingsRecord(holdingToCreate); + IndividualResource holdingResource = holdingsClient.create(holdingToCreate); UUID holdingId = holdingResource.getId(); @@ -437,7 +450,10 @@ public void canMoveHoldingsToNewInstance() throws ExecutionException, Interrupte holdingsMessageChecks.updatedMessagePublished(holdingResource.getJson(), holdingFromGet); - // item doesn't change when holding moves + JsonObject newItem = item.copy() + .put("_version", 2); + + itemMessageChecks.updatedMessagePublished(item, newItem, instanceId.toString()); } @Test @@ -511,7 +527,7 @@ public void cannotPageWithNegativeLimit() throws Exception { instancesClient.create(smallAngryPlanet(instanceId)); - postHoldingsRecord(new HoldingRequestBuilder() + holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .create()); @@ -676,27 +692,27 @@ public void canDeleteHoldingsByCql() { instancesClient.create(smallAngryPlanet(instanceId1)); instancesClient.create(nod(instanceId2)); - final var h1 = postHoldingsRecord(new HoldingRequestBuilder() + final var h1 = holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId1) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withHrid("1234") .create()).getJson(); - final var h2 = postHoldingsRecord(new HoldingRequestBuilder() + final var h2 = holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId1) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withHrid("21234") .create()).getJson(); - final var h3 = postHoldingsRecord(new HoldingRequestBuilder() + final var h3 = holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId2) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withHrid("12") .create()).getJson(); - final var h4 = postHoldingsRecord(new HoldingRequestBuilder() + final var h4 = holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId2) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withHrid("3123") .create()).getJson(); - final var h5 = postHoldingsRecord(new HoldingRequestBuilder() + final var h5 = holdingsClient.create(new HoldingRequestBuilder() .forInstance(instanceId2) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) .withHrid("123") @@ -864,7 +880,7 @@ public void updatingOrRemovingTemporaryLocationChangesEffectiveLocation() instancesClient.create(smallAngryPlanet(instanceId)); setHoldingsSequence(1); - JsonObject holding = postHoldingsRecord(new HoldingRequestBuilder() + JsonObject holding = holdingsClient.create(new HoldingRequestBuilder() .withId(holdingId) .forInstance(instanceId) .withPermanentLocation(MAIN_LIBRARY_LOCATION_ID) @@ -1599,6 +1615,7 @@ public void updatingHoldingsDoesNotUpdateItemsOnAnotherHoldings() firstHolding.put("callNumberSuffix", "updatedFirstCallNumberSuffix"); Response putResponse = update(firstHoldingsUrl, firstHolding); + Response updatedFirstHoldingResponse = get(firstHoldingsUrl); JsonObject updatedFirstHolding = updatedFirstHoldingResponse.getJson(); @@ -2308,8 +2325,7 @@ public void cannotChangeHridAfterCreation() assertThat(response.getStatusCode(), is(HttpURLConnection.HTTP_BAD_REQUEST)); assertThat(response.getBody(), - is("ERROR: Cannot change hrid of holdings record id=" + holdingsId - + ", old hrid=ho00000000001, new hrid=ABC123 (239HR)")); + is("The hrid field cannot be changed: new=ABC123, old=ho00000000001")); log.info("Finished cannotChangeHRIDAfterCreation"); } @@ -2338,11 +2354,16 @@ public void cannotRemoveHridAfterCreation() holdings.remove("hrid"); - getClient().put(holdingsStorageUrl(String.format("/%s", holdingsId)), holdings, TENANT_ID) - .get(10, TimeUnit.SECONDS); + final CompletableFuture updateCompleted = new CompletableFuture<>(); - final Response get = getById(holdingsId.toString()); - assertThat(get.getJson().getString("hrid"), is("ho00000000001")); + getClient().put(holdingsStorageUrl(String.format("/%s", holdingsId)), holdings, TENANT_ID, + text(updateCompleted)); + + final Response response = updateCompleted.get(10, TimeUnit.SECONDS); + + assertThat(response.getStatusCode(), is(HttpURLConnection.HTTP_BAD_REQUEST)); + assertThat(response.getBody(), + is("The hrid field cannot be changed: new=null, old=ho00000000001")); log.info("Finished cannotRemoveHRIDAfterCreation"); } @@ -2636,32 +2657,17 @@ public void canPostSynchronousBatchUnsafe() { } @Test + @Ignore public void canPostSynchronousBatchUnsafeAndCreateShadowInstanceWithoutUpsert() { canPostSynchronousBatchUnsafeAndCreateShadowInstance(""); } @Test + @Ignore public void canPostSynchronousBatchUnsafeAndCreateShadowInstanceWithUpsertTrue() { canPostSynchronousBatchUnsafeAndCreateShadowInstance("?upsert=true"); } - private void canPostSynchronousBatchUnsafeAndCreateShadowInstance(String subpath) { - OptimisticLockingUtil.configureAllowSuppressOptimisticLocking( - Map.of(OptimisticLockingUtil.DB_ALLOW_SUPPRESS_OPTIMISTIC_LOCKING, "9999-12-31T23:59:59Z")); - mockSharingInstance(); - - JsonArray holdingsArray = threeHoldingsWithoutInstance(); - assertThat(postSynchronousBatchUnsafe(subpath, holdingsArray, CONSORTIUM_MEMBER_TENANT), - statusCodeIs(HTTP_CREATED)); - for (Object hrObj : holdingsArray) { - final JsonObject holding = (JsonObject) hrObj; - assertExists(holding, CONSORTIUM_MEMBER_TENANT); - holdingsMessageChecks.createdMessagePublished(getById(holding.getString("id"), - CONSORTIUM_MEMBER_TENANT).getJson(), CONSORTIUM_MEMBER_TENANT, mockServer.baseUrl()); - } - } - - @Test public void canPostSynchronousBatch() { JsonArray holdingsArray = threeHoldings(); @@ -2700,21 +2706,6 @@ public void canPostSynchronousBatchAndCreateShadowInstanceWithUpsertTrue() { canPostSynchronousBatchAndCreateShadowInstance("?upsert=true"); } - private void canPostSynchronousBatchAndCreateShadowInstance(String subpath) { - mockSharingInstance(); - - JsonArray holdingsArray = threeHoldingsWithoutInstance(); - assertThat(postSynchronousBatch(subpath, holdingsArray, CONSORTIUM_MEMBER_TENANT), statusCodeIs(HTTP_CREATED)); - for (Object hrObj : holdingsArray) { - final JsonObject holding = (JsonObject) hrObj; - - assertExists(holding, CONSORTIUM_MEMBER_TENANT); - - holdingsMessageChecks.createdMessagePublished(getById(holding.getString("id"), - CONSORTIUM_MEMBER_TENANT).getJson(), CONSORTIUM_MEMBER_TENANT, mockServer.baseUrl()); - } - } - @Test public void cannotPostSynchronousBatchWithNonExistingInstanceAndNonConsortiumTenant() { JsonArray holdingsArray = threeHoldingsWithoutInstance(); @@ -2798,51 +2789,6 @@ public void canPostSynchronousBatchWithExistingIdWithUpsertTrue() { mockServer.baseUrl()); } - @Test - public void canPostSynchronousBatchWithItemCallNumber() { - final JsonArray holdings = threeHoldings(); - final JsonArray items = sixItems(holdings); - for (int i = 0; i < 3; i++) { - if (i == 0 || i == 2) { - holdings.getJsonObject(i) - .put("callNumberPrefix", "hcnp") - .put("callNumber", "hcn") - .put("callNumberSuffix", "hcns"); - } - } - for (int i = 0; i < 6; i++) { - if (i == 1 || i == 2 || i == 3 || i == 5) { - items.getJsonObject(i) - .put("itemLevelCallNumberPrefix", "icnp") - .put("itemLevelCallNumber", "icn") - .put("itemLevelCallNumberSuffix", "icns"); - } - } - final Response response1 = postSynchronousBatch("?upsert=true", holdings); - assertThat(response1, statusCodeIs(HTTP_CREATED)); - final Response itemResponse = postItemSynchronousBatch("?upsert=true", items); - assertThat(itemResponse, statusCodeIs(HTTP_CREATED)); - - assertItemEffectiveCallNumbers(items, "prefix", "hcnp", "icnp", "icnp", "icnp", "hcnp", "icnp"); - assertItemEffectiveCallNumbers(items, "callNumber", "hcn", "icn", "icn", "icn", "hcn", "icn"); - assertItemEffectiveCallNumbers(items, "suffix", "hcns", "icns", "icns", "icns", "hcns", "icns"); - - for (int i = 0; i < 3; i++) { - if (i == 0 || i == 1) { - holdings.getJsonObject(i) - .put("callNumberPrefix", "xcnp") - .put("callNumber", "xcn") - .put("callNumberSuffix", "xcns"); - } - } - final Response response2 = postSynchronousBatch("?upsert=true", holdings); - assertThat(response2, statusCodeIs(HTTP_CREATED)); - - assertItemEffectiveCallNumbers(items, "prefix", "xcnp", "icnp", "icnp", "icnp", "hcnp", "icnp"); - assertItemEffectiveCallNumbers(items, "callNumber", "xcn", "icn", "icn", "icn", "hcn", "icn"); - assertItemEffectiveCallNumbers(items, "suffix", "xcns", "icns", "icns", "icns", "hcns", "icns"); - } - @Test public void canSearchByDiscoverySuppressProperty() { final IndividualResource instance = instancesClient @@ -3055,6 +3001,21 @@ public void canSetHoldingStatementForSupplementsWithNotes() { assertThat(responseStatement.getString("staffNote"), is("Test staff note")); } + private void canPostSynchronousBatchAndCreateShadowInstance(String subpath) { + mockSharingInstance(); + + JsonArray holdingsArray = threeHoldingsWithoutInstance(); + assertThat(postSynchronousBatch(subpath, holdingsArray, CONSORTIUM_MEMBER_TENANT), statusCodeIs(HTTP_CREATED)); + for (Object hrObj : holdingsArray) { + final JsonObject holding = (JsonObject) hrObj; + + assertExists(holding, CONSORTIUM_MEMBER_TENANT); + + holdingsMessageChecks.createdMessagePublished(getById(holding.getString("id"), + CONSORTIUM_MEMBER_TENANT).getJson(), CONSORTIUM_MEMBER_TENANT, mockServer.baseUrl()); + } + } + private void setHoldingsSequence(long sequenceNumber) { final PostgresClient postgresClient = PostgresClient.getInstance(getVertx(), TENANT_ID); @@ -3112,27 +3073,6 @@ private JsonArray threeHoldingsWithoutInstance() { return holdingsArray; } - /** - * Return six item JsonObject, two for each of the three holdings. The items are not created. - */ - private JsonArray sixItems(JsonArray holdings) { - JsonArray items = new JsonArray(); - for (int i = 0; i < 6; i++) { - items.add(new JsonObject() - .put("id", UUID.randomUUID().toString()) - .put("holdingsRecordId", holdings.getJsonObject(i / 2).getString("id")) - .put("_version", 1) - .put("materialTypeId", bookMaterialTypeID) - .put("permanentLoanTypeId", canCirculateLoanTypeID) - .put("status", new JsonObject().put("name", "Available"))); - } - return items; - } - - private IndividualResource postHoldingsRecord(JsonObject holdingsRecord) { - return holdingsClient.create(holdingsRecord, TENANT_ID, Map.of(X_OKAPI_URL, mockServer.baseUrl())); - } - private Response postSynchronousBatchUnsafe(JsonArray holdingsArray) { return postSynchronousBatch(holdingsStorageSyncUnsafeUrl(""), holdingsArray); } @@ -3169,15 +3109,6 @@ private Response postSynchronousBatch(URL url, JsonArray holdingsArray, String t } } - private Response postItemSynchronousBatch(String subPath, JsonArray itemArray) { - try { - JsonObject itemCollection = new JsonObject().put("items", itemArray); - return getClient().post(itemsStorageSyncUrl(subPath), itemCollection, TENANT_ID).get(10, SECONDS); - } catch (InterruptedException | ExecutionException | TimeoutException e) { - throw new RuntimeException(e); - } - } - private static JsonObject smallAngryPlanet(UUID id) { JsonArray identifiers = new JsonArray(); identifiers.add(identifier(UUID_ISBN, "9781473619777")); @@ -3236,35 +3167,6 @@ private Response getById(String id, String tenantId) { } } - /** - * For each passed item use the id and fetch the item record from database. - */ - private List fetchItems(JsonArray items) { - try { - var result = new ArrayList(); - for (int i = 0; i < items.size(); i++) { - var id = items.getJsonObject(i).getString("id"); - result.add(getClient().get(itemsStorageUrl("/" + id), TENANT_ID).get(5, SECONDS).getJson()); - } - return result; - } catch (InterruptedException | ExecutionException | TimeoutException e) { - throw new RuntimeException(e); - } - } - - /** - * Assert that the effective call number property in the database equals the expected n1 ... n6. - * Fetch the item using the id from items. - */ - private void assertItemEffectiveCallNumbers(JsonArray items, String property, - String n1, String n2, String n3, String n4, String n5, String n6) { - - var actual = fetchItems(items).stream() - .map(item -> item.getJsonObject("effectiveCallNumberComponents").getString(property)) - .collect(Collectors.toList()); - assertThat(actual, is(List.of(n1, n2, n3, n4, n5, n6))); - } - private void assertExists(JsonObject expectedHolding) { assertExists(expectedHolding, TENANT_ID); } @@ -3375,9 +3277,11 @@ public static class ConsortiumInstanceSharingTransformer extends ResponseTransfo @SneakyThrows @Override - public com.github.tomakehurst.wiremock.http.Response transform(Request request, - com.github.tomakehurst.wiremock.http.Response response, FileSource fileSource, - com.github.tomakehurst.wiremock.extension.Parameters parameters) { + public com.github.tomakehurst.wiremock.http.Response transform( + Request request, + com.github.tomakehurst.wiremock.http.Response response, + FileSource fileSource, + com.github.tomakehurst.wiremock.extension.Parameters parameters) { SharingInstance sharingInstance = Json.getObjectMapper().readValue(request.getBody(), SharingInstance.class); diff --git a/src/test/java/org/folio/rest/api/ItemEffectiveCallNumberComponentsTest.java b/src/test/java/org/folio/rest/api/ItemEffectiveCallNumberComponentsTest.java index f6a91d53e..b62c36b04 100644 --- a/src/test/java/org/folio/rest/api/ItemEffectiveCallNumberComponentsTest.java +++ b/src/test/java/org/folio/rest/api/ItemEffectiveCallNumberComponentsTest.java @@ -224,10 +224,7 @@ public void canCalculateEffectiveCallNumberPropertyOnUpdate( final String effectivePropertyName = callNumberProperties.effectivePropertyName; final String initEffectiveValue = StringUtils.firstNonBlank(itemInitValue, holdingsInitValue); - /** expected value after holdings update */ - final String targetEffectiveValue1 = StringUtils.firstNonBlank(itemInitValue, holdingsTargetValue); - /** expected value after holdings and item update */ - final String targetEffectiveValue2 = StringUtils.firstNonBlank(itemTargetValue, holdingsTargetValue); + final String targetEffectiveValue = StringUtils.firstNonBlank(itemTargetValue, holdingsTargetValue); IndividualResource holdings = createHoldingsWithPropertySetAndInstance( holdingsPropertyName, holdingsInitValue @@ -252,9 +249,7 @@ public void canCalculateEffectiveCallNumberPropertyOnUpdate( var itemAfterHoldingsUpdate = getById(createdItem.getJson()); - if (! Objects.equals(initEffectiveValue, targetEffectiveValue1)) { - itemMessageChecks.updatedMessagePublished(createdItem.getJson(), itemAfterHoldingsUpdate); - } + itemMessageChecks.updatedMessagePublished(createdItem.getJson(), itemAfterHoldingsUpdate); if (!Objects.equals(itemInitValue, itemTargetValue)) { itemsClient.replace(createdItem.getId(), itemAfterHoldingsUpdate.copy() @@ -270,7 +265,7 @@ public void canCalculateEffectiveCallNumberPropertyOnUpdate( assertNotNull(updatedEffectiveCallNumberComponents); assertThat(updatedEffectiveCallNumberComponents.getString(effectivePropertyName), - is(targetEffectiveValue2)); + is(targetEffectiveValue)); } private IndividualResource createHoldingsWithPropertySetAndInstance( diff --git a/src/test/java/org/folio/rest/api/ItemEffectiveLocationTest.java b/src/test/java/org/folio/rest/api/ItemEffectiveLocationTest.java index 6782aa40f..661237985 100644 --- a/src/test/java/org/folio/rest/api/ItemEffectiveLocationTest.java +++ b/src/test/java/org/folio/rest/api/ItemEffectiveLocationTest.java @@ -209,10 +209,7 @@ public void canCalculateEffectiveLocationHoldingUpdate( assertThat(associatedItem.getString(EFFECTIVE_LOCATION_ID_KEY), is(effectiveLocation(holdingEndLoc, itemLoc))); - if (! associatedItem.getString(EFFECTIVE_LOCATION_ID_KEY) - .equals(createdItem.getString(EFFECTIVE_LOCATION_ID_KEY))) { - itemMessageChecks.updatedMessagePublished(createdItem, associatedItem); - } + itemMessageChecks.updatedMessagePublished(createdItem, associatedItem); holdingsMessageChecks.updatedMessagePublished(createdHolding, holdingsClient.getById(holdingsRecordId).getJson()); diff --git a/src/test/java/org/folio/rest/impl/WriteSqlTest.java b/src/test/java/org/folio/rest/impl/WriteSqlTest.java deleted file mode 100644 index 80aefaf8e..000000000 --- a/src/test/java/org/folio/rest/impl/WriteSqlTest.java +++ /dev/null @@ -1,226 +0,0 @@ -package org.folio.rest.impl; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; - -import io.vertx.core.Future; -import io.vertx.core.Vertx; -import io.vertx.core.json.JsonObject; -import io.vertx.junit5.VertxExtension; -import io.vertx.junit5.VertxTestContext; -import io.vertx.sqlclient.Tuple; -import org.folio.postgres.testing.PostgresTesterContainer; -import org.folio.rest.persist.PostgresClient; -import org.folio.util.ResourceUtil; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; -import org.junit.jupiter.params.provider.ValueSource; -import org.marc4j.callnum.DeweyCallNumber; -import org.marc4j.callnum.LCCallNumber; -import org.marc4j.callnum.NlmCallNumber; - -@ExtendWith(VertxExtension.class) -class WriteSqlTest { - private static Vertx vertx; - - @BeforeAll - static void beforeAll(Vertx vertx, VertxTestContext vtc) { - WriteSqlTest.vertx = vertx; - PostgresClient.setPostgresTester(new PostgresTesterContainer()); - var sql = "SET search_path TO 'public';\n" - + ResourceUtil.asString("/templates/db_scripts/instance-hr-item/write.sql"); - runSql(sql) - .onComplete(vtc.succeedingThenComplete()); - } - - static Future runSql(String sql) { - return PostgresClient.getInstance(vertx).runSQLFile(sql, true) - .compose(errors -> { - if (errors.isEmpty()) { - return Future.succeededFuture(); - } - return Future.failedFuture(errors.get(0)); - }); - } - - @AfterAll - static void afterAll() { - PostgresClient.stopPostgresTester(); - } - - @ParameterizedTest - @CsvSource({ - // callNumber, volume, enumeration, chronology, copyNumber, expected - "cn, v, e, ch, co, 'cn v e ch co'", - "' cn ', ' v ', ' e ', ' ch ', ' co ', 'cn v e ch co'", - "cn, v, , ch, , 'cn v ch'", - "cn, , e, , co, 'cn e co'", - ", v, e, ch, co, ", - "' ', v, e, ch, co, ", - }) - void setEffectiveShelvingOrder(String callNumber, String volume, String enumeration, String chronology, - String copyNumber, String expected, VertxTestContext vtc) { - - var jsonObject = new JsonObject().put("effectiveShelvingOrder", "old"); - if (callNumber != null) { - jsonObject.put("effectiveCallNumberComponents", new JsonObject().put("callNumber", callNumber)); - } - if (volume != null) { - jsonObject.put("volume", volume); - } - if (enumeration != null) { - jsonObject.put("enumeration", enumeration); - } - if (chronology != null) { - jsonObject.put("chronology", chronology); - } - if (copyNumber != null) { - jsonObject.put("copyNumber", copyNumber); - } - PostgresClient.getInstance(vertx) - .selectSingle("SELECT public.set_effective_shelving_order($1)", Tuple.of(jsonObject)) - .onComplete(vtc.succeeding(r -> { - assertThat(r.getJsonObject(0).getString("effectiveShelvingOrder"), is(expected)); - vtc.completeNow(); - })); - } - - @ParameterizedTest - @ValueSource(strings = { - // valid LC call number - "A1 B2 .C33", - "A1 B2 C33", - "A1 B2.C33", - "A1 B2C33", - "AB9 L3", - "BF199", - "BF199.", - "BF199.A1J7", - "G3841 .C2 1935 .M3", - "HC241.25 .I4 D47", - "HD 38.25.F8 R87 1989", - "HD38.25.F8 R87 1989", - "HE5.215 .N9/PT.A", - "HF 5549.5.T7 B294 1992", - "LD6329 1903 35TH", - "LD6353 1886", - "M1 .L33", - "M1 L33", - "M5 .L", - "M5 L3 1902", - "M5 L3 1902 V.2", - "M5 L3 1902 V2", - "M5 .L3 1902 V2 TANEYTOWN", - "M211 .M93 BMW240", - "M211 .M93 K.240", - "M211 .M93 K.240 1988 .A1", - "M211 .M93 K.240 1988 A1", - "M453 .Z29 Q1 L V.2", - "M857 .K93 H2 OP.79", - "M857 .M93 S412B M", - "M1001 .H", - "M1001 .M9 1900Z", - "M1001 .M9 K.173D B", - "M1001 .M9 K.551 1900Z M", - "M1001 .M939 S.3,13 2001", - "ML410 .M8 L25 .M95 1995", - "ML410 .M8 L25 M95 1995", - "ML410 .M9 P29 1941 M", - "MT37 2003M384", - "MT130 .M93 K96 .W83 1988", - "MT130 .M93 K96 W83 1988", - "PQ2678.K26 P54", - "PQ8550.21.R57 V5 1992", - "PR92 .L33 1990", - "PR919 .L33 1990", - "PR9199 .A39", - "PR9199.48 .B3", - "PS153 .G38 B73 2012", - "QA76", - "M1", - "BF1999.A63880 1978", - "BF1999 Aarons", - "bq1270", - "l666 15th A8", - // valid NLM (National Library of Medicine) call number - "QZ1 B2 .C33", - // "W1 B2 .C33", // bug in marc4j: https://github.com/marc4j/marc4j/pull/97 - "WR1 B2 .C33", - // invalid: - "", - "I1 B2 .C33", - "O1 B2 .C33", - "Q 11 .GA1 E53 2005", - "QSS 11 .GA1 E53 2005", - "WAA 102.5 B5315 2018", - "X1 B2 .C33", - "Y1 B2 .C33", - "Sony PDX10", - "RCA Jz(1)", - "XXKD671.G53 2012", - }) - void lcNlmCallNumber(String s, VertxTestContext vtc) { - PostgresClient.getInstance(vertx).selectSingle("SELECT public.lc_nlm_call_number($1)", Tuple.of(s)) - .onComplete(vtc.succeeding(r -> { - var nlm = new NlmCallNumber(s); - var lc = new LCCallNumber(s); - if (nlm.isValid()) { - var expected = nlm.getShelfKey(); - assertThat(s + " -> " + expected + " (NLM)", r.getString(0), is(expected)); - } else if (lc.isValid()) { - var expected = lc.getShelfKey(); - assertThat(s + " -> " + expected + " (LC)", r.getString(0), is(expected)); - } else { - assertThat(s + " -> null (for invalid)", r.getString(0), is(nullValue())); - } - vtc.completeNow(); - })); - } - - @ParameterizedTest - @ValueSource(strings = { - "1 .I39", - "1.23 .I39", - "11 .I39", - "11.34 .I39", - "11.34567 .I39", - "111 .I39", - "111 I39", - "111Q39", - "111.12 .I39", - "111.123 I39", - "111.134Q39", - "322.44 .F816 V.1 1974", - "322.45 .R513 1957", - "323 .A512RE NO.23-28", - "323 .A778 ED.2", - "323.09 .K43 V.1", - "324.54 .I39 F", - "324.548 .C425R", - "324.6 .A75CUA", - "341.7/58 / 21", - "394.1 O41b", - "9A2 C0444218 Music CD", - // invalid: - "", - "MC1 259", - "T1 105", - }) - void deweyCallNumber(String s, VertxTestContext vtc) { - PostgresClient.getInstance(vertx).selectSingle("SELECT public.dewey_call_number($1)", Tuple.of(s)) - .onComplete(vtc.succeeding(r -> { - var d = new DeweyCallNumber(s); - if (d.isValid()) { - var expected = d.getShelfKey(); - assertThat(s + " -> " + expected, r.getString(0), is(expected)); - } else { - assertThat(s + " -> null (for invalid)", r.getString(0), is(nullValue())); - } - vtc.completeNow(); - })); - } -} diff --git a/src/test/java/org/folio/rest/support/kafka/GroupedCollectedMessages.java b/src/test/java/org/folio/rest/support/kafka/GroupedCollectedMessages.java index e2ee65b1a..1be4e2d27 100644 --- a/src/test/java/org/folio/rest/support/kafka/GroupedCollectedMessages.java +++ b/src/test/java/org/folio/rest/support/kafka/GroupedCollectedMessages.java @@ -23,8 +23,7 @@ void add(String key, EventMessage eventMessage) { } List messagesByGroupKey(String key) { - // copyOf avoids sporadic ConcurrentModificationException - return List.copyOf(collectedMessages.getOrDefault(key, emptyList())); + return collectedMessages.getOrDefault(key, emptyList()); } int groupCount() { diff --git a/src/test/java/org/folio/rest/support/messages/ItemEventMessageChecks.java b/src/test/java/org/folio/rest/support/messages/ItemEventMessageChecks.java index 0435f2318..1816b8887 100644 --- a/src/test/java/org/folio/rest/support/messages/ItemEventMessageChecks.java +++ b/src/test/java/org/folio/rest/support/messages/ItemEventMessageChecks.java @@ -63,6 +63,7 @@ public void updatedMessagePublished(JsonObject oldItem, final var itemId = getId(newItem); final var newInstanceId = getInstanceIdForItem(newItem); + awaitAtMost().until(() -> kafkaConsumer.getMessagesForItem(newInstanceId, itemId), EVENT_MESSAGE_MATCHERS.hasUpdateEventMessageFor( addInstanceIdToItem(oldItem, oldInstanceId), diff --git a/src/test/java/org/folio/services/CallNumberUtilsTest.java b/src/test/java/org/folio/services/CallNumberUtilsTest.java index e846b972e..31de7a461 100644 --- a/src/test/java/org/folio/services/CallNumberUtilsTest.java +++ b/src/test/java/org/folio/services/CallNumberUtilsTest.java @@ -8,55 +8,17 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import io.vertx.core.Future; -import io.vertx.core.Vertx; -import io.vertx.core.json.JsonObject; -import io.vertx.junit5.VertxExtension; -import io.vertx.junit5.VertxTestContext; -import io.vertx.sqlclient.Tuple; import java.util.Arrays; import java.util.Collections; import java.util.Random; -import org.folio.postgres.testing.PostgresTesterContainer; import org.folio.rest.jaxrs.model.HoldingsRecord; import org.folio.rest.jaxrs.model.Item; -import org.folio.rest.persist.PostgresClient; import org.folio.rest.support.EffectiveCallNumberComponentsUtil; -import org.folio.util.ResourceUtil; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; 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.CsvSource; -@ExtendWith(VertxExtension.class) public class CallNumberUtilsTest { - private static Vertx vertx; - - @BeforeAll - static void beforeAll(Vertx vertx, VertxTestContext vtc) { - CallNumberUtilsTest.vertx = vertx; - PostgresClient.setPostgresTester(new PostgresTesterContainer()); - var sql = "SET search_path TO 'public';\n" - + ResourceUtil.asString("/templates/db_scripts/instance-hr-item/write.sql"); - runSql(sql).onComplete(vtc.succeedingThenComplete()); - } - - static Future runSql(String sql) { - return PostgresClient.getInstance(vertx).runSQLFile(sql, true) - .compose(errors -> { - if (errors.isEmpty()) { - return Future.succeededFuture(); - } - return Future.failedFuture(errors.get(0)); - }); - } - - @AfterAll - static void afterAll() { - PostgresClient.stopPostgresTester(); - } @ParameterizedTest @CsvSource({ @@ -106,8 +68,7 @@ void inputForShelvingNumber( String chronology, String copy, String suffix, - String typeId, - VertxTestContext vtc + String typeId ) { Item item = new Item(); @@ -122,15 +83,9 @@ void inputForShelvingNumber( HoldingsRecord holdingsRecord = new HoldingsRecord(); EffectiveCallNumberComponentsUtil.setCallNumberComponents(item, holdingsRecord); + EffectiveCallNumberComponentsUtil.calculateAndSetEffectiveShelvingOrder(item); - PostgresClient.getInstance(vertx) - .selectSingle("SELECT public.set_effective_shelving_order($1)", Tuple.of(JsonObject.mapFrom(item))) - .onComplete(vtc.succeeding(r -> { - EffectiveCallNumberComponentsUtil.calculateAndSetEffectiveShelvingOrder(item); - assertThat(item.getEffectiveShelvingOrder(), is(desiredShelvingOrder)); - assertThat(r.getJsonObject(0).getString("effectiveShelvingOrder"), is(desiredShelvingOrder)); - vtc.completeNow(); - })); + assertThat(item.getEffectiveShelvingOrder(), is(desiredShelvingOrder)); } @Test @@ -183,17 +138,9 @@ void testSuDocSortingOrder() { }) void checkSuDocShelvingKey( String callNumber, - String expectedShelvingKey, - VertxTestContext vtc + String expectedShelvingKey ) { var shelvingKey = new SuDocCallNumber(callNumber).getShelfKey(); assertEquals(expectedShelvingKey, shelvingKey); - - PostgresClient.getInstance(vertx) - .selectSingle("SELECT public.su_doc_call_number($1)", Tuple.of(callNumber)) - .onComplete(vtc.succeeding(r -> { - assertThat(r.getString(0), is(expectedShelvingKey)); - vtc.completeNow(); - })); } }