CIRCSTORE-344 Fix issues causing migration to v14.0.0 failure (#345)

* CIRCSTORE-344 Add request without item

* CIRCSTORE-344 Change request UUID

* CIRCSTORE-344 Fix NPE in TLR migration

* CIRCSTORE-344 Remove malformed request

* CIRCSTORE-344 Add request without ID

* CIRCSTORE-344 Change item ID

* CIRCSTORE-344 Add script to delete position

* CIRCSTORE-344 Remove unnecessary steps

* CIRCSTORE-344 Change error message

* CIRCSTORE-344 add test for position removal

* CIRCSTORE-344 change TLR migration version from 13.2.0 to 14.0.0

Co-authored-by: Oleksandr Vidinieiev <[email protected]>

Author: alexanderkurash

src/main/java/org/folio/service/tlr/TlrDataMigrationService.java

@@ -57,7 +57,7 @@ public class TlrDataMigrationService {
   // safe number of UUIDs which fits into Okapi's URL length limit (4096 characters)
   private static final int BATCH_SIZE = 80;
 
-  private static final String TLR_MIGRATION_MODULE_VERSION = "mod-circulation-storage-13.2.0";
+  private static final String TLR_MIGRATION_MODULE_VERSION = "mod-circulation-storage-14.0.0";
   private static final String REQUEST_TABLE = "request";
   private static final String ITEMS_STORAGE_URL = "/item-storage/items";
   private static final String HOLDINGS_STORAGE_URL = "/holdings-storage/holdings";
@@ -284,16 +284,23 @@ private void buildNewRequest(RequestMigrationContext context) {
     JsonObject item = migratedRequest.getJsonObject(ITEM_KEY);
     JsonObject instance = new JsonObject();
 
-    write(instance, TITLE_KEY, item.getString(TITLE_KEY));
-    write(instance, IDENTIFIERS_KEY, item.getJsonArray(IDENTIFIERS_KEY));
+    if (item != null) {
+      write(instance, TITLE_KEY, item.getString(TITLE_KEY));
+      write(instance, IDENTIFIERS_KEY, item.getJsonArray(IDENTIFIERS_KEY));
+
+      item.remove(TITLE_KEY);
+      item.remove(IDENTIFIERS_KEY);
+    }
+    else {
+      log.warn("'item' field is missing from request {}, 'instance' field will not be " +
+        "added", context.getRequestId());
+    }
+
     write(migratedRequest, INSTANCE_ID_KEY, instanceId);
     write(migratedRequest, HOLDINGS_RECORD_ID_KEY, holdingsRecordId);
     write(migratedRequest, REQUEST_LEVEL_KEY, ITEM_REQUEST_LEVEL);
     write(migratedRequest, INSTANCE_KEY, instance);
 
-    item.remove(TITLE_KEY);
-    item.remove(IDENTIFIERS_KEY);
-
     context.setNewRequest(migratedRequest);
   }
 

src/main/resources/templates/db_scripts/removePositionFromClosedRequests.sql

@@ -0,0 +1,4 @@
+UPDATE ${myuniversity}_${mymodule}.request
+SET jsonb = jsonb - 'position'
+WHERE jsonb->>'position' IS NOT NULL
+AND jsonb->>'status' IN ('Closed - Unfilled', 'Closed - Pickup expired', 'Closed - Cancelled', 'Closed - Filled');

src/main/resources/templates/db_scripts/schema.json

@@ -392,6 +392,11 @@
       "run": "after",
       "snippetPath": "add_staff_slips_hold_transit.sql",
       "fromModuleVersion": "12.3.0"
+    },
+    {
+      "run": "after",
+      "snippetPath": "removePositionFromClosedRequests.sql",
+      "fromModuleVersion": "14.0.0"
     }
   ]
 }

src/test/java/org/folio/rest/api/TenantRefApiTests.java

@@ -15,6 +15,10 @@
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
 import static org.folio.HttpStatus.HTTP_CREATED;
+import static org.folio.rest.jaxrs.model.Request.Status.CLOSED_CANCELLED;
+import static org.folio.rest.jaxrs.model.Request.Status.CLOSED_FILLED;
+import static org.folio.rest.jaxrs.model.Request.Status.CLOSED_PICKUP_EXPIRED;
+import static org.folio.rest.jaxrs.model.Request.Status.CLOSED_UNFILLED;
 
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
@@ -27,13 +31,15 @@
 import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 import org.apache.commons.io.IOUtils;
 import org.folio.postgres.testing.PostgresTesterContainer;
 import org.folio.rest.RestVerticle;
 import org.folio.rest.client.TenantClient;
 import org.folio.rest.jaxrs.model.Parameter;
+import org.folio.rest.jaxrs.model.Request;
 import org.folio.rest.jaxrs.model.TenantAttributes;
 import org.folio.rest.jaxrs.model.TenantJob;
 import org.folio.rest.persist.PostgresClient;
@@ -65,8 +71,8 @@
 public class TenantRefApiTests {
   protected static final String OLD_MODULE_VERSION = "13.0.0";
   protected static final String PREVIOUS_MODULE_VERSION = "13.1.0";
-  protected static final String MIGRATION_MODULE_VERSION = "13.2.0";
-  protected static final String NEXT_MODULE_VERSION = "13.4.0";
+  protected static final String TLR_MIGRATION_MODULE_VERSION = "14.0.0";
+  protected static final String NEXT_MODULE_VERSION = "14.1.0";
   protected static final String MODULE_NAME = "mod_circulation_storage";
   protected static final int PORT = NetworkUtils.nextFreePort();
   protected static final String URL = "http://localhost:" + PORT;
@@ -169,7 +175,7 @@ public void migrationShouldBeSkippedWhenUpgradingToLowerVersion(final TestContex
   public void migrationShouldBeSkippedWhenUpgradingFromAlreadyMigratedVersion(final TestContext context) {
     Async async = context.async();
 
-    postTenant(context, MIGRATION_MODULE_VERSION, NEXT_MODULE_VERSION)
+    postTenant(context, TLR_MIGRATION_MODULE_VERSION, NEXT_MODULE_VERSION)
       .onSuccess(job -> {
         assertThatNoRequestsWereUpdated(context);
         async.complete();
@@ -180,7 +186,7 @@ public void migrationShouldBeSkippedWhenUpgradingFromAlreadyMigratedVersion(fina
   public void jobCompletedWhenMigrationIsSuccessful(TestContext context) {
     Async async = context.async();
 
-    postTenant(context, PREVIOUS_MODULE_VERSION, MIGRATION_MODULE_VERSION)
+    postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION)
       .onSuccess(job -> {
         context.assertNull(job.getError());
         validateMigrationResult(context, async);
@@ -209,7 +215,7 @@ public void jobFailsWhenItemWasNotFound(final TestContext context) {
   }
 
   private void jobFailsWhenRemoteCallFails(TestContext context, Async async) {
-    postTenant(context, PREVIOUS_MODULE_VERSION, MIGRATION_MODULE_VERSION)
+    postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION)
       .onSuccess(job -> {
         context.assertTrue(job.getError().contains("Request failed: GET"));
         context.assertTrue(job.getError().contains("Response: [404]"));
@@ -231,7 +237,7 @@ public void useDefaultValuesForInstanceIdAndHoldingsRecordIdWhenItemWasNotFound(
     String requestId = getId(randomRequest);
 
     postgresClient.update(REQUEST_TABLE_NAME, randomRequest, requestId)
-      .compose(r -> postTenant(context, PREVIOUS_MODULE_VERSION, MIGRATION_MODULE_VERSION))
+      .compose(r -> postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION))
       .compose(job -> getRequestAsJson(requestId))
       .onFailure(context::fail)
       .onSuccess(updatedRequest -> {
@@ -261,7 +267,7 @@ public void changesMadeForAllBatchesAreRevertedInCaseOfError(TestContext context
       .whenScenarioStateIs(FIRST_CALL_MADE_SCENARIO_STATE)
       .willReturn(serverError()));
 
-    postTenant(context, PREVIOUS_MODULE_VERSION, MIGRATION_MODULE_VERSION)
+    postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION)
       .onSuccess(job -> {
         context.assertFalse(job.getError().isEmpty());
         assertThatNoRequestsWereUpdated(context);
@@ -299,11 +305,36 @@ public void jobFailsWhenRequestDoesNotHaveRequiredItemLevelRequestField(TestCont
       "request does not contain required ILR fields: " + getId(randomRequest));
   }
 
+  @Test
+  public void migrationRemovesPositionFromClosedRequests(TestContext context) {
+    Async async = context.async();
+
+    List<String> closedStatuses = Stream.of(CLOSED_FILLED, CLOSED_UNFILLED, CLOSED_PICKUP_EXPIRED,
+        CLOSED_CANCELLED)
+      .map(Request.Status::value)
+      .collect(toList());
+
+    postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION)
+      .compose(job -> getAllRequestsAsJson())
+      .onFailure(context::fail)
+      .onSuccess(requestsAfterMigration -> {
+        requestsAfterMigration.forEach(request -> {
+          Integer position = request.getInteger("position");
+          if (closedStatuses.contains(request.getString("status"))) {
+            context.assertNull(position);
+          } else {
+            context.assertNotNull(position);
+          }
+          async.complete();
+        });
+      });
+  }
+
   private void jobFailsWhenRequestValidationFails(TestContext context, Async async,
     JsonObject request, String expectedErrorMessage) {
 
     postgresClient.update(REQUEST_TABLE_NAME, request, getId(request))
-      .compose(r -> postTenant(context, PREVIOUS_MODULE_VERSION, MIGRATION_MODULE_VERSION))
+      .compose(r -> postTenant(context, PREVIOUS_MODULE_VERSION, TLR_MIGRATION_MODULE_VERSION))
       .onFailure(context::fail)
       .onSuccess(job -> {
         context.assertTrue(job.getError().contains(expectedErrorMessage));