From 45a56d337763d5997bdc51c9f509802c18c7e253 Mon Sep 17 00:00:00 2001 From: Natan Date: Wed, 29 Jul 2026 15:19:58 -0300 Subject: [PATCH] fix: pagination not slicing per page, never refreshing on update --- .../component/PaginationImpl.java | 10 ++- .../component/PaginationImplTest.java | 73 +++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/inventory-framework-core/src/main/java/me/devnatan/inventoryframework/component/PaginationImpl.java b/inventory-framework-core/src/main/java/me/devnatan/inventoryframework/component/PaginationImpl.java index 382dc753..c55b3a42 100644 --- a/inventory-framework-core/src/main/java/me/devnatan/inventoryframework/component/PaginationImpl.java +++ b/inventory-framework-core/src/main/java/me/devnatan/inventoryframework/component/PaginationImpl.java @@ -155,7 +155,10 @@ private CompletableFuture> loadSourceForTheCurrentPage() { isLoading = false; simulateStateUpdate(); - if (isLazy()) + // Computed pagination always recomputes the full source (never reuses it, see the + // condition above), so it must be sliced down to the current page just like lazy + // pagination is, otherwise every page would render the same leading slice of items. + if (isLazy() || isComputed()) return Pagination.splitSourceForPage(currentPageIndex(), getPageSize(), getPagesCount(), result); else return result; }); @@ -550,8 +553,10 @@ public void updated(@NotNull IFSlotRenderContext context) { "[Pagination] #updated(IFSlotRenderContext) called (forceUpdated = %b, pageWasChanged = %b)", forceUpdated, pageWasChanged); + // Computed pagination must be recomputed on every update, not just on page change or a + // forced update, since its whole premise is that the source is never reused/cached. // If page was changed all components will be removed, so don't trigger update on them - if (forceUpdated || pageWasChanged) { + if (forceUpdated || pageWasChanged || isComputed()) { clear(renderContext); components = new ArrayList<>(); loadCurrentPage(renderContext).thenRun(() -> { @@ -675,7 +680,6 @@ public boolean isLastPage() { @Override public boolean hasPage(int pageIndex) { - if (isComputed()) return true; if (pageIndex < 0) return false; return pageIndex < getPagesCount(); } diff --git a/inventory-framework-core/src/test/java/me/devnatan/inventoryframework/component/PaginationImplTest.java b/inventory-framework-core/src/test/java/me/devnatan/inventoryframework/component/PaginationImplTest.java index 61c0d78b..5609677d 100644 --- a/inventory-framework-core/src/test/java/me/devnatan/inventoryframework/component/PaginationImplTest.java +++ b/inventory-framework-core/src/test/java/me/devnatan/inventoryframework/component/PaginationImplTest.java @@ -1,5 +1,7 @@ package me.devnatan.inventoryframework.component; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doCallRealMethod; @@ -9,13 +11,84 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.lang.reflect.Field; import java.util.Collections; +import java.util.List; +import java.util.function.Supplier; +import me.devnatan.inventoryframework.context.IFContext; import me.devnatan.inventoryframework.context.IFSlotClickContext; +import me.devnatan.inventoryframework.state.State; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; public class PaginationImplTest { + @SuppressWarnings("unchecked") + private static PaginationImpl newComputedPagination() { + final Supplier> sourceProvider = () -> List.of(1, 2, 3); + return new PaginationImpl( + mock(State.class), + mock(IFContext.class), + 'X', + sourceProvider, + mock(PaginationElementFactory.class), + null, + /* isAsync = */ false, + /* isComputed = */ true, + Pagination.Orientation.HORIZONTAL); + } + + private static void setPagesCount(PaginationImpl pagination, int pagesCount) throws ReflectiveOperationException { + final Field field = PaginationImpl.class.getDeclaredField("pagesCount"); + field.setAccessible(true); + field.set(pagination, pagesCount); + } + + private static void setCurrentPageIndex(PaginationImpl pagination, int pageIndex) + throws ReflectiveOperationException { + final Field field = PaginationImpl.class.getDeclaredField("currPageIndex"); + field.setAccessible(true); + field.set(pagination, pageIndex); + } + + @Test + void computedPaginationCannotAdvanceOrBackWithoutMorePages() throws ReflectiveOperationException { + final PaginationImpl pagination = newComputedPagination(); + setPagesCount(pagination, 1); + setCurrentPageIndex(pagination, 0); + + // A single-page computed pagination must not report being able to advance or go back, + // regression test for https://github.com/DevNatan/inventory-framework/issues/682 + assertFalse(pagination.canAdvance()); + assertFalse(pagination.canBack()); + } + + @Test + void computedPaginationCanAdvanceAndBackWithinBounds() throws ReflectiveOperationException { + final PaginationImpl pagination = newComputedPagination(); + setPagesCount(pagination, 3); + + setCurrentPageIndex(pagination, 0); + assertTrue(pagination.canAdvance()); + assertFalse(pagination.canBack()); + + setCurrentPageIndex(pagination, 1); + assertTrue(pagination.canAdvance()); + assertTrue(pagination.canBack()); + + setCurrentPageIndex(pagination, 2); + assertFalse(pagination.canAdvance()); + assertTrue(pagination.canBack()); + } + + @Test + void hasPageRejectsNegativeIndexEvenWhenComputed() throws ReflectiveOperationException { + final PaginationImpl pagination = newComputedPagination(); + setPagesCount(pagination, 5); + + assertFalse(pagination.hasPage(-1)); + } + @Test @Disabled("This test is not working as expected, because it is passed to parent") void callAnyChildInteractionHandlerOnClickInteraction() {