From 54d42b3fac29f53e26f407fc92b4d0536fb2a0e9 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Sun, 24 May 2026 05:50:40 +0700 Subject: [PATCH 1/9] fix: paginate JCloudsStore listKeys and listFolders --- .../org/hisp/dhis/jclouds/JCloudsStore.java | 44 ++++++++++++++----- .../jclouds/BlobStoreServiceContractTest.java | 21 +++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 6f33b7ca9ca7..2c656fd563ca 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -36,8 +36,11 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.util.ArrayList; +import java.util.List; import java.util.Properties; import java.util.Set; +import java.util.function.Consumer; import javax.annotation.CheckForNull; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -58,8 +61,10 @@ import org.jclouds.blobstore.BlobStoreContext; import org.jclouds.blobstore.LocalBlobRequestSigner; import org.jclouds.blobstore.domain.Blob; +import org.jclouds.blobstore.domain.PageSet; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.blobstore.internal.RequestSigningUnsupported; +import org.jclouds.blobstore.options.ListContainerOptions; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -189,21 +194,38 @@ public void deleteDirectory(BlobKeyPrefix prefix) { public Iterable listFolders(BlobKeyPrefix prefix) { // JClouds directory listing requires a trailing "/" on the prefix String jcloudsPrefix = prefix.value() + "/"; - return getBlobStore() - .list(fileStoreConfig.container, prefix(jcloudsPrefix).delimiter("/")) - .stream() - .map(m -> BlobKeyPrefix.of(m.getName())) - .toList(); + List folders = new ArrayList<>(); + forEachPage( + prefix(jcloudsPrefix).delimiter("/"), + page -> page.forEach(m -> folders.add(BlobKeyPrefix.of(m.getName())))); + return folders; } @Override public Iterable listKeys(BlobKeyPrefix prefix) { - return getBlobStore() - .list(fileStoreConfig.container, prefix(prefix.value()).recursive()) - .stream() - .map(StorageMetadata::getName) - .map(BlobKey::new) - .toList(); + List keys = new ArrayList<>(); + forEachPage( + prefix(prefix.value()).recursive(), + page -> page.forEach(m -> keys.add(new BlobKey(m.getName())))); + return keys; + } + + /** + * Iterates every page of a JClouds {@code list} call, following {@code nextMarker} until the + * store reports no more results. Without this, the default single-page call caps at the + * provider's page size (e.g. 1000 for S3-compatible stores) and silently truncates the listing. + */ + private void forEachPage( + ListContainerOptions options, Consumer> handler) { + BlobStore bs = getBlobStore(); + PageSet page = bs.list(fileStoreConfig.container, options); + handler.accept(page); + String marker = page.getNextMarker(); + while (marker != null) { + page = bs.list(fileStoreConfig.container, options.afterMarker(marker)); + handler.accept(page); + marker = page.getNextMarker(); + } } @Override diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/jclouds/BlobStoreServiceContractTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/jclouds/BlobStoreServiceContractTest.java index 2dd2221a18cf..f46c128e8602 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/jclouds/BlobStoreServiceContractTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/jclouds/BlobStoreServiceContractTest.java @@ -314,6 +314,27 @@ void listKeys_filtersByPrefix() { assertEquals(Set.of(key("p1/x").value()), blobKeys); } + /** + * Regression test for the bundled-app cleanup bug where {@code listKeys} returned only the + * provider's first page (1000 keys on S3) and {@code JCloudsAppStorageService#deleteApp} silently + * stopped after deleting that page, leaving the rest of the app's files behind as orphans. + */ + @Test + void listKeys_paginatesPastDefaultPageSize() { + int total = 1100; // above the 1000-key default page size of S3-compatible stores + for (int i = 0; i < total; i++) { + putString(key("page/" + String.format("%05d", i)), "x"); + } + + Set keys = new HashSet<>(); + service().listKeys(BlobKeyPrefix.of(key("page").value())).forEach(k -> keys.add(k.value())); + + assertEquals( + total, + withoutDirectoryMarkers(keys).size(), + "listKeys must return every blob across all pages, not just the first page"); + } + // -------------------------------------------------------------------- presigning @Test From 2dd419e251f4c4c925629fd5388e05c46a227df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Henrik=20=C3=98verland?= Date: Fri, 22 May 2026 14:11:16 +0200 Subject: [PATCH 2/9] chore(ci): replace blocked actions-cool/maintain-one-comment with marocchino/sticky-pull-request-comment GitHub blocked actions-cool's repositories for a TOS violation on 2026-05-19, causing the Deploy/Destroy instance workflows to fail in "Set up job" with "Repository access blocked" before any step runs. Swap to marocchino/sticky-pull-request-comment@v2, which provides the same one-comment-per-PR semantics and supports delete: true. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/deploy-instance.yml | 4 ++-- .github/workflows/destroy-instance.yml | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/deploy-instance.yml b/.github/workflows/deploy-instance.yml index 7bc2c77f3f95..338dc3d1b0b1 100644 --- a/.github/workflows/deploy-instance.yml +++ b/.github/workflows/deploy-instance.yml @@ -72,6 +72,6 @@ jobs: run: curl -X POST -u "${{ secrets.DHIS2_USERNAME }}:${{ secrets.DHIS2_PASSWORD }}" "$INSTANCE_HOST/$INSTANCE_NAME/api/resourceTables/analytics" -d 'executeTei=true' - name: Comment instance URL - uses: actions-cool/maintain-one-comment@v3 + uses: marocchino/sticky-pull-request-comment@v2 with: - body: "Instance deployed to https://dev.im.dhis2.org/pr-${{ github.event.pull_request.number }}" + message: "Instance deployed to https://dev.im.dhis2.org/pr-${{ github.event.pull_request.number }}" diff --git a/.github/workflows/destroy-instance.yml b/.github/workflows/destroy-instance.yml index 338dedf020bf..8f52d209e24b 100644 --- a/.github/workflows/destroy-instance.yml +++ b/.github/workflows/destroy-instance.yml @@ -60,7 +60,6 @@ jobs: run: ./findByName.sh dev $INSTANCE_NAME && ./destroy-deployment.sh $(./findByName.sh dev $INSTANCE_NAME | jq -r '.id') - name: Delete instance URL comment - uses: actions-cool/maintain-one-comment@v3 + uses: marocchino/sticky-pull-request-comment@v2 with: - body: "Instance deployed to https://dev.im.dhis2.org/pr-${{ github.event.pull_request.number }}" delete: true From b3b65abca70d9886e98fa4a356c53b3267deb413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Henrik=20=C3=98verland?= Date: Fri, 22 May 2026 15:13:20 +0200 Subject: [PATCH 3/9] chore(ci): pin sticky-pull-request-comment to commit SHA (v3.0.4) Pin marocchino/sticky-pull-request-comment to a full 40-char commit SHA instead of a floating tag. Two reasons: 1. Correctness: there is no `v2` ref on this repo (only specific tags like v2.9.4); the previous `@v2` would have failed to resolve. 2. Defense against tag-replacement supply-chain attacks like the one that just hit actions-cool (every tag re-pointed to imposter commits exfiltrating Runner.Worker memory). A SHA pin cannot be silently moved. The trailing `# v3.0.4` comment lets Dependabot continue to manage version updates. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/deploy-instance.yml | 2 +- .github/workflows/destroy-instance.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy-instance.yml b/.github/workflows/deploy-instance.yml index 338dc3d1b0b1..37b8d5e5ed28 100644 --- a/.github/workflows/deploy-instance.yml +++ b/.github/workflows/deploy-instance.yml @@ -72,6 +72,6 @@ jobs: run: curl -X POST -u "${{ secrets.DHIS2_USERNAME }}:${{ secrets.DHIS2_PASSWORD }}" "$INSTANCE_HOST/$INSTANCE_NAME/api/resourceTables/analytics" -d 'executeTei=true' - name: Comment instance URL - uses: marocchino/sticky-pull-request-comment@v2 + uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4 with: message: "Instance deployed to https://dev.im.dhis2.org/pr-${{ github.event.pull_request.number }}" diff --git a/.github/workflows/destroy-instance.yml b/.github/workflows/destroy-instance.yml index 8f52d209e24b..3072d654f40a 100644 --- a/.github/workflows/destroy-instance.yml +++ b/.github/workflows/destroy-instance.yml @@ -60,6 +60,6 @@ jobs: run: ./findByName.sh dev $INSTANCE_NAME && ./destroy-deployment.sh $(./findByName.sh dev $INSTANCE_NAME | jq -r '.id') - name: Delete instance URL comment - uses: marocchino/sticky-pull-request-comment@v2 + uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4 with: delete: true From a0bbe6e4bd2e520c09f54b7907be815b689e21c2 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Sun, 24 May 2026 07:16:24 +0700 Subject: [PATCH 4/9] chore: clarify forEachPage javadoc --- .../src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 2c656fd563ca..69788ce6aac2 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -213,7 +213,8 @@ public Iterable listKeys(BlobKeyPrefix prefix) { /** * Iterates every page of a JClouds {@code list} call, following {@code nextMarker} until the * store reports no more results. Without this, the default single-page call caps at the - * provider's page size (e.g. 1000 for S3-compatible stores) and silently truncates the listing. + * provider's page size (e.g. 1000 keys for S3-compatible stores) and silently truncates the + * listing, which breaks any caller that needs to enumerate every blob under a prefix. */ private void forEachPage( ListContainerOptions options, Consumer> handler) { From 55ec49615b1be8714cebeda932ed62e150d71da8 Mon Sep 17 00:00:00 2001 From: tonsV2 Date: Wed, 27 May 2026 15:05:10 +0700 Subject: [PATCH 5/9] Update dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java Co-authored-by: David Mackessy <131455290+david-mackessy@users.noreply.github.com> --- .../java/org/hisp/dhis/jclouds/JCloudsStore.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 69788ce6aac2..94d5432f05b1 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -194,11 +194,13 @@ public void deleteDirectory(BlobKeyPrefix prefix) { public Iterable listFolders(BlobKeyPrefix prefix) { // JClouds directory listing requires a trailing "/" on the prefix String jcloudsPrefix = prefix.value() + "/"; - List folders = new ArrayList<>(); - forEachPage( - prefix(jcloudsPrefix).delimiter("/"), - page -> page.forEach(m -> folders.add(BlobKeyPrefix.of(m.getName())))); - return folders; + return StreamSupport.stream( + BlobStores.listAll( + getBlobStore(), fileStoreConfig.container, prefix(jcloudsPrefix).delimiter("/")) + .spliterator(), + false) + .map(m -> BlobKeyPrefix.of(m.getName())) + .toList(); } @Override From 0d4ad4fe290e3a23004989f97878c55e48cde715 Mon Sep 17 00:00:00 2001 From: tonsV2 Date: Wed, 27 May 2026 15:05:22 +0700 Subject: [PATCH 6/9] Update dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java Co-authored-by: David Mackessy <131455290+david-mackessy@users.noreply.github.com> --- .../java/org/hisp/dhis/jclouds/JCloudsStore.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 94d5432f05b1..ca6ba21458c7 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -205,11 +205,14 @@ public Iterable listFolders(BlobKeyPrefix prefix) { @Override public Iterable listKeys(BlobKeyPrefix prefix) { - List keys = new ArrayList<>(); - forEachPage( - prefix(prefix.value()).recursive(), - page -> page.forEach(m -> keys.add(new BlobKey(m.getName())))); - return keys; +return StreamSupport.stream( + BlobStores.listAll( + getBlobStore(), fileStoreConfig.container, prefix(prefix.value()).recursive()) + .spliterator(), + false) + .map(StorageMetadata::getName) + .map(BlobKey::new) + .toList(); } /** From 28a00f44b76e7a866466974b14c1de3f0f778cdc Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Wed, 27 May 2026 15:17:01 +0700 Subject: [PATCH 7/9] Remove forEachPage and clean up unused imports after switching to BlobStores.listAll --- .../org/hisp/dhis/jclouds/JCloudsStore.java | 28 ++----------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index ca6ba21458c7..4c8dc238e0b4 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -12,7 +12,7 @@ * this list of conditions and the following disclaimer in the documentation * and/or other materials provided with the distribution. * - * 3. Neither the name of the copyright holder nor the names of its contributors + * 3. Neither the name of the copyright holder nor the names of its contributors * may be used to endorse or promote products derived from this software without * specific prior written permission. * @@ -36,11 +36,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.util.ArrayList; -import java.util.List; import java.util.Properties; import java.util.Set; -import java.util.function.Consumer; import javax.annotation.CheckForNull; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -61,10 +58,8 @@ import org.jclouds.blobstore.BlobStoreContext; import org.jclouds.blobstore.LocalBlobRequestSigner; import org.jclouds.blobstore.domain.Blob; -import org.jclouds.blobstore.domain.PageSet; import org.jclouds.blobstore.domain.StorageMetadata; import org.jclouds.blobstore.internal.RequestSigningUnsupported; -import org.jclouds.blobstore.options.ListContainerOptions; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -205,7 +200,7 @@ public Iterable listFolders(BlobKeyPrefix prefix) { @Override public Iterable listKeys(BlobKeyPrefix prefix) { -return StreamSupport.stream( + return StreamSupport.stream( BlobStores.listAll( getBlobStore(), fileStoreConfig.container, prefix(prefix.value()).recursive()) .spliterator(), @@ -215,25 +210,6 @@ public Iterable listKeys(BlobKeyPrefix prefix) { .toList(); } - /** - * Iterates every page of a JClouds {@code list} call, following {@code nextMarker} until the - * store reports no more results. Without this, the default single-page call caps at the - * provider's page size (e.g. 1000 keys for S3-compatible stores) and silently truncates the - * listing, which breaks any caller that needs to enumerate every blob under a prefix. - */ - private void forEachPage( - ListContainerOptions options, Consumer> handler) { - BlobStore bs = getBlobStore(); - PageSet page = bs.list(fileStoreConfig.container, options); - handler.accept(page); - String marker = page.getNextMarker(); - while (marker != null) { - page = bs.list(fileStoreConfig.container, options.afterMarker(marker)); - handler.accept(page); - marker = page.getNextMarker(); - } - } - @Override @CheckForNull public URI signedGetUri(BlobKey key, long expirationSeconds) { From 6d3399227b9e96b643907b5f71ba1a24df502e77 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Wed, 27 May 2026 15:24:48 +0700 Subject: [PATCH 8/9] Add missing StreamSupport and BlobStores imports --- .../src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 4c8dc238e0b4..8c48d0ef15eb 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -38,6 +38,7 @@ import java.net.URI; import java.util.Properties; import java.util.Set; +import java.util.stream.StreamSupport; import javax.annotation.CheckForNull; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -55,6 +56,7 @@ import org.jclouds.ContextBuilder; import org.jclouds.blobstore.BlobRequestSigner; import org.jclouds.blobstore.BlobStore; +import org.jclouds.blobstore.BlobStores; import org.jclouds.blobstore.BlobStoreContext; import org.jclouds.blobstore.LocalBlobRequestSigner; import org.jclouds.blobstore.domain.Blob; From 9aba459b8aa9dfa0956ade952cfc11fd51e2414a Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Wed, 27 May 2026 15:28:25 +0700 Subject: [PATCH 9/9] Fix import order and restore license header trailing space required by spotless --- .../src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java index 8c48d0ef15eb..8f2bd42187a4 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/jclouds/JCloudsStore.java @@ -12,7 +12,7 @@ * this list of conditions and the following disclaimer in the documentation * and/or other materials provided with the distribution. * - * 3. Neither the name of the copyright holder nor the names of its contributors + * 3. Neither the name of the copyright holder nor the names of its contributors * may be used to endorse or promote products derived from this software without * specific prior written permission. * @@ -56,8 +56,8 @@ import org.jclouds.ContextBuilder; import org.jclouds.blobstore.BlobRequestSigner; import org.jclouds.blobstore.BlobStore; -import org.jclouds.blobstore.BlobStores; import org.jclouds.blobstore.BlobStoreContext; +import org.jclouds.blobstore.BlobStores; import org.jclouds.blobstore.LocalBlobRequestSigner; import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.domain.StorageMetadata;