fmorg-git commented on code in PR #11426:
URL: https://github.com/apache/ozone/pull/11426#discussion_r4253818054
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectNotImplemented.java:
##########
@@ -110,4 +114,12 @@ public void
deleteWithVersionIdIsNotImplementedAndDoesNotDeleteObject() throws I
assertErrorResponse(S3ErrorTable.NOT_IMPLEMENTED, () ->
delete(objectEndpoint, BUCKET_NAME, KEY_NAME));
assertKeyContent(bucket, KEY_NAME, CONTENT);
}
+
Review Comment:
should `deleteWithVersionIdIsNotImplementedAndDoesNotDeleteObject` test now
be renamed to something like
`deleteWithNonNullVersionIdIsNotImplementedAndDoesNotDeleteObject`?
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectTaggingPut.java:
##########
@@ -124,6 +124,15 @@ public void
testPutObjectTaggingWithVersionIdIsNotImplemented() throws Exception
assertThat(clientStub.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_NAME).getTags()).isEmpty();
}
+ @Test
+ public void testPutObjectTaggingWithNullVersionId() throws Exception {
+ objectEndpoint.queryParamsForTest().set(S3Consts.QueryParams.VERSION_ID,
S3Consts.NULL_VERSION_ID);
Review Comment:
should `testPutObjectTaggingWithVersionIdIsNotImplemented` test now be
renamed to something like
`testPutObjectTaggingWithNonNullVersionIdIsNotImplemented`?
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -353,7 +374,8 @@ public MultiDeleteResponse multiDelete(
}
// Deleting a specific version is not implemented; ignoring VersionId
would delete the current object instead.
Review Comment:
```suggestion
// Deleting a specific version is not implemented (except for the null
version); ignoring VersionId would delete the current object instead.
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketList.java:
##########
@@ -669,6 +672,85 @@ public void continuationTokenXmlElementName() throws
Exception {
"response must not use the non-AWS <continueToken> element");
}
+ @Test
+ public void listObjectVersionsReturnsNullVersionOfEachKey() throws
OS3Exception, IOException {
+ setup("dir1/file2", "file+1", "file3");
+
+ bucketEndpoint.queryParamsForTest().set(QueryParams.VERSIONS, "");
+ bucketEndpoint.queryParamsForTest().set(QueryParams.DELIMITER, "/");
+ bucketEndpoint.queryParamsForTest().set(QueryParams.ENCODING_TYPE,
ENCODING_TYPE);
+ ListVersionsResult result = (ListVersionsResult)
bucketEndpoint.get(BUCKET_NAME).getEntity();
+
+ assertThat(result.isTruncated()).isFalse();
+ assertThat(result.getNextKeyMarker()).isNull();
+ assertThat(result.getNextVersionIdMarker()).isNull();
+ assertThat(result.getCommonPrefixes()).extracting(p ->
p.getPrefix().getName()).containsExactly("dir1/");
+ assertThat(result.getVersions()).extracting(v ->
v.getKey().getName()).containsExactly("file+1", "file3");
+ assertThat(result.getVersions()).allSatisfy(v -> {
+ assertThat(v.getVersionId()).isEqualTo(NULL_VERSION_ID);
+ assertThat(v.getIsLatest()).isTrue();
+ assertThat(v.getOwner()).isNotNull();
+ });
+
+ StringWriter writer = new StringWriter();
+ JAXB.marshal(result, writer);
+ assertThat(writer.toString())
Review Comment:
curious - for such a small set of files, would it be worth it to have an
assertion on the actual expected output instead of a few `contains` and
`doesNotContain` checks?
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectTaggingDelete.java:
##########
@@ -115,6 +117,14 @@ public void
testDeleteTaggingWithVersionIdIsNotImplemented() throws IOException
client.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_WITH_TAG).getTags());
}
+ @Test
Review Comment:
should `testDeleteTaggingWithVersionIdIsNotImplemented` test now be renamed
to something like `testDeleteTaggingWithNonNullVersionIdIsNotImplemented`?
--
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]
