Skip to content

HDDS-13532. Refactor BucketEndpoint.get() #10045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rich7420 wants to merge 9 commits into
base: master
Choose a base branch
from
+380 −71
Open

HDDS-13532. Refactor BucketEndpoint.get() #10045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2434f5f
Refactor BucketEndpoint.get()
rich7420 Oct 7, 2025
b48f086
move test
rich7420 Oct 7, 2025
7d3f39b
fix error
rich7420 Oct 7, 2025
164422c
remove IT test for passing build-branch
rich7420 Oct 7, 2025
10500f4
rename methids and test class
rich7420 Oct 13, 2025
efb77b6
keep comment
rich7420 Oct 13, 2025
c9ad186
add audit log entry
rich7420 Nov 11, 2025
9d848d5
move if position
rich7420 Dec 6, 2025
ba657c7
Refactor BucketEndpoint.get() into helper methods
rich7420 Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
298 changes: 227 additions & 71 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,7 @@ public Response get(
String prefix = queryParams().get(QueryParams.PREFIX);
String startAfter = queryParams().get(QueryParams.START_AFTER);

Iterator<? extends OzoneKey> ozoneKeyIterator = null;
ContinueToken decodedToken =
ContinueToken.decodeFromString(continueToken);
OzoneBucket bucket = null;

// Actual bucket processing starts here
try {
final String uploads = queryParams().get(QueryParams.UPLOADS);
if (uploads != null) {
Expand All @@ -134,31 +130,22 @@ public Response get(
return listMultipartUploads(bucketName, prefix, keyMarker, uploadIdMarker, maxUploads);
}

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;
}
// Actual bucket processing starts here
// Validate and prepare parameters
BucketListingContext listingContext = validateAndPrepareParameters(
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;
// Initialize response object
ListObjectResponse response = initializeListObjectResponse(
bucketName, delimiter, encodingType, marker, listingContext.getMaxKeys(), prefix,
continueToken, startAfter);
Comment on lines +133 to +142
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here maxKeys is initialized from the query parameters (e.g., a user requests 5000), validateAndPrepareParameters is called. Inside this method, maxKeys is capped via validateMaxKeys(maxKeys) (e.g., capped to 1000). This capped value is stored inside the BucketListingContext, the maxKeys variable back in the get() method is not updated. It remains 5000. initializeListObjectResponse is called using the original maxKeys (5000).The response object now explicitly states 5000.

I think if we pass listingContext.getMaxKeys() not the local maxKeys to initializeListObjectResponse this issue would not be seen.

Could you confirm whether that’s intentional or something we should align?


// If shallow is true, only list immediate children
// delimited by OZONE_URI_DELIMITER
boolean shallow = listKeysShallowEnabled
&& OZONE_URI_DELIMITER.equals(delimiter);
// Process key listing
processKeyListing(listingContext, response);

bucket = getBucket(bucketName);
S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());

ozoneKeyIterator = bucket.listKeys(prefix, prevKey, shallow);
// Build final response
return buildFinalResponse(response, s3GAction, startNanos, perf);

} catch (OMException ex) {
auditReadFailure(s3GAction, ex);
Expand All @@ -177,71 +164,242 @@ 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);
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<? extends OzoneKey> 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<? extends OzoneKey> 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<? extends OzoneKey> 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
Comment on lines +292 to +295
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : this comment is repeated.


// Validate encoding type
if (encodingType != null && !encodingType.equals(ENCODING_TYPE)) {
throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, 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());

// 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
//
Iterator<? extends OzoneKey> 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);
response.setMaxKeys(maxKeys);
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<? extends OzoneKey> 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
Expand All @@ -254,7 +412,7 @@ public Response get(
count++;
}

if (count == maxKeys) {
if (count == context.getMaxKeys()) {
lastKey = next.getName();
break;
}
Expand All @@ -263,7 +421,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);
Expand All @@ -274,27 +432,25 @@ 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, PerformanceStringBuilder perf) {
int keyCount = response.getCommonPrefixes().size() + response.getContents().size();
long opLatencyNs = getMetrics().updateGetBucketSuccessStats(startNanos);
getMetrics().incListKeyCount(keyCount);
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,
Expand Down
Loading
Loading