Skip to content

Commit b968353

Browse files
Jimmyweng006jojochuangpeterxcli
authored
HDDS-13447. [S3G] ListObjectsV2 should accept maxKeys=0 (apache#8833)
Co-authored-by: Wei-Chiu Chuang <weichiu@apache.org> Co-authored-by: peterxcli <peterxcli@gmail.com>
1 parent 4d360f4 commit b968353

File tree

3 files changed

+88
-55
lines changed

3 files changed

+88
-55
lines changed

‎hadoop-ozone/dist/src/main/smoketest/s3/objectlist.robot‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ List objects with negative max-keys should fail
4141
${result} = Execute AWSS3APICli and checkrc list-objects-v2 --bucket ${BUCKET} --max-keys -1 255
4242
Should Contain ${result} InvalidArgument
4343

44-
List objects with zero max-keys should fail
45-
${result} = Execute AWSS3APICli and checkrc list-objects-v2 --bucket ${BUCKET} --max-keys 0 255
46-
Should Contain ${result} InvalidArgument
44+
List objects with zero max-keys should not fail
45+
${result} = Execute AWSS3APICli and checkrc list-objects-v2 --bucket ${BUCKET} --max-keys 0 0
46+
Should not Contain ${result} InvalidArgument
4747

4848
List objects with max-keys exceeding config limit should not return more than limit
4949
Prepare Many Objects In Bucket 1100

‎hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java‎

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -225,61 +225,63 @@ public Response get(
225225
}
226226
String lastKey = null;
227227
int count = 0;
228-
while (ozoneKeyIterator != null && ozoneKeyIterator.hasNext()) {
229-
OzoneKey next = ozoneKeyIterator.next();
230-
if (bucket != null && bucket.getBucketLayout().isFileSystemOptimized() &&
231-
StringUtils.isNotEmpty(prefix) &&
232-
!next.getName().startsWith(prefix)) {
233-
// prefix has delimiter but key don't have
234-
// example prefix: dir1/ key: dir123
235-
continue;
236-
}
237-
if (startAfter != null && count == 0 && Objects.equals(startAfter, next.getName())) {
238-
continue;
239-
}
240-
String relativeKeyName = next.getName().substring(prefix.length());
241-
242-
int depth = StringUtils.countMatches(relativeKeyName, delimiter);
243-
if (!StringUtils.isEmpty(delimiter)) {
244-
if (depth > 0) {
245-
// means key has multiple delimiters in its value.
246-
// ex: dir/dir1/dir2, where delimiter is "/" and prefix is dir/
247-
String dirName = relativeKeyName.substring(0, relativeKeyName
248-
.indexOf(delimiter));
249-
if (!dirName.equals(prevDir)) {
250-
response.addPrefix(EncodingTypeObject.createNullable(
251-
prefix + dirName + delimiter, encodingType));
252-
prevDir = dirName;
228+
if (maxKeys > 0) {
229+
while (ozoneKeyIterator != null && ozoneKeyIterator.hasNext()) {
230+
OzoneKey next = ozoneKeyIterator.next();
231+
if (bucket != null && bucket.getBucketLayout().isFileSystemOptimized() &&
232+
StringUtils.isNotEmpty(prefix) &&
233+
!next.getName().startsWith(prefix)) {
234+
// prefix has delimiter but key don't have
235+
// example prefix: dir1/ key: dir123
236+
continue;
237+
}
238+
if (startAfter != null && count == 0 && Objects.equals(startAfter, next.getName())) {
239+
continue;
240+
}
241+
String relativeKeyName = next.getName().substring(prefix.length());
242+
243+
int depth = StringUtils.countMatches(relativeKeyName, delimiter);
244+
if (!StringUtils.isEmpty(delimiter)) {
245+
if (depth > 0) {
246+
// means key has multiple delimiters in its value.
247+
// ex: dir/dir1/dir2, where delimiter is "/" and prefix is dir/
248+
String dirName = relativeKeyName.substring(0, relativeKeyName
249+
.indexOf(delimiter));
250+
if (!dirName.equals(prevDir)) {
251+
response.addPrefix(EncodingTypeObject.createNullable(
252+
prefix + dirName + delimiter, encodingType));
253+
prevDir = dirName;
254+
count++;
255+
}
256+
} else if (relativeKeyName.endsWith(delimiter)) {
257+
// means or key is same as prefix with delimiter at end and ends with
258+
// delimiter. ex: dir/, where prefix is dir and delimiter is /
259+
response.addPrefix(
260+
EncodingTypeObject.createNullable(relativeKeyName, encodingType));
261+
count++;
262+
} else {
263+
// means our key is matched with prefix if prefix is given and it
264+
// does not have any common prefix.
265+
addKey(response, next);
253266
count++;
254267
}
255-
} else if (relativeKeyName.endsWith(delimiter)) {
256-
// means or key is same as prefix with delimiter at end and ends with
257-
// delimiter. ex: dir/, where prefix is dir and delimiter is /
258-
response.addPrefix(
259-
EncodingTypeObject.createNullable(relativeKeyName, encodingType));
260-
count++;
261268
} else {
262-
// means our key is matched with prefix if prefix is given and it
263-
// does not have any common prefix.
264269
addKey(response, next);
265270
count++;
266271
}
267-
} else {
268-
addKey(response, next);
269-
count++;
270-
}
271272

272-
if (count == maxKeys) {
273-
lastKey = next.getName();
274-
break;
273+
if (count == maxKeys) {
274+
lastKey = next.getName();
275+
break;
276+
}
275277
}
276278
}
277279

278280
response.setKeyCount(count);
279281

280282
if (count < maxKeys) {
281283
response.setTruncated(false);
282-
} else if (ozoneKeyIterator.hasNext()) {
284+
} else if (ozoneKeyIterator.hasNext() && lastKey != null) {
283285
response.setTruncated(true);
284286
ContinueToken nextToken = new ContinueToken(lastKey, prevDir);
285287
response.setNextToken(nextToken.encodeToString());
@@ -303,8 +305,8 @@ public Response get(
303305
}
304306

305307
private int validateMaxKeys(int maxKeys) throws OS3Exception {
306-
if (maxKeys <= 0) {
307-
throw newError(S3ErrorTable.INVALID_ARGUMENT, "maxKeys must be > 0");
308+
if (maxKeys < 0) {
309+
throw newError(S3ErrorTable.INVALID_ARGUMENT, "maxKeys must be >= 0");
308310
}
309311

310312
return Math.min(maxKeys, maxKeysLimit);

‎hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketList.java‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -523,26 +523,57 @@ public void testEncodingTypeException() throws IOException {
523523
}
524524

525525
@Test
526-
public void testListObjectsWithInvalidMaxKeys() throws Exception {
527-
OzoneClient client = createClientWithKeys("file1");
526+
public void testListObjectsWithNegativeMaxKeys() throws Exception {
527+
OzoneClient client = new OzoneClientStub();
528528
client.getObjectStore().createS3Bucket("bucket");
529529
BucketEndpoint bucketEndpoint = EndpointBuilder.newBucketEndpointBuilder()
530530
.setClient(client)
531531
.build();
532532

533-
// maxKeys < 0
533+
// maxKeys < 0 should throw InvalidArgument
534534
OS3Exception e1 = assertThrows(OS3Exception.class, () ->
535535
bucketEndpoint.get("bucket", null, null, null, -1, null,
536536
null, null, null, null, null, null, 1000)
537537
);
538538
assertEquals(S3ErrorTable.INVALID_ARGUMENT.getCode(), e1.getCode());
539+
}
539540

540-
// maxKeys == 0
541-
OS3Exception e2 = assertThrows(OS3Exception.class, () ->
542-
bucketEndpoint.get("bucket", null, null, null, 0, null,
543-
null, null, null, null, null, null, 1000)
544-
);
545-
assertEquals(S3ErrorTable.INVALID_ARGUMENT.getCode(), e2.getCode());
541+
@Test
542+
public void testListObjectsWithZeroMaxKeys() throws Exception {
543+
OzoneClient client = new OzoneClientStub();
544+
client.getObjectStore().createS3Bucket("bucket");
545+
BucketEndpoint bucketEndpoint = EndpointBuilder.newBucketEndpointBuilder()
546+
.setClient(client)
547+
.build();
548+
549+
// maxKeys = 0, should return empty list and not throw.
550+
ListObjectResponse response = (ListObjectResponse) bucketEndpoint.get(
551+
"bucket", null, null, null, 0, null,
552+
null, null, null, null, null, null, 1000).getEntity();
553+
554+
assertEquals(0, response.getContents().size());
555+
assertFalse(response.isTruncated());
556+
}
557+
558+
@Test
559+
public void testListObjectsWithZeroMaxKeysInNonEmptyBucket() throws Exception {
560+
OzoneClient client = createClientWithKeys("file1", "file2", "file3", "file4", "file5");
561+
BucketEndpoint bucketEndpoint = EndpointBuilder.newBucketEndpointBuilder()
562+
.setClient(client)
563+
.build();
564+
565+
ListObjectResponse response = (ListObjectResponse) bucketEndpoint.get(
566+
"b1", null, null, null, 0, null,
567+
null, null, null, null, null, null, 1000).getEntity();
568+
569+
// Should return empty list and not throw.
570+
assertEquals(0, response.getContents().size());
571+
assertFalse(response.isTruncated());
572+
573+
ListObjectResponse fullResponse = (ListObjectResponse) bucketEndpoint.get(
574+
"b1", null, null, null, 1000, null,
575+
null, null, null, null, null, null, 1000).getEntity();
576+
assertEquals(5, fullResponse.getContents().size());
546577
}
547578

548579
@Test

0 commit comments

Comments
 (0)