diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index ee6d284b668..2788cb05d86 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -100,7 +100,6 @@ public Response get( ) throws OS3Exception, IOException { long startNanos = Time.monotonicNowNanos(); S3GAction s3GAction = S3GAction.GET_BUCKET; - PerformanceStringBuilder perf = new PerformanceStringBuilder(); // Chain of responsibility: let each handler try to handle the request for (BucketOperationHandler handler : handlers) { @@ -119,11 +118,6 @@ public Response get( String prefix = queryParams().get(QueryParams.PREFIX); String startAfter = queryParams().get(QueryParams.START_AFTER); - Iterator ozoneKeyIterator = null; - ContinueToken decodedToken = - ContinueToken.decodeFromString(continueToken); - OzoneBucket bucket = null; - try { final String uploads = queryParams().get(QueryParams.UPLOADS); if (uploads != null) { @@ -133,31 +127,22 @@ public Response get( return listMultipartUploads(bucketName, prefix, keyMarker, uploadIdMarker, maxUploads); } - maxKeys = validateMaxKeys(maxKeys); - - if (prefix == null) { - prefix = ""; - } + // Actual bucket processing starts here + // Validate and prepare parameters + BucketListingContext context = validateAndPrepareParameters( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter); - // Assign marker to startAfter. for the compatibility of aws api v1 - if (startAfter == null && marker != null) { - startAfter = marker; - } + // Initialize response object + ListObjectResponse response = initializeListObjectResponse( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter); - // If continuation token and start after both are provided, then we - // ignore start After - String prevKey = continueToken != null ? decodedToken.getLastKey() - : startAfter; + // Process key listing + processKeyListing(context, response); - // If shallow is true, only list immediate children - // delimited by OZONE_URI_DELIMITER - boolean shallow = listKeysShallowEnabled - && OZONE_URI_DELIMITER.equals(delimiter); - - bucket = getBucket(bucketName); - S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner()); - - ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow); + // Build final response + return buildFinalResponse(response, s3GAction, startNanos); } catch (OMException ex) { auditReadFailure(s3GAction, ex); @@ -176,11 +161,140 @@ public Response get( throw ex; } - // The valid encodingType Values is "url" + // Return empty response if no keys found + // This path is reached when FILE_NOT_FOUND exception is caught and handled + ListObjectResponse emptyResponse = new ListObjectResponse(); + emptyResponse.setName(bucketName); + emptyResponse.setKeyCount(0); + + // Log audit entry for empty response to align with previous behavior + long opLatencyNs = getMetrics().updateGetBucketSuccessStats(startNanos); + PerformanceStringBuilder perf = new PerformanceStringBuilder(); + perf.appendCount(0); + perf.appendOpLatencyNanos(opLatencyNs); + auditReadSuccess(s3GAction, perf); + + return Response.ok(emptyResponse).build(); + } + + private int validateMaxKeys(int maxKeys) throws OS3Exception { + if (maxKeys < 0) { + throw newError(S3ErrorTable.INVALID_ARGUMENT, "maxKeys must be >= 0"); + } + + return Math.min(maxKeys, maxKeysLimit); + } + + /** + * Context class to hold bucket listing parameters and state. + */ + static class BucketListingContext { + private final String bucketName; + private final String delimiter; + private final String encodingType; + private final String marker; + private final int maxKeys; + private final String prefix; + private final String continueToken; + private final String startAfter; + private final String prevKey; + private final boolean shallow; + private final OzoneBucket bucket; + private final Iterator ozoneKeyIterator; + private final ContinueToken decodedToken; + + @SuppressWarnings("parameternumber") + BucketListingContext(String bucketName, String delimiter, String encodingType, + String marker, int maxKeys, String prefix, String continueToken, + String startAfter, String prevKey, boolean shallow, + OzoneBucket bucket, Iterator ozoneKeyIterator, + ContinueToken decodedToken) { + this.bucketName = bucketName; + this.delimiter = delimiter; + this.encodingType = encodingType; + this.marker = marker; + this.maxKeys = maxKeys; + this.prefix = prefix; + this.continueToken = continueToken; + this.startAfter = startAfter; + this.prevKey = prevKey; + this.shallow = shallow; + this.bucket = bucket; + this.ozoneKeyIterator = ozoneKeyIterator; + this.decodedToken = decodedToken; + } + + // Getters + public String getBucketName() { + return bucketName; + } + + public String getDelimiter() { + return delimiter; + } + + public String getEncodingType() { + return encodingType; + } + + public String getMarker() { + return marker; + } + + public int getMaxKeys() { + return maxKeys; + } + + public String getPrefix() { + return prefix; + } + + public String getContinueToken() { + return continueToken; + } + + public String getStartAfter() { + return startAfter; + } + + public String getPrevKey() { + return prevKey; + } + + public boolean isShallow() { + return shallow; + } + + public OzoneBucket getBucket() { + return bucket; + } + + public Iterator getOzoneKeyIterator() { + return ozoneKeyIterator; + } + + public ContinueToken getDecodedToken() { + return decodedToken; + } + } + + /** + * Validate and prepare parameters for bucket listing. + */ + @SuppressWarnings({"parameternumber", "checkstyle:ParameterNumber"}) + BucketListingContext validateAndPrepareParameters( + String bucketName, String delimiter, String encodingType, String marker, + int maxKeys, String prefix, String continueToken, String startAfter) + throws OS3Exception, IOException { + + // If you specify the encoding-type request parameter, should return encoded key name values + // in the following response elements: Delimiter, Prefix, Key, and StartAfter. + // For detail refer: https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#AmazonS3-ListObjectsV2-response-EncodingType + + // Validate encoding type if (encodingType != null && !encodingType.equals(ENCODING_TYPE)) { throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, encodingType); } - // If you specify the encoding-type request parameter,should return // encoded key name values in the following response elements: // Delimiter, Prefix, Key, and StartAfter. @@ -189,9 +303,47 @@ public Response get( // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html // #AmazonS3-ListObjectsV2-response-EncodingType // + maxKeys = validateMaxKeys(maxKeys); + + if (prefix == null) { + prefix = ""; + } + + // Assign marker to startAfter. for the compatibility of aws api v1 + if (startAfter == null && marker != null) { + startAfter = marker; + } + + ContinueToken decodedToken = ContinueToken.decodeFromString(continueToken); + + // If continuation token and start after both are provided, then we + // ignore start After + String prevKey = continueToken != null ? decodedToken.getLastKey() : startAfter; + + // If shallow is true, only list immediate children + // delimited by OZONE_URI_DELIMITER + boolean shallow = listKeysShallowEnabled && OZONE_URI_DELIMITER.equals(delimiter); + + OzoneBucket bucket = getBucket(bucketName); + S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner()); + + Iterator ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow); + + return new BucketListingContext(bucketName, delimiter, encodingType, marker, + maxKeys, prefix, continueToken, startAfter, prevKey, shallow, + bucket, ozoneKeyIterator, decodedToken); + } + + /** + * Initialize ListObjectResponse object. + */ + @SuppressWarnings({"parameternumber", "checkstyle:ParameterNumber"}) + ListObjectResponse initializeListObjectResponse( + String bucketName, String delimiter, String encodingType, String marker, + int maxKeys, String prefix, String continueToken, String startAfter) { + ListObjectResponse response = new ListObjectResponse(); - response.setDelimiter( - EncodingTypeObject.createNullable(delimiter, encodingType)); + response.setDelimiter(EncodingTypeObject.createNullable(delimiter, encodingType)); response.setName(bucketName); response.setPrefix(EncodingTypeObject.createNullable(prefix, encodingType)); response.setMarker(marker == null ? "" : marker); @@ -199,48 +351,60 @@ public Response get( response.setEncodingType(encodingType); response.setTruncated(false); response.setContinueToken(continueToken); - response.setStartAfter( - EncodingTypeObject.createNullable(startAfter, encodingType)); + response.setStartAfter(EncodingTypeObject.createNullable(startAfter, encodingType)); + + return response; + } + /** + * Process key listing logic. + */ + void processKeyListing(BucketListingContext context, ListObjectResponse response) { String prevDir = null; - if (continueToken != null) { - prevDir = decodedToken.getLastDir(); + if (context.getContinueToken() != null) { + prevDir = context.getDecodedToken().getLastDir(); } + String lastKey = null; int count = 0; - if (maxKeys > 0) { + Iterator ozoneKeyIterator = context.getOzoneKeyIterator(); + + if (context.getMaxKeys() > 0) { while (ozoneKeyIterator != null && ozoneKeyIterator.hasNext()) { OzoneKey next = ozoneKeyIterator.next(); - if (bucket != null && bucket.getBucketLayout().isFileSystemOptimized() && - StringUtils.isNotEmpty(prefix) && - !next.getName().startsWith(prefix)) { + + if (context.getBucket() != null && + context.getBucket().getBucketLayout().isFileSystemOptimized() && + StringUtils.isNotEmpty(context.getPrefix()) && + !next.getName().startsWith(context.getPrefix())) { // prefix has delimiter but key don't have // example prefix: dir1/ key: dir123 continue; } - if (startAfter != null && count == 0 && Objects.equals(startAfter, next.getName())) { + + if (context.getStartAfter() != null && count == 0 && + Objects.equals(context.getStartAfter(), next.getName())) { continue; } - String relativeKeyName = next.getName().substring(prefix.length()); - - int depth = StringUtils.countMatches(relativeKeyName, delimiter); - if (!StringUtils.isEmpty(delimiter)) { + + String relativeKeyName = next.getName().substring(context.getPrefix().length()); + int depth = StringUtils.countMatches(relativeKeyName, context.getDelimiter()); + + if (!StringUtils.isEmpty(context.getDelimiter())) { if (depth > 0) { // means key has multiple delimiters in its value. // ex: dir/dir1/dir2, where delimiter is "/" and prefix is dir/ - String dirName = relativeKeyName.substring(0, relativeKeyName - .indexOf(delimiter)); + String dirName = relativeKeyName.substring(0, relativeKeyName.indexOf(context.getDelimiter())); if (!dirName.equals(prevDir)) { response.addPrefix(EncodingTypeObject.createNullable( - prefix + dirName + delimiter, encodingType)); + context.getPrefix() + dirName + context.getDelimiter(), context.getEncodingType())); prevDir = dirName; count++; } - } else if (relativeKeyName.endsWith(delimiter)) { + } else if (relativeKeyName.endsWith(context.getDelimiter())) { // means or key is same as prefix with delimiter at end and ends with // delimiter. ex: dir/, where prefix is dir and delimiter is / - response.addPrefix( - EncodingTypeObject.createNullable(relativeKeyName, encodingType)); + response.addPrefix(EncodingTypeObject.createNullable(relativeKeyName, context.getEncodingType())); count++; } else { // means our key is matched with prefix if prefix is given and it @@ -253,7 +417,7 @@ public Response get( count++; } - if (count == maxKeys) { + if (count == context.getMaxKeys()) { lastKey = next.getName(); break; } @@ -262,7 +426,7 @@ public Response get( response.setKeyCount(count); - if (count < maxKeys) { + if (count < context.getMaxKeys()) { response.setTruncated(false); } else if (ozoneKeyIterator.hasNext() && lastKey != null) { response.setTruncated(true); @@ -273,27 +437,26 @@ public Response get( } else { response.setTruncated(false); } + } - int keyCount = - response.getCommonPrefixes().size() + response.getContents().size(); - long opLatencyNs = - getMetrics().updateGetBucketSuccessStats(startNanos); + /** + * Build final response with metrics and audit logging. + */ + Response buildFinalResponse(ListObjectResponse response, S3GAction s3GAction, long startNanos) { + int keyCount = response.getCommonPrefixes().size() + response.getContents().size(); + long opLatencyNs = getMetrics().updateGetBucketSuccessStats(startNanos); getMetrics().incListKeyCount(keyCount); + + PerformanceStringBuilder perf = new PerformanceStringBuilder(); perf.appendCount(keyCount); perf.appendOpLatencyNanos(opLatencyNs); + auditReadSuccess(s3GAction, perf); response.setKeyCount(keyCount); + return Response.ok(response).build(); } - private int validateMaxKeys(int maxKeys) throws OS3Exception { - if (maxKeys < 0) { - throw newError(S3ErrorTable.INVALID_ARGUMENT, "maxKeys must be >= 0"); - } - - return Math.min(maxKeys, maxKeysLimit); - } - @PUT public Response put( @PathParam(BUCKET) String bucketName, diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketEndpoint.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketEndpoint.java new file mode 100644 index 00000000000..9bd6b2a3633 --- /dev/null +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketEndpoint.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.s3.endpoint; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import javax.ws.rs.core.Response; +import org.apache.hadoop.ozone.client.OzoneClientStub; +import org.apache.hadoop.ozone.s3.exception.OS3Exception; +import org.apache.hadoop.ozone.s3.exception.S3ErrorTable; +import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Simple unit tests for the refactored BucketEndpoint methods. + */ +public class TestBucketEndpoint { + + private static final String TEST_BUCKET_NAME = "test-bucket"; + private BucketEndpoint bucketEndpoint; + + @BeforeEach + public void setUp() throws IOException { + OzoneClientStub client = new OzoneClientStub(); + + bucketEndpoint = EndpointBuilder.newBucketEndpointBuilder() + .setClient(client) + .build(); + + // Create a test bucket + client.getObjectStore().createS3Bucket(TEST_BUCKET_NAME); + } + + // Note: validateMaxKeys is a private helper and is covered indirectly by + // public API tests elsewhere. We avoid calling private helpers directly + // from unit tests in this module to keep encapsulation intact. + + @Test + public void testBucketListingContextCreation() throws OS3Exception, IOException { + // Test basic context creation + String bucketName = TEST_BUCKET_NAME; + String delimiter = "/"; + String encodingType = null; + String marker = null; + int maxKeys = 100; + String prefix = "test/"; + String continueToken = null; + String startAfter = null; + + BucketEndpoint.BucketListingContext context = bucketEndpoint.validateAndPrepareParameters( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter); + + assertNotNull(context); + assertEquals(bucketName, context.getBucketName()); + assertEquals(delimiter, context.getDelimiter()); + assertEquals(maxKeys, context.getMaxKeys()); + assertEquals(prefix, context.getPrefix()); + } + + @Test + public void testInitializeListObjectResponse() { + // Test response initialization + String bucketName = TEST_BUCKET_NAME; + String delimiter = "/"; + String encodingType = "url"; + String marker = "test-marker"; + int maxKeys = 100; + String prefix = "test/"; + String continueToken = "test-token"; + String startAfter = "test-start"; + + Object response = bucketEndpoint.initializeListObjectResponse( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter); + + assertNotNull(response); + // Basic validation only; avoid depending on ListObjectResponse type here. + assertEquals("ListObjectResponse", response.getClass().getSimpleName()); + } + + @Test + public void testValidateAndPrepareParametersWithInvalidEncodingType() { + // Test invalid encoding type + String bucketName = TEST_BUCKET_NAME; + String delimiter = "/"; + String encodingType = "invalid"; + String marker = null; + int maxKeys = 100; + String prefix = "test/"; + String continueToken = null; + String startAfter = null; + + OS3Exception exception = assertThrows(OS3Exception.class, () -> + bucketEndpoint.validateAndPrepareParameters( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter)); + + assertEquals(S3ErrorTable.INVALID_ARGUMENT.getCode(), exception.getCode()); + } + + @Test + public void testValidateAndPrepareParametersWithNonexistentBucket() { + // Test with non-existent bucket + String bucketName = "nonexistent-bucket"; + String delimiter = "/"; + String encodingType = null; + String marker = null; + int maxKeys = 100; + String prefix = "test/"; + String continueToken = null; + String startAfter = null; + + OS3Exception exception = assertThrows(OS3Exception.class, () -> + bucketEndpoint.validateAndPrepareParameters( + bucketName, delimiter, encodingType, marker, maxKeys, prefix, + continueToken, startAfter)); + + assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), exception.getCode()); + } + + @Test + public void testGetDelegatesToAclHandler() throws Exception { + // When ?acl is present, BucketEndpoint should delegate to BucketAclHandler. + bucketEndpoint.queryParamsForTest().set(QueryParams.ACL, ""); + + Response response = bucketEndpoint.get(TEST_BUCKET_NAME); + + assertNotNull(response); + assertEquals(200, response.getStatus()); + assertNotNull(response.getEntity()); + assertEquals("S3BucketAcl", response.getEntity().getClass().getSimpleName()); + } +}