Skip to content

Commit 3d13b99

Browse files
mziccardmziccard
committed
Remove md5 and crc32c options from StorageRpc
- BlobWriteOption extends Serializable instead of Option - add BlobTargetObject.convert to convert BlobWriteOption and possibly mask md5 and crc32c fields - add private Storage.create(blobInfo, inputStream, blobTargetOption) - update Storage.create, Storage.writer and Blob.writer javadoc
1 parent cebb0bb commit 3d13b99

File tree

6 files changed

+105
-68
lines changed

6 files changed

+105
-68
lines changed

‎gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java

-13
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_GENERATION_NOT_MATCH;
2424
import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_METAGENERATION_MATCH;
2525
import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_METAGENERATION_NOT_MATCH;
26-
import static com.google.gcloud.spi.StorageRpc.Option.IF_MD5_MATCH;
27-
import static com.google.gcloud.spi.StorageRpc.Option.IF_CRC32C_MATCH;
2826
import static com.google.gcloud.spi.StorageRpc.Option.MAX_RESULTS;
2927
import static com.google.gcloud.spi.StorageRpc.Option.PAGE_TOKEN;
3028
import static com.google.gcloud.spi.StorageRpc.Option.PREDEFINED_ACL;
@@ -108,15 +106,6 @@ private static StorageException translate(GoogleJsonError exception) {
108106
return new StorageException(exception.getCode(), exception.getMessage(), retryable);
109107
}
110108

111-
private static void applyOptions(StorageObject storageObject, Map<Option, ?> options) {
112-
if (IF_MD5_MATCH.getBoolean(options) == null) {
113-
storageObject.setMd5Hash(null);
114-
}
115-
if (IF_CRC32C_MATCH.getBoolean(options) == null) {
116-
storageObject.setCrc32c(null);
117-
}
118-
}
119-
120109
@Override
121110
public Bucket create(Bucket bucket, Map<Option, ?> options) throws StorageException {
122111
try {
@@ -134,7 +123,6 @@ public Bucket create(Bucket bucket, Map<Option, ?> options) throws StorageExcept
134123
@Override
135124
public StorageObject create(StorageObject storageObject, final InputStream content,
136125
Map<Option, ?> options) throws StorageException {
137-
applyOptions(storageObject, options);
138126
try {
139127
Storage.Objects.Insert insert = storage.objects()
140128
.insert(storageObject.getBucket(), storageObject,
@@ -503,7 +491,6 @@ public void write(String uploadId, byte[] toWrite, int toWriteOffset, StorageObj
503491
@Override
504492
public String open(StorageObject object, Map<Option, ?> options)
505493
throws StorageException {
506-
applyOptions(object, options);
507494
try {
508495
Insert req = storage.objects().insert(object.getBucket(), object);
509496
GenericUrl url = req.buildHttpRequest().getUrl();

‎gcloud-java-storage/src/main/java/com/google/gcloud/spi/StorageRpc.java

-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ enum Option {
4242
IF_SOURCE_METAGENERATION_NOT_MATCH("ifSourceMetagenerationNotMatch"),
4343
IF_SOURCE_GENERATION_MATCH("ifSourceGenerationMatch"),
4444
IF_SOURCE_GENERATION_NOT_MATCH("ifSourceGenerationNotMatch"),
45-
IF_MD5_MATCH("md5Hash"),
46-
IF_CRC32C_MATCH("crc32c"),
4745
PREFIX("prefix"),
4846
MAX_RESULTS("maxResults"),
4947
PAGE_TOKEN("pageToken"),

‎gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,9 @@ public BlobReadChannel reader(BlobSourceOption... options) {
270270
}
271271

272272
/**
273-
* Returns a {@code BlobWriteChannel} object for writing to this blob.
273+
* Returns a {@code BlobWriteChannel} object for writing to this blob. By default any md5 and
274+
* crc32c values in the current blob are ignored unless requested via the
275+
* {@code BlobWriteOption.md5Match} and {@code BlobWriteOption.crc32cMatch} options.
274276
*
275277
* @param options target blob options
276278
* @throws StorageException upon failure

‎gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java

+71-26
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121

2222
import com.google.common.collect.ImmutableList;
2323
import com.google.common.collect.Iterables;
24+
import com.google.common.collect.Lists;
2425
import com.google.gcloud.AuthCredentials.ServiceAccountAuthCredentials;
2526
import com.google.gcloud.Service;
2627
import com.google.gcloud.spi.StorageRpc;
28+
import com.google.gcloud.spi.StorageRpc.Tuple;
2729

2830
import java.io.InputStream;
2931
import java.io.Serializable;
@@ -33,6 +35,7 @@
3335
import java.util.LinkedHashSet;
3436
import java.util.LinkedList;
3537
import java.util.List;
38+
import java.util.Objects;
3639
import java.util.Set;
3740
import java.util.concurrent.TimeUnit;
3841

@@ -146,65 +149,103 @@ public static BlobTargetOption metagenerationNotMatch() {
146149
return new BlobTargetOption(StorageRpc.Option.IF_METAGENERATION_NOT_MATCH);
147150
}
148151

149-
static BlobWriteOption[] convert(BlobTargetOption[] options, BlobWriteOption... optionsToAdd) {
150-
BlobWriteOption[] writeOptions = new BlobWriteOption[options.length + optionsToAdd.length];
151-
int index = 0;
152-
for (BlobTargetOption option : options) {
153-
writeOptions[index++] = new BlobWriteOption(option);
154-
}
155-
for (BlobWriteOption option : optionsToAdd) {
156-
writeOptions[index++] = option;
152+
static Tuple<BlobInfo, BlobTargetOption[]> convert(BlobInfo info, BlobWriteOption... options) {
153+
BlobInfo.Builder infoBuilder = info.toBuilder().crc32c(null).md5(null);
154+
List<BlobTargetOption> targetOptions = Lists.newArrayListWithCapacity(options.length);
155+
for (BlobWriteOption option : options) {
156+
switch (option.option) {
157+
case IF_CRC32C_MATCH:
158+
infoBuilder.crc32c(info.crc32c());
159+
break;
160+
case IF_MD5_MATCH:
161+
infoBuilder.md5(info.md5());
162+
break;
163+
default:
164+
targetOptions.add(option.toTargetOption());
165+
break;
166+
}
157167
}
158-
return writeOptions;
168+
return Tuple.of(infoBuilder.build(),
169+
targetOptions.toArray(new BlobTargetOption[targetOptions.size()]));
159170
}
160171
}
161172

162-
class BlobWriteOption extends Option {
173+
class BlobWriteOption implements Serializable {
163174

164175
private static final long serialVersionUID = -3880421670966224580L;
165176

166-
BlobWriteOption(BlobTargetOption option) {
167-
super(option.rpcOption(), option.value());
177+
private final Option option;
178+
private final Object value;
179+
180+
enum Option {
181+
PREDEFINED_ACL, IF_GENERATION_MATCH, IF_GENERATION_NOT_MATCH, IF_METAGENERATION_MATCH,
182+
IF_METAGENERATION_NOT_MATCH, IF_MD5_MATCH, IF_CRC32C_MATCH;
183+
184+
StorageRpc.Option toRpcOption() {
185+
return StorageRpc.Option.valueOf(this.name());
186+
}
168187
}
169188

170-
private BlobWriteOption(StorageRpc.Option rpcOption, Object value) {
171-
super(rpcOption, value);
189+
BlobTargetOption toTargetOption() {
190+
return new BlobTargetOption(this.option.toRpcOption(), this.value);
172191
}
173192

174-
private BlobWriteOption(StorageRpc.Option rpcOption) {
175-
this(rpcOption, null);
193+
private BlobWriteOption(Option option, Object value) {
194+
this.option = option;
195+
this.value = value;
196+
}
197+
198+
private BlobWriteOption(Option option) {
199+
this(option, null);
200+
}
201+
202+
@Override
203+
public int hashCode() {
204+
return Objects.hash(option, value);
205+
}
206+
207+
@Override
208+
public boolean equals(Object obj) {
209+
if (obj == null) {
210+
return false;
211+
}
212+
if (!(obj instanceof BlobWriteOption)) {
213+
return false;
214+
}
215+
final BlobWriteOption other = (BlobWriteOption) obj;
216+
return this.option == other.option && Objects.equals(this.value, other.value);
176217
}
177218

178219
public static BlobWriteOption predefinedAcl(PredefinedAcl acl) {
179-
return new BlobWriteOption(StorageRpc.Option.PREDEFINED_ACL, acl.entry());
220+
return new BlobWriteOption(Option.PREDEFINED_ACL, acl.entry());
180221
}
181222

182223
public static BlobWriteOption doesNotExist() {
183-
return new BlobWriteOption(StorageRpc.Option.IF_GENERATION_MATCH, 0L);
224+
return new BlobWriteOption(Option.IF_GENERATION_MATCH, 0L);
184225
}
185226

186227
public static BlobWriteOption generationMatch() {
187-
return new BlobWriteOption(StorageRpc.Option.IF_GENERATION_MATCH);
228+
return new BlobWriteOption(Option.IF_GENERATION_MATCH);
188229
}
189230

190231
public static BlobWriteOption generationNotMatch() {
191-
return new BlobWriteOption(StorageRpc.Option.IF_GENERATION_NOT_MATCH);
232+
return new BlobWriteOption(Option.IF_GENERATION_NOT_MATCH);
192233
}
193234

194235
public static BlobWriteOption metagenerationMatch() {
195-
return new BlobWriteOption(StorageRpc.Option.IF_METAGENERATION_MATCH);
236+
return new BlobWriteOption(Option.IF_METAGENERATION_MATCH);
196237
}
197238

198239
public static BlobWriteOption metagenerationNotMatch() {
199-
return new BlobWriteOption(StorageRpc.Option.IF_METAGENERATION_NOT_MATCH);
240+
return new BlobWriteOption(Option.IF_METAGENERATION_NOT_MATCH);
200241
}
201242

202243
public static BlobWriteOption md5Match() {
203-
return new BlobWriteOption(StorageRpc.Option.IF_MD5_MATCH, true);
244+
return new BlobWriteOption(Option.IF_MD5_MATCH, true);
204245
}
205246

206247
public static BlobWriteOption crc32cMatch() {
207-
return new BlobWriteOption(StorageRpc.Option.IF_CRC32C_MATCH, true);
248+
return new BlobWriteOption(Option.IF_CRC32C_MATCH, true);
208249
}
209250
}
210251

@@ -582,7 +623,9 @@ public static Builder builder() {
582623

583624
/**
584625
* Create a new blob. Direct upload is used to upload {@code content}. For large content,
585-
* {@link #writer} is recommended as it uses resumable upload.
626+
* {@link #writer} is recommended as it uses resumable upload. By default any md5 and crc32c
627+
* values in the given {@code blobInfo} are ignored unless requested via the
628+
* {@code BlobWriteOption.md5Match} and {@code BlobWriteOption.crc32cMatch} options.
586629
*
587630
* @return a complete blob information.
588631
* @throws StorageException upon failure
@@ -742,7 +785,9 @@ public static Builder builder() {
742785
BlobReadChannel reader(BlobId blob, BlobSourceOption... options);
743786

744787
/**
745-
* Create a blob and return a channel for writing its content.
788+
* Create a blob and return a channel for writing its content. By default any md5 and crc32c
789+
* values in the given {@code blobInfo} are ignored unless requested via the
790+
* {@code BlobWriteOption.md5Match} and {@code BlobWriteOption.crc32cMatch} options.
746791
*
747792
* @throws StorageException upon failure
748793
*/

‎gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java

+14-7
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,7 @@ public BlobInfo create(BlobInfo blobInfo, BlobTargetOption... options) {
129129
.md5(EMPTY_BYTE_ARRAY_MD5)
130130
.crc32c(EMPTY_BYTE_ARRAY_CRC32C)
131131
.build();
132-
return create(updatedInfo, new ByteArrayInputStream(EMPTY_BYTE_ARRAY),
133-
BlobTargetOption.convert(
134-
options, BlobWriteOption.md5Match(), BlobWriteOption.crc32cMatch()));
132+
return create(updatedInfo, new ByteArrayInputStream(EMPTY_BYTE_ARRAY), options);
135133
}
136134

137135
@Override
@@ -142,14 +140,18 @@ public BlobInfo create(BlobInfo blobInfo, byte[] content, BlobTargetOption... op
142140
.crc32c(BaseEncoding.base64().encode(
143141
Ints.toByteArray(Hashing.crc32c().hashBytes(content).asInt())))
144142
.build();
145-
return create(updatedInfo, new ByteArrayInputStream(content), BlobTargetOption.convert(
146-
options, BlobWriteOption.md5Match(), BlobWriteOption.crc32cMatch()));
143+
return create(updatedInfo, new ByteArrayInputStream(content), options);
147144
}
148145

149146
@Override
150147
public BlobInfo create(BlobInfo blobInfo, final InputStream content, BlobWriteOption... options) {
151-
final StorageObject blobPb = blobInfo.toPb();
152-
final Map<StorageRpc.Option, ?> optionsMap = optionMap(blobInfo, options);
148+
Tuple<BlobInfo, BlobTargetOption[]> targetOptions = BlobTargetOption.convert(blobInfo, options);
149+
return create(targetOptions.x(), content, targetOptions.y());
150+
}
151+
152+
private BlobInfo create(BlobInfo info, final InputStream content, BlobTargetOption... options) {
153+
final StorageObject blobPb = info.toPb();
154+
final Map<StorageRpc.Option, ?> optionsMap = optionMap(info, options);
153155
try {
154156
return BlobInfo.fromPb(runWithRetries(new Callable<StorageObject>() {
155157
@Override
@@ -558,6 +560,11 @@ public BlobReadChannel reader(BlobId blob, BlobSourceOption... options) {
558560

559561
@Override
560562
public BlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options) {
563+
Tuple<BlobInfo, BlobTargetOption[]> targetOptions = BlobTargetOption.convert(blobInfo, options);
564+
return writer(targetOptions.x(), targetOptions.y());
565+
}
566+
567+
private BlobWriteChannel writer(BlobInfo blobInfo, BlobTargetOption... options) {
561568
final Map<StorageRpc.Option, ?> optionsMap = optionMap(blobInfo, options);
562569
return new BlobWriteChannelImpl(options(), blobInfo, optionsMap);
563570
}

‎gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java

+17-19
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,6 @@ public class StorageImplTest {
134134
Storage.BlobWriteOption.md5Match();
135135
private static final Storage.BlobWriteOption BLOB_WRITE_CRC2C =
136136
Storage.BlobWriteOption.crc32cMatch();
137-
private static final Map<StorageRpc.Option, ?> BLOB_WRITE_OPTIONS_SIMPLE = ImmutableMap.of(
138-
StorageRpc.Option.IF_MD5_MATCH, true,
139-
StorageRpc.Option.IF_CRC32C_MATCH, true);
140-
private static final Map<StorageRpc.Option, ?> BLOB_WRITE_OPTIONS_COMPLEX = ImmutableMap.of(
141-
StorageRpc.Option.IF_METAGENERATION_MATCH, BLOB_INFO1.metageneration(),
142-
StorageRpc.Option.IF_GENERATION_MATCH, 0L,
143-
StorageRpc.Option.PREDEFINED_ACL, BUCKET_TARGET_PREDEFINED_ACL.value(),
144-
StorageRpc.Option.IF_MD5_MATCH, true,
145-
StorageRpc.Option.IF_CRC32C_MATCH, true);
146137

147138
// Bucket source options
148139
private static final Storage.BucketSourceOption BUCKET_SOURCE_METAGENERATION =
@@ -276,7 +267,7 @@ public void testCreateBlob() throws IOException {
276267
EasyMock.expect(storageRpcMock.create(
277268
EasyMock.eq(BLOB_INFO1.toBuilder().md5(CONTENT_MD5).crc32c(CONTENT_CRC32C).build().toPb()),
278269
EasyMock.capture(capturedStream),
279-
EasyMock.eq(BLOB_WRITE_OPTIONS_SIMPLE)))
270+
EasyMock.eq(EMPTY_RPC_OPTIONS)))
280271
.andReturn(BLOB_INFO1.toPb());
281272
EasyMock.replay(optionsMock, storageRpcMock);
282273
storage = StorageFactory.instance().get(optionsMock);
@@ -301,7 +292,7 @@ public void testCreateEmptyBlob() throws IOException {
301292
.build()
302293
.toPb()),
303294
EasyMock.capture(capturedStream),
304-
EasyMock.eq(BLOB_WRITE_OPTIONS_SIMPLE)))
295+
EasyMock.eq(EMPTY_RPC_OPTIONS)))
305296
.andReturn(BLOB_INFO1.toPb());
306297
EasyMock.replay(optionsMock, storageRpcMock);
307298
storage = StorageFactory.instance().get(optionsMock);
@@ -324,7 +315,7 @@ public void testCreateBlobWithOptions() throws IOException {
324315
.build()
325316
.toPb()),
326317
EasyMock.capture(capturedStream),
327-
EasyMock.eq(BLOB_WRITE_OPTIONS_COMPLEX)))
318+
EasyMock.eq(BLOB_TARGET_OPTIONS_CREATE)))
328319
.andReturn(BLOB_INFO1.toPb());
329320
EasyMock.replay(optionsMock, storageRpcMock);
330321
storage = StorageFactory.instance().get(optionsMock);
@@ -344,11 +335,14 @@ public void testCreateBlobFromStream() throws IOException {
344335
EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock);
345336
EasyMock.expect(optionsMock.retryParams()).andReturn(RetryParams.noRetries());
346337
ByteArrayInputStream fileStream = new ByteArrayInputStream(BLOB_CONTENT);
347-
EasyMock.expect(storageRpcMock.create(BLOB_INFO1.toPb(), fileStream, EMPTY_RPC_OPTIONS))
338+
BlobInfo.Builder infoBuilder = BLOB_INFO1.toBuilder();
339+
BlobInfo infoWithHashes = infoBuilder.md5(CONTENT_MD5).crc32c(CONTENT_CRC32C).build();
340+
BlobInfo infoWithoutHashes = infoBuilder.md5(null).crc32c(null).build();
341+
EasyMock.expect(storageRpcMock.create(infoWithoutHashes.toPb(), fileStream, EMPTY_RPC_OPTIONS))
348342
.andReturn(BLOB_INFO1.toPb());
349343
EasyMock.replay(optionsMock, storageRpcMock);
350344
storage = StorageFactory.instance().get(optionsMock);
351-
BlobInfo blob = storage.create(BLOB_INFO1, fileStream);
345+
BlobInfo blob = storage.create(infoWithHashes, fileStream);
352346
assertEquals(BLOB_INFO1, blob);
353347
}
354348

@@ -807,24 +801,28 @@ public void testReaderWithOptions() throws IOException {
807801
@Test
808802
public void testWriter() {
809803
EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock).times(2);
810-
EasyMock.expect(storageRpcMock.open(BLOB_INFO1.toPb(), EMPTY_RPC_OPTIONS))
804+
BlobInfo.Builder infoBuilder = BLOB_INFO1.toBuilder();
805+
BlobInfo infoWithHashes = infoBuilder.md5(CONTENT_MD5).crc32c(CONTENT_CRC32C).build();
806+
BlobInfo infoWithoutHashes = infoBuilder.md5(null).crc32c(null).build();
807+
EasyMock.expect(storageRpcMock.open(infoWithoutHashes.toPb(), EMPTY_RPC_OPTIONS))
811808
.andReturn("upload-id");
812809
EasyMock.replay(optionsMock, storageRpcMock);
813810
storage = StorageFactory.instance().get(optionsMock);
814-
BlobWriteChannel channel = storage.writer(BLOB_INFO1);
811+
BlobWriteChannel channel = storage.writer(infoWithHashes);
815812
assertNotNull(channel);
816813
assertTrue(channel.isOpen());
817814
}
818815

819816
@Test
820817
public void testWriterWithOptions() {
821818
EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock).times(2);
822-
EasyMock.expect(storageRpcMock.open(BLOB_INFO1.toPb(), BLOB_WRITE_OPTIONS_COMPLEX))
819+
BlobInfo info = BLOB_INFO1.toBuilder().md5(CONTENT_MD5).crc32c(CONTENT_CRC32C).build();
820+
EasyMock.expect(storageRpcMock.open(info.toPb(), BLOB_TARGET_OPTIONS_CREATE))
823821
.andReturn("upload-id");
824822
EasyMock.replay(optionsMock, storageRpcMock);
825823
storage = StorageFactory.instance().get(optionsMock);
826-
BlobWriteChannel channel = storage.writer(BLOB_INFO1, BLOB_WRITE_METAGENERATION,
827-
BLOB_WRITE_NOT_EXIST, BLOB_WRITE_PREDEFINED_ACL, BLOB_WRITE_CRC2C, BLOB_WRITE_MD5_HASH);
824+
BlobWriteChannel channel = storage.writer(info, BLOB_WRITE_METAGENERATION, BLOB_WRITE_NOT_EXIST,
825+
BLOB_WRITE_PREDEFINED_ACL, BLOB_WRITE_CRC2C, BLOB_WRITE_MD5_HASH);
828826
assertNotNull(channel);
829827
assertTrue(channel.isOpen());
830828
}

0 commit comments

Comments
 (0)