Skip to content

Commit

Permalink
MODINV-977 Mark suppress from discovery for SRS holdings record
Browse files Browse the repository at this point in the history
  • Loading branch information
dmytrokrutii authored Jun 19, 2024
1 parent 8d0546e commit a06ee8e
Show file tree
Hide file tree
Showing 11 changed files with 205 additions and 7 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/inventory/InventoryVerticle.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void start(Promise<Void> 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);
Expand Down
55 changes: 50 additions & 5 deletions src/main/java/org/folio/inventory/resources/Holdings.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {

Expand All @@ -41,18 +48,25 @@ 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";
private static final String HRID_FIELD = "hrid";
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();
}

Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}

}
83 changes: 82 additions & 1 deletion src/test/java/api/HoldingsApiExamples.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/api/support/ApiTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/api/support/builders/HoldingRequestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
26 changes: 26 additions & 0 deletions src/test/java/api/support/builders/SourceRecordRequestBuilder.java
Original file line number Diff line number Diff line change
@@ -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);
}

}
5 changes: 5 additions & 0 deletions src/test/java/api/support/http/ResourceClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/api/support/http/StorageInterfaceUrls.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down
9 changes: 9 additions & 0 deletions src/test/java/support/fakes/FakeOkapi.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public void start(Promise<Void> startFuture) {
registerFakePubSubModule(router);
registerFakeRequestsModule(router);
registerFakeSourceRecordStorage(router);
registerFakeHoldingSourcesModule(router);

server.requestHandler(router)
.listen(PORT_TO_USE, result -> {
Expand Down Expand Up @@ -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")
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/support/fakes/FakeStorageModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit a06ee8e

Please sign in to comment.