andrewgaul requested changes on this pull request.

Please address @timuralp comments which suggest a misimplementation of the API 
with the segments parameter.  Will add more comments after you address this set.

> +
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMap.Builder;
+
+/**
+ * Updates Content Metadata.
+ *
+ * Required to update the Content Length of Object File as Sum of Content 
length
+ * of Segments
+ * 
+ * @param contentMetadata
+ * 
+ * @return Map with Content Metadata.
+ */
+
+public class ContentMetaDataToMap {

But if you don't use it anywhere...?

> +    * @param metadata
+    *           corresponds to {@link SwiftObject#getMetadata()}.
+    * @param headers
+    *           Binds the map to headers, without prefixing/escaping the header
+    *           name/key.
+    *
+    * @return {@link SwiftObject#getEtag()} of the object, which is the MD5
+    *         checksum of the concatenated ETag values of the {@code segments}.
+    */
+   
+   @Deprecated
+   @Named("dynamicLargeObject:uploadLargeFile")
+   @PUT
+   @Headers(keys = EXPECT, values = "100-continue")
+   @ResponseParser(ETagHeader.class)
+   String uploadPart(@PathParam("containerName") String container, 
@PathParam("objectName") String objectName,

Agree this seems useless.

This pull request should scope exactly to creating and getting DLO objects 
since the Swift API offers only this.  The portable abstraction will largely 
not interact with it, with the exception of getting legacy DLO objects.

> +      for (int i = partNumber; i < 3; partNumber++) {
+         String objName = String.format("%s/%s/%s", objectName, "dlo", 
partNumber);
+         String data = String.format("%s%s", "data", partNumber);
+         String etag = getApi().getDynamicLargeObjectApi(regionId, 
defaultContainerName).uploadPart(
+               defaultContainerName, objName, Payloads.newPayload(data), 
ImmutableMap.of("myfoo", "Bar"),
+               ImmutableMap.of("myfoo", "Bar"));
+         Segment s = new Segment(objName, etag, data.length());
+         assertNotNull(etag);
+         segmentList.add(s);
+         total_size += data.length();
+      }
+      String etagOfEtags = getApi().getDynamicLargeObjectApi(regionId, 
defaultContainerName).replaceManifest(
+            objectName, segmentList, ImmutableMap.of("MyFoo", "Bar"), 
ImmutableMap.of("MyFoo", "Bar"));
+
+      SwiftObject bigObject = getApi().getObjectApi(regionId, 
defaultContainerName).get(objectName);
+      assertThat(bigObject.getETag().equals(etagOfEtags));

You misused `assertThat` -- all of these checks are no-ops.  You need to write 
`assertThat(...).isEqualTo` or `.isTrue`.

> +   @Override
+   @BeforeClass(groups = "live")
+   public void setup() {
+      super.setup();
+      for (String regionId : regions) {
+         boolean created = 
getApi().getContainerApi(regionId).create(defaultContainerName);
+         if (!created) {
+            deleteAllObjectsInContainer(regionId, defaultContainerName);
+         }
+      }
+
+     /* megOf1s = new byte[1024 * 1024];
+      megOf2s = new byte[1024 * 1024];
+
+      Arrays.fill(megOf1s, (byte) 1);
+      Arrays.fill(megOf2s, (byte) 2);*/

Again, remove commented code!

> +      super.setup();
+      for (String regionId : regions) {
+         boolean created = 
getApi().getContainerApi(regionId).create(defaultContainerName);
+         if (!created) {
+            deleteAllObjectsInContainer(regionId, defaultContainerName);
+         }
+      }
+
+     /* megOf1s = new byte[1024 * 1024];
+      megOf2s = new byte[1024 * 1024];
+
+      Arrays.fill(megOf1s, (byte) 1);
+      Arrays.fill(megOf2s, (byte) 2);*/
+      
+      megOf1s = TestUtils.randomByteSource().slice(0, 1048576);
+      megOf2s = TestUtils.randomByteSource().slice(0, 1048576);

Prefer 1024 * 1024.

> +
+      awaitConsistency();
+
+      SwiftObject bigObject = getApi().getObjectApi(regionId, 
containerName).get(name);
+      assertThat(bigObject.getETag().equals(etagOfEtags));
+      
assertThat(bigObject.getPayload().getContentMetadata().getContentLength().equals(Long.valueOf(2
 * 1024 * 1024)));
+      assertThat(bigObject.getMetadata().equals(ImmutableMap.of("myfoo", 
"Bar")));
+
+      // segments are visible
+      
assertThat(getApi().getContainerApi(regionId).get(containerName).getObjectCount().equals(Long.valueOf(3)));
+   }
+
+   protected void assertMegabyteAndETagMatches(String regionId, String 
containerName, String name, String etag1s) {
+      SwiftObject object1s = getApi().getObjectApi(regionId, 
containerName).get(name);
+      assertThat(object1s.getETag().equals(etag1s));
+      
assertThat(object1s.getPayload().getContentMetadata().getContentLength().equals(Long.valueOf(1024
 * 1024)));

Prefer 1024L * 1024L over `valueOf`.

> +   public void setup() {
+      super.setup();
+      for (String regionId : regions) {
+         boolean created = 
getApi().getContainerApi(regionId).create(defaultContainerName);
+         if (!created) {
+            deleteAllObjectsInContainer(regionId, defaultContainerName);
+         }
+      }
+
+     /* megOf1s = new byte[1024 * 1024];
+      megOf2s = new byte[1024 * 1024];
+
+      Arrays.fill(megOf1s, (byte) 1);
+      Arrays.fill(megOf2s, (byte) 2);*/
+      
+      megOf1s = TestUtils.randomByteSource().slice(0, 1048576);

Move initialization of these constants to their declarations and mark `static 
final`.

> +      SwiftObject object1s = getApi().getObjectApi(regionId, 
> containerName).get(name);
+      assertThat(object1s.getETag().equals(etag1s));
+      
assertThat(object1s.getPayload().getContentMetadata().getContentLength().equals(Long.valueOf(1024
 * 1024)));
+   }
+
+   protected void deleteAllObjectsInContainerDLO(String regionId, final String 
containerName) {
+       ObjectList objects = getApi().getObjectApi(regionId, 
containerName).list(new ListContainerOptions());
+      if (objects == null) {
+         return;
+      }
+      List<String> pathsToDelete = Lists.transform(objects, new 
Function<SwiftObject, String>() {
+         public String apply(SwiftObject input) {
+            return containerName + "/" + input.getName();
+         }
+      });
+      if (!pathsToDelete.isEmpty()) {

Remove useless conditional.

> +      // segments are visible
+      
assertThat(getApi().getContainerApi(regionId).get(containerName).getObjectCount().equals(Long.valueOf(3)));
+   }
+
+   protected void assertMegabyteAndETagMatches(String regionId, String 
containerName, String name, String etag1s) {
+      SwiftObject object1s = getApi().getObjectApi(regionId, 
containerName).get(name);
+      assertThat(object1s.getETag().equals(etag1s));
+      
assertThat(object1s.getPayload().getContentMetadata().getContentLength().equals(Long.valueOf(1024
 * 1024)));
+   }
+
+   protected void deleteAllObjectsInContainerDLO(String regionId, final String 
containerName) {
+       ObjectList objects = getApi().getObjectApi(regionId, 
containerName).list(new ListContainerOptions());
+      if (objects == null) {
+         return;
+      }
+      List<String> pathsToDelete = Lists.transform(objects, new 
Function<SwiftObject, String>() {

Why do you need this `Function` if you loop over the paths below?

-- 
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/1105#pullrequestreview-40140287

Reply via email to