Skip to content

Commit

Permalink
Remove checks for contentType in CopyRequest
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@mziccard
mziccard committed Mar 19, 2016
1 parent 8840b33 commit 22636e2
Show file tree
Hide file tree
Showing 10 changed files with 288 additions and 139 deletions.
57 changes: 38 additions & 19 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ public class CopyWriter implements Restorable<CopyWriter> {
*
* @throws StorageException upon failure
*/
public BlobInfo result() {
public Blob result() {
while (!isDone()) {
copyChunk();
}
return BlobInfo.fromPb(rewriteResponse.result);
return Blob.fromPb(serviceOptions.service(), rewriteResponse.result);
}

/**
Expand Down Expand Up @@ -120,8 +120,12 @@ public RestorableState<CopyWriter> capture() {
serviceOptions,
BlobId.fromPb(rewriteResponse.rewriteRequest.source),
rewriteResponse.rewriteRequest.sourceOptions,
BlobInfo.fromPb(rewriteResponse.rewriteRequest.target),
BlobId.of(rewriteResponse.rewriteRequest.targetBucket,
rewriteResponse.rewriteRequest.targetName),
rewriteResponse.rewriteRequest.targetOptions)
.targetInfo(rewriteResponse.rewriteRequest.targetObject != null
? BlobInfo.fromPb(rewriteResponse.rewriteRequest.targetObject) : null)
.result(rewriteResponse.result != null ? BlobInfo.fromPb(rewriteResponse.result) : null)
.blobSize(blobSize())
.isDone(isDone())
.megabytesCopiedPerChunk(rewriteResponse.rewriteRequest.megabytesRewrittenPerCall)
Expand All @@ -132,12 +136,13 @@ public RestorableState<CopyWriter> capture() {

static class StateImpl implements RestorableState<CopyWriter>, Serializable {

private static final long serialVersionUID = 8279287678903181701L;
private static final long serialVersionUID = 1693964441435822700L;

private final StorageOptions serviceOptions;
private final BlobId source;
private final Map<StorageRpc.Option, ?> sourceOptions;
private final BlobInfo target;
private final BlobId targetId;
private final BlobInfo targetInfo;
private final Map<StorageRpc.Option, ?> targetOptions;
private final BlobInfo result;
private final long blobSize;
Expand All @@ -150,7 +155,8 @@ static class StateImpl implements RestorableState<CopyWriter>, Serializable {
this.serviceOptions = builder.serviceOptions;
this.source = builder.source;
this.sourceOptions = builder.sourceOptions;
this.target = builder.target;
this.targetId = builder.targetId;
this.targetInfo = builder.targetInfo;
this.targetOptions = builder.targetOptions;
this.result = builder.result;
this.blobSize = builder.blobSize;
Expand All @@ -165,8 +171,9 @@ static class Builder {
private final StorageOptions serviceOptions;
private final BlobId source;
private final Map<StorageRpc.Option, ?> sourceOptions;
private final BlobInfo target;
private final BlobId targetId;
private final Map<StorageRpc.Option, ?> targetOptions;
private BlobInfo targetInfo;
private BlobInfo result;
private long blobSize;
private boolean isDone;
Expand All @@ -176,14 +183,19 @@ static class Builder {

private Builder(StorageOptions options, BlobId source,
Map<StorageRpc.Option, ?> sourceOptions,
BlobInfo target, Map<StorageRpc.Option, ?> targetOptions) {
BlobId targetId, Map<StorageRpc.Option, ?> targetOptions) {
this.serviceOptions = options;
this.source = source;
this.sourceOptions = sourceOptions;
this.target = target;
this.targetId = targetId;
this.targetOptions = targetOptions;
}

Builder targetInfo(BlobInfo targetInfo) {
this.targetInfo = targetInfo;
return this;
}

Builder result(BlobInfo result) {
this.result = result;
return this;
Expand Down Expand Up @@ -220,15 +232,16 @@ RestorableState<CopyWriter> build() {
}

static Builder builder(StorageOptions options, BlobId source,
Map<StorageRpc.Option, ?> sourceOptions, BlobInfo target,
Map<StorageRpc.Option, ?> sourceOptions, BlobId targetId,
Map<StorageRpc.Option, ?> targetOptions) {
return new Builder(options, source, sourceOptions, target, targetOptions);
return new Builder(options, source, sourceOptions, targetId, targetOptions);
}

@Override
public CopyWriter restore() {
RewriteRequest rewriteRequest = new RewriteRequest(
source.toPb(), sourceOptions, target.toPb(), targetOptions, megabytesCopiedPerChunk);
RewriteRequest rewriteRequest = new RewriteRequest(source.toPb(), sourceOptions,
targetId.bucket(), targetId.name(), targetInfo != null ? targetInfo.toPb() : null,
targetOptions, megabytesCopiedPerChunk);
RewriteResponse rewriteResponse = new RewriteResponse(rewriteRequest,
result != null ? result.toPb() : null, blobSize, isDone, rewriteToken,
totalBytesCopied);
Expand All @@ -237,8 +250,9 @@ public CopyWriter restore() {

@Override
public int hashCode() {
return Objects.hash(serviceOptions, source, sourceOptions, target, targetOptions, result,
blobSize, isDone, megabytesCopiedPerChunk, rewriteToken, totalBytesCopied);
return Objects.hash(serviceOptions, source, sourceOptions, targetId, targetInfo,
targetOptions, result, blobSize, isDone, megabytesCopiedPerChunk, rewriteToken,
totalBytesCopied);
}

@Override
Expand All @@ -253,7 +267,8 @@ public boolean equals(Object obj) {
return Objects.equals(this.serviceOptions, other.serviceOptions)
&& Objects.equals(this.source, other.source)
&& Objects.equals(this.sourceOptions, other.sourceOptions)
&& Objects.equals(this.target, other.target)
&& Objects.equals(this.targetId, other.targetId)
&& Objects.equals(this.targetInfo, other.targetInfo)
&& Objects.equals(this.targetOptions, other.targetOptions)
&& Objects.equals(this.result, other.result)
&& Objects.equals(this.rewriteToken, other.rewriteToken)
Expand All @@ -267,10 +282,14 @@ public boolean equals(Object obj) {
public String toString() {
return MoreObjects.toStringHelper(this)
.add("source", source)
.add("target", target)
.add("isDone", isDone)
.add("totalBytesRewritten", totalBytesCopied)
.add("targetId", targetId)
.add("targetInfo", targetInfo)
.add("result", result)
.add("blobSize", blobSize)
.add("isDone", isDone)
.add("rewriteToken", rewriteToken)
.add("totalBytesCopied", totalBytesCopied)
.add("megabytesCopiedPerChunk", megabytesCopiedPerChunk)
.toString();
}
}
Expand Down
80 changes: 39 additions & 41 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,8 @@ class CopyRequest implements Serializable {

private final BlobId source;
private final List<BlobSourceOption> sourceOptions;
private final BlobInfo target;
private final BlobId targetId;
private final BlobInfo targetInfo;
private final List<BlobTargetOption> targetOptions;
private final Long megabytesCopiedPerChunk;

Expand All @@ -971,7 +972,8 @@ public static class Builder {
private final Set<BlobSourceOption> sourceOptions = new LinkedHashSet<>();
private final Set<BlobTargetOption> targetOptions = new LinkedHashSet<>();
private BlobId source;
private BlobInfo target;
private BlobId targetId;
private BlobInfo targetInfo;
private Long megabytesCopiedPerChunk;

/**
Expand Down Expand Up @@ -1019,39 +1021,37 @@ public Builder sourceOptions(Iterable<BlobSourceOption> options) {
*
* @return the builder
*/
public Builder target(BlobId target) {
this.target = BlobInfo.builder(target).build();
public Builder target(BlobId targetId) {
this.targetId = targetId;
return this;
}

/**
* Sets the copy target and target options. {@code target} parameter is used to override
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code
* target.contentType} is a required field.
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
* information is set exactly to {@code target}, no information is inherited from the source
* blob. If not set, target blob information is inherited from the source blob.
*
* @return the builder
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
*/
public Builder target(BlobInfo target, BlobTargetOption... options)
throws IllegalArgumentException {
checkContentType(target);
this.target = target;
public Builder target(BlobInfo targetInfo, BlobTargetOption... options) {
this.targetId = targetInfo.blobId();
this.targetInfo = targetInfo;
Collections.addAll(targetOptions, options);
return this;
}

/**
* Sets the copy target and target options. {@code target} parameter is used to override
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code
* target.contentType} is a required field.
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
* information is set exactly to {@code target}, no information is inherited from the source
* blob. If not set, target blob information is inherited from the source blob.
*
* @return the builder
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
*/
public Builder target(BlobInfo target, Iterable<BlobTargetOption> options)
throws IllegalArgumentException {
checkContentType(target);
this.target = target;
public Builder target(BlobInfo targetInfo, Iterable<BlobTargetOption> options) {
this.targetId = targetInfo.blobId();
this.targetInfo = targetInfo;
Iterables.addAll(targetOptions, options);
return this;
}
Expand All @@ -1072,16 +1072,15 @@ public Builder megabytesCopiedPerChunk(Long megabytesCopiedPerChunk) {
* Creates a {@code CopyRequest} object.
*/
public CopyRequest build() {
checkNotNull(source);
checkNotNull(target);
return new CopyRequest(this);
}
}

private CopyRequest(Builder builder) {
source = checkNotNull(builder.source);
sourceOptions = ImmutableList.copyOf(builder.sourceOptions);
target = checkNotNull(builder.target);
targetId = checkNotNull(builder.targetId);
targetInfo = builder.targetInfo;
targetOptions = ImmutableList.copyOf(builder.targetOptions);
megabytesCopiedPerChunk = builder.megabytesCopiedPerChunk;
}
Expand All @@ -1101,10 +1100,20 @@ public List<BlobSourceOption> sourceOptions() {
}

/**
* Returns the {@link BlobInfo} for the target blob.
* Returns the {@link BlobId} for the target blob.
*/
public BlobInfo target() {
return target;
public BlobId targetId() {
return targetId;
}

/**
* Returns the {@link BlobInfo} for the target blob. If set, this value is used to replace
* source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob
* information is set exactly to this value, no information is inherited from the source blob.
* If not set, target blob information is inherited from the source blob.
*/
public BlobInfo targetInfo() {
return targetInfo;
}

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

/**
* Creates a copy request. {@code target} parameter is used to override source blob information
* (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required
* field.
* (e.g. {@code contentType}, {@code contentLanguage}).
*
* @param sourceBucket name of the bucket containing the source blob
* @param sourceBlob name of the source blob
* @param target a {@code BlobInfo} object for the target blob
* @return a copy request
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
*/
public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target)
throws IllegalArgumentException {
checkContentType(target);
public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target) {
return builder().source(sourceBucket, sourceBlob).target(target).build();
}

/**
* Creates a copy request. {@code target} parameter is used to override source blob information
* (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required
* field.
* Creates a copy request. {@code target} parameter is used to replace source blob information
* (e.g. {@code contentType}, {@code contentLanguage}). Target blob information is set exactly
* to {@code target}, no information is inherited from the source blob.
*
* @param sourceBlobId a {@code BlobId} object for the source blob
* @param target a {@code BlobInfo} object for the target blob
* @return a copy request
* @throws IllegalArgumentException if {@code target.contentType} is {@code null}
*/
public static CopyRequest of(BlobId sourceBlobId, BlobInfo target)
throws IllegalArgumentException {
checkContentType(target);
public static CopyRequest of(BlobId sourceBlobId, BlobInfo target) {
return builder().source(sourceBlobId).target(target).build();
}

Expand Down Expand Up @@ -1214,10 +1216,6 @@ public static CopyRequest of(BlobId sourceBlobId, BlobId targetBlobId) {
public static Builder builder() {
return new Builder();
}

private static void checkContentType(BlobInfo blobInfo) throws IllegalArgumentException {
checkArgument(blobInfo.contentType() != null, "Blob content type can not be null");
}
}

/**
Expand Down
15 changes: 10 additions & 5 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,15 +413,20 @@ public CopyWriter copy(final CopyRequest copyRequest) {
final StorageObject source = copyRequest.source().toPb();
final Map<StorageRpc.Option, ?> sourceOptions =
optionMap(copyRequest.source().generation(), null, copyRequest.sourceOptions(), true);
final StorageObject target = copyRequest.target().toPb();
final Map<StorageRpc.Option, ?> targetOptions = optionMap(copyRequest.target().generation(),
copyRequest.target().metageneration(), copyRequest.targetOptions());
final BlobId targetId = copyRequest.targetId();
final StorageObject targetObject =
copyRequest.targetInfo() != null ? copyRequest.targetInfo().toPb() : null;
final Map<StorageRpc.Option, ?> targetOptions = optionMap(
copyRequest.targetInfo() != null ? copyRequest.targetInfo().generation() : null,
copyRequest.targetInfo() != null ? copyRequest.targetInfo().metageneration() : null,
copyRequest.targetOptions());
try {
RewriteResponse rewriteResponse = runWithRetries(new Callable<RewriteResponse>() {
@Override
public RewriteResponse call() {
return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions, target,
targetOptions, copyRequest.megabytesCopiedPerChunk()));
return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions,
targetId.bucket(), targetId.name(), targetObject, targetOptions,
copyRequest.megabytesCopiedPerChunk()));
}
}, options().retryParams(), EXCEPTION_HANDLER);
return new CopyWriter(options(), rewriteResponse);
Expand Down
4 changes: 2 additions & 2 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ private RewriteResponse rewrite(RewriteRequest req, String token) {
Long maxBytesRewrittenPerCall = req.megabytesRewrittenPerCall != null
? req.megabytesRewrittenPerCall * MEGABYTE : null;
com.google.api.services.storage.model.RewriteResponse rewriteResponse = storage.objects()
.rewrite(req.source.getBucket(), req.source.getName(), req.target.getBucket(),
req.target.getName(), req.target.getContentType() != null ? req.target : null)
.rewrite(req.source.getBucket(), req.source.getName(), req.targetBucket,
req.targetName, req.targetObject)
.setSourceGeneration(req.source.getGeneration())
.setRewriteToken(token)
.setMaxBytesRewrittenPerCall(maxBytesRewrittenPerCall)
Expand Down
19 changes: 13 additions & 6 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,20 @@ class RewriteRequest {

public final StorageObject source;
public final Map<StorageRpc.Option, ?> sourceOptions;
public final StorageObject target;
public final String targetBucket;
public final String targetName;
public final StorageObject targetObject;
public final Map<StorageRpc.Option, ?> targetOptions;
public final Long megabytesRewrittenPerCall;

public RewriteRequest(StorageObject source, Map<StorageRpc.Option, ?> sourceOptions,
StorageObject target, Map<StorageRpc.Option, ?> targetOptions,
Long megabytesRewrittenPerCall) {
String targetBucket, String targetName, StorageObject targetObject,
Map<StorageRpc.Option, ?> targetOptions, Long megabytesRewrittenPerCall) {
this.source = source;
this.sourceOptions = sourceOptions;
this.target = target;
this.targetBucket = targetBucket;
this.targetName = targetName;
this.targetObject = targetObject;
this.targetOptions = targetOptions;
this.megabytesRewrittenPerCall = megabytesRewrittenPerCall;
}
Expand All @@ -163,14 +167,17 @@ public boolean equals(Object obj) {
final RewriteRequest other = (RewriteRequest) obj;
return Objects.equals(this.source, other.source)
&& Objects.equals(this.sourceOptions, other.sourceOptions)
&& Objects.equals(this.target, other.target)
&& Objects.equals(this.targetBucket, other.targetBucket)
&& Objects.equals(this.targetName, other.targetName)
&& Objects.equals(this.targetObject, other.targetObject)
&& Objects.equals(this.targetOptions, other.targetOptions)
&& Objects.equals(this.megabytesRewrittenPerCall, other.megabytesRewrittenPerCall);
}

@Override
public int hashCode() {
return Objects.hash(source, sourceOptions, target, targetOptions, megabytesRewrittenPerCall);
return Objects.hash(source, sourceOptions, targetBucket, targetName, targetObject,
targetOptions, megabytesRewrittenPerCall);
}
}

Expand Down
Loading

0 comments on commit 22636e2

Please sign in to comment.