Remove BlobBuilder and Payloads.calculateMD5

Callers should instead explicitly set contentMD5, usually with the
results from Guava Hashing.md5(). This narrows the API and removes a
strange IOException from callers.  Further it removes a dangerous
rebuffering of arbitrarily-large non-repeatable Payloads.
This commit is contained in:
Andrew Gaul 2014-05-26 20:37:56 -07:00
parent c6d4e6645c
commit 186f052022
15 changed files with 84 additions and 116 deletions

View File

@ -16,6 +16,7 @@
*/
package org.jclouds.atmos;
import static com.google.common.base.Charsets.UTF_8;
import static com.google.common.base.Preconditions.checkState;
import static org.jclouds.util.Predicates2.retry;
import static org.testng.Assert.assertEquals;
@ -49,6 +50,8 @@ import com.google.common.base.Predicate;
import com.google.common.base.Throwables;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing;
/**
* Tests behavior of {@code AtmosClient}
@ -147,9 +150,11 @@ public class AtmosClientLiveTest extends BaseBlobStoreIntegrationTest {
@Test(timeOut = 5 * 60 * 1000, dependsOnMethods = { "testCreateDirectory" })
public void testListOptions() throws Exception {
createOrReplaceObject("object2", "here is my data!", "meta-value1");
createOrReplaceObject("object3", "here is my data!", "meta-value1");
createOrReplaceObject("object4", "here is my data!", "meta-value1");
String data = "here is my data!";
HashCode hashCode = Hashing.md5().hashString(data, UTF_8);
createOrReplaceObject("object2", data, hashCode, "meta-value1");
createOrReplaceObject("object3", data, hashCode, "meta-value1");
createOrReplaceObject("object4", data, hashCode, "meta-value1");
BoundedSet<? extends DirectoryEntry> r2 = getApi().listDirectory(privateDirectory, ListOptions.Builder.limit(1));
assertEquals(r2.size(), 1);
assert r2.getToken() != null;
@ -164,13 +169,15 @@ public class AtmosClientLiveTest extends BaseBlobStoreIntegrationTest {
public void testFileOperations() throws Exception {
// create the object
System.err.printf("creating%n");
createOrReplaceObject("object", "here is my data!", "meta-value1");
String data1 = "here is my data!";
createOrReplaceObject("object", data1, Hashing.md5().hashString(data1, UTF_8), "meta-value1");
assertEventuallyObjectMatches("object", "here is my data!", "meta-value1");
assertEventuallyHeadMatches("object", "meta-value1");
// try overwriting the object
System.err.printf("overwriting%n");
createOrReplaceObject("object", "here is my data?", "meta-value?");
String data2 = "here is my data?";
createOrReplaceObject("object", data2, Hashing.md5().hashString(data2, UTF_8), "meta-value?");
assertEventuallyObjectMatches("object", "here is my data?", "meta-value?");
// loop to gather metrics
@ -199,7 +206,7 @@ public class AtmosClientLiveTest extends BaseBlobStoreIntegrationTest {
}
private void createOrUpdateWithErrorLoop(boolean stream, String data, String metadataValue) throws Exception {
createOrReplaceObject("object", makeData(data, stream), metadataValue);
createOrReplaceObject("object", makeData(data, stream), Hashing.md5().hashString(data, UTF_8), metadataValue);
assertEventuallyObjectMatches("object", data, metadataValue);
}
@ -207,13 +214,13 @@ public class AtmosClientLiveTest extends BaseBlobStoreIntegrationTest {
return stream ? Strings2.toInputStream(in) : in;
}
private void createOrReplaceObject(String name, Object data, String metadataValue) throws Exception {
private void createOrReplaceObject(String name, Object data, HashCode hashCode, String metadataValue) throws Exception {
// Test PUT with string data, ETag hash, and a piece of metadata
AtmosObject object = getApi().newObject();
object.getContentMetadata().setName(name);
object.setPayload(Payloads.newPayload(data));
object.getContentMetadata().setContentLength(16l);
Payloads.calculateMD5(object);
object.getContentMetadata().setContentMD5(hashCode.asBytes());
object.getContentMetadata().setContentType("text/plain");
object.getUserMetadata().getMetadata().put("Metadata", metadataValue);
replaceObject(object);

View File

@ -49,6 +49,7 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.hash.Hashing;
import com.google.common.hash.HashingInputStream;
import com.google.common.io.ByteSource;
import com.google.common.io.Closeables;
import com.google.common.io.Files;
@ -176,9 +177,11 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
public Blob getBlob(final String container, final String key) {
BlobBuilder builder = blobBuilders.get();
builder.name(key);
File file = getFileForBlobKey(container, key);
ByteSource byteSource = Files.asByteSource(getFileForBlobKey(container, key));
try {
builder.payload(file).calculateMD5();
builder.payload(byteSource)
.contentLength(byteSource.size())
.contentMD5(byteSource.hash(Hashing.md5()).asBytes());
} catch (IOException e) {
logger.error("An error occurred calculating MD5 for blob %s from container ", key, container);
Throwables.propagateIfPossible(e);

View File

@ -36,7 +36,6 @@ import org.jclouds.blobstore.domain.PageSet;
import org.jclouds.blobstore.integration.internal.BaseBlobStoreIntegrationTest;
import org.jclouds.http.HttpResponseException;
import org.jclouds.http.options.GetOptions;
import org.jclouds.io.Payloads;
import org.jclouds.openstack.keystone.v2_0.config.KeystoneProperties;
import org.jclouds.openstack.swift.domain.AccountMetadata;
import org.jclouds.openstack.swift.domain.ContainerMetadata;
@ -50,6 +49,7 @@ import org.testng.annotations.Test;
import com.google.common.base.Charsets;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.hash.Hashing;
/**
* Tests behavior of {@code JaxrsAnnotationProcessor}
@ -365,7 +365,7 @@ public abstract class CommonSwiftClientLiveTest<C extends CommonSwiftClient> ext
SwiftObject object = getApi().newSwiftObject();
object.getInfo().setName(key);
object.setPayload(data);
Payloads.calculateMD5(object);
object.getPayload().getContentMetadata().setContentMD5(Hashing.md5().hashString(data, Charsets.UTF_8).asBytes());
object.getInfo().setContentType("text/plain");
object.getInfo().getMetadata().put("Metadata", "metadata-value");
return object;

View File

@ -291,10 +291,8 @@ Options can also be specified for extension modules
The payload argument can be anything accepted by the PayloadSource protocol."
([^String name &
{:keys [payload content-type content-length content-md5 calculate-md5
{:keys [payload content-type content-length content-md5
content-disposition content-encoding content-language metadata]}]
{:pre [(not (and content-md5 calculate-md5))
(not (and (nil? payload) calculate-md5))]}
(let [blob-builder (.name (BlobBuilderImpl.) name)
blob-builder (if payload
(.payload blob-builder
@ -306,11 +304,9 @@ Options can also be specified for extension modules
blob-builder (if content-type
(.contentType blob-builder content-type)
blob-builder)
blob-builder (if calculate-md5 ;; Only do calculateMD5 OR contentMD5.
(.calculateMD5 blob-builder)
(if content-md5
(.contentMD5 blob-builder content-md5)
blob-builder))]
blob-builder (if content-md5
(.contentMD5 blob-builder content-md5)
blob-builder)]
(doto blob-builder
(.contentDisposition content-disposition)
(.contentEncoding content-encoding)

View File

@ -46,6 +46,7 @@ import com.google.common.base.Supplier;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimaps;
import com.google.common.hash.Hashing;
import com.google.common.io.ByteStreams;
import com.google.common.net.HttpHeaders;
@ -125,7 +126,6 @@ public class TransientStorageStrategy implements LocalStorageStrategy {
Blob newBlob = createUpdatedCopyOfBlobInContainer(containerName, blob);
Map<String, Blob> map = containerToBlobs.get(containerName);
map.put(newBlob.getMetadata().getName(), newBlob);
Payloads.calculateMD5(newBlob);
return base16().lowerCase().encode(newBlob.getPayload().getContentMetadata().getContentMD5());
}
@ -159,11 +159,13 @@ public class TransientStorageStrategy implements LocalStorageStrategy {
if (payload == null || !(payload instanceof ByteArrayPayload)) {
MutableContentMetadata oldMd = in.getPayload().getContentMetadata();
byte[] out = ByteStreams.toByteArray(in.getPayload());
payload = (ByteArrayPayload) Payloads.calculateMD5(Payloads.newPayload(out));
payload = Payloads.newByteArrayPayload(out);
payload.getContentMetadata().setContentMD5(Hashing.md5().hashBytes(out).asBytes());
HttpUtils.copy(oldMd, payload.getContentMetadata());
} else {
if (payload.getContentMetadata().getContentMD5() == null)
Payloads.calculateMD5(in);
if (payload.getContentMetadata().getContentMD5() == null) {
payload.getContentMetadata().setContentMD5(ByteStreams.hash(payload, Hashing.md5()).asBytes());
}
}
} catch (IOException e) {
Throwables.propagate(e);

View File

@ -131,14 +131,5 @@ public interface BlobBuilder {
PayloadBlobBuilder contentEncoding(String contentEncoding);
PayloadBlobBuilder expires(Date expires);
/**
* @deprecated Callers should instead call BlobBuilder.contentMD5,
* usually with the results from Guava Hashing.md5().
* @see Payloads#calculateMD5
*/
@Deprecated
PayloadBlobBuilder calculateMD5() throws IOException;
}
}

View File

@ -153,16 +153,6 @@ public class BlobBuilderImpl implements BlobBuilder {
return builder.payload(payload);
}
/**
* @deprecated Callers should instead call BlobBuilder.contentMD5,
* usually with the results from Guava Hashing.md5().
*/
@Deprecated
@Override
public PayloadBlobBuilder calculateMD5() throws IOException {
return builder.payload(Payloads.calculateMD5(payload));
}
@Override
public PayloadBlobBuilder payload(InputStream payload) {
return builder.payload(payload);

View File

@ -21,6 +21,7 @@
(:import [java.io ByteArrayInputStream ByteArrayOutputStream
StringBufferInputStream]
[org.jclouds.util Strings2]
com.google.common.hash.Hashing
com.google.common.io.ByteSource))
(defn clean-stub-fixture
@ -64,14 +65,14 @@
(is (create-container blobstore-stub "container"))
(is (empty? (blobs blobstore-stub "container")))
(is (put-blob blobstore-stub "container"
(blob "blob1" :payload "blob1" :calculate-md5 true)))
(blob "blob1" :payload "blob1")))
(is (put-blob blobstore-stub "container"
(blob "blob2" :payload "blob2" :calculate-md5 true)))
(blob "blob2" :payload "blob2")))
(is (= 2 (count (blobs blobstore-stub "container"))))
(is (= 1 (count (blobs blobstore-stub "container" :max-results 1))))
(create-directory blobstore-stub "container" "dir")
(is (put-blob blobstore-stub "container"
(blob "dir/blob2" :payload "blob2" :calculate-md5 true)))
(blob "dir/blob2" :payload "blob2")))
(is (= 3 (count-blobs blobstore-stub "container")))
(is (= 3 (count (blobs blobstore-stub "container"))))
(is (= 4 (count (blobs blobstore-stub "container" :recursive true))))
@ -85,8 +86,7 @@
(create-container blobstore-stub container-name)
(dotimes [i total-blobs] (put-blob blobstore-stub container-name
(blob (str i)
:payload (str i)
:calculate-md5 true)))
:payload (str i))))
;; verify
(is (= total-blobs (count-blobs blobstore-stub container-name)))))
@ -98,9 +98,9 @@
(deftest get-blob-test
(is (create-container blobstore-stub "blob"))
(is (put-blob blobstore-stub "blob"
(blob "blob1" :payload "blob1" :calculate-md5 true)))
(blob "blob1" :payload "blob1")))
(is (put-blob blobstore-stub "blob"
(blob "blob2" :payload "blob2" :calculate-md5 true)))
(blob "blob2" :payload "blob2")))
(is (= "blob2" (Strings2/toStringAndClose (get-blob-stream blobstore-stub
"blob" "blob2")))))
@ -148,9 +148,10 @@
(is (= "DELETE" (.getMethod request)))))
(deftest blob-test
(let [a-blob (blob "test-name"
:payload "test-payload"
:calculate-md5 true)]
(let [byte-source (ByteSource/wrap (.getBytes "test-payload"))
a-blob (blob "test-name"
:payload byte-source
:content-md5 (.asBytes (.hash byte-source (Hashing/md5))))]
(is (= (seq (.. a-blob (getPayload) (getContentMetadata) (getContentMD5)))
(seq (.digest (doto (java.security.MessageDigest/getInstance "MD5")
(.reset)

View File

@ -30,6 +30,10 @@ import org.jclouds.rest.internal.BaseAsyncClientTest;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import com.google.common.base.Charsets;
import com.google.common.hash.Hashing;
import com.google.common.io.ByteSource;
/**
* Tests behavior of {@code LocalBlobRequestSigner}
*
@ -88,7 +92,12 @@ public class TransientBlobRequestSignerTest extends BaseAsyncClientTest<LocalAsy
public void testSignPutBlobWithGenerate() throws ArrayIndexOutOfBoundsException, SecurityException,
IllegalArgumentException, NoSuchMethodException, IOException {
Blob blob = blobFactory.get().name(blobName).payload("foo").calculateMD5().contentType("text/plain").build();
ByteSource byteSource = ByteSource.wrap("foo".getBytes(Charsets.UTF_8));
Blob blob = blobFactory.get().name(blobName)
.payload(byteSource)
.contentLength(byteSource.size())
.contentMD5(byteSource.hash(Hashing.md5()).asBytes())
.contentType("text/plain").build();
byte[] md5 = { -84, -67, 24, -37, 76, -62, -8, 92, -19, -17, 101, 79, -52, -60, -92, -40 };
assertEquals(blob.getPayload().getContentMetadata().getContentMD5(), md5);

View File

@ -515,9 +515,6 @@ public class BaseBlobIntegrationTest extends BaseBlobStoreIntegrationTest {
PayloadBlobBuilder blobBuilder = view.getBlobStore().blobBuilder(name).payload(Payloads.newPayload(content))
.contentType(type);
addContentMetadata(blobBuilder);
if (content instanceof InputStream) {
blobBuilder.calculateMD5();
}
Blob blob = blobBuilder.build();
String container = getContainerName();
try {
@ -621,7 +618,9 @@ public class BaseBlobIntegrationTest extends BaseBlobStoreIntegrationTest {
// normalize the
// providers.
Blob blob = view.getBlobStore().blobBuilder(name).userMetadata(ImmutableMap.of("Adrian", "powderpuff"))
.payload(TEST_STRING).contentType(MediaType.TEXT_PLAIN).calculateMD5().build();
.payload(TEST_STRING).contentType(MediaType.TEXT_PLAIN)
.contentMD5(md5().hashString(TEST_STRING, Charsets.UTF_8).asBytes())
.build();
String container = getContainerName();
try {
assertNull(view.getBlobStore().blobMetadata(container, "powderpuff"));

View File

@ -77,7 +77,9 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
// NOTE all metadata in jclouds comes out as lowercase, in an effort to
// normalize the providers.
view.getBlobStore().blobBuilder(key).userMetadata(ImmutableMap.of("Adrian", "powderpuff"))
.payload(TEST_STRING).contentType(MediaType.TEXT_PLAIN).calculateMD5().build());
.payload(TEST_STRING).contentType(MediaType.TEXT_PLAIN)
.contentMD5(md5().hashString(TEST_STRING, UTF_8).asBytes())
.build());
validateContent(containerName, key);
PageSet<? extends StorageMetadata> container = view.getBlobStore().list(containerName,

View File

@ -100,44 +100,4 @@ public class Payloads {
public static UrlEncodedFormPayload newUrlEncodedFormPayload(Multimap<String, String> formParams) {
return new UrlEncodedFormPayload(formParams);
}
/**
* Calculates and sets {@link Payload#setContentMD5} on the payload.
*
* <p/>
* note that this will rebuffer in memory if the payload is not repeatable.
*
* @param payload
* payload to calculate
* @return new Payload with md5 set.
* @throws IOException
*/
public static Payload calculateMD5(Payload payload) throws IOException {
checkNotNull(payload, "payload");
if (!payload.isRepeatable()) {
MutableContentMetadata oldContentMetadata = payload.getContentMetadata();
Payload oldPayload = payload;
try {
payload = newByteArrayPayload(toByteArray(payload));
} finally {
oldPayload.release();
}
oldContentMetadata.setContentLength(payload.getContentMetadata().getContentLength());
oldContentMetadata.setContentMD5(payload.getContentMetadata().getContentMD5());
payload.setContentMetadata(oldContentMetadata);
}
payload.getContentMetadata().setContentMD5(ByteStreams.hash(payload, md5()).asBytes());
return payload;
}
/**
* Calculates the md5 on a payload, replacing as necessary.
*/
public static <T extends PayloadEnclosing> T calculateMD5(T payloadEnclosing) throws IOException {
checkNotNull(payloadEnclosing, "payloadEnclosing");
Payload newPayload = calculateMD5(payloadEnclosing.getPayload());
if (newPayload != payloadEnclosing.getPayload())
payloadEnclosing.setPayload(newPayload);
return payloadEnclosing;
}
}

View File

@ -123,12 +123,8 @@ public abstract class BaseRestApiTest {
}
assertEquals(payload, toMatch);
Long length = Long.valueOf(payload.getBytes().length);
try {
assertContentHeadersEqual(request, contentType, contentDispositon, contentEncoding, contentLanguage,
length, contentMD5 ? ByteStreams.hash(request.getPayload(), md5()).asBytes() : null, expires);
} catch (IOException e) {
propagate(e);
}
assertContentHeadersEqual(request, contentType, contentDispositon, contentEncoding, contentLanguage,
length, contentMD5 ? md5().hashBytes(payload.getBytes()).asBytes() : null, expires);
}
}

View File

@ -18,7 +18,6 @@ package org.jclouds.rest.internal;
import static com.google.common.base.Charsets.UTF_8;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.jclouds.io.Payloads.calculateMD5;
import static org.jclouds.io.Payloads.newInputStreamPayload;
import static org.jclouds.io.Payloads.newStringPayload;
import static org.jclouds.reflect.Reflection2.method;
@ -140,6 +139,8 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.hash.Hashing;
import com.google.common.io.ByteSource;
import com.google.common.io.Files;
import com.google.common.net.HttpHeaders;
import com.google.common.reflect.Invokable;
@ -1970,8 +1971,11 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
public void testPutPayloadEnclosingGenerateMD5() throws SecurityException, NoSuchMethodException, IOException {
Invokable<?, ?> method = method(TestTransformers.class, "put", PayloadEnclosing.class);
PayloadEnclosing payloadEnclosing = new PayloadEnclosingImpl(newStringPayload("whoops"));
calculateMD5(payloadEnclosing);
ByteSource byteSource = ByteSource.wrap("whoops".getBytes(UTF_8));
PayloadEnclosing payloadEnclosing = new PayloadEnclosingImpl(Payloads.newByteSourcePayload(byteSource));
payloadEnclosing.getPayload().getContentMetadata().setContentLength(byteSource.size());
payloadEnclosing.getPayload().getContentMetadata().setContentMD5(byteSource.hash(Hashing.md5()).asBytes());
GeneratedHttpRequest request = processor.apply(Invocation.create(method,
ImmutableList.<Object> of(payloadEnclosing)));
assertRequestLineEquals(request, "PUT http://localhost:9999 HTTP/1.1");
@ -1983,10 +1987,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
public void testPutInputStreamPayloadEnclosingGenerateMD5() throws SecurityException, NoSuchMethodException,
IOException {
Invokable<?, ?> method = method(TestTransformers.class, "put", PayloadEnclosing.class);
ByteSource byteSource = ByteSource.wrap("whoops".getBytes(UTF_8));
PayloadEnclosing payloadEnclosing = new PayloadEnclosingImpl(
newInputStreamPayload(Strings2.toInputStream("whoops")));
newInputStreamPayload(byteSource.openStream()));
payloadEnclosing.getPayload().getContentMetadata().setContentLength(byteSource.size());
payloadEnclosing.getPayload().getContentMetadata().setContentMD5(byteSource.hash(Hashing.md5()).asBytes());
calculateMD5(payloadEnclosing);
GeneratedHttpRequest request = processor.apply(Invocation.create(method,
ImmutableList.<Object> of(payloadEnclosing)));
assertRequestLineEquals(request, "PUT http://localhost:9999 HTTP/1.1");
@ -2048,8 +2054,11 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
public void testPutPayloadWithGeneratedMD5AndNoContentType() throws SecurityException, NoSuchMethodException,
IOException {
Payload payload = newStringPayload("whoops");
calculateMD5(payload);
ByteSource byteSource = ByteSource.wrap("whoops".getBytes(UTF_8));
Payload payload = Payloads.newByteSourcePayload(byteSource);
payload.getContentMetadata().setContentLength(byteSource.size());
payload.getContentMetadata().setContentMD5(byteSource.hash(Hashing.md5()).asBytes());
Invokable<?, ?> method = method(TestTransformers.class, "put", Payload.class);
GeneratedHttpRequest request = processor.apply(Invocation.create(method,
ImmutableList.<Object> of(payload)));
@ -2071,8 +2080,11 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
public void testPutInputStreamPayloadWithMD5() throws NoSuchAlgorithmException, IOException, SecurityException,
NoSuchMethodException {
Payload payload = newStringPayload("whoops");
calculateMD5(payload);
ByteSource byteSource = ByteSource.wrap("whoops".getBytes(UTF_8));
Payload payload = Payloads.newByteSourcePayload(byteSource);
payload.getContentMetadata().setContentLength(byteSource.size());
payload.getContentMetadata().setContentMD5(byteSource.hash(Hashing.md5()).asBytes());
Invokable<?, ?> method = method(TestTransformers.class, "put", Payload.class);
GeneratedHttpRequest request = processor.apply(Invocation.create(method,
ImmutableList.<Object> of(payload)));

View File

@ -45,7 +45,6 @@ import org.jclouds.blobstore.ContainerNotFoundException;
import org.jclouds.blobstore.integration.internal.BaseBlobStoreIntegrationTest;
import org.jclouds.http.HttpResponseException;
import org.jclouds.http.options.GetOptions;
import org.jclouds.io.Payloads;
import org.jclouds.io.payloads.ByteArrayPayload;
import org.jclouds.util.Strings2;
import org.jclouds.util.Throwables2;
@ -55,6 +54,7 @@ import com.google.common.base.Charsets;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables;
import com.google.common.hash.Hashing;
/**
* Tests behavior of {@code AzureBlobClient}
@ -223,7 +223,7 @@ public class AzureBlobClientLiveTest extends BaseBlobStoreIntegrationTest {
AzureBlob object = getApi().newBlob();
object.getProperties().setName("object");
object.setPayload(data);
Payloads.calculateMD5(object);
object.getProperties().getContentMetadata().setContentMD5(Hashing.md5().hashString(data, Charsets.UTF_8).asBytes());
object.getProperties().getContentMetadata().setContentType("text/plain");
object.getProperties().getMetadata().put("mykey", "metadata-value");
byte[] md5 = object.getProperties().getContentMetadata().getContentMD5();