timuralp commented on this pull request.
Thanks for the review @andrewgaul! I reworked the tests to avoid code
duplication -- thanks for the suggestions.
> @@ -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
Done
> @@ -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());
I'm not mutating the iterated container, though -- `rs` is the result set.
Regardless, will rework to the suggested approach.
> @@ -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);
Sorry about that -- the test surely fails and I must have failed to run it and
didn't catch this.
> @@ -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");
Yes -- will combine these. I wasn't sure about other providers' behavior, but I
suspect it should either match or ignore the test.
> @@ -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 &&
Done
--
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-53887958
