ArafatKhan2198 commented on code in PR #11242:
URL: https://github.com/apache/ozone/pull/11242#discussion_r4063717015
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -204,8 +214,10 @@ private GetObjectAttributesResponse buildResponse(String
keyPath, OzoneKey key,
if (eTag != null) {
String partsCountStr = extractPartsCount(eTag);
if (partsCountStr != null && completedPartSizes != null) {
+ boolean directoryBucketLayout =
+ context.getBucket().getBucketLayout().isFileSystemOptimized();
Review Comment:
This also adds one more OM call for every multipart `ObjectParts` request.
Since `headS3ObjectAttributes()` is already calling OM, can we return the
bucket layout as part of that response and avoid another OM call?
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -204,8 +214,10 @@ private GetObjectAttributesResponse buildResponse(String
keyPath, OzoneKey key,
if (eTag != null) {
String partsCountStr = extractPartsCount(eTag);
if (partsCountStr != null && completedPartSizes != null) {
+ boolean directoryBucketLayout =
+ context.getBucket().getBucketLayout().isFileSystemOptimized();
Review Comment:
This adds an extra bucket lookup just to check the bucket layout.
`getBucket()` may require READ permission on the bucket. A user may have
permission to read the object but not the bucket, so this could make
`GetObjectAttributes` fail with `AccessDenied`.
Can we get the bucket layout from the existing `headS3ObjectAttributes()`
call instead, so we don't change the current permission requirements?
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectAttributesGet.java:
##########
@@ -299,13 +304,52 @@ public void
testGetObjectAttributesNonContiguousMultipartParts() throws IOExcept
assertEquals(partThreeContent.length(),
paginatedParts.getParts().get(0).getSize());
}
- private void completeMultipartUploadWithParts(String key, String...
partContents)
+ @Test
+ public void testGetObjectAttrFsoMPUPartsWithoutChecksum()
throws IOException, OS3Exception {
- String uploadID = initiateMultipartUpload(rest, BUCKET_NAME, key);
+ OzoneClient client = rest.getClient();
+ String volumeName =
rest.getOzoneConfiguration().get(OzoneConfigKeys.OZONE_S3_VOLUME_NAME,
+ OzoneConfigKeys.OZONE_S3_VOLUME_NAME_DEFAULT);
+ OzoneVolume volume = client.getObjectStore().getVolume(volumeName);
+ volume.createBucket(FSO_BUCKET_NAME, BucketArgs.newBuilder()
+ .setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED)
+ .build());
+
+ final String key = "fso-mpu-key";
+ completeMultipartUploadWithPartsInBucket(FSO_BUCKET_NAME, key, "part-one",
"part-two");
+
+ Response response = getObjectAttributes(rest, FSO_BUCKET_NAME, key,
"ObjectParts");
+
+ assertEquals(HTTP_OK, response.getStatus());
+ GetObjectAttributesResponse.ObjectParts objectParts =
+ ((GetObjectAttributesResponse) response.getEntity()).getObjectParts();
+ assertNotNull(objectParts);
+ assertEquals(2, objectParts.getPartsCount().intValue());
+ assertEquals(2, objectParts.getParts().size());
+ assertEquals(1, objectParts.getParts().get(0).getPartNumber());
+ assertEquals("part-one".length(), objectParts.getParts().get(0).getSize());
+ assertEquals(2, objectParts.getParts().get(1).getPartNumber());
+ assertEquals("part-two".length(), objectParts.getParts().get(1).getSize());
+ }
+
+ @Test
+ public void testShouldIncludePartElements() {
Review Comment:
Can we add a few more cases here?
It would be good to cover:
* OBS without checksum -> false
* OBS with checksum -> true
* FSO without checksum -> true
* FSO with checksum -> true
This will make the expected behavior clearer.
--
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]
