andrewgaul requested changes on this pull request.
@archupsg03 I have more comments but let's address the first set. As you make
changes, please reply "Done" to each individual comment.
@timuralp please review `DynamicLargeObjectApi`.
> + @BinderParam(SetPayload.class) Payload blob,
+ @BinderParam(BindObjectMetadataToHeaders.class) Map<String, String>
metadata,
+ @BinderParam(BindToHeaders.class) Map<String, String> headers);
+
+ /**
+ * Delete objects.
+ *
+ * @param objectName
+ * corresponds to {@link SwiftObject#getName()}.
+ */
+
+ @Deprecated
+ @Named("dynamicLargeObject: removeLargeFile")
+ @DELETE
+ @Fallback(VoidOnNotFoundOr404.class)
+ void removeLargeFile(@PathParam("containerName") String container,
@PathParam("objectName") String name);
What does this do differently than `ObjectApi.delete`?
> + * ordered parts which will be concatenated upon download.
+ * @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:replaceManifest")
+ @PUT
+ @ResponseParser(ETagHeader.class)
+ @Headers(keys = "X-Object-Manifest", values =
"{containerName}/{objectName}/")
+ String replaceManifest(@PathParam("containerName") String containerName,
@PathParam("objectName") String objectName,
Why do these methods take a `containerName` parameter but equivalent
`StaticLargeObjectApi` methods do not?
> + @Test
+ public void testReplaceManifest() throws Exception {
+ for (String regionId : regions) {
+ assertReplaceManifest(regionId, defaultContainerName, defaultName);
+ uploadLargeFile(regionId);
+ removeLargeFile(regionId);
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void uploadLargeFile(String regionId) throws IOException,
InterruptedException {
+ /*
+ * createRandomFile(SIZE, bigFile); Payload payLoad = new
+ * FilePayload(bigFile);
+ */
Remove commented-out code.
> + * @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:replaceManifest")
+ @PUT
+ @ResponseParser(ETagHeader.class)
+ @Headers(keys = "X-Object-Manifest", values =
"{containerName}/{objectName}/")
+ String replaceManifest(@PathParam("containerName") String containerName,
@PathParam("objectName") String objectName,
+ @BinderParam(BindToJsonPayload.class) List<Segment> segments,
Could `segments` be a `Collection`?
> +
+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 {
What does this do and where is it used?
> +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 {
+
+ public ImmutableMap<String, String>
getContentMetadataForManifest(ContentMetadata contentMetadata) {
Return `Map`.
> + * @param objectName
+ * corresponds to {@link SwiftObject#getName()}.
+ * @param segments
+ * ordered parts which will be concatenated upon download.
+ * @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")
Remove space after colon (repeated below).
> + * @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 uploadLargeFile(@PathParam("containerName") String container,
@PathParam("objectName") String objectName,
I don't understand this API. Shouldn't callers upload single-part objects of
the form, "obj1", "obj2", then call a method which creates the DLO with the
name "obj"?
> + getApi().getObjectApi(regionId, containerName).delete(name);
+ }
+ }
+
+ @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];
Use `TestUtils.randomByteSource`.
> + assertEquals(bigObject.getETag(), etagOfEtags);
+
assertEquals(bigObject.getPayload().getContentMetadata().getContentLength(),
Long.valueOf(2 * 1024 * 1024));
+ assertEquals(bigObject.getMetadata(), ImmutableMap.of("myfoo", "Bar"));
+
+ // segments are visible
+
assertEquals(getApi().getContainerApi(regionId).get(containerName).getObjectCount(),
Long.valueOf(3));
+ }
+
+ protected void assertMegabyteAndETagMatches(String regionId, String
containerName, String name, String etag1s) {
+ SwiftObject object1s = getApi().getObjectApi(regionId,
containerName).get(name);
+ assertEquals(object1s.getETag(), etag1s);
+
assertEquals(object1s.getPayload().getContentMetadata().getContentLength(),
Long.valueOf(1024 * 1024));
+ }
+
+ protected void deleteAllObjectsInContainerDLO(String regionId, final String
containerName) {
+ Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
Remove sleep.
> +
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.io.ByteSource;
+import com.google.common.util.concurrent.Uninterruptibles;
+
+@Test(groups = "live", testName = "DynamicLargeObjectApiLiveTest",
singleThreaded = true)
+public class DynamicLargeObjectApiLiveTest extends BaseSwiftApiLiveTest {
+
+ private String defaultName = getClass().getSimpleName();
+ private String defaultContainerName = getClass().getSimpleName() +
"Container";
+ private byte[] megOf1s;
+ private byte[] megOf2s;
+ String objectName = "myObject";
Add `private` access.
> + @SuppressWarnings("deprecation")
+ @Test
+ public void uploadLargeFile(String regionId) throws IOException,
InterruptedException {
+ /*
+ * createRandomFile(SIZE, bigFile); Payload payLoad = new
+ * FilePayload(bigFile);
+ */
+ List<Segment> list1 = new ArrayList<Segment>();
+ int partNumber = 1;
+ int total_size = 0;
+ // configure the blobstore to use multipart uploading of the file
+ /*
+ * payLoad.getContentMetadata().setContentLength(bigFile.length());
+ * BasePayloadSlicer slicer = new BasePayloadSlicer();
+ */
+ while (partNumber < 3) {
Prefer `for` loop.
> + @Test
+ public void uploadLargeFile(String regionId) throws IOException,
InterruptedException {
+ /*
+ * createRandomFile(SIZE, bigFile); Payload payLoad = new
+ * FilePayload(bigFile);
+ */
+ List<Segment> list1 = new ArrayList<Segment>();
+ int partNumber = 1;
+ int total_size = 0;
+ // configure the blobstore to use multipart uploading of the file
+ /*
+ * payLoad.getContentMetadata().setContentLength(bigFile.length());
+ * BasePayloadSlicer slicer = new BasePayloadSlicer();
+ */
+ while (partNumber < 3) {
+ String objName = objectName + "/dlo/" + String.valueOf(partNumber);
Remove call to `String.valueOf`.
> + // configure the blobstore to use multipart uploading of the file
+ /*
+ * payLoad.getContentMetadata().setContentLength(bigFile.length());
+ * BasePayloadSlicer slicer = new BasePayloadSlicer();
+ */
+ while (partNumber < 3) {
+ String objName = objectName + "/dlo/" + String.valueOf(partNumber);
+ String data = "data".concat(String.valueOf(partNumber));
+ String etag = getApi().getDynamicLargeObjectApi(regionId,
defaultContainerName).uploadLargeFile(
+ defaultContainerName, objName, Payloads.newPayload(data),
ImmutableMap.of("myfoo", "Bar"),
+ ImmutableMap.of("myfoo", "Bar"));
+ Segment s = new Segment(objName, etag, data.length());
+ assertNotNull(etag);
+ list1.add(s);
+ partNumber++;
+ total_size = total_size + data.length();
Prefer `+=`.
> + String objName = objectName + "/dlo/" + String.valueOf(partNumber);
+ String data = "data".concat(String.valueOf(partNumber));
+ String etag = getApi().getDynamicLargeObjectApi(regionId,
defaultContainerName).uploadLargeFile(
+ defaultContainerName, objName, Payloads.newPayload(data),
ImmutableMap.of("myfoo", "Bar"),
+ ImmutableMap.of("myfoo", "Bar"));
+ Segment s = new Segment(objName, etag, data.length());
+ assertNotNull(etag);
+ list1.add(s);
+ partNumber++;
+ total_size = total_size + data.length();
+ }
+ String etagOfEtags = getApi().getDynamicLargeObjectApi(regionId,
defaultContainerName).replaceManifest(
+ defaultContainerName, objectName, list1, ImmutableMap.of("MyFoo",
"Bar"), ImmutableMap.of("MyFoo", "Bar"));
+
+ SwiftObject bigObject = getApi().getObjectApi(regionId,
defaultContainerName).get(objectName);
+ assertEquals(bigObject.getETag(), etagOfEtags);
Prefer `assertThat`. Repeated throughout.
--
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-39905447
