jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Gaul <notificati...@github.com>
Subject Re: [jclouds/jclouds] DLO - Lower Level Provider API changes (#1105)
Date Sat, 22 Jul 2017 14:42:40 GMT
andrewgaul commented on this pull request.



> +      // 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 mix imperative and functional styles?  You can more clearly write:

```java
for (SwiftObject object : objects) {
   String name = containerName + "/" + input.getName();
   getApi().getObjectApi(regionId, containerName).delete(name);
}
```

> +import org.jclouds.rest.annotations.BinderParam;
+import org.jclouds.rest.annotations.Headers;
+import org.jclouds.rest.annotations.RequestFilters;
+import org.jclouds.rest.annotations.ResponseParser;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * Provides access to the OpenStack Object Storage (Swift) Static Large Object
+ * API features.
+ * <p/>
+ * This API is new to jclouds and hence is in Beta. That means we need people to
+ * use it and give us feedback. Based on that feedback, minor changes to the
+ * interfaces may happen. This code will replace
+ * org.jclouds.openstack.swift.SwiftClient in jclouds 2.0 and it is recommended
+ * you adopt it sooner than later.

Remove useless comment.

> +@Consumes(APPLICATION_JSON)
+@Path("/{objectName}")
+public interface DynamicLargeObjectApi {
+   /**
+    * Creates or updates a dynamic large object's manifest.
+    *
+    * @param objectName
+    *           corresponds to {@link SwiftObject#getName()}.
+    * @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}.

Add `@deprecated` Javadoc pointing users to SLO API.

> +import org.jclouds.openstack.swift.v1.options.ListContainerOptions;
+import org.jclouds.utils.TestUtils;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Charsets;
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.io.ByteSource;
+
+@Test(groups = "live", testName = "DynamicLargeObjectApiLiveTest", singleThreaded = true)
+public class DynamicLargeObjectApiLiveTest extends BaseSwiftApiLiveTest {
+
+   private String defaultName = getClass().getSimpleName();
+   private static final ByteSource megOf1s = TestUtils.randomByteSource().slice(0, 1024 *
1024);;
+   private static final ByteSource megOf2s = TestUtils.randomByteSource().slice(0, 1024 *
1024);;

Extra semicolons.

> +         assertReplaceManifest(regionId, defaultName);
+         uploadLargeFile(regionId);
+      }
+   }
+
+   @SuppressWarnings("deprecation")
+   @Test
+   public void uploadLargeFile(String regionId) throws IOException, InterruptedException
{
+      int total_size = 0;
+      RegionScopedBlobStoreContext ctx = RegionScopedBlobStoreContext.class.cast(view);
+      BlobStore blobStore = ctx.getBlobStore();
+      String defaultContainerName = getContainerName();
+      // configure the blobstore to use multipart uploading of the file
+      for (int partNumber = 0; partNumber < 3; partNumber++) {
+         String objName = String.format("%s/%s/%s", objectName, "dlo", partNumber);
+         String data = String.format("%s%s", "data", partNumber);

Simplify to:

```java
String data = "data" + partNumber;
```

> +   private static final ByteSource megOf1s = TestUtils.randomByteSource().slice(0, 1024
* 1024);;
+   private static final ByteSource megOf2s = TestUtils.randomByteSource().slice(0, 1024 *
1024);;
+   private String objectName = "myObject";
+
+   @Test
+   public void testReplaceManifest() throws Exception {
+      for (String regionId : regions) {
+         assertReplaceManifest(regionId, defaultName);
+         uploadLargeFile(regionId);
+      }
+   }
+
+   @SuppressWarnings("deprecation")
+   @Test
+   public void uploadLargeFile(String regionId) throws IOException, InterruptedException
{
+      int total_size = 0;

Prefer `long totalSize` which will eliminate the `Long.valueOf` call below.

> +
+import org.jclouds.io.Payloads;
+import org.jclouds.openstack.swift.v1.SwiftApi;
+import org.jclouds.openstack.v2_0.internal.BaseOpenStackMockTest;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.net.HttpHeaders;
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.MockWebServer;
+import com.squareup.okhttp.mockwebserver.RecordedRequest;
+
+@Test(groups = "unit", testName = "DynamicLargeObjectApiMockTest")
+public class DynamicLargeObjectApiMockTest extends BaseOpenStackMockTest<SwiftApi>
{
+
+   String containerName = "myContainer";

`private` visibility.

-- 
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-48767678
Mime
View raw message