You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by GitBox <gi...@apache.org> on 2022/02/06 12:48:11 UTC
[GitHub] [jclouds] gaul commented on a change in pull request #130: JCLOUDS-1587 - S3 Blobstore: Support Bucket Configuration Options - Versioning, Lifecycle, Encryption
gaul commented on a change in pull request #130:
URL: https://github.com/apache/jclouds/pull/130#discussion_r800174777
##########
File path: blobstore/src/main/java/org/jclouds/blobstore/options/ListContainerOptions.java
##########
@@ -58,6 +59,12 @@ public ListContainerOptions() {
this.delimiter = delimiter;
}
+ ListContainerOptions(Integer maxKeys, String marker, String dir, boolean recursive,
+ boolean detailed, String prefix, String delimiter, boolean versions) {
+ this(maxKeys, marker, dir, recursive, detailed, prefix, delimiter);
+ this.versions = versions;
Review comment:
Same comment about making this the primary method and add a `@Deprecated` annotation to the old method.
##########
File path: blobstore/src/main/java/org/jclouds/blobstore/domain/internal/StorageMetadataImpl.java
##########
@@ -40,6 +40,10 @@
@Nullable
private final String eTag;
@Nullable
+ private final String versionId;
+ @Nullable
+ private final String isLatest;
Review comment:
`boolean`? Need to propagate this throughout.
##########
File path: apis/s3/src/test/java/org/jclouds/s3/S3ClientTest.java
##########
@@ -501,6 +504,86 @@ public void testDisableBucketLogging() throws SecurityException, NoSuchMethodExc
checkFilters(request);
}
+ public void testGetBucketVersion() throws SecurityException, NoSuchMethodException, IOException {
+ Invokable<?, ?> method = method(S3Client.class, "getBucketConfiguration", String.class, BucketConfigOptions[].class);
+ GeneratedHttpRequest request = processor.createRequest(method, ImmutableList.<Object> of("bucket", new BucketConfigOptions().versioning()));
+
+ assertRequestLineEquals(request, "GET https://bucket." + url + "/?versioning=true HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+ assertPayloadEquals(request, null, null, false);
+
+ assertResponseParserClassEquals(method, request, BucketConfigurationHandler.class);
+ assertSaxResponseParserClassEquals(method, null);
+ assertFallbackClassEquals(method, null);
+
+ checkFilters(request);
+ }
+
+ public void testGetBucketLifeCycles() throws SecurityException, NoSuchMethodException, IOException {
+ Invokable<?, ?> method = method(S3Client.class, "getBucketConfiguration", String.class, BucketConfigOptions[].class);
+ GeneratedHttpRequest request = processor.createRequest(method, ImmutableList.<Object> of("bucket", new BucketConfigOptions().lifecycle()));
+
+ assertRequestLineEquals(request, "GET https://bucket." + url + "/?lifecycle=true HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+ assertPayloadEquals(request, null, null, false);
+
+ assertResponseParserClassEquals(method, request, BucketConfigurationHandler.class);
+ assertSaxResponseParserClassEquals(method, null);
+ assertFallbackClassEquals(method, null);
+
+ checkFilters(request);
+ }
+
+ public void testGetBucketEncryption() throws SecurityException, NoSuchMethodException, IOException {
+ Invokable<?, ?> method = method(S3Client.class, "getBucketConfiguration", String.class, BucketConfigOptions[].class);
+ GeneratedHttpRequest request = processor.createRequest(method, ImmutableList.<Object> of("bucket", new BucketConfigOptions().encryption()));
+
+ assertRequestLineEquals(request, "GET https://bucket." + url + "/?encryption=true HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+ assertPayloadEquals(request, null, null, false);
+
+ assertResponseParserClassEquals(method, request, BucketConfigurationHandler.class);
+ assertSaxResponseParserClassEquals(method, null);
+ assertFallbackClassEquals(method, null);
+
+ checkFilters(request);
+ }
+
+ public void testDeleteBucketConfiguration() throws SecurityException, NoSuchMethodException, IOException {
+ Invokable<?, ?> method = method(S3Client.class, "deleteBucketConfiguration", String.class, BucketConfigOptions[].class);
+ GeneratedHttpRequest request = processor.createRequest(method, ImmutableList.<Object> of("bucket"));
+
+ assertRequestLineEquals(request, "DELETE https://bucket." + url + "/ HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+ assertPayloadEquals(request, null, null, false);
+
+ assertResponseParserClassEquals(method, request, BucketConfigurationHandler.class);
+ assertSaxResponseParserClassEquals(method, null);
+ assertFallbackClassEquals(method, MapHttp4xxCodesToExceptions.class);
+
+ checkFilters(request);
+ }
+
+ public void testPutBucketConfiguration() throws SecurityException, NoSuchMethodException, IOException {
+
+ Invokable<?, ?> method = method(S3Client.class, "putBucketConfiguration", String.class, S3Object.class,
+ BucketConfigOptions[].class);
+ GeneratedHttpRequest request = processor.createRequest(method, ImmutableList.<Object> of("bucket",
+ blobToS3Object.apply(BindBlobToMultipartFormTest.TEST_BLOB), new BucketConfigOptions().lifecycle()));
+
+ assertRequestLineEquals(request, "PUT https://bucket." + url + "/?lifecycle=true HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+ assertPayloadEquals(request, "hello", "text/plain",
+ false);
+
+ assertResponseParserClassEquals(method, request, BucketConfigurationHandler.class);
+ assertSaxResponseParserClassEquals(method, null);
+ assertFallbackClassEquals(method, null);
+
+ checkFilters(request);
Review comment:
Can you add a live test for adding multiple objects with the same name and different versions? These unit tests are good but comparing against live AWS will give me more confidence about this PR.
##########
File path: apis/s3/src/main/java/org/jclouds/s3/options/ListBucketOptions.java
##########
@@ -96,6 +96,16 @@ public String getDelimiter() {
return getFirstQueryOrNull("delimiter");
}
+ public ListBucketOptions versions(Boolean versions) {
Review comment:
Should this be a `boolean` if it cannot be `null`?
##########
File path: blobstore/src/main/java/org/jclouds/blobstore/domain/internal/StorageMetadataImpl.java
##########
@@ -60,6 +64,23 @@ public StorageMetadataImpl(StorageType type, @Nullable String id, @Nullable Stri
this.type = checkNotNull(type, "type");
this.size = size;
this.tier = tier;
+ this.versionId = null;
+ this.isLatest = null;
+ }
+
+ public StorageMetadataImpl(StorageType type, @Nullable String id, @Nullable String name,
+ @Nullable Location location, @Nullable URI uri, @Nullable String eTag,
+ @Nullable Date creationDate, @Nullable Date lastModified,
+ Map<String, String> userMetadata, @Nullable Long size, Tier tier, @Nullable String versionId) {
+ super(id, name, location, uri, userMetadata);
Review comment:
Please make the constructor with all parameters the only one that sets fields and the existing one with fewer parameters can call `this(...)` with a default value for `versionId`.
##########
File path: blobstore/src/main/java/org/jclouds/blobstore/domain/internal/MutableStorageMetadataImpl.java
##########
@@ -38,6 +38,8 @@
private Date lastModified;
private Long size;
private Tier tier;
+ private String versionId;
+ private String isLatest;
Review comment:
Should this be a `boolean`?
##########
File path: apis/s3/src/main/java/org/jclouds/s3/blobstore/S3BlobStore.java
##########
@@ -133,6 +134,18 @@ public boolean containerExists(String container) {
return sync.bucketExists(container);
}
+ public String getBucketConfiguration(String bucketName, BucketConfigOptions httpOptions ) {
+ return sync.getBucketConfiguration(bucketName, httpOptions);
+ }
+
+ public String deleteBucketConfiguration(String bucketName, BucketConfigOptions httpOptions ) {
Review comment:
Extra whitespace here and above.
--
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: notifications-unsubscribe@jclouds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org