Skip to content

Commit 3e5db7a

Browse files
mziccardmziccard
committed
Refactor CopyRequest: remove targetId, add boolean overrideInfo
1 parent 22636e2 commit 3e5db7a

9 files changed

Lines changed: 102 additions & 111 deletions

File tree

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

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,9 @@ public RestorableState<CopyWriter> capture() {
120120
serviceOptions,
121121
BlobId.fromPb(rewriteResponse.rewriteRequest.source),
122122
rewriteResponse.rewriteRequest.sourceOptions,
123-
BlobId.of(rewriteResponse.rewriteRequest.targetBucket,
124-
rewriteResponse.rewriteRequest.targetName),
123+
rewriteResponse.rewriteRequest.overrideInfo,
124+
BlobInfo.fromPb(rewriteResponse.rewriteRequest.target),
125125
rewriteResponse.rewriteRequest.targetOptions)
126-
.targetInfo(rewriteResponse.rewriteRequest.targetObject != null
127-
? BlobInfo.fromPb(rewriteResponse.rewriteRequest.targetObject) : null)
128126
.result(rewriteResponse.result != null ? BlobInfo.fromPb(rewriteResponse.result) : null)
129127
.blobSize(blobSize())
130128
.isDone(isDone())
@@ -141,8 +139,8 @@ static class StateImpl implements RestorableState<CopyWriter>, Serializable {
141139
private final StorageOptions serviceOptions;
142140
private final BlobId source;
143141
private final Map<StorageRpc.Option, ?> sourceOptions;
144-
private final BlobId targetId;
145-
private final BlobInfo targetInfo;
142+
private final boolean overrideInfo;
143+
private final BlobInfo target;
146144
private final Map<StorageRpc.Option, ?> targetOptions;
147145
private final BlobInfo result;
148146
private final long blobSize;
@@ -155,8 +153,8 @@ static class StateImpl implements RestorableState<CopyWriter>, Serializable {
155153
this.serviceOptions = builder.serviceOptions;
156154
this.source = builder.source;
157155
this.sourceOptions = builder.sourceOptions;
158-
this.targetId = builder.targetId;
159-
this.targetInfo = builder.targetInfo;
156+
this.overrideInfo = builder.overrideInfo;
157+
this.target = builder.target;
160158
this.targetOptions = builder.targetOptions;
161159
this.result = builder.result;
162160
this.blobSize = builder.blobSize;
@@ -171,9 +169,9 @@ static class Builder {
171169
private final StorageOptions serviceOptions;
172170
private final BlobId source;
173171
private final Map<StorageRpc.Option, ?> sourceOptions;
174-
private final BlobId targetId;
172+
private final boolean overrideInfo;
173+
private BlobInfo target;
175174
private final Map<StorageRpc.Option, ?> targetOptions;
176-
private BlobInfo targetInfo;
177175
private BlobInfo result;
178176
private long blobSize;
179177
private boolean isDone;
@@ -182,20 +180,16 @@ static class Builder {
182180
private Long megabytesCopiedPerChunk;
183181

184182
private Builder(StorageOptions options, BlobId source,
185-
Map<StorageRpc.Option, ?> sourceOptions,
186-
BlobId targetId, Map<StorageRpc.Option, ?> targetOptions) {
183+
Map<StorageRpc.Option, ?> sourceOptions, boolean overrideInfo, BlobInfo target,
184+
Map<StorageRpc.Option, ?> targetOptions) {
187185
this.serviceOptions = options;
188186
this.source = source;
189187
this.sourceOptions = sourceOptions;
190-
this.targetId = targetId;
188+
this.overrideInfo = overrideInfo;
189+
this.target = target;
191190
this.targetOptions = targetOptions;
192191
}
193192

194-
Builder targetInfo(BlobInfo targetInfo) {
195-
this.targetInfo = targetInfo;
196-
return this;
197-
}
198-
199193
Builder result(BlobInfo result) {
200194
this.result = result;
201195
return this;
@@ -232,16 +226,15 @@ RestorableState<CopyWriter> build() {
232226
}
233227

234228
static Builder builder(StorageOptions options, BlobId source,
235-
Map<StorageRpc.Option, ?> sourceOptions, BlobId targetId,
229+
Map<StorageRpc.Option, ?> sourceOptions, boolean overrideInfo, BlobInfo target,
236230
Map<StorageRpc.Option, ?> targetOptions) {
237-
return new Builder(options, source, sourceOptions, targetId, targetOptions);
231+
return new Builder(options, source, sourceOptions, overrideInfo, target, targetOptions);
238232
}
239233

240234
@Override
241235
public CopyWriter restore() {
242236
RewriteRequest rewriteRequest = new RewriteRequest(source.toPb(), sourceOptions,
243-
targetId.bucket(), targetId.name(), targetInfo != null ? targetInfo.toPb() : null,
244-
targetOptions, megabytesCopiedPerChunk);
237+
overrideInfo, target.toPb(), targetOptions, megabytesCopiedPerChunk);
245238
RewriteResponse rewriteResponse = new RewriteResponse(rewriteRequest,
246239
result != null ? result.toPb() : null, blobSize, isDone, rewriteToken,
247240
totalBytesCopied);
@@ -250,7 +243,7 @@ public CopyWriter restore() {
250243

251244
@Override
252245
public int hashCode() {
253-
return Objects.hash(serviceOptions, source, sourceOptions, targetId, targetInfo,
246+
return Objects.hash(serviceOptions, source, sourceOptions, overrideInfo, target,
254247
targetOptions, result, blobSize, isDone, megabytesCopiedPerChunk, rewriteToken,
255248
totalBytesCopied);
256249
}
@@ -267,8 +260,8 @@ public boolean equals(Object obj) {
267260
return Objects.equals(this.serviceOptions, other.serviceOptions)
268261
&& Objects.equals(this.source, other.source)
269262
&& Objects.equals(this.sourceOptions, other.sourceOptions)
270-
&& Objects.equals(this.targetId, other.targetId)
271-
&& Objects.equals(this.targetInfo, other.targetInfo)
263+
&& Objects.equals(this.overrideInfo, other.overrideInfo)
264+
&& Objects.equals(this.target, other.target)
272265
&& Objects.equals(this.targetOptions, other.targetOptions)
273266
&& Objects.equals(this.result, other.result)
274267
&& Objects.equals(this.rewriteToken, other.rewriteToken)
@@ -282,8 +275,8 @@ public boolean equals(Object obj) {
282275
public String toString() {
283276
return MoreObjects.toStringHelper(this)
284277
.add("source", source)
285-
.add("targetId", targetId)
286-
.add("targetInfo", targetInfo)
278+
.add("overrideInfo", overrideInfo)
279+
.add("target", target)
287280
.add("result", result)
288281
.add("blobSize", blobSize)
289282
.add("isDone", isDone)

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

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

963963
private final BlobId source;
964964
private final List<BlobSourceOption> sourceOptions;
965-
private final BlobId targetId;
966-
private final BlobInfo targetInfo;
965+
private final boolean overrideInfo;
966+
private final BlobInfo target;
967967
private final List<BlobTargetOption> targetOptions;
968968
private final Long megabytesCopiedPerChunk;
969969

@@ -972,8 +972,8 @@ public static class Builder {
972972
private final Set<BlobSourceOption> sourceOptions = new LinkedHashSet<>();
973973
private final Set<BlobTargetOption> targetOptions = new LinkedHashSet<>();
974974
private BlobId source;
975-
private BlobId targetId;
976-
private BlobInfo targetInfo;
975+
private boolean overrideInfo;
976+
private BlobInfo target;
977977
private Long megabytesCopiedPerChunk;
978978

979979
/**
@@ -1022,21 +1022,22 @@ public Builder sourceOptions(Iterable<BlobSourceOption> options) {
10221022
* @return the builder
10231023
*/
10241024
public Builder target(BlobId targetId) {
1025-
this.targetId = targetId;
1025+
this.overrideInfo = false;
1026+
this.target = BlobInfo.builder(targetId).build();
10261027
return this;
10271028
}
10281029

10291030
/**
10301031
* Sets the copy target and target options. {@code target} parameter is used to override
10311032
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
10321033
* 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.
1034+
* blob.
10341035
*
10351036
* @return the builder
10361037
*/
1037-
public Builder target(BlobInfo targetInfo, BlobTargetOption... options) {
1038-
this.targetId = targetInfo.blobId();
1039-
this.targetInfo = targetInfo;
1038+
public Builder target(BlobInfo target, BlobTargetOption... options) {
1039+
this.overrideInfo = true;
1040+
this.target = checkNotNull(target);
10401041
Collections.addAll(targetOptions, options);
10411042
return this;
10421043
}
@@ -1045,13 +1046,13 @@ public Builder target(BlobInfo targetInfo, BlobTargetOption... options) {
10451046
* Sets the copy target and target options. {@code target} parameter is used to override
10461047
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
10471048
* 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.
1049+
* blob.
10491050
*
10501051
* @return the builder
10511052
*/
1052-
public Builder target(BlobInfo targetInfo, Iterable<BlobTargetOption> options) {
1053-
this.targetId = targetInfo.blobId();
1054-
this.targetInfo = targetInfo;
1053+
public Builder target(BlobInfo target, Iterable<BlobTargetOption> options) {
1054+
this.overrideInfo = true;
1055+
this.target = checkNotNull(target);
10551056
Iterables.addAll(targetOptions, options);
10561057
return this;
10571058
}
@@ -1079,8 +1080,8 @@ public CopyRequest build() {
10791080
private CopyRequest(Builder builder) {
10801081
source = checkNotNull(builder.source);
10811082
sourceOptions = ImmutableList.copyOf(builder.sourceOptions);
1082-
targetId = checkNotNull(builder.targetId);
1083-
targetInfo = builder.targetInfo;
1083+
overrideInfo = builder.overrideInfo;
1084+
target = checkNotNull(builder.target);
10841085
targetOptions = ImmutableList.copyOf(builder.targetOptions);
10851086
megabytesCopiedPerChunk = builder.megabytesCopiedPerChunk;
10861087
}
@@ -1100,20 +1101,21 @@ public List<BlobSourceOption> sourceOptions() {
11001101
}
11011102

11021103
/**
1103-
* Returns the {@link BlobId} for the target blob.
1104+
* Returns the {@link BlobInfo} for the target blob.
11041105
*/
1105-
public BlobId targetId() {
1106-
return targetId;
1106+
public BlobInfo target() {
1107+
return target;
11071108
}
11081109

11091110
/**
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.
1111+
* Returns whether to override the target blob information with {@link #target()}.
1112+
* If {@code true}, the value of {@link #target()} is used to replace source blob information
1113+
* (e.g. {@code contentType}, {@code contentLanguage}). Target blob information is set exactly
1114+
* to this value, no information is inherited from the source blob. If {@code false}, target
1115+
* blob information is inherited from the source blob.
11141116
*/
1115-
public BlobInfo targetInfo() {
1116-
return targetInfo;
1117+
public boolean overrideInfo() {
1118+
return overrideInfo;
11171119
}
11181120

11191121
/**

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -413,19 +413,15 @@ 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 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());
416+
final StorageObject targetObject = copyRequest.target().toPb();
417+
final Map<StorageRpc.Option, ?> targetOptions = optionMap(copyRequest.target().generation(),
418+
copyRequest.target().metageneration(), copyRequest.targetOptions());
423419
try {
424420
RewriteResponse rewriteResponse = runWithRetries(new Callable<RewriteResponse>() {
425421
@Override
426422
public RewriteResponse call() {
427423
return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions,
428-
targetId.bucket(), targetId.name(), targetObject, targetOptions,
424+
copyRequest.overrideInfo(), targetObject, targetOptions,
429425
copyRequest.megabytesCopiedPerChunk()));
430426
}
431427
}, options().retryParams(), EXCEPTION_HANDLER);

‎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.targetBucket,
584-
req.targetName, req.targetObject)
583+
.rewrite(req.source.getBucket(), req.source.getName(), req.target.getBucket(),
584+
req.target.getName(), req.overrideInfo ? req.target : null)
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: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,20 +138,18 @@ class RewriteRequest {
138138

139139
public final StorageObject source;
140140
public final Map<StorageRpc.Option, ?> sourceOptions;
141-
public final String targetBucket;
142-
public final String targetName;
143-
public final StorageObject targetObject;
141+
public final boolean overrideInfo;
142+
public final StorageObject target;
144143
public final Map<StorageRpc.Option, ?> targetOptions;
145144
public final Long megabytesRewrittenPerCall;
146145

147146
public RewriteRequest(StorageObject source, Map<StorageRpc.Option, ?> sourceOptions,
148-
String targetBucket, String targetName, StorageObject targetObject,
149-
Map<StorageRpc.Option, ?> targetOptions, Long megabytesRewrittenPerCall) {
147+
boolean overrideInfo, StorageObject target, Map<StorageRpc.Option, ?> targetOptions,
148+
Long megabytesRewrittenPerCall) {
150149
this.source = source;
151150
this.sourceOptions = sourceOptions;
152-
this.targetBucket = targetBucket;
153-
this.targetName = targetName;
154-
this.targetObject = targetObject;
151+
this.overrideInfo = overrideInfo;
152+
this.target = target;
155153
this.targetOptions = targetOptions;
156154
this.megabytesRewrittenPerCall = megabytesRewrittenPerCall;
157155
}
@@ -167,17 +165,16 @@ public boolean equals(Object obj) {
167165
final RewriteRequest other = (RewriteRequest) obj;
168166
return Objects.equals(this.source, other.source)
169167
&& Objects.equals(this.sourceOptions, other.sourceOptions)
170-
&& Objects.equals(this.targetBucket, other.targetBucket)
171-
&& Objects.equals(this.targetName, other.targetName)
172-
&& Objects.equals(this.targetObject, other.targetObject)
168+
&& Objects.equals(this.overrideInfo, other.overrideInfo)
169+
&& Objects.equals(this.target, other.target)
173170
&& Objects.equals(this.targetOptions, other.targetOptions)
174171
&& Objects.equals(this.megabytesRewrittenPerCall, other.megabytesRewrittenPerCall);
175172
}
176173

177174
@Override
178175
public int hashCode() {
179-
return Objects.hash(source, sourceOptions, targetBucket, targetName, targetObject,
180-
targetOptions, megabytesRewrittenPerCall);
176+
return Objects.hash(source, sourceOptions, overrideInfo, target, targetOptions,
177+
megabytesRewrittenPerCall);
181178
}
182179
}
183180

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ public void testDelete() throws Exception {
222222
@Test
223223
public void testCopyToBucket() throws Exception {
224224
initializeExpectedBlob(2);
225+
BlobInfo target = BlobInfo.builder(BlobId.of("bt", "n")).build();
225226
CopyWriter copyWriter = createMock(CopyWriter.class);
226227
Capture<CopyRequest> capturedCopyRequest = Capture.newInstance();
227228
expect(storage.options()).andReturn(mockOptions);
@@ -231,15 +232,16 @@ public void testCopyToBucket() throws Exception {
231232
CopyWriter returnedCopyWriter = blob.copyTo("bt");
232233
assertEquals(copyWriter, returnedCopyWriter);
233234
assertEquals(capturedCopyRequest.getValue().source(), blob.blobId());
234-
assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "n"));
235-
assertNull(capturedCopyRequest.getValue().targetInfo());
235+
assertEquals(capturedCopyRequest.getValue().target(), target);
236+
assertFalse(capturedCopyRequest.getValue().overrideInfo());
236237
assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty());
237238
assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty());
238239
}
239240

240241
@Test
241242
public void testCopyTo() throws Exception {
242243
initializeExpectedBlob(2);
244+
BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build();
243245
CopyWriter copyWriter = createMock(CopyWriter.class);
244246
Capture<CopyRequest> capturedCopyRequest = Capture.newInstance();
245247
expect(storage.options()).andReturn(mockOptions);
@@ -249,15 +251,16 @@ public void testCopyTo() throws Exception {
249251
CopyWriter returnedCopyWriter = blob.copyTo("bt", "nt");
250252
assertEquals(copyWriter, returnedCopyWriter);
251253
assertEquals(capturedCopyRequest.getValue().source(), blob.blobId());
252-
assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "nt"));
253-
assertNull(capturedCopyRequest.getValue().targetInfo());
254+
assertEquals(capturedCopyRequest.getValue().target(), target);
255+
assertFalse(capturedCopyRequest.getValue().overrideInfo());
254256
assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty());
255257
assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty());
256258
}
257259

258260
@Test
259261
public void testCopyToBlobId() throws Exception {
260262
initializeExpectedBlob(2);
263+
BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build();
261264
BlobId targetId = BlobId.of("bt", "nt");
262265
CopyWriter copyWriter = createMock(CopyWriter.class);
263266
Capture<CopyRequest> capturedCopyRequest = Capture.newInstance();
@@ -268,8 +271,8 @@ public void testCopyToBlobId() throws Exception {
268271
CopyWriter returnedCopyWriter = blob.copyTo(targetId);
269272
assertEquals(copyWriter, returnedCopyWriter);
270273
assertEquals(capturedCopyRequest.getValue().source(), blob.blobId());
271-
assertEquals(capturedCopyRequest.getValue().targetId(), targetId);
272-
assertNull(capturedCopyRequest.getValue().targetInfo());
274+
assertEquals(capturedCopyRequest.getValue().target(), target);
275+
assertFalse(capturedCopyRequest.getValue().overrideInfo());
273276
assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty());
274277
assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty());
275278
}

0 commit comments

Comments
 (0)