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-unsubscr...@jclouds.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
