diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/RootEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/RootEndpoint.java index f5d24fa8de2..1af40b7094d 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/RootEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/RootEndpoint.java @@ -114,7 +114,7 @@ private Response listDirectoryBuckets() String accountId = S3Owner.DEFAULT_S3OWNER_ID; int count = 0; String lastBucketName = null; - while (bucketIterator.hasNext() && count < maxDirectoryBuckets) { + while (count < maxDirectoryBuckets && bucketIterator.hasNext()) { OzoneBucket bucket = bucketIterator.next(); if (!bucket.getBucketLayout().isFileSystemOptimized()) { continue; @@ -130,7 +130,7 @@ private Response listDirectoryBuckets() count++; } - if (lastBucketName != null && bucketIterator.hasNext()) { + if (count == maxDirectoryBuckets && lastBucketName != null && bucketIterator.hasNext()) { response.setContinuationToken( new ContinueToken(lastBucketName, null).encodeToString()); } @@ -182,7 +182,7 @@ private Response listAllBuckets() int count = 0; String lastBucketName = null; - while (bucketIterator.hasNext() && count < maxBuckets) { + while (count < maxBuckets && bucketIterator.hasNext()) { OzoneBucket next = bucketIterator.next(); BucketMetadata bucketMetadata = new BucketMetadata(); bucketMetadata.setName(next.getName()); @@ -192,7 +192,7 @@ private Response listAllBuckets() count++; } - if (paginated && lastBucketName != null && bucketIterator.hasNext()) { + if (paginated && count == maxBuckets && lastBucketName != null && bucketIterator.hasNext()) { response.setContinuationToken( new ContinueToken(lastBucketName, null).encodeToString()); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java index 4622564e743..c90967afea7 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java @@ -17,18 +17,42 @@ package org.apache.hadoop.ozone.s3.endpoint; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; - +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; +import javax.ws.rs.core.Response; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.OzoneConfigKeys; +import org.apache.hadoop.ozone.client.ObjectStore; +import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.client.OzoneClientStub; +import org.apache.hadoop.ozone.client.OzoneVolume; +import org.apache.hadoop.ozone.client.protocol.ClientProtocol; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.s3.exception.OS3Exception; +import org.apache.hadoop.ozone.s3.signature.SignatureInfo; +import org.apache.hadoop.ozone.s3.util.ContinueToken; import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; /** * This class test HeadBucket functionality. @@ -156,6 +180,91 @@ public void testListAllBucketsUnpaginatedReturnsAll() throws Exception { assertNull(response.getContinuationToken()); } + @ParameterizedTest(name = "directory={0}, buckets={1}, limit={2}, previous={3}") + @CsvSource({ + // directory, total buckets, response limit, previous index, expected names, continuation token, RPC calls + "false, 0, 2, -1, '', false, 1", + "false, 1, 2, -1, bucket-0, false, 2", + "false, 2, 2, -1, bucket-0;bucket-1, false, 2", + "false, 3, 2, -1, bucket-0;bucket-1, true, 2", + "false, 2, 1, -1, bucket-0, true, 1", + "false, 3, 5, -1, bucket-0;bucket-1;bucket-2, false, 3", + "false, 3, 2, 1, bucket-2, false, 2", + "false, 3, , -1, bucket-0;bucket-1;bucket-2, false, 3", + "true, 0, 2, -1, '', false, 1", + "true, 1, 2, -1, bucket-0, false, 2", + "true, 2, 2, -1, bucket-0, false, 2", + "true, 3, 2, -1, bucket-0;bucket-2, false, 3", + "true, 4, 2, -1, bucket-0;bucket-2, true, 2", + "true, 2, 1, -1, bucket-0, true, 1", + "true, 3, 5, -1, bucket-0;bucket-2, false, 3", + "true, 3, 0, -1, '', false, 1", + "true, 5, 2, 2, bucket-4, false, 2", + "true, 2, 2, 0, '', false, 2" + }) + void testListBucketsRpcCount(boolean directory, int total, Integer limit, int previous, + String expectedNames, boolean hasToken, int expectedCalls) throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setInt(OzoneConfigKeys.OZONE_CLIENT_LIST_CACHE_SIZE, 2); + ClientProtocol proxy = mock(ClientProtocol.class); + List buckets = new ArrayList<>(); + for (int i = 0; i < total; i++) { + buckets.add(OzoneBucket.newBuilder(conf, proxy).setVolumeName(DEFAULT_VOLUME).setName("bucket-" + i) + .setBucketLayout(i % 2 == 0 ? BucketLayout.FILE_SYSTEM_OPTIMIZED : BucketLayout.OBJECT_STORE).build()); + } + when(proxy.listBuckets(eq(DEFAULT_VOLUME), isNull(), nullable(String.class), eq(2), eq(false))) + .thenAnswer(invocation -> { + String marker = invocation.getArgument(2); + return buckets.stream() + .filter(bucket -> marker == null || bucket.getName().compareTo(marker) > 0) + .limit(2).collect(Collectors.toList()); + }); + OzoneVolume volume = OzoneVolume.newBuilder(conf, proxy).setName(DEFAULT_VOLUME).setOwner("root") + .setAcls(Collections.emptyList()).build(); + ObjectStore store = mock(ObjectStore.class); + when(store.getS3Volume()).thenReturn(volume); + when(store.getClientProxy()).thenReturn(proxy); + OzoneClient client = mock(OzoneClient.class); + when(client.getObjectStore()).thenReturn(store); + when(client.getProxy()).thenReturn(proxy); + RootEndpoint endpoint = EndpointBuilder.newRootEndpointBuilder().setClient(client) + .setSignatureInfo(new SignatureInfo.Builder(SignatureInfo.Version.V4) + .setCredentialScope("20260101/us-west-2/" + (directory ? "s3express" : "s3") + "/aws4_request").build()) + .build(); + if (limit != null) { + endpoint.queryParamsForTest().setInt( + directory ? QueryParams.MAX_DIRECTORY_BUCKETS : QueryParams.MAX_BUCKETS, limit); + } + if (previous >= 0) { + endpoint.queryParamsForTest().set(QueryParams.CONTINUATION_TOKEN, + new ContinueToken("bucket-" + previous, null).encodeToString()); + } + try (Response httpResponse = endpoint.get()) { + Object entity = httpResponse.getEntity(); + List names; + String token; + if (directory) { + ListDirectoryBucketsResponse response = (ListDirectoryBucketsResponse) entity; + names = response.getBuckets().stream().map(bucket -> bucket.getName()).collect(Collectors.toList()); + token = response.getContinuationToken(); + } else { + ListBucketResponse response = (ListBucketResponse) entity; + names = response.getBuckets().stream().map(bucket -> bucket.getName()).collect(Collectors.toList()); + token = response.getContinuationToken(); + assertThat(response.getOwner().getDisplayName()).isEqualTo("root"); + } + assertThat(names).containsExactly(StringUtils.split(expectedNames, ';')); + if (hasToken) { + assertThat(token).isNotNull(); + assertThat(ContinueToken.decodeFromString(token).getLastKey()).isEqualTo(names.get(names.size() - 1)); + } else { + assertThat(token).isNull(); + } + } + verify(proxy, times(expectedCalls)) + .listBuckets(eq(DEFAULT_VOLUME), isNull(), nullable(String.class), eq(2), eq(false)); + } + private ListBucketResponse listWithMaxBuckets(int maxBuckets) throws Exception { rootEndpoint.queryParamsForTest().unset(QueryParams.CONTINUATION_TOKEN); rootEndpoint.queryParamsForTest().setInt(QueryParams.MAX_BUCKETS, maxBuckets);