andrewgaul requested changes on this pull request.
> @@ -266,7 +266,8 @@ public StorageMetadata apply(String key) {
if (options != null) {
if (options.getDir() != null && !options.getDir().isEmpty()) {
contents = filterDirectory(contents, options);
- } else if (options.getPrefix() != null) {
+ } else if (options.getPrefix() != null &&
`Strings.isNullOrEmpty`
> @@ -688,4 +691,26 @@ public void testUpdateObjectCannedACL() throws Exception
> {
returnContainer(containerName);
}
}
+
+ public void testList_EmptyOptionSingleContainer() throws Exception {
+ String containerName = getContainerName();
+ try {
+ S3Object object = getApi().newS3Object();
+ object.getMetadata().setKey("a");
+ object.setPayload(TEST_STRING);
+ getApi().putObject(containerName, object);
+ // Test listing where we set the prefix and delimiter to empty string
+ ListBucketResponse rs = getApi().listBucket(containerName,
+
ListBucketOptions.Builder.delimiter("").withPrefix("").afterMarker(""));
+ assertEquals(rs.size(), 3);
+ Set<String> expected = Sets.newHashSet("a");
This s3 test is similar to the filesystem test -- should you add one test to
`BaseContainerIntegrationTest` instead of duplicating code?
> @@ -182,6 +182,25 @@ public void testList_NoOptionSingleContainer() throws
> IOException {
checkForContainerContent(CONTAINER_NAME, blobsExpected);
}
+ public void testList_EmptyOptionSingleContainer() throws IOException {
+ blobStore.createContainerInLocation(null, CONTAINER_NAME);
+
+ checkForContainerContent(CONTAINER_NAME, null);
+
+ TestUtils.createBlobsInContainer(CONTAINER_NAME, "a", "b", "c");
+ // Test listing where we set the prefix and delimiter to empty string
Make this a method javadoc instead?
> @@ -182,6 +182,25 @@ public void testList_NoOptionSingleContainer() throws
> IOException {
checkForContainerContent(CONTAINER_NAME, blobsExpected);
}
+ public void testList_EmptyOptionSingleContainer() throws IOException {
+ blobStore.createContainerInLocation(null, CONTAINER_NAME);
+
+ checkForContainerContent(CONTAINER_NAME, null);
+
+ TestUtils.createBlobsInContainer(CONTAINER_NAME, "a", "b", "c");
+ // Test listing where we set the prefix and delimiter to empty string
+ ListContainerOptions options =
ListContainerOptions.Builder.delimiter("")
+ .prefix("").afterMarker("");
+ PageSet<? extends StorageMetadata> rs = blobStore.list(CONTAINER_NAME,
options);
+ assertEquals(rs.size(), 3);
+ Set<String> expected = Sets.newHashSet("a", "b", "c");
+ for (StorageMetadata sm : rs) {
+ assertTrue(expected.contains(sm.getName()));
+ expected.remove(sm.getName());
Iterating while removing requires use of `Iterator.remove` to avoid
invalidating the iterator. Instead could you use assertj:
```java
ImmutableList.Builder<String> builder = ImmutableList.builder();
for (StorageMetadata sm : rs) {
builder.add(sm.getName());
}
assertThat(builder.build()).containsExactly(expected);
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1121#pullrequestreview-53853763
