chihsuan commented on code in PR #11358:
URL: https://github.com/apache/ozone/pull/11358#discussion_r4135943960
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListParts.java:
##########
@@ -99,6 +117,83 @@ public void testListPartsWithUnknownUploadID() {
() -> EndpointTestUtils.listParts(rest, OzoneConsts.S3_BUCKET,
"no-such-key", "no-such-upload", 2, 0));
}
+ @ParameterizedTest
+ @CsvSource({", 0", "'', 0", "owner, 1"})
+ public void testListPartsOnlyLooksUpBucketForOwner(String expectedOwner, int
bucketLookups) throws Exception {
+ ClientProtocol proxy = mock(ClientProtocol.class);
+ HttpHeaders headers = mock(HttpHeaders.class);
+
when(headers.getHeaderString(EXPECTED_BUCKET_OWNER_HEADER)).thenReturn(expectedOwner);
+ ObjectEndpoint endpoint = newEndpoint(proxy, headers);
+ OzoneMultipartUploadPartListParts parts = new
OzoneMultipartUploadPartListParts(
+
RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE), 4,
true);
+ parts.addPart(new OzoneMultipartUploadPartListParts.PartInfo(4, "part4",
0, 10, "etag4"));
+ when(proxy.listParts("volume1", "bucket1", "key1", "upload1", 3,
2)).thenReturn(parts);
+
+ try (Response response = EndpointTestUtils.listParts(endpoint, "bucket1",
"key1", "upload1", 2, 3)) {
+ assertThat(response.getStatus()).isEqualTo(200);
+ ListPartsResponse result = (ListPartsResponse) response.getEntity();
+ assertThat(result.getBucket()).isEqualTo("bucket1");
+ assertThat(result.getPartNumberMarker()).isEqualTo(3);
+ assertThat(result.getMaxParts()).isEqualTo(2);
+ assertThat(result.getNextPartNumberMarker()).isEqualTo(4);
+ assertThat(result.getTruncated()).isTrue();
+ assertThat(result.getPartList()).hasSize(1);
+ assertThat(result.getPartList().get(0).getETag()).isEqualTo("etag4");
+ }
+ verify(proxy, times(bucketLookups)).getBucketDetails("volume1", "bucket1");
Review Comment:
Just curious, do we need tests to guard this optimization? These check how
many times we look up the bucket, which feels tied to implementation details. I
didn't see similar guards for other S3 APIs.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/MultipartKeyHandler.java:
##########
@@ -123,17 +121,18 @@ private Response abortMultipartUpload(OzoneVolume volume,
String bucket,
* @throws IOException
* @throws OS3Exception
*/
- private Response listParts(OzoneBucket ozoneBucket, String key, String
uploadId,
+ private Response listParts(ObjectEndpoint.ObjectRequestContext context,
String key, String uploadId,
Review Comment:
nit: Could we pass `context.getVolume()` and `context.getBucketName()` into
`listParts`, like `abortMultipartUpload` does? That keeps the two helpers
consistent.
--
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]
