From a752ab27eda1fb1bcee9e7ace0ea0da49b9fa2f2 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 25 Jan 2024 10:50:49 +0530 Subject: [PATCH 1/2] Fix limit check for listing S3 objects Signed-off-by: Bhumika Saini --- .../repositories/s3/S3BlobContainer.java | 2 +- .../s3/S3BlobStoreContainerTests.java | 39 +++++++++++-------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java index 3a55fcb0bdbcd..25f361b40636e 100644 --- a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java +++ b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java @@ -514,7 +514,7 @@ private static List executeListing( for (ListObjectsV2Response listObjectsV2Response : listObjectsIterable) { results.add(listObjectsV2Response); totalObjects += listObjectsV2Response.contents().size(); - if (limit != -1 && totalObjects > limit) { + if (limit != -1 && totalObjects >= limit) { break; } } diff --git a/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java index 58ad290a31e85..21392768f6cb1 100644 --- a/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java @@ -32,6 +32,20 @@ package org.opensearch.repositories.s3; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobMetadata; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.blobstore.BlobStoreException; +import org.opensearch.common.blobstore.DeleteResult; +import org.opensearch.common.blobstore.stream.read.ReadContext; +import org.opensearch.common.collect.Tuple; +import org.opensearch.common.io.InputStreamContainer; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.test.OpenSearchTestCase; import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.exception.SdkException; @@ -70,19 +84,6 @@ import software.amazon.awssdk.services.s3.model.UploadPartResponse; import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Iterable; -import org.opensearch.action.LatchedActionListener; -import org.opensearch.common.blobstore.BlobContainer; -import org.opensearch.common.blobstore.BlobMetadata; -import org.opensearch.common.blobstore.BlobPath; -import org.opensearch.common.blobstore.BlobStoreException; -import org.opensearch.common.blobstore.DeleteResult; -import org.opensearch.common.blobstore.stream.read.ReadContext; -import org.opensearch.common.collect.Tuple; -import org.opensearch.common.io.InputStreamContainer; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.common.unit.ByteSizeUnit; -import org.opensearch.test.OpenSearchTestCase; - import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -103,9 +104,6 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatchers; - import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.ArgumentMatchers.eq; @@ -916,6 +914,15 @@ public void testListBlobsByPrefixInLexicographicOrderWithLimitLessThanPageSize() testListBlobsByPrefixInLexicographicOrder(2, 1, BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC); } + /** + * Test the boundary value at page size to ensure + * unnecessary calls are not made to S3 by fetching the next page. + * @throws IOException + */ + public void testListBlobsByPrefixInLexicographicOrderWithLimitEqualToPageSize() throws IOException { + testListBlobsByPrefixInLexicographicOrder(5, 1, BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC); + } + public void testListBlobsByPrefixInLexicographicOrderWithLimitGreaterThanPageSize() throws IOException { testListBlobsByPrefixInLexicographicOrder(8, 2, BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC); } From b83f2e0e620f82968997e8f75f64df65a77be849 Mon Sep 17 00:00:00 2001 From: Bhumika Saini Date: Thu, 25 Jan 2024 11:33:37 +0530 Subject: [PATCH 2/2] Apply spotless fix Signed-off-by: Bhumika Saini --- .../s3/S3BlobStoreContainerTests.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java index 21392768f6cb1..2b45e9cfe2d4b 100644 --- a/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3BlobStoreContainerTests.java @@ -32,20 +32,6 @@ package org.opensearch.repositories.s3; -import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatchers; -import org.opensearch.action.LatchedActionListener; -import org.opensearch.common.blobstore.BlobContainer; -import org.opensearch.common.blobstore.BlobMetadata; -import org.opensearch.common.blobstore.BlobPath; -import org.opensearch.common.blobstore.BlobStoreException; -import org.opensearch.common.blobstore.DeleteResult; -import org.opensearch.common.blobstore.stream.read.ReadContext; -import org.opensearch.common.collect.Tuple; -import org.opensearch.common.io.InputStreamContainer; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.common.unit.ByteSizeUnit; -import org.opensearch.test.OpenSearchTestCase; import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.exception.SdkException; @@ -84,6 +70,19 @@ import software.amazon.awssdk.services.s3.model.UploadPartResponse; import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Iterable; +import org.opensearch.action.LatchedActionListener; +import org.opensearch.common.blobstore.BlobContainer; +import org.opensearch.common.blobstore.BlobMetadata; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.blobstore.BlobStoreException; +import org.opensearch.common.blobstore.DeleteResult; +import org.opensearch.common.blobstore.stream.read.ReadContext; +import org.opensearch.common.collect.Tuple; +import org.opensearch.common.io.InputStreamContainer; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.test.OpenSearchTestCase; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -104,6 +103,9 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.ArgumentMatchers.eq;