Ramsons Bug Fix release v12.9.2 (#1051)

* [MODORDERS-1211] ECS | User in member tenant cannot bind pieces related to order from Central tenant (#1049) * MODORDERS-1211. ECS | User in member tenant cannot bind pieces related to order from Central tenant (cherry picked from commit 7e13e49) * MODORDERS-1211. Prepare for 12.9.2 Ramsons Bug Fix release * MODORDERS-1218. Improve logging for change instance connection (#1052) * MODORDERS-1218. Improve logging for change instance connection (cherry picked from commit f796e85) * [maven-release-plugin] prepare release v12.9.2 * [maven-release-plugin] prepare for next development iteration
folio-org · Nov 20, 2024 · 3e9e6f2 · 3e9e6f2
1 parent 3da6b7c
commit 3e9e6f2
Show file tree

Hide file tree

Showing 13 changed files with 191 additions and 32 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,14 @@
 ## 13.0.0 - Unreleased
 
+## 12.9.2 - Released (Ramsons R2 2024)
+The primary focus of this release was to fix issue with binding items in member tenants.
+
+[Full Changelog](https://github.com/folio-org/mod-orders/compare/v12.9.1...v12.9.2)
+
+### Bug Fixes
+* [MODORDERS-1211](https://issues.folio.org/browse/MODORDERS-1211) - ECS | User in member tenant cannot bind pieces related to order from Central tenant
+
+
 ## 12.9.1 - Released (Ramsons R2 2024)
 The primary focus of this release was to fix order processing and multi-tenant issues.
 

diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json
@@ -1131,6 +1131,7 @@
             "circulation.requests.item.move.post",
             "circulation.requests.item.put",
             "user-tenants.collection.get",
+            "orders-storage.settings.collection.get",
             "consortia.sharing-instances.item.post"
           ]
         },

diff --git a/pom.xml b/pom.xml
@@ -2,7 +2,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>org.folio</groupId>
   <artifactId>mod-orders</artifactId>
-  <version>12.9.2-SNAPSHOT</version>
+  <version>12.9.3-SNAPSHOT</version>
 
   <name>Orders Business Logic</name>
   <description>Business logic to manage orders in FOLIO</description>

diff --git a/src/main/java/org/folio/config/ApplicationConfig.java b/src/main/java/org/folio/config/ApplicationConfig.java
@@ -848,8 +848,8 @@ OrderTemplatesService orderTemplatesService() {
   }
 
   @Bean
-  ConsortiumConfigurationService consortiumConfigurationService(RestClient restClient) {
-    return new ConsortiumConfigurationService(restClient);
+  ConsortiumConfigurationService consortiumConfigurationService(RestClient restClient, SettingsRetriever settingsRetriever) {
+    return new ConsortiumConfigurationService(restClient, settingsRetriever);
   }
 
   @Bean

diff --git a/src/main/java/org/folio/helper/BindHelper.java b/src/main/java/org/folio/helper/BindHelper.java
@@ -21,6 +21,7 @@
 import org.folio.rest.jaxrs.model.ReceivedItem;
 import org.folio.rest.jaxrs.model.Title;
 import org.folio.rest.tools.utils.TenantTool;
+import org.folio.service.consortium.ConsortiumConfigurationService;
 import org.folio.service.inventory.InventoryInstanceManager;
 import org.folio.service.inventory.InventoryItemRequestService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -49,6 +50,9 @@ public class BindHelper extends CheckinReceivePiecesHelper<BindPiecesCollection>
   @Autowired
   private InventoryInstanceManager inventoryInstanceManager;
 
+  @Autowired
+  private ConsortiumConfigurationService consortiumConfigurationService;
+
   public BindHelper(BindPiecesCollection bindPiecesCollection,
                     Map<String, String> okapiHeaders, Context ctx) {
     super(okapiHeaders, ctx);
@@ -111,8 +115,9 @@ private Future<Void> clearTitleBindItemsIfNeeded(String titleId, String bindItem
   }
 
   public Future<BindPiecesResult> bindPieces(BindPiecesCollection bindPiecesCollection, RequestContext requestContext) {
-    return removeForbiddenEntities(requestContext)
-      .compose(vVoid -> processBindPieces(bindPiecesCollection, requestContext));
+    return consortiumConfigurationService.overrideContextToCentralTenantIfNeeded(requestContext)
+      .compose(ctx -> removeForbiddenEntities(ctx)
+        .compose(vVoid -> processBindPieces(bindPiecesCollection, ctx)));
   }
 
   private Future<BindPiecesResult> processBindPieces(BindPiecesCollection bindPiecesCollection, RequestContext requestContext) {

diff --git a/src/main/java/org/folio/service/consortium/ConsortiumConfigurationService.java b/src/main/java/org/folio/service/consortium/ConsortiumConfigurationService.java
@@ -8,17 +8,21 @@
 import org.apache.logging.log4j.Logger;
 import org.folio.models.consortium.ConsortiumConfiguration;
 import org.folio.orders.utils.RequestContextUtil;
+import org.folio.rest.acq.model.Setting;
 import org.folio.rest.core.RestClient;
 import org.folio.rest.core.models.RequestContext;
 import org.folio.rest.core.models.RequestEntry;
 import org.folio.rest.jaxrs.model.Location;
 import org.folio.rest.tools.utils.TenantTool;
+import org.folio.service.settings.SettingsRetriever;
+import org.folio.service.settings.util.SettingKey;
 import org.springframework.beans.factory.annotation.Value;
 
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
 import static org.folio.orders.utils.CacheUtils.buildAsyncCache;
+import static org.folio.orders.utils.RequestContextUtil.createContextWithNewTenantId;
 
 public class ConsortiumConfigurationService {
   private static final Logger logger = LogManager.getLogger(ConsortiumConfigurationService.class);
@@ -34,8 +38,11 @@ public class ConsortiumConfigurationService {
   private final RestClient restClient;
   private final AsyncCache<String, Optional<ConsortiumConfiguration>> asyncCache;
 
-  public ConsortiumConfigurationService(RestClient restClient) {
+  private SettingsRetriever settingsRetriever;
+
+  public ConsortiumConfigurationService(RestClient restClient, SettingsRetriever settingsRetriever) {
     this.restClient = restClient;
+    this.settingsRetriever = settingsRetriever;
     asyncCache = buildAsyncCache(Vertx.currentContext(), cacheExpirationTime);
   }
 
@@ -74,4 +81,26 @@ public Future<RequestContext> cloneRequestContextIfNeeded(RequestContext request
       .map(config -> config.isEmpty() ? requestContext : RequestContextUtil.createContextWithNewTenantId(requestContext, location.getTenantId()));
   }
 
+  public Future<RequestContext> overrideContextToCentralTenantIfNeeded(RequestContext requestContext) {
+    return getConsortiumConfiguration(requestContext)
+      .compose(consortiumConfiguration -> {
+        if (consortiumConfiguration.isEmpty()) {
+          return Future.succeededFuture(requestContext);
+        }
+        String tenantId = TenantTool.tenantId(requestContext.getHeaders());
+        var configuration = consortiumConfiguration.get();
+        logger.info("overrideContextToCentralTenantIdNeeded:: tenantId from request: {}, centralTenantId: {}",
+          tenantId, configuration.centralTenantId());
+        if (StringUtils.equals(tenantId, configuration.centralTenantId())) {
+          return Future.succeededFuture(requestContext);
+        }
+        RequestContext centralContext = createContextWithNewTenantId(requestContext, configuration.centralTenantId());
+        return settingsRetriever.getSettingByKey(SettingKey.CENTRAL_ORDERING_ENABLED, centralContext)
+          .map(centralOrdering -> {
+            logger.info("overrideContextToCentralTenantIdNeeded:: central ordering enabled: {}", centralOrdering);
+            return centralOrdering.map(Setting::getValue).orElse(null);
+          })
+          .compose(orderingEnabled -> Future.succeededFuture(Boolean.parseBoolean(orderingEnabled) ? centralContext : requestContext));
+      });
+  }
 }
diff --git a/src/main/java/org/folio/service/orders/lines/update/OrderLinePatchOperationService.java b/src/main/java/org/folio/service/orders/lines/update/OrderLinePatchOperationService.java
@@ -69,10 +69,13 @@ public OrderLinePatchOperationService(RestClient restClient,
   }
 
   public Future<Void> patch(String lineId, PatchOrderLineRequest request, RequestContext requestContext) {
+    logger.info("patch:: start patching operation: {} for poLineId: {}", request.getOperation(), lineId);
     String newInstanceId = request.getReplaceInstanceRef().getNewInstanceId();
     return inventoryInstanceManager.createShadowInstanceIfNeeded(newInstanceId, requestContext)
       .compose(v -> patchOrderLine(request, lineId, requestContext))
-      .compose(v -> updateInventoryInstanceInformation(request, lineId, requestContext));
+      .compose(v -> updateInventoryInstanceInformation(request, lineId, requestContext))
+      .onSuccess(v -> logger.info("patch:: successfully patched operation: {} for poLineId: {}", request.getOperation(), lineId))
+      .onFailure(e -> logger.error("Failed to patch operation: {} for poLineId: {}", request.getOperation(), lineId, e));
   }
 
   private Future<Void> patchOrderLine(PatchOrderLineRequest request, String lineId, RequestContext requestContext) {

diff --git a/...a/org/folio/service/orders/lines/update/instance/BaseOrderLineUpdateInstanceStrategy.java b/...a/org/folio/service/orders/lines/update/instance/BaseOrderLineUpdateInstanceStrategy.java
@@ -2,6 +2,8 @@
 
 import io.vertx.core.Future;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.folio.models.orders.lines.update.OrderLineUpdateInstanceHolder;
 import org.folio.okapi.common.GenericCompositeFuture;
 import org.folio.orders.utils.RequestContextUtil;
@@ -14,6 +16,8 @@
 
 public abstract class BaseOrderLineUpdateInstanceStrategy implements OrderLineUpdateInstanceStrategy {
 
+  private static final Logger logger = LogManager.getLogger(BaseOrderLineUpdateInstanceStrategy.class);
+
   InventoryInstanceManager inventoryInstanceManager;
   InventoryItemManager inventoryItemManager;
   InventoryHoldingManager inventoryHoldingManager;
@@ -46,7 +50,9 @@ Future<Void> deleteAbandonedHoldings(boolean isDeleteAbandonedHoldings, PoLine p
           .filter(location -> StringUtils.isNotEmpty(location.getHoldingId()))
           .map(location -> {
             var locationContext = RequestContextUtil.createContextWithNewTenantId(requestContext, location.getTenantId());
-            return deleteHoldingWithoutItems(location.getHoldingId(), locationContext);
+            return deleteHoldingWithoutItems(location.getHoldingId(), locationContext)
+              .onSuccess(v -> logger.info("deleteAbandonedHoldings:: operation succeeded for holdingId: {}", location.getHoldingId()))
+              .onFailure(e -> logger.error("Failed to delete abandoned holdings for holdingId: {}", location.getHoldingId(), e));
           }).toList()
       )
       .mapEmpty();

diff --git a/...olio/service/orders/lines/update/instance/WithHoldingOrderLineUpdateInstanceStrategy.java b/...olio/service/orders/lines/update/instance/WithHoldingOrderLineUpdateInstanceStrategy.java
@@ -83,6 +83,7 @@ protected Future<Void> processHoldings(OrderLineUpdateInstanceHolder holder, Req
 
   private Future<Void> moveHoldings(OrderLineUpdateInstanceHolder holder, String newInstanceId, RequestContext requestContext) {
     PoLine poLine = holder.getStoragePoLine();
+    logger.info("moveHoldings:: start processing for poLineId: {} and new instanceId: {}", poLine.getId(), newInstanceId);
     CompositePoLine compPOL = PoLineCommonUtil.convertToCompositePoLine(poLine);
     return pieceStorageService.getPiecesByPoLineId(compPOL, requestContext)
       .map(pieces -> getHoldingsByTenants(holder, pieces, requestContext))
@@ -93,7 +94,11 @@ private Future<Void> moveHoldings(OrderLineUpdateInstanceHolder holder, String n
               removeHoldingUnrecognizedFields(holdings);
               var locationContext = RequestContextUtil.createContextWithNewTenantId(requestContext, entry.getKey());
               return inventoryInstanceManager.createShadowInstanceIfNeeded(newInstanceId, locationContext)
-                .compose(instance -> inventoryHoldingManager.updateInstanceForHoldingRecords(holdings, newInstanceId, locationContext));
+                .compose(instance -> inventoryHoldingManager.updateInstanceForHoldingRecords(holdings, newInstanceId, locationContext))
+                .onSuccess(v -> logger.info("moveHoldings:: {} holdings for tenantId: {} have been updated with new instanceId: {}",
+                  holdings.size(), entry.getKey(), newInstanceId))
+                .onFailure(e -> logger.error("Failed to update holdings for tenantId: {} with new instanceId: {}",
+                  entry.getKey(), newInstanceId, e));
             }))
             .toList();
           return GenericCompositeFuture.all(updateHoldings).mapEmpty();
@@ -227,7 +232,10 @@ private Future<Map<String, List<Location>>> retrieveUniqueLocations(PoLine poLin
 
   private Future<Void> updateItemsHolding(String holdingId, String newHoldingId, String poLineId, RequestContext requestContext) {
     return inventoryItemManager.getItemsByHoldingIdAndOrderLineId(holdingId, poLineId, requestContext)
-      .compose(items -> updateItemsInInventory(items, newHoldingId, requestContext));
+      .compose(items -> updateItemsInInventory(items, newHoldingId, requestContext))
+      .onSuccess(v -> logger.info("updateItemsHolding:: existing items for holdingId: {} have been updated with new holdingId: {}",
+        holdingId, newHoldingId))
+      .onFailure(e -> logger.error("Failed to update items for holdingId: {} with new holdingId: {}", holdingId, newHoldingId, e));
   }
 
   private Future<Void> updateItemsInInventory(List<JsonObject> items, String newHoldingId, RequestContext requestContext) {

diff --git a/src/test/java/org/folio/service/CirculationRequestsRetrieverTest.java b/src/test/java/org/folio/service/CirculationRequestsRetrieverTest.java
@@ -251,8 +251,8 @@ public CirculationRequestsRetriever circulationRequestsRetriever(PieceStorageSer
     }
 
     @Bean
-    ConsortiumConfigurationService consortiumConfigurationService(RestClient restClient) {
-      return new ConsortiumConfigurationService(restClient);
+    ConsortiumConfigurationService consortiumConfigurationService(RestClient restClient, SettingsRetriever settingsRetriever) {
+      return new ConsortiumConfigurationService(restClient, settingsRetriever);
     }
 
     @Bean

diff --git a/src/test/java/org/folio/service/consortium/ConsortiumConfigurationServiceTest.java b/src/test/java/org/folio/service/consortium/ConsortiumConfigurationServiceTest.java
@@ -3,30 +3,34 @@
 import io.vertx.core.Future;
 import io.vertx.core.json.JsonArray;
 import io.vertx.core.json.JsonObject;
-import io.vertx.junit5.VertxExtension;
-import io.vertx.junit5.VertxTestContext;
 import org.folio.okapi.common.XOkapiHeaders;
+import org.folio.rest.acq.model.Setting;
 import org.folio.rest.core.RestClient;
 import org.folio.rest.core.models.RequestContext;
 import org.folio.rest.core.models.RequestEntry;
 import org.folio.rest.jaxrs.model.Location;
+import org.folio.service.settings.SettingsRetriever;
+import org.folio.service.settings.util.SettingKey;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.MockitoAnnotations;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
 
-@ExtendWith({VertxExtension.class, MockitoExtension.class})
 public class ConsortiumConfigurationServiceTest {
+  private AutoCloseable mockitoMocks;
 
   @InjectMocks
   private ConsortiumConfigurationService consortiumConfigurationService;
@@ -37,28 +41,122 @@ public class ConsortiumConfigurationServiceTest {
   @Mock
   private RequestContext requestContext;
 
+  @Mock
+  private SettingsRetriever settingsRetriever;
+
+  @BeforeEach
+  public void initMocks(){
+    mockitoMocks = MockitoAnnotations.openMocks(this);
+  }
+
+  @AfterEach
+  void afterEach() throws Exception {
+    mockitoMocks.close();
+  }
+
   @Test
-  void testCloneRequestContextIfNeeded(VertxTestContext vertxTestContext) {
+  void testCloneRequestContextIfNeeded() {
     // given
     JsonObject consortiumConfiguration = new JsonObject(Map.of(
       "userTenants", new JsonArray(List.of(
         new JsonObject(Map.of("consortiumId", "cid", "centralTenantId", "ctid"))
       ))
     ));
-    Mockito.when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
+    when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
       .thenReturn(Future.succeededFuture(consortiumConfiguration));
 
     // when
     Future<RequestContext> future = consortiumConfigurationService.cloneRequestContextIfNeeded(requestContext, new Location().withTenantId("tenantId"));
 
     // then
-    vertxTestContext.assertComplete(future)
-      .onComplete(ar -> {
-        assertTrue(ar.succeeded());
-        assertEquals("tenantId", ar.result().getHeaders().get(XOkapiHeaders.TENANT));
-        verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
-        vertxTestContext.completeNow();
-      });
+    assertEquals("tenantId", future.result().getHeaders().get(XOkapiHeaders.TENANT));
+    verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
+
+  }
+
+  @Test
+  void overrideContextToCentralTenantIfNeeded_withEmptyConsortiumConfiguration() {
+    // given
+    JsonObject consortiumConfiguration = new JsonObject(Map.of(
+      "userTenants", new JsonArray(List.of())
+    ));
+    when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
+      .thenReturn(Future.succeededFuture(consortiumConfiguration));
+
+    // when
+    Future<RequestContext> future = consortiumConfigurationService.overrideContextToCentralTenantIfNeeded(requestContext);
+
+    // then
+    assertEquals(requestContext, future.result());
+    verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
+    verifyNoInteractions(settingsRetriever);
+  }
+
+  @Test
+  void overrideContextToCentralTenantIdNeeded_withSameTenantIf() {
+    // given
+    JsonObject consortiumConfiguration = new JsonObject(Map.of(
+      "userTenants", new JsonArray(List.of(
+        new JsonObject(Map.of("consortiumId", "cid", "centralTenantId", "tenantId"))
+      ))
+    ));
+    when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
+      .thenReturn(Future.succeededFuture(consortiumConfiguration));
+    when(requestContext.getHeaders()).thenReturn(Map.of(XOkapiHeaders.TENANT, "tenantId"));
+
+    // when
+    Future<RequestContext> future = consortiumConfigurationService.overrideContextToCentralTenantIfNeeded(requestContext);
+
+    // then
+    assertEquals(requestContext, future.result());
+    verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
+    verifyNoInteractions(settingsRetriever);
+  }
+
+  @Test
+  void overrideContextToCentralTenantIdNeeded_withCentralOrderingEnabled() {
+    // given
+    JsonObject consortiumConfiguration = new JsonObject(Map.of(
+      "userTenants", new JsonArray(List.of(
+        new JsonObject(Map.of("consortiumId", "cid", "centralTenantId", "centralTenantId"))
+      ))
+    ));
+    when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
+      .thenReturn(Future.succeededFuture(consortiumConfiguration));
+    when(requestContext.getHeaders()).thenReturn(Map.of(XOkapiHeaders.TENANT, "tenantId"));
+    when(settingsRetriever.getSettingByKey(any(SettingKey.class), any(RequestContext.class)))
+      .thenReturn(Future.succeededFuture(Optional.of(new Setting().withValue("true"))));
+
+    // when
+    Future<RequestContext> future = consortiumConfigurationService.overrideContextToCentralTenantIfNeeded(requestContext);
+
+    // then
+    assertEquals("centralTenantId", future.result().getHeaders().get(XOkapiHeaders.TENANT));
+    verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
+    verify(settingsRetriever).getSettingByKey(any(SettingKey.class), any(RequestContext.class));
+  }
+
+  @Test
+  void overrideContextToCentralTenantIdNeeded_withCentralOrderingDisabled() {
+    // given
+    JsonObject consortiumConfiguration = new JsonObject(Map.of(
+      "userTenants", new JsonArray(List.of(
+        new JsonObject(Map.of("consortiumId", "cid", "centralTenantId", "centralTenantId"))
+      ))
+    ));
+    when(restClient.getAsJsonObject(any(RequestEntry.class), any()))
+      .thenReturn(Future.succeededFuture(consortiumConfiguration));
+    when(requestContext.getHeaders()).thenReturn(Map.of(XOkapiHeaders.TENANT, "tenantId"));
+    when(settingsRetriever.getSettingByKey(any(SettingKey.class), any(RequestContext.class)))
+      .thenReturn(Future.succeededFuture(Optional.of(new Setting().withValue("false"))));
+
+    // when
+    Future<RequestContext> future = consortiumConfigurationService.overrideContextToCentralTenantIfNeeded(requestContext);
+
+    // then
+    assertEquals(requestContext, future.result());
+    verify(restClient).getAsJsonObject(any(RequestEntry.class), any());
+    verify(settingsRetriever).getSettingByKey(any(SettingKey.class), any(RequestContext.class));
   }
 
 }