diff --git a/NEWS.md b/NEWS.md index 7c189959d..451cd9405 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,7 @@ * Error appears when edit via quickMARC MARC Instance shared from Member tenant [MODDATAIMP-1052](https://folio-org.atlassian.net/browse/MODDATAIMP-1052) * Fix mod-inventory OOM issue [MODINV-1023](https://folio-org.atlassian.net/browse/MODINV-1023) * Replace GET with POST request for fetching instances and holdings on /items endpoint to omit 414 error [MODINV-943](https://folio-org.atlassian.net/browse/MODINV-943) +* Call suppress-on-discovery for source record on holding update if discoverySuppress is true [MODINV-977](https://folio-org.atlassian.net/browse/MODINV-977) * Requires `holdings-storage 2.0 3.0 4.0 5.0 6.0 7.0` ## 20.2.0 2023-03-20 diff --git a/src/main/java/org/folio/inventory/InventoryVerticle.java b/src/main/java/org/folio/inventory/InventoryVerticle.java index 8079714c6..26af54824 100644 --- a/src/main/java/org/folio/inventory/InventoryVerticle.java +++ b/src/main/java/org/folio/inventory/InventoryVerticle.java @@ -64,7 +64,7 @@ public void start(Promise started) { new Items(storage, client).register(router); new MoveApi(storage, client).register(router); new Instances(storage, client, consortiumService).register(router); - new Holdings(storage).register(router); + new Holdings(storage, client).register(router); new InstancesBatch(storage, client, consortiumService).register(router); new IsbnUtilsApi().register(router); new ItemsByHoldingsRecordId(storage, client).register(router); diff --git a/src/main/java/org/folio/inventory/resources/Holdings.java b/src/main/java/org/folio/inventory/resources/Holdings.java index effe021dd..561e9bd0f 100644 --- a/src/main/java/org/folio/inventory/resources/Holdings.java +++ b/src/main/java/org/folio/inventory/resources/Holdings.java @@ -1,19 +1,23 @@ package org.folio.inventory.resources; import static io.netty.util.internal.StringUtil.COMMA; +import static java.lang.String.format; import static java.util.concurrent.CompletableFuture.completedFuture; import static org.folio.inventory.support.CompletableFutures.failedFuture; import static org.folio.inventory.support.EndpointFailureHandler.handleFailure; +import static org.folio.inventory.support.http.server.ServerErrorResponse.internalError; import static org.folio.inventory.support.http.server.SuccessResponse.noContent; import java.lang.invoke.MethodHandles; import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import io.vertx.core.http.HttpClient; import io.vertx.core.json.JsonObject; import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; @@ -24,14 +28,17 @@ import org.apache.logging.log4j.Logger; import org.folio.HoldingsRecord; +import org.folio.HttpStatus; import org.folio.inventory.common.WebContext; import org.folio.inventory.config.InventoryConfiguration; import org.folio.inventory.config.InventoryConfigurationImpl; import org.folio.inventory.domain.HoldingsRecordCollection; +import org.folio.inventory.domain.HoldingsRecordsSourceCollection; import org.folio.inventory.exceptions.NotFoundException; import org.folio.inventory.exceptions.UnprocessableEntityException; import org.folio.inventory.storage.Storage; import org.folio.inventory.support.http.server.FailureResponseConsumer; +import org.folio.rest.client.SourceStorageRecordsClient; public class Holdings { @@ -41,6 +48,11 @@ public class Holdings { private static final String HOLDINGS_NOT_FOUND_ERROR_MSG = "Holdings not found"; private static final String BLOCKED_FIELDS_UPDATED_ERROR_MSG = "Holdings is controlled by MARC record, " + "these fields are blocked and can not be updated: "; + private static final String SUPPRESS_FROM_DISCOVERY_ERROR_MSG = "Suppress from discovery wasn't changed for SRS record" + + ". Holdings id:'%s', status code: '%s'"; + + private static final String MARC = "MARC"; + public static final String HOLDING_ID_TYPE = "HOLDINGS"; private static final String INVENTORY_PATH = "/inventory"; private static final String HOLDINGS_PATH = INVENTORY_PATH + "/holdings"; @@ -48,11 +60,13 @@ public class Holdings { private static final String ID_FIELD = "id"; private static final String MARC_SOURCE_ID = "036ee84a-6afd-4c3c-9ad3-4a12ab875f59"; + private final HttpClient client; private final Storage storage; private final InventoryConfiguration config; - public Holdings(final Storage storage) { + public Holdings(final Storage storage, final HttpClient client) { this.storage = storage; + this.client = client; this.config = new InventoryConfigurationImpl(); } @@ -67,13 +81,14 @@ private void update(RoutingContext rContext) { var holdingsRequest = rContext.getBodyAsJson(); var updatedHoldings = holdingsRequest.mapTo(HoldingsRecord.class); var holdingsRecordCollection = storage.getHoldingsRecordCollection(wContext); + var holdingRecordSourceCollection = storage.getHoldingsRecordsSourceCollection(wContext); completedFuture(updatedHoldings) .thenCompose(holdingsRecord -> holdingsRecordCollection.findById(rContext.request().getParam(ID_FIELD))) .thenCompose(this::refuseWhenHoldingsNotFound) .thenCompose(existingHoldings -> refuseWhenBlockedFieldsChanged(existingHoldings, updatedHoldings)) .thenCompose(existingHoldings -> refuseWhenHridChanged(existingHoldings, updatedHoldings)) - .thenAccept(existingHoldings -> updateHoldings(updatedHoldings, holdingsRecordCollection, rContext)) + .thenAccept(existingHoldings -> updateHoldings(updatedHoldings, holdingsRecordCollection, holdingRecordSourceCollection, rContext, wContext)) .exceptionally(throwable -> { LOGGER.error(throwable); handleFailure(throwable, rContext); @@ -129,11 +144,41 @@ private boolean areHoldingsBlockedFieldsChanged(HoldingsRecord existingHoldings, return ObjectUtils.notEqual(existingBlockedFields, updatedBlockedFields); } - private void updateHoldings(HoldingsRecord holdingsRecord, HoldingsRecordCollection holdingsRecordCollection, - RoutingContext rContext) { + private void updateHoldings(HoldingsRecord holdingsRecord, HoldingsRecordCollection holdingsRecordCollection, HoldingsRecordsSourceCollection recordsSourceCollection, + RoutingContext rContext, WebContext wContext) { holdingsRecordCollection.update(holdingsRecord, - voidSuccess -> noContent(rContext.response()), + v -> { + if (Optional.ofNullable(holdingsRecord.getDiscoverySuppress()).orElse(false)) { + recordsSourceCollection.findById(holdingsRecord.getSourceId()).thenAccept(source -> + { + if (MARC.equals(source.getName())) { + updateSuppressFromDiscoveryFlag(wContext, rContext,holdingsRecord); + } else noContent(rContext.response()); + }); + } else noContent(rContext.response()); + }, FailureResponseConsumer.serverError(rContext.response()) ); } + + private void updateSuppressFromDiscoveryFlag(WebContext wContext, RoutingContext rContext, HoldingsRecord updatedHoldings) { + try { + new SourceStorageRecordsClient(wContext.getOkapiLocation(), wContext.getTenantId(), wContext.getToken(), client) + .putSourceStorageRecordsSuppressFromDiscoveryById(updatedHoldings.getId(), HOLDING_ID_TYPE, updatedHoldings.getDiscoverySuppress(), httpClientResponse -> { + if (httpClientResponse.result().statusCode() == HttpStatus.HTTP_OK.toInt()) { + LOGGER.info(format("Suppress from discovery flag was updated for record in SRS. Holding id: %s", + updatedHoldings.getId())); + noContent(rContext.response()); + } else { + var errMsg = format(SUPPRESS_FROM_DISCOVERY_ERROR_MSG, updatedHoldings.getId(), httpClientResponse.result().statusCode()); + LOGGER.error(errMsg); + internalError(rContext.response(), errMsg); + } + }); + } catch (Exception e) { + LOGGER.error("Error during updating suppress from discovery flag for record in SRS", e); + handleFailure(e, rContext); + } + } + } diff --git a/src/test/java/api/HoldingsApiExamples.java b/src/test/java/api/HoldingsApiExamples.java index ffb451a8d..e75d4e37a 100644 --- a/src/test/java/api/HoldingsApiExamples.java +++ b/src/test/java/api/HoldingsApiExamples.java @@ -1,8 +1,9 @@ package api; +import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR; import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND; -import static io.netty.handler.codec.http.HttpResponseStatus.NO_CONTENT; import static io.netty.handler.codec.http.HttpResponseStatus.OK; +import static io.netty.handler.codec.http.HttpResponseStatus.NO_CONTENT; import static java.util.concurrent.TimeUnit.SECONDS; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -19,6 +20,7 @@ import api.support.ApiRoot; import api.support.ApiTests; import api.support.builders.HoldingRequestBuilder; +import api.support.builders.SourceRecordRequestBuilder; import api.support.fixtures.InstanceRequestExamples; import io.netty.handler.codec.http.HttpResponseStatus; import io.vertx.core.json.JsonArray; @@ -75,6 +77,85 @@ public void canUpdateAnExistingHoldings() throws Exception { assertThat(updatedHoldings.getString("permanentLocationId"), is("fcd64ce1-6995-48f0-840e-89ffa2288371")); } + @Test + public void canSuppressFromDiscoveryOnUpdateForMarcRecord() throws Exception { + var instanceId = instancesClient.create(InstanceRequestExamples.smallAngryPlanet()).getId(); + var newHoldings = holdingsStorageClient.create(new HoldingRequestBuilder() + .forInstance(instanceId) + .withMarcSource()) + .getJson(); + + holdingsSourceStorageClient.create(new HoldingRequestBuilder().createMarcHoldingsSource()); + sourceRecordStorageClient.create(new SourceRecordRequestBuilder(newHoldings.getString("id"))); + var updateHoldingsRequest = newHoldings.copy() + .put("discoverySuppress", true); + + var putResponse = updateHoldings(updateHoldingsRequest); + + assertThat(putResponse.getStatusCode(), is(NO_CONTENT.code())); + + var getResponse = holdingsStorageClient.getById(getId(newHoldings)); + var getRecordResponse = sourceRecordStorageClient.getById(UUID.fromString(newHoldings.getString(("id")))); + + assertThat(getResponse.getStatusCode(), is(OK.code())); + assertThat(getRecordResponse.getStatusCode(), is(OK.code())); + + var updatedHoldings = getResponse.getJson(); + var updatedRecord = getRecordResponse.getJson(); + + assertThat(updatedHoldings.getString("id"), is(newHoldings.getString("id"))); + assertThat(updatedHoldings.getBoolean("discoverySuppress"), is(Boolean.TRUE)); + assertThat(updatedRecord.getJsonObject("additionalInfo").getBoolean("suppressDiscovery"), is(Boolean.TRUE)); + } + + @Test + public void cannotSuppressFromDiscoveryForSourceOnUpdateForFolioRecord() throws Exception { + var instanceId = instancesClient.create(InstanceRequestExamples.smallAngryPlanet()).getId(); + var newHoldings = holdingsStorageClient.create(new HoldingRequestBuilder().forInstance(instanceId)) + .getJson(); + + holdingsSourceStorageClient.create(new HoldingRequestBuilder().createFolioHoldingsSource()); + sourceRecordStorageClient.create(new SourceRecordRequestBuilder(newHoldings.getString("id"))); + var updateHoldingsRequest = newHoldings.copy() + .put("discoverySuppress", true); + + var putResponse = updateHoldings(updateHoldingsRequest); + + assertThat(putResponse.getStatusCode(), is(NO_CONTENT.code())); + + var getResponse = holdingsStorageClient.getById(getId(newHoldings)); + var getRecordResponse = sourceRecordStorageClient.getById(UUID.fromString(newHoldings.getString(("id")))); + + assertThat(getResponse.getStatusCode(), is(OK.code())); + assertThat(getRecordResponse.getStatusCode(), is(OK.code())); + + var updatedHoldings = getResponse.getJson(); + var updatedRecord = getRecordResponse.getJson(); + + assertThat(updatedHoldings.getString("id"), is(newHoldings.getString("id"))); + assertThat(updatedHoldings.getBoolean("discoverySuppress"), is(Boolean.TRUE)); + assertThat(updatedRecord.getJsonObject("additionalInfo").getBoolean("suppressDiscovery"), is(Boolean.FALSE)); + } + + @Test + public void cannotSuppressFromDiscoveryForSourceOnUpdateIfThatDoesNotExist() throws Exception { + var instanceId = instancesClient.create(InstanceRequestExamples.smallAngryPlanet()).getId(); + var newHoldings = holdingsStorageClient.create(new HoldingRequestBuilder() + .forInstance(instanceId) + .withMarcSource()) + .getJson(); + + holdingsSourceStorageClient.create(new HoldingRequestBuilder().createMarcHoldingsSource()); + + var updateHoldingsRequest = newHoldings.copy() + .put("discoverySuppress", true); + + var putResponse = updateHoldings(updateHoldingsRequest); + + assertThat(putResponse.getStatusCode(), is(INTERNAL_SERVER_ERROR.code())); + assertThat(putResponse.getBody().contains(NOT_FOUND.codeAsText()),is(true)); + } + @Test public void cannotUpdateAnHoldingsThatDoesNotExist() throws Exception { UUID instanceId = instancesClient.create(InstanceRequestExamples.smallAngryPlanet()).getId(); diff --git a/src/test/java/api/support/ApiTests.java b/src/test/java/api/support/ApiTests.java index 2d079664f..b23394342 100644 --- a/src/test/java/api/support/ApiTests.java +++ b/src/test/java/api/support/ApiTests.java @@ -20,6 +20,7 @@ public abstract class ApiTests { protected static OkapiHttpClient okapiClient; protected static OkapiHttpClient consortiumOkapiClient; protected final ResourceClient holdingsStorageClient; + protected final ResourceClient holdingsSourceStorageClient; protected final ResourceClient itemsStorageClient; protected final ResourceClient itemsClient; protected final ResourceClient instancesClient; @@ -40,6 +41,7 @@ public abstract class ApiTests { public ApiTests() { holdingsStorageClient = ResourceClient.forHoldingsStorage(okapiClient); + holdingsSourceStorageClient = ResourceClient.forHoldingSourceRecord(okapiClient); itemsStorageClient = ResourceClient.forItemsStorage(okapiClient); itemsClient = ResourceClient.forItems(okapiClient); instancesClient = ResourceClient.forInstances(okapiClient); diff --git a/src/test/java/api/support/builders/HoldingRequestBuilder.java b/src/test/java/api/support/builders/HoldingRequestBuilder.java index ceb712021..cff460332 100644 --- a/src/test/java/api/support/builders/HoldingRequestBuilder.java +++ b/src/test/java/api/support/builders/HoldingRequestBuilder.java @@ -101,6 +101,18 @@ private HoldingRequestBuilder withTemporaryLocation(UUID temporaryLocationId) { this.administrativeNotes); } + public JsonObject createFolioHoldingsSource() { + return new JsonObject() + .put("id", FOLIO_SOURCE_HOLDINGS_ID) + .put("name", "FOLIO"); + } + + public JsonObject createMarcHoldingsSource() { + return new JsonObject() + .put("id", MARC_SOURCE_HOLDINGS_ID) + .put("name", "MARC"); + } + public HoldingRequestBuilder permanentlyInMainLibrary() { return withPermanentLocation(UUID.fromString(ApiTestSuite.getMainLibraryLocation())); } diff --git a/src/test/java/api/support/builders/SourceRecordRequestBuilder.java b/src/test/java/api/support/builders/SourceRecordRequestBuilder.java new file mode 100644 index 000000000..79e022a89 --- /dev/null +++ b/src/test/java/api/support/builders/SourceRecordRequestBuilder.java @@ -0,0 +1,26 @@ +package api.support.builders; + +import io.vertx.core.json.JsonObject; + +public class SourceRecordRequestBuilder extends AbstractBuilder { + private static final String MARC_TYPE = "MARC"; + + private final String id; + private final String recordType; + private final JsonObject additionalInfo; + + public SourceRecordRequestBuilder(String id) { + this.id = id; + this.recordType = MARC_TYPE; + this.additionalInfo = new JsonObject().put("suppressDiscovery", false); + } + + @Override + public JsonObject create() { + return new JsonObject() + .put("id", this.id) + .put("recordType", this.recordType) + .put("additionalInfo", this.additionalInfo); + } + +} diff --git a/src/test/java/api/support/http/ResourceClient.java b/src/test/java/api/support/http/ResourceClient.java index 39dbd0fd8..52c80e9ce 100644 --- a/src/test/java/api/support/http/ResourceClient.java +++ b/src/test/java/api/support/http/ResourceClient.java @@ -138,6 +138,11 @@ public static ResourceClient forSourceRecordStorage(OkapiHttpClient okapiClient) "source record storage", "records"); } + public static ResourceClient forHoldingSourceRecord(OkapiHttpClient okapiClient) { + return new ResourceClient(okapiClient, StorageInterfaceUrls::holdingRecordSourcesUrl, + "holdings source record", "holdingsRecordsSources"); + } + public static ResourceClient forBoundWithPartsStorage (OkapiHttpClient okapiClient) { return new ResourceClient( okapiClient, StorageInterfaceUrls::boundWithPartsUrl, "bound-with parts storage", "boundWithParts"); diff --git a/src/test/java/api/support/http/StorageInterfaceUrls.java b/src/test/java/api/support/http/StorageInterfaceUrls.java index 0cf2def4a..a0f418a35 100644 --- a/src/test/java/api/support/http/StorageInterfaceUrls.java +++ b/src/test/java/api/support/http/StorageInterfaceUrls.java @@ -65,6 +65,10 @@ public static URL sourceRecordStorageUrl(String subPath) { return viaOkapiURL("/source-storage/records" + subPath); } + public static URL holdingRecordSourcesUrl(String subPath) { + return viaOkapiURL("/holdings-sources" + subPath); + } + public static URL boundWithPartsUrl(String subPath) { return viaOkapiURL( "/inventory-storage/bound-with-parts" + subPath ); } diff --git a/src/test/java/support/fakes/FakeOkapi.java b/src/test/java/support/fakes/FakeOkapi.java index a00307ec5..ae75b0210 100644 --- a/src/test/java/support/fakes/FakeOkapi.java +++ b/src/test/java/support/fakes/FakeOkapi.java @@ -41,6 +41,7 @@ public void start(Promise startFuture) { registerFakePubSubModule(router); registerFakeRequestsModule(router); registerFakeSourceRecordStorage(router); + registerFakeHoldingSourcesModule(router); server.requestHandler(router) .listen(PORT_TO_USE, result -> { @@ -139,6 +140,14 @@ private void registerFakeHoldingStorageModule(Router router) { .create().register(router); } + private void registerFakeHoldingSourcesModule(Router router) { + new FakeStorageModuleBuilder() + .withRecordName("Holding record sources") + .withRootPath("/holdings-sources") + .withCollectionPropertyName("holdingsRecordsSources") + .create().register(router); + } + private void registerFakeAuthorityStorageModule(Router router) { new FakeStorageModuleBuilder() .withRecordName("authority") diff --git a/src/test/java/support/fakes/FakeStorageModule.java b/src/test/java/support/fakes/FakeStorageModule.java index c60495492..5f985f0aa 100644 --- a/src/test/java/support/fakes/FakeStorageModule.java +++ b/src/test/java/support/fakes/FakeStorageModule.java @@ -82,6 +82,8 @@ void register(Router router) { router.route(pathTree).handler(this::emulateFailureIfNeeded); router.route(pathTree).handler(this::checkTokenHeader); + router.put(rootPath + "/:id/suppress-from-discovery").handler(this::successSuppressFromDiscovery); + router.post(rootPath + "/retrieve").handler(this::retrieveMany); router.post(rootPath).handler(this::checkRequiredProperties); router.post(rootPath).handler(this::checkUniqueProperties); @@ -319,6 +321,17 @@ private void empty(RoutingContext routingContext) { SuccessResponse.noContent(routingContext.response()); } + private void successSuppressFromDiscovery(RoutingContext routingContext) { + var id = routingContext.request().getParam("id"); + var resourcesForTenant = getResourcesForTenant(new WebContext(routingContext)); + if (resourcesForTenant.containsKey(id)) { + resourcesForTenant.get(id).getJsonObject("additionalInfo").put("suppressDiscovery", true); + JsonResponse.success(routingContext.response(), new JsonObject()); + } else { + ClientErrorResponse.notFound(routingContext.response()); + } + } + private void delete(RoutingContext routingContext) { WebContext context = new WebContext(routingContext);