rich7420 commented on code in PR #11358:
URL: https://github.com/apache/ozone/pull/11358#discussion_r4136377040
##########
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:
Thanks, that makes sense. Updated to match `abortMultipartUpload`.
##########
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:
I added this because the responses would still look correct if we
accidentally brought back the extra RPC. I’ve kept the count check in one test
and removed the repeated checks from the error cases.
--
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]
