Skip to content

Commit 22636e2

Browse files
mziccardmziccard
committed
Remove checks for contentType in CopyRequest
- update CopyRequest to hold both targetId and targetInfo - avoid sending storage object in StorageRpc.rewrite when only targetId is set - refactor CopyWriter and state - add integration tests
1 parent 8840b33 commit 22636e2

File tree

10 files changed

+288
-139
lines changed

10 files changed

+288
-139
lines changed

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

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@ public class CopyWriter implements Restorable<CopyWriter> {
6565
*
6666
* @throws StorageException upon failure
6767
*/
68-
public BlobInfo result() {
68+
public Blob result() {
6969
while (!isDone()) {
7070
copyChunk();
7171
}
72-
return BlobInfo.fromPb(rewriteResponse.result);
72+
return Blob.fromPb(serviceOptions.service(), rewriteResponse.result);
7373
}
7474

7575
/**
@@ -120,8 +120,12 @@ public RestorableState<CopyWriter> capture() {
120120
serviceOptions,
121121
BlobId.fromPb(rewriteResponse.rewriteRequest.source),
122122
rewriteResponse.rewriteRequest.sourceOptions,
123-
BlobInfo.fromPb(rewriteResponse.rewriteRequest.target),
123+
BlobId.of(rewriteResponse.rewriteRequest.targetBucket,
124+
rewriteResponse.rewriteRequest.targetName),
124125
rewriteResponse.rewriteRequest.targetOptions)
126+
.targetInfo(rewriteResponse.rewriteRequest.targetObject != null
127+
? BlobInfo.fromPb(rewriteResponse.rewriteRequest.targetObject) : null)
128+
.result(rewriteResponse.result != null ? BlobInfo.fromPb(rewriteResponse.result) : null)
125129
.blobSize(blobSize())
126130
.isDone(isDone())
127131
.megabytesCopiedPerChunk(rewriteResponse.rewriteRequest.megabytesRewrittenPerCall)
@@ -132,12 +136,13 @@ public RestorableState<CopyWriter> capture() {
132136

133137
static class StateImpl implements RestorableState<CopyWriter>, Serializable {
134138

135-
private static final long serialVersionUID = 8279287678903181701L;
139+
private static final long serialVersionUID = 1693964441435822700L;
136140

137141
private final StorageOptions serviceOptions;
138142
private final BlobId source;
139143
private final Map<StorageRpc.Option, ?> sourceOptions;
140-
private final BlobInfo target;
144+
private final BlobId targetId;
145+
private final BlobInfo targetInfo;
141146
private final Map<StorageRpc.Option, ?> targetOptions;
142147
private final BlobInfo result;
143148
private final long blobSize;
@@ -150,7 +155,8 @@ static class StateImpl implements RestorableState<CopyWriter>, Serializable {
150155
this.serviceOptions = builder.serviceOptions;
151156
this.source = builder.source;
152157
this.sourceOptions = builder.sourceOptions;
153-
this.target = builder.target;
158+
this.targetId = builder.targetId;
159+
this.targetInfo = builder.targetInfo;
154160
this.targetOptions = builder.targetOptions;
155161
this.result = builder.result;
156162
this.blobSize = builder.blobSize;
@@ -165,8 +171,9 @@ static class Builder {
165171
private final StorageOptions serviceOptions;
166172
private final BlobId source;
167173
private final Map<StorageRpc.Option, ?> sourceOptions;
168-
private final BlobInfo target;
174+
private final BlobId targetId;
169175
private final Map<StorageRpc.Option, ?> targetOptions;
176+
private BlobInfo targetInfo;
170177
private BlobInfo result;
171178
private long blobSize;
172179
private boolean isDone;
@@ -176,14 +183,19 @@ static class Builder {
176183

177184
private Builder(StorageOptions options, BlobId source,
178185
Map<StorageRpc.Option, ?> sourceOptions,
179-
BlobInfo target, Map<StorageRpc.Option, ?> targetOptions) {
186+
BlobId targetId, Map<StorageRpc.Option, ?> targetOptions) {
180187
this.serviceOptions = options;
181188
this.source = source;
182189
this.sourceOptions = sourceOptions;
183-
this.target = target;
190+
this.targetId = targetId;
184191
this.targetOptions = targetOptions;
185192
}
186193

194+
Builder targetInfo(BlobInfo targetInfo) {
195+
this.targetInfo = targetInfo;
196+
return this;
197+
}
198+
187199
Builder result(BlobInfo result) {
188200
this.result = result;
189201
return this;
@@ -220,15 +232,16 @@ RestorableState<CopyWriter> build() {
220232
}
221233

222234
static Builder builder(StorageOptions options, BlobId source,
223-
Map<StorageRpc.Option, ?> sourceOptions, BlobInfo target,
235+
Map<StorageRpc.Option, ?> sourceOptions, BlobId targetId,
224236
Map<StorageRpc.Option, ?> targetOptions) {
225-
return new Builder(options, source, sourceOptions, target, targetOptions);
237+
return new Builder(options, source, sourceOptions, targetId, targetOptions);
226238
}
227239

228240
@Override
229241
public CopyWriter restore() {
230-
RewriteRequest rewriteRequest = new RewriteRequest(
231-
source.toPb(), sourceOptions, target.toPb(), targetOptions, megabytesCopiedPerChunk);
242+
RewriteRequest rewriteRequest = new RewriteRequest(source.toPb(), sourceOptions,
243+
targetId.bucket(), targetId.name(), targetInfo != null ? targetInfo.toPb() : null,
244+
targetOptions, megabytesCopiedPerChunk);
232245
RewriteResponse rewriteResponse = new RewriteResponse(rewriteRequest,
233246
result != null ? result.toPb() : null, blobSize, isDone, rewriteToken,
234247
totalBytesCopied);
@@ -237,8 +250,9 @@ public CopyWriter restore() {
237250

238251
@Override
239252
public int hashCode() {
240-
return Objects.hash(serviceOptions, source, sourceOptions, target, targetOptions, result,
241-
blobSize, isDone, megabytesCopiedPerChunk, rewriteToken, totalBytesCopied);
253+
return Objects.hash(serviceOptions, source, sourceOptions, targetId, targetInfo,
254+
targetOptions, result, blobSize, isDone, megabytesCopiedPerChunk, rewriteToken,
255+
totalBytesCopied);
242256
}
243257

244258
@Override
@@ -253,7 +267,8 @@ public boolean equals(Object obj) {
253267
return Objects.equals(this.serviceOptions, other.serviceOptions)
254268
&& Objects.equals(this.source, other.source)
255269
&& Objects.equals(this.sourceOptions, other.sourceOptions)
256-
&& Objects.equals(this.target, other.target)
270+
&& Objects.equals(this.targetId, other.targetId)
271+
&& Objects.equals(this.targetInfo, other.targetInfo)
257272
&& Objects.equals(this.targetOptions, other.targetOptions)
258273
&& Objects.equals(this.result, other.result)
259274
&& Objects.equals(this.rewriteToken, other.rewriteToken)
@@ -267,10 +282,14 @@ public boolean equals(Object obj) {
267282
public String toString() {
268283
return MoreObjects.toStringHelper(this)
269284
.add("source", source)
270-
.add("target", target)
271-
.add("isDone", isDone)
272-
.add("totalBytesRewritten", totalBytesCopied)
285+
.add("targetId", targetId)
286+
.add("targetInfo", targetInfo)
287+
.add("result", result)
273288
.add("blobSize", blobSize)
289+
.add("isDone", isDone)
290+
.add("rewriteToken", rewriteToken)
291+
.add("totalBytesCopied", totalBytesCopied)
292+
.add("megabytesCopiedPerChunk", megabytesCopiedPerChunk)
274293
.toString();
275294
}
276295
}

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

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,8 @@ class CopyRequest implements Serializable {
962962

963963
private final BlobId source;
964964
private final List<BlobSourceOption> sourceOptions;
965-
private final BlobInfo target;
965+
private final BlobId targetId;
966+
private final BlobInfo targetInfo;
966967
private final List<BlobTargetOption> targetOptions;
967968
private final Long megabytesCopiedPerChunk;
968969

@@ -971,7 +972,8 @@ public static class Builder {
971972
private final Set<BlobSourceOption> sourceOptions = new LinkedHashSet<>();
972973
private final Set<BlobTargetOption> targetOptions = new LinkedHashSet<>();
973974
private BlobId source;
974-
private BlobInfo target;
975+
private BlobId targetId;
976+
private BlobInfo targetInfo;
975977
private Long megabytesCopiedPerChunk;
976978

977979
/**
@@ -1019,39 +1021,37 @@ public Builder sourceOptions(Iterable<BlobSourceOption> options) {
10191021
*
10201022
* @return the builder
10211023
*/
1022-
public Builder target(BlobId target) {
1023-
this.target = BlobInfo.builder(target).build();
1024+
public Builder target(BlobId targetId) {
1025+
this.targetId = targetId;
10241026
return this;
10251027
}
10261028

10271029
/**
10281030
* Sets the copy target and target options. {@code target} parameter is used to override
1029-
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code
1030-
* target.contentType} is a required field.
1031+
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
1032+
* information is set exactly to {@code target}, no information is inherited from the source
1033+
* blob. If not set, target blob information is inherited from the source blob.
10311034
*
10321035
* @return the builder
1033-
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
10341036
*/
1035-
public Builder target(BlobInfo target, BlobTargetOption... options)
1036-
throws IllegalArgumentException {
1037-
checkContentType(target);
1038-
this.target = target;
1037+
public Builder target(BlobInfo targetInfo, BlobTargetOption... options) {
1038+
this.targetId = targetInfo.blobId();
1039+
this.targetInfo = targetInfo;
10391040
Collections.addAll(targetOptions, options);
10401041
return this;
10411042
}
10421043

10431044
/**
10441045
* Sets the copy target and target options. {@code target} parameter is used to override
1045-
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code
1046-
* target.contentType} is a required field.
1046+
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
1047+
* information is set exactly to {@code target}, no information is inherited from the source
1048+
* blob. If not set, target blob information is inherited from the source blob.
10471049
*
10481050
* @return the builder
1049-
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
10501051
*/
1051-
public Builder target(BlobInfo target, Iterable<BlobTargetOption> options)
1052-
throws IllegalArgumentException {
1053-
checkContentType(target);
1054-
this.target = target;
1052+
public Builder target(BlobInfo targetInfo, Iterable<BlobTargetOption> options) {
1053+
this.targetId = targetInfo.blobId();
1054+
this.targetInfo = targetInfo;
10551055
Iterables.addAll(targetOptions, options);
10561056
return this;
10571057
}
@@ -1072,16 +1072,15 @@ public Builder megabytesCopiedPerChunk(Long megabytesCopiedPerChunk) {
10721072
* Creates a {@code CopyRequest} object.
10731073
*/
10741074
public CopyRequest build() {
1075-
checkNotNull(source);
1076-
checkNotNull(target);
10771075
return new CopyRequest(this);
10781076
}
10791077
}
10801078

10811079
private CopyRequest(Builder builder) {
10821080
source = checkNotNull(builder.source);
10831081
sourceOptions = ImmutableList.copyOf(builder.sourceOptions);
1084-
target = checkNotNull(builder.target);
1082+
targetId = checkNotNull(builder.targetId);
1083+
targetInfo = builder.targetInfo;
10851084
targetOptions = ImmutableList.copyOf(builder.targetOptions);
10861085
megabytesCopiedPerChunk = builder.megabytesCopiedPerChunk;
10871086
}
@@ -1101,10 +1100,20 @@ public List<BlobSourceOption> sourceOptions() {
11011100
}
11021101

11031102
/**
1104-
* Returns the {@link BlobInfo} for the target blob.
1103+
* Returns the {@link BlobId} for the target blob.
11051104
*/
1106-
public BlobInfo target() {
1107-
return target;
1105+
public BlobId targetId() {
1106+
return targetId;
1107+
}
1108+
1109+
/**
1110+
* Returns the {@link BlobInfo} for the target blob. If set, this value is used to replace
1111+
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
1112+
* information is set exactly to this value, no information is inherited from the source blob.
1113+
* If not set, target blob information is inherited from the source blob.
1114+
*/
1115+
public BlobInfo targetInfo() {
1116+
return targetInfo;
11081117
}
11091118

11101119
/**
@@ -1125,34 +1134,27 @@ public Long megabytesCopiedPerChunk() {
11251134

11261135
/**
11271136
* Creates a copy request. {@code target} parameter is used to override source blob information
1128-
* (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required
1129-
* field.
1137+
* (e.g. {@code contentType}, {@code contentLanguage}).
11301138
*
11311139
* @param sourceBucket name of the bucket containing the source blob
11321140
* @param sourceBlob name of the source blob
11331141
* @param target a {@code BlobInfo} object for the target blob
11341142
* @return a copy request
1135-
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
11361143
*/
1137-
public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target)
1138-
throws IllegalArgumentException {
1139-
checkContentType(target);
1144+
public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target) {
11401145
return builder().source(sourceBucket, sourceBlob).target(target).build();
11411146
}
11421147

11431148
/**
1144-
* Creates a copy request. {@code target} parameter is used to override source blob information
1145-
* (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required
1146-
* field.
1149+
* Creates a copy request. {@code target} parameter is used to replace source blob information
1150+
* (e.g. {@code contentType}, {@code contentLanguage}). Target blob information is set exactly
1151+
* to {@code target}, no information is inherited from the source blob.
11471152
*
11481153
* @param sourceBlobId a {@code BlobId} object for the source blob
11491154
* @param target a {@code BlobInfo} object for the target blob
11501155
* @return a copy request
1151-
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
11521156
*/
1153-
public static CopyRequest of(BlobId sourceBlobId, BlobInfo target)
1154-
throws IllegalArgumentException {
1155-
checkContentType(target);
1157+
public static CopyRequest of(BlobId sourceBlobId, BlobInfo target) {
11561158
return builder().source(sourceBlobId).target(target).build();
11571159
}
11581160

@@ -1214,10 +1216,6 @@ public static CopyRequest of(BlobId sourceBlobId, BlobId targetBlobId) {
12141216
public static Builder builder() {
12151217
return new Builder();
12161218
}
1217-
1218-
private static void checkContentType(BlobInfo blobInfo) throws IllegalArgumentException {
1219-
checkArgument(blobInfo.contentType() != null, "Blob content type can not be null");
1220-
}
12211219
}
12221220

12231221
/**

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,15 +413,20 @@ public CopyWriter copy(final CopyRequest copyRequest) {
413413
final StorageObject source = copyRequest.source().toPb();
414414
final Map<StorageRpc.Option, ?> sourceOptions =
415415
optionMap(copyRequest.source().generation(), null, copyRequest.sourceOptions(), true);
416-
final StorageObject target = copyRequest.target().toPb();
417-
final Map<StorageRpc.Option, ?> targetOptions = optionMap(copyRequest.target().generation(),
418-
copyRequest.target().metageneration(), copyRequest.targetOptions());
416+
final BlobId targetId = copyRequest.targetId();
417+
final StorageObject targetObject =
418+
copyRequest.targetInfo() != null ? copyRequest.targetInfo().toPb() : null;
419+
final Map<StorageRpc.Option, ?> targetOptions = optionMap(
420+
copyRequest.targetInfo() != null ? copyRequest.targetInfo().generation() : null,
421+
copyRequest.targetInfo() != null ? copyRequest.targetInfo().metageneration() : null,
422+
copyRequest.targetOptions());
419423
try {
420424
RewriteResponse rewriteResponse = runWithRetries(new Callable<RewriteResponse>() {
421425
@Override
422426
public RewriteResponse call() {
423-
return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions, target,
424-
targetOptions, copyRequest.megabytesCopiedPerChunk()));
427+
return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions,
428+
targetId.bucket(), targetId.name(), targetObject, targetOptions,
429+
copyRequest.megabytesCopiedPerChunk()));
425430
}
426431
}, options().retryParams(), EXCEPTION_HANDLER);
427432
return new CopyWriter(options(), rewriteResponse);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,8 @@ private RewriteResponse rewrite(RewriteRequest req, String token) {
580580
Long maxBytesRewrittenPerCall = req.megabytesRewrittenPerCall != null
581581
? req.megabytesRewrittenPerCall * MEGABYTE : null;
582582
com.google.api.services.storage.model.RewriteResponse rewriteResponse = storage.objects()
583-
.rewrite(req.source.getBucket(), req.source.getName(), req.target.getBucket(),
584-
req.target.getName(), req.target.getContentType() != null ? req.target : null)
583+
.rewrite(req.source.getBucket(), req.source.getName(), req.targetBucket,
584+
req.targetName, req.targetObject)
585585
.setSourceGeneration(req.source.getGeneration())
586586
.setRewriteToken(token)
587587
.setMaxBytesRewrittenPerCall(maxBytesRewrittenPerCall)

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,20 @@ class RewriteRequest {
138138

139139
public final StorageObject source;
140140
public final Map<StorageRpc.Option, ?> sourceOptions;
141-
public final StorageObject target;
141+
public final String targetBucket;
142+
public final String targetName;
143+
public final StorageObject targetObject;
142144
public final Map<StorageRpc.Option, ?> targetOptions;
143145
public final Long megabytesRewrittenPerCall;
144146

145147
public RewriteRequest(StorageObject source, Map<StorageRpc.Option, ?> sourceOptions,
146-
StorageObject target, Map<StorageRpc.Option, ?> targetOptions,
147-
Long megabytesRewrittenPerCall) {
148+
String targetBucket, String targetName, StorageObject targetObject,
149+
Map<StorageRpc.Option, ?> targetOptions, Long megabytesRewrittenPerCall) {
148150
this.source = source;
149151
this.sourceOptions = sourceOptions;
150-
this.target = target;
152+
this.targetBucket = targetBucket;
153+
this.targetName = targetName;
154+
this.targetObject = targetObject;
151155
this.targetOptions = targetOptions;
152156
this.megabytesRewrittenPerCall = megabytesRewrittenPerCall;
153157
}
@@ -163,14 +167,17 @@ public boolean equals(Object obj) {
163167
final RewriteRequest other = (RewriteRequest) obj;
164168
return Objects.equals(this.source, other.source)
165169
&& Objects.equals(this.sourceOptions, other.sourceOptions)
166-
&& Objects.equals(this.target, other.target)
170+
&& Objects.equals(this.targetBucket, other.targetBucket)
171+
&& Objects.equals(this.targetName, other.targetName)
172+
&& Objects.equals(this.targetObject, other.targetObject)
167173
&& Objects.equals(this.targetOptions, other.targetOptions)
168174
&& Objects.equals(this.megabytesRewrittenPerCall, other.megabytesRewrittenPerCall);
169175
}
170176

171177
@Override
172178
public int hashCode() {
173-
return Objects.hash(source, sourceOptions, target, targetOptions, megabytesRewrittenPerCall);
179+
return Objects.hash(source, sourceOptions, targetBucket, targetName, targetObject,
180+
targetOptions, megabytesRewrittenPerCall);
174181
}
175182
}
176183

0 commit comments

Comments
 (0)