Skip to content

Commit

Permalink
Compute Operation: remove creationTimestamp field and fix isDone() (#914
Browse files Browse the repository at this point in the history 
)

* Remove Compute Operation's creation timestamp field

* Compute's Operation.isDone() return true if operation does not exist
  • Loading branch information
@mziccard @aozarov
mziccard authored and aozarov committed Apr 13, 2016
1 parent b7f6b86 commit a6c68af
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 46 deletions.
1 change: 0 additions & 1 deletion gcloud-java-compute/src/main/java/com/google/gcloud/compute/Compute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ static String selector(LicenseField... fields) {
*/
enum OperationField {
CLIENT_OPERATION_ID("clientOperationId"),
CREATION_TIMESTAMP("creationTimestamp"),
DESCRIPTION("description"),
END_TIME("endTime"),
ERROR("error"),
Expand Down
38 changes: 7 additions & 31 deletions gcloud-java-compute/src/main/java/com/google/gcloud/compute/Operation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ public class Operation implements Serializable {
private final ComputeOptions options;
private final String id;
private final OperationId operationId;
private final Long creationTimestamp;
private final String clientOperationId;
private final String operationType;
private final String targetLink;
Expand Down Expand Up @@ -296,7 +295,6 @@ static final class Builder {

private Compute compute;
private String id;
private Long creationTimestamp;
private OperationId operationId;
private String clientOperationId;
private String operationType;
Expand Down Expand Up @@ -324,9 +322,6 @@ static final class Builder {
if (operationPb.getId() != null) {
id = operationPb.getId().toString();
}
if (operationPb.getCreationTimestamp() != null) {
creationTimestamp = TIMESTAMP_FORMATTER.parseMillis(operationPb.getCreationTimestamp());
}
if (RegionOperationId.matchesUrl(operationPb.getSelfLink())) {
operationId = RegionOperationId.fromUrl(operationPb.getSelfLink());
} else if (ZoneOperationId.matchesUrl(operationPb.getSelfLink())) {
Expand Down Expand Up @@ -372,11 +367,6 @@ Builder id(String id) {
return this;
}

Builder creationTimestamp(Long creationTimestamp) {
this.creationTimestamp = creationTimestamp;
return this;
}

Builder operationId(OperationId operationId) {
this.operationId = checkNotNull(operationId);
return this;
Expand Down Expand Up @@ -471,7 +461,6 @@ private Operation(Builder builder) {
this.compute = checkNotNull(builder.compute);
this.options = compute.options();
this.id = builder.id;
this.creationTimestamp = builder.creationTimestamp;
this.operationId = checkNotNull(builder.operationId);
this.clientOperationId = builder.clientOperationId;
this.operationType = builder.operationType;
Expand Down Expand Up @@ -505,13 +494,6 @@ public String id() {
return id;
}

/**
* Returns the creation timestamp in milliseconds since epoch.
*/
public Long creationTimestamp() {
return creationTimestamp;
}

/**
* Returns the operation's identity. This method returns an {@link GlobalOperationId} for global
* operations, a {@link RegionOperationId} for region operations and a {@link ZoneOperationId} for
Expand Down Expand Up @@ -658,23 +640,21 @@ public boolean exists() throws ComputeException {

/**
* Checks if this operation has completed its execution, either failing or succeeding. If the
* operation does not exist this method returns {@code false}. To correctly wait for operation's
* completion, check that the operation exists first using {@link #exists()}:
* operation does not exist this method returns {@code true}. You can wait for operation
* completion with:
* <pre> {@code
* if (operation.exists()) {
* while(!operation.isDone()) {
* Thread.sleep(1000L);
* }
* while(!operation.isDone()) {
* Thread.sleep(1000L);
* }}</pre>
*
* @return {@code true} if this operation is in {@link Operation.Status#DONE} state, {@code false}
* if the state is not {@link Operation.Status#DONE} or the operation does not exist
* @return {@code true} if this operation is in {@link Operation.Status#DONE} state or if it does
* not exist, {@code false} if the state is not {@link Operation.Status#DONE}
* @throws ComputeException upon failure
*/
public boolean isDone() throws ComputeException {
Operation operation =
compute.get(operationId, Compute.OperationOption.fields(Compute.OperationField.STATUS));
return operation != null && operation.status() == Status.DONE;
return operation == null || operation.status() == Status.DONE;
}

/**
Expand Down Expand Up @@ -705,7 +685,6 @@ public String toString() {
return MoreObjects.toStringHelper(this)
.add("id", id)
.add("operationsId", operationId)
.add("creationTimestamp", creationTimestamp)
.add("clientOperationId", clientOperationId)
.add("operationType", operationType)
.add("targetLink", targetLink)
Expand Down Expand Up @@ -743,9 +722,6 @@ com.google.api.services.compute.model.Operation toPb() {
if (id != null) {
operationPb.setId(new BigInteger(id));
}
if (creationTimestamp != null) {
operationPb.setCreationTimestamp(TIMESTAMP_FORMATTER.print(creationTimestamp));
}
operationPb.setName(operationId.operation());
operationPb.setClientOperationId(clientOperationId);
switch (operationId.type()) {
Expand Down
3 changes: 0 additions & 3 deletions gcloud-java-compute/src/test/java/com/google/gcloud/compute/ComputeImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,6 @@ public void setUp() {
Compute otherService = options.toBuilder().build().service();
globalOperation = new Operation.Builder(otherService)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(GLOBAL_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand All @@ -448,7 +447,6 @@ public void setUp() {
.build();
zoneOperation = new Operation.Builder(otherService)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(ZONE_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand All @@ -469,7 +467,6 @@ public void setUp() {
.build();
regionOperation = new Operation.Builder(otherService)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(REGION_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand Down
9 changes: 1 addition & 8 deletions gcloud-java-compute/src/test/java/com/google/gcloud/compute/OperationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ private void initializeExpectedOperation(int optionsCalls) {
replay(serviceMockReturnsOptions);
globalOperation = new Operation.Builder(serviceMockReturnsOptions)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(GLOBAL_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand All @@ -110,7 +109,6 @@ private void initializeExpectedOperation(int optionsCalls) {
.build();
zoneOperation = new Operation.Builder(serviceMockReturnsOptions)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(ZONE_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand All @@ -131,7 +129,6 @@ private void initializeExpectedOperation(int optionsCalls) {
.build();
regionOperation = new Operation.Builder(serviceMockReturnsOptions)
.id(ID)
.creationTimestamp(CREATION_TIMESTAMP)
.operationId(REGION_OPERATION_ID)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
Expand All @@ -157,7 +154,6 @@ private void initializeOperation() {
operation = new Operation.Builder(compute)
.id(ID)
.operationId(GLOBAL_OPERATION_ID)
.creationTimestamp(CREATION_TIMESTAMP)
.clientOperationId(CLIENT_OPERATION_ID)
.operationType(OPERATION_TYPE)
.targetLink(TARGET_LINK)
Expand All @@ -183,7 +179,6 @@ public void tearDown() throws Exception {
}

private void assertEqualsCommonFields(Operation operation) {
assertEquals(CREATION_TIMESTAMP, operation.creationTimestamp());
assertEquals(ID, operation.id());
assertEquals(CLIENT_OPERATION_ID, operation.clientOperationId());
assertEquals(OPERATION_TYPE, operation.operationType());
Expand All @@ -205,7 +200,6 @@ private void assertEqualsCommonFields(Operation operation) {
}

private void assertNullCommonFields(Operation operation) {
assertNull(operation.creationTimestamp());
assertNull(operation.id());
assertNull(operation.clientOperationId());
assertNull(operation.operationType());
Expand Down Expand Up @@ -358,7 +352,7 @@ public void testIsDone_NotExists() throws Exception {
expect(compute.get(GLOBAL_OPERATION_ID, expectedOptions)).andReturn(null);
replay(compute);
initializeOperation();
assertFalse(operation.isDone());
assertTrue(operation.isDone());
verify(compute);
}

Expand Down Expand Up @@ -401,7 +395,6 @@ public void testReloadWithOptions() throws Exception {
private void compareOperation(Operation expected, Operation value) {
assertEquals(expected, value);
assertEquals(expected.compute().options(), value.compute().options());
assertEquals(expected.creationTimestamp(), value.creationTimestamp());
assertEquals(expected.operationId(), value.operationId());
assertEquals(expected.clientOperationId(), value.clientOperationId());
assertEquals(expected.operationType(), value.operationType());
Expand Down
3 changes: 0 additions & 3 deletions gcloud-java-compute/src/test/java/com/google/gcloud/compute/it/ITComputeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@ public void testListGlobalOperationsWithSelectedFields() {
assertNull(operation.operationType());
assertNull(operation.targetLink());
assertNull(operation.targetId());
assertNull(operation.creationTimestamp());
assertNull(operation.operationType());
assertNull(operation.status());
assertNull(operation.statusMessage());
Expand Down Expand Up @@ -564,7 +563,6 @@ public void testListRegionOperationsWithSelectedFields() {
assertNull(operation.operationType());
assertNull(operation.targetLink());
assertNull(operation.targetId());
assertNull(operation.creationTimestamp());
assertNull(operation.operationType());
assertNull(operation.status());
assertNull(operation.statusMessage());
Expand Down Expand Up @@ -628,7 +626,6 @@ public void testListZoneOperationsWithSelectedFields() {
assertNull(operation.operationType());
assertNull(operation.targetLink());
assertNull(operation.targetId());
assertNull(operation.creationTimestamp());
assertNull(operation.operationType());
assertNull(operation.status());
assertNull(operation.statusMessage());
Expand Down

0 comments on commit a6c68af

Please sign in to comment.