diff --git a/NEWS.md b/NEWS.md index e4ee90fa..162553db 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +## v7.0.1 2026-07-06 +### Bug fixes +* Fix offset handling when retrieving holdings from HoldingsIQ. ([MODKBEKBJ-825](https://folio-org.atlassian.net/browse/MODKBEKBJ-825)) + ## v7.0.0 2026-04-16 ### Breaking changes * Upgrade the module for Vert.X 5.0.x ([FHIQC-48](https://issues.folio.org/browse/FHIQC-48)) diff --git a/pom.xml b/pom.xml index 2cc971dd..ff3065d5 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.folio mod-kb-ebsco-java - 7.0.1-SNAPSHOT + 7.0.2-SNAPSHOT jar EBSCO KB Broker diff --git a/src/main/java/org/folio/service/holdings/AbstractLoadServiceFacade.java b/src/main/java/org/folio/service/holdings/AbstractLoadServiceFacade.java index aa0de7e8..957f960a 100644 --- a/src/main/java/org/folio/service/holdings/AbstractLoadServiceFacade.java +++ b/src/main/java/org/folio/service/holdings/AbstractLoadServiceFacade.java @@ -137,18 +137,18 @@ protected int getRequestCount(Integer totalCount, int maxRequestCount) { } protected CompletableFuture loadWithPagination(Integer totalPages, - IntFunction> pageLoader) { + IntFunction> offsetLoader) { CompletableFuture future = CompletableFuture.completedFuture(null); List pagesToLoad = IntStream.range(1, totalPages + 1).boxed().toList(); for (Integer page : pagesToLoad) { future = future.thenCompose( - o -> retryOnFailure(loadPageRetries, loadPageDelay, retries -> calculatePage(pageLoader, page, retries))); + o -> retryOnFailure(loadPageRetries, loadPageDelay, retries -> calculateOffset(offsetLoader, page, retries))); } return future; } - protected CompletableFuture calculatePage(IntFunction> pageLoader, - Integer page, Integer retries) { + protected CompletableFuture calculateOffset(IntFunction> pageLoader, + Integer page, Integer retries) { if (loadPageRetries > retries) { page = Math.max(page / 2, loadPageSizeMin); } diff --git a/src/main/java/org/folio/service/holdings/DefaultLoadServiceFacade.java b/src/main/java/org/folio/service/holdings/DefaultLoadServiceFacade.java index 0223847b..441a0e79 100644 --- a/src/main/java/org/folio/service/holdings/DefaultLoadServiceFacade.java +++ b/src/main/java/org/folio/service/holdings/DefaultLoadServiceFacade.java @@ -2,6 +2,7 @@ import io.vertx.core.Vertx; import java.util.concurrent.CompletableFuture; +import java.util.function.IntFunction; import org.folio.holdingsiq.model.HoldingsLoadStatus; import org.folio.holdingsiq.service.LoadService; import org.folio.repository.holdings.LoadStatus; @@ -32,10 +33,9 @@ public DefaultLoadServiceFacade(@Value("${holdings.status.check.delay}") long st @Override protected CompletableFuture loadHoldings(LoadHoldingsMessage message, LoadService loadingService) { - return - loadWithPagination(message.getTotalPages(), page -> loadingService.loadHoldings(getMaxPageSize(), page) - .thenAccept(holdings -> holdingsService.saveHolding( - new HoldingsMessage(holdings.getHoldingsList(), message.getTenantId(), null, message.getCredentialsId())))); + return loadWithPagination(message.getTotalPages(), offset -> loadingService.loadHoldings(getMaxPageSize(), offset) + .thenAccept(holdings -> holdingsService.saveHolding( + new HoldingsMessage(holdings.getHoldingsList(), message.getTenantId(), null, message.getCredentialsId())))); } @Override @@ -59,6 +59,20 @@ protected int getMaxPageSize() { return loadPageSize; } + /** + * Convert the page index (1..totalPages) into a record offset based on the page size. + * page 1 -> offset 1 (records 1..2500) + * page 2 -> offset 2501 (records 2501..5000) + * + * @return CompletableFuture that will be completed when offset is loaded + */ + @Override + protected CompletableFuture calculateOffset(IntFunction> offsetLoader, Integer page, + Integer retries) { + int offset = (page - 1) * getMaxPageSize() + 1; + return offsetLoader.apply(offset); + } + private HoldingsStatus mapToStatus(HoldingsLoadStatus status) { if (status == null) { return createNoneStatus(); diff --git a/src/main/resources/liquibase/tenant/changelog-7.0.1.xml b/src/main/resources/liquibase/tenant/changelog-7.0.1.xml new file mode 100644 index 00000000..ac0d0698 --- /dev/null +++ b/src/main/resources/liquibase/tenant/changelog-7.0.1.xml @@ -0,0 +1,7 @@ + + + + diff --git a/src/main/resources/liquibase/tenant/changelog.xml b/src/main/resources/liquibase/tenant/changelog.xml index ac296dcb..cb4d62e5 100644 --- a/src/main/resources/liquibase/tenant/changelog.xml +++ b/src/main/resources/liquibase/tenant/changelog.xml @@ -10,4 +10,5 @@ + diff --git a/src/main/resources/liquibase/tenant/scripts/v7.0.1/increase-holdings-publisher-name-size.xml b/src/main/resources/liquibase/tenant/scripts/v7.0.1/increase-holdings-publisher-name-size.xml new file mode 100644 index 00000000..6e955bd3 --- /dev/null +++ b/src/main/resources/liquibase/tenant/scripts/v7.0.1/increase-holdings-publisher-name-size.xml @@ -0,0 +1,10 @@ + + + + + + + diff --git a/src/test/java/org/folio/service/holdings/DefaultLoadServiceFacadeTest.java b/src/test/java/org/folio/service/holdings/DefaultLoadServiceFacadeTest.java index f33c1c6c..4b60f9ff 100644 --- a/src/test/java/org/folio/service/holdings/DefaultLoadServiceFacadeTest.java +++ b/src/test/java/org/folio/service/holdings/DefaultLoadServiceFacadeTest.java @@ -1,12 +1,21 @@ package org.folio.service.holdings; +import static org.folio.repository.holdings.LoadStatus.COMPLETED; +import static org.folio.repository.holdings.LoadStatus.FAILED; +import static org.folio.repository.holdings.LoadStatus.IN_PROGRESS; import static org.folio.repository.holdings.LoadStatus.NONE; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import io.vertx.core.Vertx; import java.util.concurrent.CompletableFuture; +import java.util.function.IntFunction; import lombok.SneakyThrows; +import org.folio.holdingsiq.model.HoldingsLoadStatus; import org.folio.holdingsiq.service.impl.LoadServiceImpl; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -21,8 +30,23 @@ public class DefaultLoadServiceFacadeTest { private final Vertx vertx = Vertx.vertx(); - private final DefaultLoadServiceFacade defaultLoadServiceFacade = - new DefaultLoadServiceFacade(1L, 1, 1, 1, 1, 1, 1, vertx); + private final int pageSize = 2500; + private final int pageRetryCount = 3; + + private DefaultLoadServiceFacade defaultLoadServiceFacade; + + @Before + public void setUp() { + defaultLoadServiceFacade = new DefaultLoadServiceFacade( + 1L, // statusRetryDelay + 1, // statusRetryCount + 100, // loadPageRetryDelay + 50, // snapshotRefreshPeriod + pageRetryCount, // loadPageRetryCount + pageSize, // loadPageSize + 500, // loadPageSizeMin + vertx); + } @Test @SneakyThrows @@ -33,4 +57,157 @@ public void shouldNotFailOnEmptyStatusFromHoldingsIq() { var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); assertEquals(NONE, result.get().getStatus()); } + + @Test + @SneakyThrows + public void shouldMapLoadStatusCompletedCorrectly() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("Completed") + .created("2024-01-01 10:00:00") + .totalCount(5000) + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); + HoldingsStatus holdingsStatus = result.get(); + + assertEquals(COMPLETED, holdingsStatus.getStatus()); + assertEquals("2024-01-01 10:00:00", holdingsStatus.getCreated()); + assertEquals(Integer.valueOf(5000), holdingsStatus.getTotalCount()); + } + + @Test + @SneakyThrows + public void shouldMapLoadStatusInProgressCorrectly() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("In progress") + .created("2024-01-01 09:00:00") + .totalCount(3000) + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); + HoldingsStatus holdingsStatus = result.get(); + + assertEquals(IN_PROGRESS, holdingsStatus.getStatus()); + assertEquals("2024-01-01 09:00:00", holdingsStatus.getCreated()); + assertEquals(Integer.valueOf(3000), holdingsStatus.getTotalCount()); + } + + @Test + @SneakyThrows + public void shouldMapLoadStatusFailedCorrectly() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("Failed") + .created("2024-01-01 08:00:00") + .totalCount(0) + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); + HoldingsStatus holdingsStatus = result.get(); + + assertEquals(FAILED, holdingsStatus.getStatus()); + assertEquals("2024-01-01 08:00:00", holdingsStatus.getCreated()); + assertEquals(Integer.valueOf(0), holdingsStatus.getTotalCount()); + } + + @Test + @SneakyThrows + public void shouldGetLoadingStatusReturnLastLoadingStatusIgnoringTransactionId() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("Completed") + .created("2024-01-01 10:00:00") + .totalCount(5000) + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLoadingStatus(loadService, "tx-123"); + HoldingsStatus holdingsStatus = result.get(); + + assertEquals(COMPLETED, holdingsStatus.getStatus()); + assertEquals(Integer.valueOf(5000), holdingsStatus.getTotalCount()); + } + + @Test + public void shouldGetMaxPageSizeReturnConfiguredPageSize() { + assertEquals(pageSize, defaultLoadServiceFacade.getMaxPageSize()); + } + + @Test + @SneakyThrows + public void shouldCalculateOffsetForPages() { + verifyOffset(1, 1); + verifyOffset(2, 2501); + verifyOffset(3, 5001); + } + + private void verifyOffset(int pageNumber, int expectedOffset) { + CompletableFuture callbackFuture = CompletableFuture.completedFuture(null); + IntFunction> offsetLoader = offset -> { + assertEquals(expectedOffset, offset); + return callbackFuture; + }; + + defaultLoadServiceFacade.calculateOffset(offsetLoader, pageNumber, pageRetryCount); + + assertTrue(callbackFuture.isDone()); + } + + @Test + @SneakyThrows + public void shouldPopulateHoldingsReturnNullTransactionId() { + Mockito.when(loadService.populateHoldingsForce()) + .thenReturn(CompletableFuture.completedFuture(null)); + + var result = defaultLoadServiceFacade.populateHoldings(loadService); + assertNull(result.get()); + } + + @Test + @SneakyThrows + public void shouldGetLastLoadingStatusWithDefaultValues() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("Completed") + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); + HoldingsStatus holdingsStatus = result.get(); + + assertEquals(COMPLETED, holdingsStatus.getStatus()); + assertNull(holdingsStatus.getCreated()); + assertNull(holdingsStatus.getTotalCount()); + } + + @Test + @SneakyThrows + public void shouldGetLastLoadingStatusPreserveCreatedAndCount() { + HoldingsLoadStatus holdingsLoadStatus = HoldingsLoadStatus.builder() + .status("Completed") + .created("2024-01-01 10:00:00") + .totalCount(5000) + .build(); + + Mockito.when(loadService.getLoadingStatus()) + .thenReturn(CompletableFuture.completedFuture(holdingsLoadStatus)); + + var result = defaultLoadServiceFacade.getLastLoadingStatus(loadService); + HoldingsStatus holdingsStatus = result.get(); + + assertNotNull(holdingsStatus); + assertEquals(COMPLETED, holdingsStatus.getStatus()); + assertEquals("2024-01-01 10:00:00", holdingsStatus.getCreated()); + assertEquals(Integer.valueOf(5000), holdingsStatus.getTotalCount()); + } }