Repository: jclouds Updated Branches: refs/heads/master 04ab255d9 -> d41101df5
JCLOUDS-1264: Swift Unicode multipart manifests This fixes a bug where previously BindManifestToJsonPayload used the character length as the ContentLength, instead of the byte length, which caused issues if the JSON contained multi-byte Unicode characters. Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/d41101df Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/d41101df Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/d41101df Branch: refs/heads/master Commit: d41101df5932043c3a8614552f25ab91194595ba Parents: 04ab255 Author: Mat Mannion <[email protected]> Authored: Mon Apr 3 12:19:21 2017 +0100 Committer: Andrew Gaul <[email protected]> Committed: Tue Apr 4 05:05:53 2017 -0700 ---------------------------------------------------------------------- .../v1/binders/BindManifestToJsonPayload.java | 65 --------- .../blobstore/RegionScopedSwiftBlobStore.java | 2 +- .../swift/v1/features/StaticLargeObjectApi.java | 3 +- .../features/StaticLargeObjectApiLiveTest.java | 133 ++++++++++++------- .../features/StaticLargeObjectApiMockTest.java | 48 +++++++ 5 files changed, 132 insertions(+), 119 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/d41101df/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindManifestToJsonPayload.java ---------------------------------------------------------------------- diff --git a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindManifestToJsonPayload.java b/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindManifestToJsonPayload.java deleted file mode 100644 index 76ad60f..0000000 --- a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindManifestToJsonPayload.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.jclouds.openstack.swift.v1.binders; - -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.Map; - -import javax.inject.Inject; - -import org.jclouds.http.HttpRequest; -import org.jclouds.json.Json; -import org.jclouds.rest.MapBinder; - -/** - * Binds the object to the request as a json object. - */ -public class BindManifestToJsonPayload implements MapBinder { - - protected final Json jsonBinder; - - @Inject - BindManifestToJsonPayload(Json jsonBinder) { - this.jsonBinder = jsonBinder; - } - - @Override - public <R extends HttpRequest> R bindToRequest(R request, Map<String, Object> postParams) { - return bindToRequest(request, (Object) postParams); - } - - @Override - public <R extends HttpRequest> R bindToRequest(R request, Object payload) { - String json = jsonBinder.toJson(checkNotNull(payload, "payload")); - request.setPayload(json); - /** - * The Content-Length request header must contain the length of the JSON content, not the length of the segment - * objects. However, after the PUT operation is complete, the Content-Length metadata is set to the total length - * of all the object segments. A similar situation applies to the ETag header. If it is used in the PUT - * operation, it must contain the MD5 checksum of the JSON content. The ETag metadata value is then set to be - * the MD5 checksum of the concatenated ETag values of the object segments. You can also set the Content-Type - * request header and custom object metadata. - * When the PUT operation sees the ?multipart-manifest=put query string, it reads the request body and verifies - * that each segment object exists and that the sizes and ETags match. If there is a mismatch, the PUT operation - * fails. - */ - request.getPayload().getContentMetadata().setContentLength((long)json.length()); - return request; - } - -} http://git-wip-us.apache.org/repos/asf/jclouds/blob/d41101df/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java ---------------------------------------------------------------------- diff --git a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java b/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java index c8b52ab..bd77e2b 100644 --- a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java +++ b/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java @@ -494,7 +494,7 @@ public class RegionScopedSwiftBlobStore implements BlobStore { mapBuilder.put("content-type", contentMetadata.getContentType()); } /** - * Do not set content-length. Set automatically to manifest json string length by BindManifestToJsonPayload + * Do not set content-length. Set automatically to manifest json string length by BindToJsonPayload */ if (contentMetadata.getContentDisposition() != null) { mapBuilder.put("content-disposition", contentMetadata.getContentDisposition()); http://git-wip-us.apache.org/repos/asf/jclouds/blob/d41101df/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java ---------------------------------------------------------------------- diff --git a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java b/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java index 16a5e1e..d5d490d 100644 --- a/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java +++ b/apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java @@ -32,7 +32,6 @@ import javax.ws.rs.PathParam; import org.jclouds.Fallbacks.EmptyListOnNotFoundOr404; import org.jclouds.Fallbacks.VoidOnNotFoundOr404; import org.jclouds.openstack.keystone.v2_0.filters.AuthenticateRequest; -import org.jclouds.openstack.swift.v1.binders.BindManifestToJsonPayload; import org.jclouds.openstack.swift.v1.binders.BindMetadataToHeaders.BindObjectMetadataToHeaders; import org.jclouds.openstack.swift.v1.binders.BindToHeaders; import org.jclouds.openstack.swift.v1.domain.DeleteStaticLargeObjectResponse; @@ -101,7 +100,7 @@ public interface StaticLargeObjectApi { @ResponseParser(ETagHeader.class) @QueryParams(keys = "multipart-manifest", values = "put") String replaceManifest(@PathParam("objectName") String objectName, - @BinderParam(BindManifestToJsonPayload.class) List<Segment> segments, + @BinderParam(BindToJsonPayload.class) List<Segment> segments, @BinderParam(BindObjectMetadataToHeaders.class) Map<String, String> metadata, @BinderParam(BindToHeaders.class) Map<String, String> headers); http://git-wip-us.apache.org/repos/asf/jclouds/blob/d41101df/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java ---------------------------------------------------------------------- diff --git a/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java b/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java index 5aebe13..ae1dbd4 100644 --- a/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java +++ b/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java @@ -41,14 +41,16 @@ import com.google.common.io.ByteSource; @Test(groups = "live", testName = "StaticLargeObjectApiLiveTest", singleThreaded = true) public class StaticLargeObjectApiLiveTest extends BaseSwiftApiLiveTest { - private String name = getClass().getSimpleName(); - private String containerName = getClass().getSimpleName() + "Container"; + private String defaultName = getClass().getSimpleName(); + private String defaultContainerName = getClass().getSimpleName() + "Container"; + private String unicodeName = getClass().getSimpleName() + "unicâªde"; + private String unicodeContainerName = getClass().getSimpleName() + "unicâªdeContainer"; private byte[] megOf1s; private byte[] megOf2s; public void testNotPresentWhenDeleting() throws Exception { for (String regionId : regions) { - DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, containerName).delete(UUID.randomUUID().toString()); + DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, defaultContainerName).delete(UUID.randomUUID().toString()); assertThat(resp.status()).isEqualTo("200 OK"); assertThat(resp.deleted()).isZero(); assertThat(resp.notFound()).isEqualTo(1); @@ -59,69 +61,91 @@ public class StaticLargeObjectApiLiveTest extends BaseSwiftApiLiveTest { @Test public void testReplaceManifest() throws Exception { for (String regionId : regions) { - ObjectApi objectApi = getApi().getObjectApi(regionId, containerName); - - String etag1s = objectApi.put(name + "/1", newByteSourcePayload(ByteSource.wrap(megOf1s))); - awaitConsistency(); - assertMegabyteAndETagMatches(regionId, name + "/1", etag1s); - - String etag2s = objectApi.put(name + "/2", newByteSourcePayload(ByteSource.wrap(megOf2s))); - awaitConsistency(); - assertMegabyteAndETagMatches(regionId, name + "/2", etag2s); - - List<Segment> segments = ImmutableList.<Segment> builder() - .add(Segment.builder() - .path(format("%s/%s/1", containerName, name)).etag(etag1s).sizeBytes(1024 * 1024) - .build()) - .add(Segment.builder() - .path(format("%s/%s/2", containerName, name)).etag(etag2s).sizeBytes(1024 * 1024) - .build()) - .build(); - - awaitConsistency(); - String etagOfEtags = getApi().getStaticLargeObjectApi(regionId, containerName).replaceManifest( - name, segments, ImmutableMap.of("myfoo", "Bar")); - - assertNotNull(etagOfEtags); - - awaitConsistency(); - - SwiftObject bigObject = getApi().getObjectApi(regionId, containerName).get(name); - 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)); + assertReplaceManifest(regionId, defaultContainerName, defaultName); } } @Test(dependsOnMethods = "testReplaceManifest") public void testDelete() throws Exception { for (String regionId : regions) { - DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, containerName).delete(name); - assertThat(resp.status()).isEqualTo("200 OK"); - assertThat(resp.deleted()).isEqualTo(3); - assertThat(resp.notFound()).isZero(); - assertThat(resp.errors()).isEmpty(); - assertEquals(getApi().getContainerApi(regionId).get(containerName).getObjectCount(), Long.valueOf(0)); + assertDelete(regionId, defaultContainerName, defaultName); + } + } + + @Test + public void testReplaceManifestUnicode() throws Exception { + for (String regionId : regions) { + assertReplaceManifest(regionId, unicodeContainerName, unicodeName); + } + } + + @Test(dependsOnMethods = "testReplaceManifestUnicode") + public void testDeleteUnicode() throws Exception { + for (String regionId : regions) { + assertDelete(regionId, unicodeContainerName, unicodeName); } } public void testDeleteSinglePartObjectWithMultiPartDelete() throws Exception { String objectName = "testDeleteSinglePartObjectWithMultiPartDelete"; for (String regionId : regions) { - getApi().getObjectApi(regionId, containerName).put(objectName, newByteSourcePayload(ByteSource.wrap("swifty".getBytes()))); - DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, containerName).delete(objectName); + getApi().getObjectApi(regionId, defaultContainerName).put(objectName, newByteSourcePayload(ByteSource.wrap("swifty".getBytes()))); + DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, defaultContainerName).delete(objectName); assertThat(resp.status()).isEqualTo("400 Bad Request"); assertThat(resp.deleted()).isZero(); assertThat(resp.notFound()).isZero(); assertThat(resp.errors()).hasSize(1); - getApi().getObjectApi(regionId, containerName).delete(objectName); + getApi().getObjectApi(regionId, defaultContainerName).delete(objectName); } } - protected void assertMegabyteAndETagMatches(String regionId, String name, String etag1s) { + protected void assertReplaceManifest(String regionId, String containerName, String name) { + ObjectApi objectApi = getApi().getObjectApi(regionId, containerName); + + String etag1s = objectApi.put(name + "/1", newByteSourcePayload(ByteSource.wrap(megOf1s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/1", etag1s); + + String etag2s = objectApi.put(name + "/2", newByteSourcePayload(ByteSource.wrap(megOf2s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/2", etag2s); + + List<Segment> segments = ImmutableList.<Segment> builder() + .add(Segment.builder() + .path(format("%s/%s/1", containerName, name)).etag(etag1s).sizeBytes(1024 * 1024) + .build()) + .add(Segment.builder() + .path(format("%s/%s/2", containerName, name)).etag(etag2s).sizeBytes(1024 * 1024) + .build()) + .build(); + + awaitConsistency(); + String etagOfEtags = getApi().getStaticLargeObjectApi(regionId, containerName).replaceManifest( + name, segments, ImmutableMap.of("myfoo", "Bar")); + + assertNotNull(etagOfEtags); + + awaitConsistency(); + + SwiftObject bigObject = getApi().getObjectApi(regionId, containerName).get(name); + 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 assertDelete(String regionId, String containerName, String name) { + DeleteStaticLargeObjectResponse resp = getApi().getStaticLargeObjectApi(regionId, containerName).delete(name); + assertThat(resp.status()).isEqualTo("200 OK"); + assertThat(resp.deleted()).isEqualTo(3); + assertThat(resp.notFound()).isZero(); + assertThat(resp.errors()).isEmpty(); + assertEquals(getApi().getContainerApi(regionId).get(containerName).getObjectCount(), Long.valueOf(0)); + } + + 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)); @@ -132,9 +156,14 @@ public class StaticLargeObjectApiLiveTest extends BaseSwiftApiLiveTest { public void setup() { super.setup(); for (String regionId : regions) { - boolean created = getApi().getContainerApi(regionId).create(containerName); + boolean created = getApi().getContainerApi(regionId).create(defaultContainerName); + if (!created) { + deleteAllObjectsInContainer(regionId, defaultContainerName); + } + + created = getApi().getContainerApi(regionId).create(unicodeContainerName); if (!created) { - deleteAllObjectsInContainer(regionId, containerName); + deleteAllObjectsInContainer(regionId, unicodeContainerName); } } @@ -148,8 +177,10 @@ public class StaticLargeObjectApiLiveTest extends BaseSwiftApiLiveTest { @AfterClass(groups = "live") public void tearDown() { for (String regionId : regions) { - deleteAllObjectsInContainer(regionId, containerName); - getApi().getContainerApi(regionId).deleteIfEmpty(containerName); + deleteAllObjectsInContainer(regionId, defaultContainerName); + getApi().getContainerApi(regionId).deleteIfEmpty(defaultContainerName); + deleteAllObjectsInContainer(regionId, unicodeContainerName); + getApi().getContainerApi(regionId).deleteIfEmpty(unicodeContainerName); } } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/d41101df/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java ---------------------------------------------------------------------- diff --git a/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java b/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java index 708875e..5389023 100644 --- a/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java +++ b/apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java @@ -19,9 +19,11 @@ package org.jclouds.openstack.swift.v1.features; import static org.assertj.core.api.Assertions.assertThat; import static org.jclouds.openstack.swift.v1.reference.SwiftHeaders.OBJECT_METADATA_PREFIX; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotEquals; import java.util.List; +import com.google.common.base.Charsets; import org.jclouds.openstack.swift.v1.SwiftApi; import org.jclouds.openstack.swift.v1.domain.DeleteStaticLargeObjectResponse; import org.jclouds.openstack.swift.v1.domain.Segment; @@ -74,6 +76,52 @@ public class StaticLargeObjectApiMockTest extends BaseOpenStackMockTest<SwiftApi } } + public void testReplaceManifestUnicodeUTF8() throws Exception { + MockWebServer server = mockOpenStackServer(); + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json")))); + server.enqueue(addCommonHeaders(new MockResponse().addHeader(HttpHeaders.ETAG, "\"abcd\""))); + + try { + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift"); + assertEquals( + api.getStaticLargeObjectApi("DFW", "myContainer").replaceManifest( + "unicâªde", + ImmutableList + .<Segment> builder() + .add(Segment.builder().path("/mycontainer/unicâªde/slo/1").etag("0228c7926b8b642dfb29554cd1f00963") + .sizeBytes(1468006).build()) + .add(Segment.builder().path("/mycontainer/unicâªde/slo/2") + .etag("5bfc9ea51a00b790717eeb934fb77b9b").sizeBytes(1572864).build()) + .add(Segment.builder().path("/mycontainer/unicâªde/slo/3") + .etag("b9c3da507d2557c1ddc51f27c54bae51").sizeBytes(256).build()).build(), + ImmutableMap.of("MyFoo", "Bar")), "abcd"); + + assertEquals(server.getRequestCount(), 2); + assertAuthentication(server); + + RecordedRequest replaceRequest = server.takeRequest(); + assertRequest(replaceRequest, "PUT", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer/unic%E2%82%AAde?multipart-manifest=put"); + assertEquals(replaceRequest.getHeader(OBJECT_METADATA_PREFIX + "myfoo"), "Bar"); + + String expectedManifest = + "[{\"path\":\"/mycontainer/unicâªde/slo/1\",\"etag\":\"0228c7926b8b642dfb29554cd1f00963\",\"size_bytes\":1468006}," + + "{\"path\":\"/mycontainer/unicâªde/slo/2\",\"etag\":\"5bfc9ea51a00b790717eeb934fb77b9b\",\"size_bytes\":1572864}," + + "{\"path\":\"/mycontainer/unicâªde/slo/3\",\"etag\":\"b9c3da507d2557c1ddc51f27c54bae51\",\"size_bytes\":256}]"; + + long characterLength = expectedManifest.length(); + long byteLength = expectedManifest.getBytes(Charsets.UTF_8).length; + + assertNotEquals(characterLength, byteLength); + assertEquals(replaceRequest.getHeader("content-length"), Long.toString(byteLength)); + + assertEquals( + new String(replaceRequest.getBody()), + expectedManifest); + } finally { + server.shutdown(); + } + } + public void testReplaceManifestWithHeaders() throws Exception { MockWebServer server = mockOpenStackServer(); server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
