jojochuang commented on code in PR #8833:
URL: https://github.com/apache/ozone/pull/8833#discussion_r2218032188
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -225,53 +225,55 @@ public Response get(
}
String lastKey = null;
int count = 0;
- while (ozoneKeyIterator != null && ozoneKeyIterator.hasNext()) {
Review Comment:
The get() mthod is too long, containing unrelated code pieces. Suggest to
refactor it into multiple smaller helper methods in a follow up task.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -225,53 +225,55 @@ public Response get(
}
String lastKey = null;
int count = 0;
- while (ozoneKeyIterator != null && ozoneKeyIterator.hasNext()) {
- OzoneKey next = ozoneKeyIterator.next();
- if (bucket != null && bucket.getBucketLayout().isFileSystemOptimized() &&
- StringUtils.isNotEmpty(prefix) &&
- !next.getName().startsWith(prefix)) {
- // prefix has delimiter but key don't have
- // example prefix: dir1/ key: dir123
- continue;
- }
- if (startAfter != null && count == 0 && Objects.equals(startAfter,
next.getName())) {
- continue;
- }
- String relativeKeyName = next.getName().substring(prefix.length());
-
- int depth = StringUtils.countMatches(relativeKeyName, delimiter);
- if (!StringUtils.isEmpty(delimiter)) {
- 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));
- if (!dirName.equals(prevDir)) {
- response.addPrefix(EncodingTypeObject.createNullable(
- prefix + dirName + delimiter, encodingType));
- prevDir = dirName;
+ if (maxKeys > 0) {
Review Comment:
potential bug:
If maxKeys = 0, it does not enter this if statement. Later, lastKey would
still be null. If the bucket is not emtpy, and key iterator has next item, this
line looks problematic:
```
ContinueToken nextToken = new ContinueToken(lastKey, prevDir);
response.setNextToken(nextToken.encodeToString());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
