timuralp commented on this pull request.
I think this is getting better. Please addres the outstanding comments.
> @@ -37,7 +37,7 @@
@Named("size_bytes")
private final long sizeBytes;
- private Segment(String path, String etag, long sizeBytes) {
+ public Segment(String path, String etag, long sizeBytes) {
Is this change still necessary? Is it only used in tests?
> +@RequestFilters(AuthenticateRequest.class)
+@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
As a result of a PUT of a 0-sized object with X-Object-Manifest header set, I'm
observing that Swift returns an ETag of the 0-sized object. Are you observing a
different behavior in your tests?
> + @PUT
+ @ResponseParser(ETagHeader.class)
+ @Headers(keys = "X-Object-Manifest", values =
"{containerName}/{objectName}/")
+ String putManifest(@PathParam("objectName") String objectName,
+ @BinderParam(BindObjectMetadataToHeaders.class) Map<String, String>
metadata,
+ @BinderParam(BindToHeaders.class) Map<String, String> headers);
+
+ /**
+ * Creates or updates a dynamic large object's manifest.
+ *
+ * @param objectName
+ * corresponds to {@link SwiftObject#getName()}.
+ * @param metadata
+ * corresponds to {@link SwiftObject#getMetadata()}.
+ *
+ * @return {@link SwiftObject#getEtag()} of the object, which is the MD5
Same comment as above about the ETag applies here.
> + * @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,
@archupsg03 can you address the questions above? I'm still unclear on why this
method is required. The point is that callers can and should call `putObject()`
to upload new or replace existing parts of the DLO. Could you explain how you
expect this method to be used and why it would be preferred to calling
`putObject()`?
> + 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, defaultContainerName, defaultName);
+ uploadLargeFile(regionId);
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void uploadLargeFile(String regionId) throws IOException,
InterruptedException {
+ List<Segment> segmentList = new ArrayList<Segment>();
This is no longer used in the test. Can we remove this code entirely? On the
other hand, it would be useful to upload some blobs and verify that the ETag is
returned as expected from a HEAD.
> +
> assertThat(getApi().getContainerApi(regionId).get(defaultContainerName).getObjectCount()).isEqualTo(Long.valueOf(3));
+ }
+
+ @SuppressWarnings("deprecation")
+ protected void assertReplaceManifest(String regionId, String containerName,
String name) {
+ ObjectApi objectApi = getApi().getObjectApi(regionId, containerName);
+
+ String etag1s = objectApi.put(name + "/1",
newByteSourcePayload(megOf1s));
+ awaitConsistency();
+ assertMegabyteAndETagMatches(regionId, containerName, name + "/1",
etag1s);
+
+ String etag2s = objectApi.put(name + "/2",
newByteSourcePayload(megOf2s));
+ awaitConsistency();
+ assertMegabyteAndETagMatches(regionId, containerName, name + "/2",
etag2s);
+
+ List<Segment> segments = ImmutableList.<Segment> builder()
I don't think these segments are ever uploaded?
> + 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()) {
If `pathsToDelete` is empty, the for loop would be a NOOP.
--
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-42512966
