From 6e5f7139a7c37e7ec46f62121a42a26a759c2cde Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 16 Aug 2018 18:34:19 -0400 Subject: [PATCH 1/7] WIP: Bigtable: add CRUD instances Another step in continuing the work in: https://github.com/spollapally/google-cloud-java/tree/instance-admin-client/google-cloud-clients/google-cloud-bigtable --- .../admin/v2/BigtableInstanceAdminClient.java | 190 +++++++++++++++++- .../admin/v2/models/CreateClusterRequest.java | 125 ++++++++++++ .../v2/models/CreateInstanceRequest.java | 156 ++++++++++++++ .../bigtable/admin/v2/models/Instance.java | 92 +++++++++ .../v2/models/UpdateInstanceRequest.java | 101 ++++++++++ 5 files changed, 658 insertions(+), 6 deletions(-) create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java index b2a2f741126d..fd9218bb2e95 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java @@ -15,9 +15,26 @@ */ package com.google.cloud.bigtable.admin.v2; +import com.google.api.core.ApiFunction; +import com.google.api.core.ApiFuture; +import com.google.api.core.ApiFutures; +import com.google.bigtable.admin.v2.DeleteInstanceRequest; +import com.google.bigtable.admin.v2.GetInstanceRequest; +import com.google.bigtable.admin.v2.InstanceName; +import com.google.bigtable.admin.v2.ListInstancesRequest; +import com.google.bigtable.admin.v2.ListInstancesResponse; +import com.google.bigtable.admin.v2.LocationName; import com.google.bigtable.admin.v2.ProjectName; +import com.google.cloud.bigtable.admin.v2.models.CreateInstanceRequest; +import com.google.cloud.bigtable.admin.v2.models.Instance; +import com.google.cloud.bigtable.admin.v2.models.UpdateInstanceRequest; import com.google.cloud.bigtable.admin.v2.stub.BigtableInstanceAdminStub; +import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.protobuf.Empty; import java.io.IOException; +import java.util.List; import javax.annotation.Nonnull; /** @@ -27,14 +44,11 @@ *

See the individual methods for example code. * * 

{@code
- * try(BigtableInstanceAdminClient client =  BigtableInstanceAdminClient.create(ProjectName.of("my-project"))) {
+ * try(BigtableInstanceAdminClient client =  BigtableInstanceAdminClient.create(ProjectName.of("[PROJECT]"))) {
  *   CreateInstanceRequest request = CreateInstanceRequest.of("my-instance")
- *     .addFamily("cf1")
- *     .addFamily("cf2", GCRULES.maxVersions(10))
- *     .addSplit(ByteString.copyFromUtf8("b"))
- *     .addSplit(ByteString.copyFromUtf8("q"));
+ *     .addCluster("my-cluster", "us-east1-c", 3, StorageType.SSD);
  *
- *   client.createInstance(request);
+ *   Instance instance = client.createInstance(request);
  * }
  * }
* @@ -105,4 +119,168 @@ public ProjectName getProjectName() { public void close() { stub.close(); } + + /** + * Creates a new instance and returns its representation. + * + * @see CreateInstanceRequest for details. + */ + public Instance createInstance(CreateInstanceRequest request) { + return awaitFuture(createInstanceAsync(request)); + } + + /** + * Asynchronously creates a new instance and returns its representation wrapped in a future. + * + * @see CreateInstanceRequest for details. + */ + public ApiFuture createInstanceAsync(CreateInstanceRequest request) { + return ApiFutures.transform( + stub.createInstanceOperationCallable().futureCall(request.toProto(projectName)), + Instance.PROTO_TRANSFORMER, + MoreExecutors.directExecutor()); + } + + /** + * Updates a new instance and returns its representation. + * + * @see UpdateInstanceRequest for details. + */ + public Instance updateInstance(UpdateInstanceRequest request) { + return awaitFuture(updateInstanceAsync(request)); + } + + /** + * Asynchronously updates a new instance and returns its representation wrapped in a future. + * + * @see UpdateInstanceRequest for details. + */ + public ApiFuture updateInstanceAsync(UpdateInstanceRequest request) { + return ApiFutures.transform( + stub.partialUpdateInstanceOperationCallable().futureCall(request.toProto(projectName)), + Instance.PROTO_TRANSFORMER, + MoreExecutors.directExecutor()); + } + + /** Get the instance representation. */ + public Instance getInstance(String id) { + return awaitFuture(getInstanceAsync(id)); + } + + /** Asynchronously gets the instance representation wrapped in a future. */ + public ApiFuture getInstanceAsync(String instanceId) { + + InstanceName name = InstanceName.of(projectName.getProject(), instanceId); + + GetInstanceRequest request = GetInstanceRequest.newBuilder() + .setName(name.toString()) + .build(); + + return ApiFutures.transform( + stub.getInstanceCallable().futureCall(request), + Instance.PROTO_TRANSFORMER, + MoreExecutors.directExecutor()); + } + + /** Lists all of the instances in the current project. */ + public List listInstances() { + return awaitFuture(listInstancesAsync()); + } + + /** Asynchronously lists all of the instances in the current project. */ + public ApiFuture> listInstancesAsync() { + ListInstancesRequest request = ListInstancesRequest.newBuilder() + .setParent(projectName.toString()) + .build(); + + ApiFuture responseFuture = stub.listInstancesCallable() + .futureCall(request); + + return ApiFutures.transform(responseFuture, new ApiFunction>() { + @Override + public List apply(ListInstancesResponse proto) { + // NOTE: pagination is intentionally ignored. The server does not implement it. + Verify.verify(proto.getNextPageToken().isEmpty(), + "Server returned an unexpected paginated response"); + + ImmutableList.Builder instances = ImmutableList.builder(); + + for (com.google.bigtable.admin.v2.Instance protoInstance : proto.getInstancesList()) { + instances.add(Instance.PROTO_TRANSFORMER.apply(protoInstance)); + } + + ImmutableList.Builder failedZones = ImmutableList.builder(); + for (String locationStr : proto.getFailedLocationsList()) { + failedZones.add(LocationName.parse(locationStr).getLocation()); + } + + + if (!failedZones.build().isEmpty()) { + throw new PartialListInstancesException(failedZones.build(), instances.build()); + } + + return instances.build(); + } + }, MoreExecutors.directExecutor()); + } + + /** Deletes the specified instance. */ + public void deleteInstance(String instanceId) { + awaitFuture(deleteInstanceAsync(instanceId)); + } + + /** Asynchronously deletes the specified instance. */ + private ApiFuture deleteInstanceAsync(String instanceId) { + InstanceName instanceName = InstanceName.of(projectName.getProject(), instanceId); + + DeleteInstanceRequest request = DeleteInstanceRequest.newBuilder() + .setName(instanceName.toString()) + .build(); + + return ApiFutures.transform(stub.deleteInstanceCallable().futureCall(request), + new ApiFunction() { + @Override + public Void apply(Empty input) { + return null; + } + }, + MoreExecutors.directExecutor() + ); + } + + + private T awaitFuture(ApiFuture future) { + try { + return future.get(); + } catch(Throwable t) { + // TODO(igorbernstein2): figure out a better wrapper exception. + throw new RuntimeException(t); + } + } + + /** + * Exception thrown when some zones are unavailable and listInstances is unable to return a full + * instance list. This exception can be inspected to get a partial list. + */ + public static class PartialListInstancesException extends RuntimeException { + private final List failedZones; + private final List instances; + + PartialListInstancesException(List failedZones, List instances) { + super("Failed to list all instances, some zones where unavailable"); + + this.failedZones = failedZones; + this.instances = instances; + } + + /** A list of zones, whose unavailability caused this error. */ + public List getFailedZones() { + return failedZones; + } + + /** A partial list of instances that were found in the available zones. */ + public List getInstances() { + return instances; + } + } } diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java new file mode 100644 index 000000000000..09f8c20634ee --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java @@ -0,0 +1,125 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import com.google.api.core.InternalApi; +import com.google.bigtable.admin.v2.InstanceName; +import com.google.bigtable.admin.v2.LocationName; +import com.google.bigtable.admin.v2.ProjectName; +import com.google.bigtable.admin.v2.StorageType; + +/** + * Parameters for creating a new Bigtable cluster. + * + *

A cluster represents the actual Cloud Bigtable service. Each cluster belongs to a single Cloud + * Bigtable instance. When your application sends requests to a Cloud Bigtable instance, those + * requests are actually handled by one of the clusters in the instance. + * + *

Each cluster is located in a single zone. An instance's clusters must be in unique zones that + * are within the same region. For example, if the first cluster is in us-east1-b, then us-east1-c + * is a valid zone for the second cluster. For a list of zones and regions where Cloud Bigtable is + * available, see Cloud Bigtable Locations. + * + * + * Examples: + * + * 

{@code
+ * // Small production instance:
+ * CreateClusterRequest clusterRequest = CreateClusterRequest.of("my-existing-instance", "my-cluster")
+ *   .setZone("us-east1-c")
+ *   .setServeNodes(3)
+ *   .setStorageType(StorageType.SSD);
+ * }
+ * + * @see For more details + */ +public final class CreateClusterRequest { + private final com.google.bigtable.admin.v2.CreateClusterRequest.Builder proto = com.google.bigtable.admin.v2.CreateClusterRequest + .newBuilder(); + private final String instanceId; + private String zone; + + + /** + * Builds a new request to create a new cluster to the specified instance with the specified + * cluster id. */ + public static CreateClusterRequest of(String instanceId, String clusterId) { + return new CreateClusterRequest(instanceId, clusterId); + } + + private CreateClusterRequest(String instanceId, String clusterId) { + this.instanceId = instanceId; + proto.setClusterId(clusterId); + proto.getClusterBuilder().setDefaultStorageType(StorageType.SSD); + } + + /** + * Sets the zone where the new cluster will be located. Must be different from the existing + * cluster. + */ + public CreateClusterRequest setZone(String zone) { + this.zone = zone; + return this; + } + + /** Sets the type of storage used by this cluster to serve its parent instance's tables. */ + public CreateClusterRequest setServeNodes(int numNodes) { + proto.getClusterBuilder().setServeNodes(numNodes); + return this; + } + + /** + * Sets the type of storage used by this cluster to serve its parent instance's tables. + * Defaults to {@code SSD}. + */ + // TODO(igorbernstein2): try to avoid leaking protobuf generated enums + public CreateClusterRequest setStorageType(StorageType storageType) { + proto.getClusterBuilder().setDefaultStorageType(storageType); + return this; + } + + /** + * Creates the request protobuf. This method is considered an internal implementation detail and + * not meant to be used by applications. + */ + @InternalApi + public com.google.bigtable.admin.v2.CreateClusterRequest toProto(ProjectName projectName) { + proto.setParent(InstanceName.of(projectName.getProject(), instanceId).toString()); + proto.getClusterBuilder().setLocation(LocationName.of(projectName.getProject(), zone).toString()); + + return proto.build(); + } + + /** + * Gets the clusterId. This method is meant to be used by {@link CreateClusterRequest} and is + * considered an internal implementation detail and not meant to be used by applications. + */ + @InternalApi + String getClusterId() { + return proto.getClusterId(); + } + + /** + * Creates the request protobuf. This method is considered an internal implementation detail and + * not meant to be used by applications. + */ + @InternalApi + com.google.bigtable.admin.v2.Cluster toEmbeddedProto(ProjectName projectName) { + proto.getClusterBuilder().setLocation(LocationName.of(projectName.getProject(), zone).toString()); + + return proto.getClusterBuilder().build(); + } +} diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java new file mode 100644 index 000000000000..26b41c76218e --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java @@ -0,0 +1,156 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import com.google.api.core.InternalApi; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.ProjectName; +import com.google.bigtable.admin.v2.StorageType; +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; +import java.util.List; +import javax.annotation.Nonnull; + +/** + * Parameters for creating a new Bigtable Instance. + * + *

A Cloud Bigtable instance is mostly just a container for your clusters and nodes, + * which do all of the real work. Instances come in 2 flavors: + * + *

+ *
Production + *
A standard instance with either 1 or 2 clusters, as well as 3 or more nodes in each cluster. + * You cannot downgrade a production instance to a development instance. + * + *
Development + *
A low-cost instance for development and testing, with performance limited to the equivalent + * of a 1-node cluster. Development instances only support a single 1 node cluster. + *
+ * + * When creating an Instance, you must create at least one cluster in it. + * + * Examples: + * + * 
{@code
+ * // Small production instance:
+ * CreateInstanceRequest smallProdInstanceRequest = CreateInstanceRequest.of("my-small-instance")
+ *   .addCluster("cluster1", "us-east1-c", 3, StorageType.SSD);
+ *
+ * // Development instance:
+ * CreateInstanceRequest smallProdInstanceRequest = CreateInstanceRequest.of("my-dev-instance")
+ *   .setType(Type.DEVELOPMENT)
+ *   .addCluster("cluster1", "us-east1-c", 1, StorageType.SSD);
+ *
+ * }
+ * + * @see For more details + */ +public final class CreateInstanceRequest { + private final com.google.bigtable.admin.v2.CreateInstanceRequest.Builder builder = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder(); + + private final List clusterRequests = Lists.newArrayList(); + + /** Builds a new request to create a new instance with the specified id. */ + public static CreateInstanceRequest of(String instanceId) { + return new CreateInstanceRequest(instanceId); + } + + private CreateInstanceRequest(@Nonnull String instanceId) { + Preconditions.checkNotNull(instanceId, "InstanceId can't be null"); + + builder.setInstanceId(instanceId); + builder.getInstanceBuilder().setDisplayName(instanceId); + builder.getInstanceBuilder().setType(Type.PRODUCTION); + } + + /** + * Sets the friendly display name of the instance. If left unspecified, it will default to the id + */ + public CreateInstanceRequest setDisplayName(@Nonnull String displayName) { + Preconditions.checkNotNull(displayName); + builder.getInstanceBuilder().setDisplayName(displayName); + return this; + } + + /** + * Sets the type of instance. + * + *

Can be either DEVELOPMENT or PRODUCTION. Defaults to PRODUCTION. + * Please see class javadoc for details. + */ + // TODO(igorbernstein2): try to avoid leaking protobuf generated enums + public CreateInstanceRequest setType(@Nonnull Type type) { + Preconditions.checkNotNull(type); + builder.getInstanceBuilder().setType(type); + return this; + } + + /** + * Adds an arbitrary label to the instance. + * + *

Labels are key-value pairs that you can use to group related instances and store metadata + * about an instance. + * + * @see For more details + */ + public CreateInstanceRequest addLabel(@Nonnull String key, @Nonnull String value) { + Preconditions.checkNotNull(key, "Key can't be null"); + Preconditions.checkNotNull(value, "Value can't be null"); + builder.getInstanceBuilder().putLabels(key, value); + return this; + } + + /** + * Adds a cluster to the instance request. + * + *

All new instances must have at least one cluster. DEVELOPMENT instances must have exactly + * one cluster. + * + * @param clusterId The name of the cluster. + * @param zone The zone where the cluster will be created. + * @param serveNodes The number of nodes that cluster will contain. DEVELOPMENT instance clusters must have exactly one node. + * @param storageType The type of storage used by this cluster to serve its parent instance's tables. + */ + // TODO(igorbernstein2): try to avoid leaking protobuf generated enums + public CreateInstanceRequest addCluster(String clusterId, String zone, int serveNodes, StorageType storageType) { + CreateClusterRequest clusterRequest = CreateClusterRequest + .of("ignored-instance-id", clusterId) + .setZone(zone) + .setServeNodes(serveNodes) + .setStorageType(storageType); + clusterRequests.add(clusterRequest); + + return this; + } + + /** + * Creates the request protobuf. This method is considered an internal implementation detail and + * not meant to be used by applications. + */ + @InternalApi + public com.google.bigtable.admin.v2.CreateInstanceRequest toProto(ProjectName projectName) { + builder + .setParent(projectName.toString()) + .clearClusters(); + + for (CreateClusterRequest clusterRequest : clusterRequests) { + builder.putClusters(clusterRequest.getClusterId(), clusterRequest.toEmbeddedProto(projectName)); + } + + return builder.build(); + } +} diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java new file mode 100644 index 000000000000..3be4e4e02c9a --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java @@ -0,0 +1,92 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import com.google.api.core.ApiFunction; +import com.google.api.core.InternalApi; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.InstanceName; +import com.google.common.base.Objects; +import java.util.Map; +import javax.annotation.Nonnull; + +/** + * Represents an existing Cloud Bigtable instance. + * + *

A Cloud Bigtable instance is mostly just a container for your clusters and nodes, which do all + * of the real work. + */ +public class Instance { + @Nonnull + private final com.google.bigtable.admin.v2.Instance proto; + + /** + * Wraps the protobuf. This method is considered an internal implementation detail and + * not meant to be used by applications. + */ + @InternalApi + public static ApiFunction PROTO_TRANSFORMER = new ApiFunction() { + @Override + public Instance apply(com.google.bigtable.admin.v2.Instance proto) { + return new Instance(proto); + } + }; + + private Instance(com.google.bigtable.admin.v2.Instance proto) { + this.proto = proto; + } + + /** Gets the instance's id. */ + public String getId() { + return InstanceName.parse(proto.getName()).getInstance(); + } + + /** Gets the instance's friendly name. */ + public String getDisplayName() { + return proto.getDisplayName(); + } + + /** Gets the instance's current type. Can be DEVELOPMENT or PRODUCTION. */ + public Type getType() { + return proto.getType(); + } + + /** + * Gets the current labels associated with the instance. + * + * @see For more details + */ + public Map getLabels() { + return proto.getLabelsMap(); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Instance instance = (Instance) o; + return Objects.equal(proto, instance.proto); + } + + @Override + public int hashCode() { + return Objects.hashCode(proto); + } +} diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java new file mode 100644 index 000000000000..01d1ebbf88d3 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import com.google.api.core.InternalApi; +import com.google.bigtable.admin.v2.Instance; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.InstanceName; +import com.google.bigtable.admin.v2.PartialUpdateInstanceRequest; +import com.google.bigtable.admin.v2.ProjectName; +import com.google.common.base.Preconditions; +import com.google.protobuf.FieldMask; +import com.google.protobuf.util.FieldMaskUtil; +import java.util.Map; +import javax.annotation.Nonnull; + +/** + * Parameters for updating an existing Bigtable instance. + * + *

Existing instances maybe updated to change their superficial appearance (ie. display name) and + * can also be upgraded from a DEVELOPMENT instance to a PRODUCTION instance. Please note that + * upgrading to a PRODUCTION instance cannot be undone. + */ +public class UpdateInstanceRequest { + private final String instanceId; + private final PartialUpdateInstanceRequest.Builder builder = PartialUpdateInstanceRequest.newBuilder(); + + /** Builds a new request to update an existing instance with the specified id. */ + public static UpdateInstanceRequest of(@Nonnull String instanceId) { + return new UpdateInstanceRequest(instanceId); + } + + private UpdateInstanceRequest(@Nonnull String instanceId) { + Preconditions.checkNotNull(instanceId, "instanceId can't be null"); + this.instanceId = instanceId; + } + + /** Changes the display name of the instance. */ + public UpdateInstanceRequest withDisplayName(@Nonnull String displayName) { + Preconditions.checkNotNull(displayName); + builder.getInstanceBuilder().setDisplayName(displayName); + updateFieldMask(Instance.DISPLAY_NAME_FIELD_NUMBER); + + return this; + } + + /** + * Upgrades the instance from a DEVELOPMENT instance to a PRODUCTION instance. This cannot be + * undone. + */ + public UpdateInstanceRequest withProductionType() { + builder.getInstanceBuilder().setType(Type.PRODUCTION); + updateFieldMask(Instance.TYPE_FIELD_NUMBER); + + return this; + } + + /** + * Replaces the labels associated with the instance. + * + * @see For more details + */ + public UpdateInstanceRequest withLabels(@Nonnull Map labels) { + Preconditions.checkNotNull(labels, "labels can't be null"); + builder.getInstanceBuilder().clearLabels(); + builder.getInstanceBuilder().putAllLabels(labels); + updateFieldMask(Instance.LABELS_FIELD_NUMBER); + + return this; + } + + private void updateFieldMask(int fieldNumber) { + FieldMask newMask = FieldMaskUtil.fromFieldNumbers(Instance.class, fieldNumber); + builder.setUpdateMask(FieldMaskUtil.union(builder.getUpdateMask(), newMask)); + } + + /** + * Creates the request protobuf. This method is considered an internal implementation detail and + * not meant to be used by applications. + */ + @InternalApi + public PartialUpdateInstanceRequest toProto(ProjectName projectName) { + InstanceName instanceName = InstanceName.of(projectName.getProject(), instanceId); + builder.getInstanceBuilder().setName(instanceName.toString()); + + return builder.build(); + } +} From b366abd4b4886162db0188dbe881402f7aaa0d1d Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Fri, 17 Aug 2018 16:44:47 -0400 Subject: [PATCH 2/7] Polish instance crudl --- .../google-cloud-bigtable-admin/pom.xml | 6 + .../admin/v2/BigtableInstanceAdminClient.java | 145 ++++++---- .../admin/v2/models/CreateClusterRequest.java | 8 +- .../bigtable/admin/v2/models/Instance.java | 50 +++- .../models/PartialListInstancesException.java | 44 +++ .../v2/models/UpdateInstanceRequest.java | 23 +- .../v2/BigtableInstanceAdminClientTest.java | 269 +++++++++++++++++- .../v2/models/CreateClusterRequestTest.java | 163 +++++++++++ .../admin/v2/models/InstanceTest.java | 71 +++++ .../v2/models/UpdateInstanceRequestTest.java | 116 ++++++++ 10 files changed, 809 insertions(+), 86 deletions(-) create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java create mode 100644 google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java diff --git a/google-cloud-clients/google-cloud-bigtable-admin/pom.xml b/google-cloud-clients/google-cloud-bigtable-admin/pom.xml index 75603a302648..7de8dd203a18 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/pom.xml +++ b/google-cloud-clients/google-cloud-bigtable-admin/pom.xml @@ -61,6 +61,12 @@ test-jar test + + com.google.api + gax + testlib + test + junit junit diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java index fd9218bb2e95..8e49ced04ece 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java @@ -27,11 +27,14 @@ import com.google.bigtable.admin.v2.ProjectName; import com.google.cloud.bigtable.admin.v2.models.CreateInstanceRequest; import com.google.cloud.bigtable.admin.v2.models.Instance; +import com.google.cloud.bigtable.admin.v2.models.PartialListInstancesException; import com.google.cloud.bigtable.admin.v2.models.UpdateInstanceRequest; import com.google.cloud.bigtable.admin.v2.stub.BigtableInstanceAdminStub; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.protobuf.Empty; import java.io.IOException; import java.util.List; @@ -96,7 +99,7 @@ public static BigtableInstanceAdminClient create(@Nonnull BigtableInstanceAdminS return create(settings.getProjectName(), settings.getStubSettings().createStub()); } - /** Constructs an instance of BigtableInstanceAdminClient with the given Projectname and stub. */ + /** Constructs an instance of BigtableInstanceAdminClient with the given ProjectName and stub. */ public static BigtableInstanceAdminClient create(@Nonnull ProjectName projectName, @Nonnull BigtableInstanceAdminStub stub) { return new BigtableInstanceAdminClient(projectName, stub); @@ -110,6 +113,7 @@ private BigtableInstanceAdminClient( } /** Gets the ProjectName this client is associated with. */ + @SuppressWarnings("WeakerAccess") public ProjectName getProjectName() { return projectName; } @@ -125,6 +129,7 @@ public void close() { * * @see CreateInstanceRequest for details. */ + @SuppressWarnings("WeakerAccess") public Instance createInstance(CreateInstanceRequest request) { return awaitFuture(createInstanceAsync(request)); } @@ -134,10 +139,16 @@ public Instance createInstance(CreateInstanceRequest request) { * * @see CreateInstanceRequest for details. */ + @SuppressWarnings("WeakerAccess") public ApiFuture createInstanceAsync(CreateInstanceRequest request) { return ApiFutures.transform( stub.createInstanceOperationCallable().futureCall(request.toProto(projectName)), - Instance.PROTO_TRANSFORMER, + new ApiFunction() { + @Override + public Instance apply(com.google.bigtable.admin.v2.Instance proto) { + return Instance.fromProto(proto); + } + }, MoreExecutors.directExecutor()); } @@ -146,6 +157,7 @@ public ApiFuture createInstanceAsync(CreateInstanceRequest request) { * * @see UpdateInstanceRequest for details. */ + @SuppressWarnings("WeakerAccess") public Instance updateInstance(UpdateInstanceRequest request) { return awaitFuture(updateInstanceAsync(request)); } @@ -155,21 +167,28 @@ public Instance updateInstance(UpdateInstanceRequest request) { * * @see UpdateInstanceRequest for details. */ + @SuppressWarnings("WeakerAccess") public ApiFuture updateInstanceAsync(UpdateInstanceRequest request) { return ApiFutures.transform( stub.partialUpdateInstanceOperationCallable().futureCall(request.toProto(projectName)), - Instance.PROTO_TRANSFORMER, + new ApiFunction() { + @Override + public Instance apply(com.google.bigtable.admin.v2.Instance proto) { + return Instance.fromProto(proto); + } + }, MoreExecutors.directExecutor()); } /** Get the instance representation. */ + @SuppressWarnings("WeakerAccess") public Instance getInstance(String id) { return awaitFuture(getInstanceAsync(id)); } /** Asynchronously gets the instance representation wrapped in a future. */ + @SuppressWarnings("WeakerAccess") public ApiFuture getInstanceAsync(String instanceId) { - InstanceName name = InstanceName.of(projectName.getProject(), instanceId); GetInstanceRequest request = GetInstanceRequest.newBuilder() @@ -178,16 +197,23 @@ public ApiFuture getInstanceAsync(String instanceId) { return ApiFutures.transform( stub.getInstanceCallable().futureCall(request), - Instance.PROTO_TRANSFORMER, + new ApiFunction() { + @Override + public Instance apply(com.google.bigtable.admin.v2.Instance proto) { + return Instance.fromProto(proto); + } + }, MoreExecutors.directExecutor()); } /** Lists all of the instances in the current project. */ + @SuppressWarnings("WeakerAccess") public List listInstances() { return awaitFuture(listInstancesAsync()); } /** Asynchronously lists all of the instances in the current project. */ + @SuppressWarnings("WeakerAccess") public ApiFuture> listInstancesAsync() { ListInstancesRequest request = ListInstancesRequest.newBuilder() .setParent(projectName.toString()) @@ -196,41 +222,47 @@ public ApiFuture> listInstancesAsync() { ApiFuture responseFuture = stub.listInstancesCallable() .futureCall(request); - return ApiFutures.transform(responseFuture, new ApiFunction>() { - @Override - public List apply(ListInstancesResponse proto) { - // NOTE: pagination is intentionally ignored. The server does not implement it. - Verify.verify(proto.getNextPageToken().isEmpty(), - "Server returned an unexpected paginated response"); - - ImmutableList.Builder instances = ImmutableList.builder(); - - for (com.google.bigtable.admin.v2.Instance protoInstance : proto.getInstancesList()) { - instances.add(Instance.PROTO_TRANSFORMER.apply(protoInstance)); - } - - ImmutableList.Builder failedZones = ImmutableList.builder(); - for (String locationStr : proto.getFailedLocationsList()) { - failedZones.add(LocationName.parse(locationStr).getLocation()); - } - - - if (!failedZones.build().isEmpty()) { - throw new PartialListInstancesException(failedZones.build(), instances.build()); - } - - return instances.build(); - } - }, MoreExecutors.directExecutor()); + return ApiFutures + .transform(responseFuture, new ApiFunction>() { + @Override + public List apply(ListInstancesResponse proto) { + // NOTE: pagination is intentionally ignored. The server does not implement it. + Verify.verify(proto.getNextPageToken().isEmpty(), + "Server returned an unexpected paginated response"); + + ImmutableList.Builder instances = ImmutableList.builder(); + + for (com.google.bigtable.admin.v2.Instance protoInstance : proto.getInstancesList()) { + instances.add(Instance.fromProto(protoInstance)); + } + + ImmutableList.Builder failedZones = ImmutableList.builder(); + for (String locationStr : proto.getFailedLocationsList()) { + LocationName fullLocation = LocationName.parse(locationStr); + if (fullLocation == null) { + continue; + } + failedZones.add(fullLocation.getLocation()); + } + + if (!failedZones.build().isEmpty()) { + throw new PartialListInstancesException(failedZones.build(), instances.build()); + } + + return instances.build(); + } + }, MoreExecutors.directExecutor()); } /** Deletes the specified instance. */ + @SuppressWarnings("WeakerAccess") public void deleteInstance(String instanceId) { awaitFuture(deleteInstanceAsync(instanceId)); } /** Asynchronously deletes the specified instance. */ - private ApiFuture deleteInstanceAsync(String instanceId) { + @SuppressWarnings("WeakerAccess") + public ApiFuture deleteInstanceAsync(String instanceId) { InstanceName instanceName = InstanceName.of(projectName.getProject(), instanceId); DeleteInstanceRequest request = DeleteInstanceRequest.newBuilder() @@ -248,39 +280,32 @@ public Void apply(Empty input) { ); } - - private T awaitFuture(ApiFuture future) { - try { - return future.get(); - } catch(Throwable t) { - // TODO(igorbernstein2): figure out a better wrapper exception. - throw new RuntimeException(t); - } - } - /** - * Exception thrown when some zones are unavailable and listInstances is unable to return a full - * instance list. This exception can be inspected to get a partial list. + * Awaits the result of a future, taking care to propagate errors while maintaining the call site + * in a suppressed exception. This allows semantic errors to be caught across threads, while + * preserving the call site in the error. The caller's stacktrace will be made available as a + * suppressed exception. */ - public static class PartialListInstancesException extends RuntimeException { - private final List failedZones; - private final List instances; - - PartialListInstancesException(List failedZones, List instances) { - super("Failed to list all instances, some zones where unavailable"); + // TODO(igorbernstein2): try to move this into gax + private T awaitFuture(ApiFuture future) { + RuntimeException error; - this.failedZones = failedZones; - this.instances = instances; + try { + return Futures.getUnchecked(future); + } catch (UncheckedExecutionException e) { + if (e.getCause() instanceof RuntimeException) { + error = (RuntimeException) e.getCause(); + } else { + error = e; + } + } catch (RuntimeException e) { + error = e; } - /** A list of zones, whose unavailability caused this error. */ - public List getFailedZones() { - return failedZones; - } + // Add the caller's stack as a suppressed exception + error.addSuppressed(new RuntimeException("Encountered error while awaiting future")); - /** A partial list of instances that were found in the available zones. */ - public List getInstances() { - return instances; - } + throw error; } + } diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java index 09f8c20634ee..130cf46d5dfb 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java @@ -49,6 +49,7 @@ public final class CreateClusterRequest { private final com.google.bigtable.admin.v2.CreateClusterRequest.Builder proto = com.google.bigtable.admin.v2.CreateClusterRequest .newBuilder(); + // Partial ids will be set when the project is passed to toProto private final String instanceId; private String zone; @@ -70,12 +71,14 @@ private CreateClusterRequest(String instanceId, String clusterId) { * Sets the zone where the new cluster will be located. Must be different from the existing * cluster. */ + @SuppressWarnings("WeakerAccess") public CreateClusterRequest setZone(String zone) { this.zone = zone; return this; } /** Sets the type of storage used by this cluster to serve its parent instance's tables. */ + @SuppressWarnings("WeakerAccess") public CreateClusterRequest setServeNodes(int numNodes) { proto.getClusterBuilder().setServeNodes(numNodes); return this; @@ -86,6 +89,7 @@ public CreateClusterRequest setServeNodes(int numNodes) { * Defaults to {@code SSD}. */ // TODO(igorbernstein2): try to avoid leaking protobuf generated enums + @SuppressWarnings("WeakerAccess") public CreateClusterRequest setStorageType(StorageType storageType) { proto.getClusterBuilder().setDefaultStorageType(storageType); return this; @@ -113,8 +117,8 @@ String getClusterId() { } /** - * Creates the request protobuf. This method is considered an internal implementation detail and - * not meant to be used by applications. + * Creates the request protobuf to be used in {@link CreateInstanceRequest}. This method is + * considered an internal implementation detail and not meant to be used by applications. */ @InternalApi com.google.bigtable.admin.v2.Cluster toEmbeddedProto(ProjectName projectName) { diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java index 3be4e4e02c9a..b6cde0128f5b 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java @@ -15,51 +15,61 @@ */ package com.google.cloud.bigtable.admin.v2.models; -import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; +import com.google.bigtable.admin.v2.Instance.State; import com.google.bigtable.admin.v2.Instance.Type; import com.google.bigtable.admin.v2.InstanceName; import com.google.common.base.Objects; +import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import java.util.Map; import javax.annotation.Nonnull; /** * Represents an existing Cloud Bigtable instance. * - *

A Cloud Bigtable instance is mostly just a container for your clusters and nodes, which do all - * of the real work. + *

A Cloud Bigtable instance is mostly just a container for your clusters and nodes, which do + * all of the real work. */ -public class Instance { +public final class Instance { @Nonnull private final com.google.bigtable.admin.v2.Instance proto; /** - * Wraps the protobuf. This method is considered an internal implementation detail and - * not meant to be used by applications. + * Wraps the protobuf. This method is considered an internal implementation detail and not meant + * to be used by applications. */ @InternalApi - public static ApiFunction PROTO_TRANSFORMER = new ApiFunction() { - @Override - public Instance apply(com.google.bigtable.admin.v2.Instance proto) { - return new Instance(proto); - } - }; + public static Instance fromProto(com.google.bigtable.admin.v2.Instance proto) { + return new Instance(proto); + } - private Instance(com.google.bigtable.admin.v2.Instance proto) { + private Instance(@Nonnull com.google.bigtable.admin.v2.Instance proto) { + Preconditions.checkNotNull(proto); + Preconditions.checkArgument(!proto.getName().isEmpty(), "Name must be set"); this.proto = proto; } /** Gets the instance's id. */ + @SuppressWarnings("WeakerAccess") public String getId() { - return InstanceName.parse(proto.getName()).getInstance(); + // Constructor ensures that name is not null + InstanceName fullName = Verify.verifyNotNull( + InstanceName.parse(proto.getName()), + "Name can never be null"); + + //noinspection ConstantConditions + return fullName.getInstance(); } /** Gets the instance's friendly name. */ + @SuppressWarnings("WeakerAccess") public String getDisplayName() { return proto.getDisplayName(); } /** Gets the instance's current type. Can be DEVELOPMENT or PRODUCTION. */ + @SuppressWarnings("WeakerAccess") public Type getType() { return proto.getType(); } @@ -67,12 +77,22 @@ public Type getType() { /** * Gets the current labels associated with the instance. * - * @see For more details + * @see For more + * details */ + @SuppressWarnings("WeakerAccess") public Map getLabels() { return proto.getLabelsMap(); } + + /** The current state of the instance. */ + // TODO(igorbernstein2): Try to avoid leaking protobuf enums + @SuppressWarnings("WeakerAccess") + public State getState() { + return proto.getState(); + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java new file mode 100644 index 000000000000..54bb98b98979 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java @@ -0,0 +1,44 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import java.util.List; + +/** + * Exception thrown when some zones are unavailable and listInstances is unable to return a full + * instance list. This exception can be inspected to get a partial list. + */ +public class PartialListInstancesException extends RuntimeException { + private final List failedZones; + private final List instances; + + public PartialListInstancesException(List failedZones, List instances) { + super("Failed to list all instances, some zones where unavailable"); + + this.failedZones = failedZones; + this.instances = instances; + } + + /** A list of zones, whose unavailability caused this error. */ + public List getFailedZones() { + return failedZones; + } + + /** A partial list of instances that were found in the available zones. */ + public List getInstances() { + return instances; + } +} diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java index 01d1ebbf88d3..3203971d9695 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java @@ -30,13 +30,14 @@ /** * Parameters for updating an existing Bigtable instance. * - *

Existing instances maybe updated to change their superficial appearance (ie. display name) and - * can also be upgraded from a DEVELOPMENT instance to a PRODUCTION instance. Please note that + *

Existing instances maybe updated to change their superficial appearance (ie. display name) + * and can also be upgraded from a DEVELOPMENT instance to a PRODUCTION instance. Please note that * upgrading to a PRODUCTION instance cannot be undone. */ public class UpdateInstanceRequest { private final String instanceId; - private final PartialUpdateInstanceRequest.Builder builder = PartialUpdateInstanceRequest.newBuilder(); + private final PartialUpdateInstanceRequest.Builder builder = PartialUpdateInstanceRequest + .newBuilder(); /** Builds a new request to update an existing instance with the specified id. */ public static UpdateInstanceRequest of(@Nonnull String instanceId) { @@ -49,7 +50,8 @@ private UpdateInstanceRequest(@Nonnull String instanceId) { } /** Changes the display name of the instance. */ - public UpdateInstanceRequest withDisplayName(@Nonnull String displayName) { + @SuppressWarnings("WeakerAccess") + public UpdateInstanceRequest setDisplayName(@Nonnull String displayName) { Preconditions.checkNotNull(displayName); builder.getInstanceBuilder().setDisplayName(displayName); updateFieldMask(Instance.DISPLAY_NAME_FIELD_NUMBER); @@ -61,7 +63,8 @@ public UpdateInstanceRequest withDisplayName(@Nonnull String displayName) { * Upgrades the instance from a DEVELOPMENT instance to a PRODUCTION instance. This cannot be * undone. */ - public UpdateInstanceRequest withProductionType() { + @SuppressWarnings("WeakerAccess") + public UpdateInstanceRequest setProductionType() { builder.getInstanceBuilder().setType(Type.PRODUCTION); updateFieldMask(Instance.TYPE_FIELD_NUMBER); @@ -71,9 +74,11 @@ public UpdateInstanceRequest withProductionType() { /** * Replaces the labels associated with the instance. * - * @see For more details + * @see For more + * details */ - public UpdateInstanceRequest withLabels(@Nonnull Map labels) { + @SuppressWarnings("WeakerAccess") + public UpdateInstanceRequest setLabels(@Nonnull Map labels) { Preconditions.checkNotNull(labels, "labels can't be null"); builder.getInstanceBuilder().clearLabels(); builder.getInstanceBuilder().putAllLabels(labels); @@ -93,6 +98,10 @@ private void updateFieldMask(int fieldNumber) { */ @InternalApi public PartialUpdateInstanceRequest toProto(ProjectName projectName) { + // Empty field mask implies full resource replacement, which would clear all fields in an empty + // update request. + Preconditions.checkState(!builder.getUpdateMask().getPathsList().isEmpty(), "Update request is empty"); + InstanceName instanceName = InstanceName.of(projectName.getProject(), instanceId); builder.getInstanceBuilder().setName(instanceName.toString()); diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java index d76193b7afb5..82c6a78dbacf 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java @@ -17,30 +17,88 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.api.core.ApiFuture; +import com.google.api.core.ApiFutures; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.longrunning.OperationFutures; +import com.google.api.gax.longrunning.OperationSnapshot; +import com.google.api.gax.rpc.OperationCallable; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.rpc.testing.FakeOperationSnapshot; +import com.google.bigtable.admin.v2.Cluster; +import com.google.bigtable.admin.v2.CreateInstanceMetadata; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.InstanceName; +import com.google.bigtable.admin.v2.ListInstancesResponse; +import com.google.bigtable.admin.v2.LocationName; +import com.google.bigtable.admin.v2.PartialUpdateInstanceRequest; import com.google.bigtable.admin.v2.ProjectName; +import com.google.bigtable.admin.v2.StorageType; +import com.google.bigtable.admin.v2.UpdateInstanceMetadata; +import com.google.cloud.bigtable.admin.v2.models.CreateInstanceRequest; +import com.google.cloud.bigtable.admin.v2.models.Instance; +import com.google.cloud.bigtable.admin.v2.models.PartialListInstancesException; +import com.google.cloud.bigtable.admin.v2.models.UpdateInstanceRequest; import com.google.cloud.bigtable.admin.v2.stub.BigtableInstanceAdminStub; +import com.google.protobuf.Empty; +import com.google.protobuf.FieldMask; +import io.grpc.Status.Code; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; @RunWith(MockitoJUnitRunner.class) public class BigtableInstanceAdminClientTest { + private static final ProjectName PROJECT_NAME = ProjectName.of("my-project"); + private static final InstanceName INSTANCE_NAME = + InstanceName.of(PROJECT_NAME.getProject(), "my-instance"); + private BigtableInstanceAdminClient adminClient; + @Mock private BigtableInstanceAdminStub mockStub; + @Mock + private OperationCallable mockCreateInstanceCallable; + + @Mock + private OperationCallable mockUpdateInstanceCallable; + + @Mock + private UnaryCallable mockGetInstanceCallable; + + @Mock + private UnaryCallable mockListInstancesCallable; + + @Mock + private UnaryCallable mockDeleteInstanceCallable; + + @Before public void setUp() { adminClient = BigtableInstanceAdminClient - .create(ProjectName.of("[PROJECT]"), mockStub); + .create(PROJECT_NAME, mockStub); + + Mockito.when(mockStub.createInstanceOperationCallable()).thenReturn(mockCreateInstanceCallable); + Mockito.when(mockStub.partialUpdateInstanceOperationCallable()) + .thenReturn(mockUpdateInstanceCallable); + + Mockito.when(mockStub.getInstanceCallable()).thenReturn(mockGetInstanceCallable); + Mockito.when(mockStub.listInstancesCallable()).thenReturn(mockListInstancesCallable); + Mockito.when(mockStub.deleteInstanceCallable()).thenReturn(mockDeleteInstanceCallable); } @Test public void testProjectName() { - assertThat(adminClient.getProjectName()).isEqualTo(ProjectName.of("[PROJECT]")); + assertThat(adminClient.getProjectName()).isEqualTo(PROJECT_NAME); } @Test @@ -48,4 +106,211 @@ public void testClose() { adminClient.close(); Mockito.verify(mockStub).close(); } + + @Test + public void testCreateInstance() { + // Setup + com.google.bigtable.admin.v2.CreateInstanceRequest expectedRequest = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder() + .setParent(PROJECT_NAME.toString()) + .setInstanceId(INSTANCE_NAME.getInstance()) + .setInstance( + com.google.bigtable.admin.v2.Instance.newBuilder() + .setType(Type.DEVELOPMENT) + .setDisplayName(INSTANCE_NAME.getInstance()) + ) + .putClusters("cluster1", Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-c") + .setServeNodes(1) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .build(); + + com.google.bigtable.admin.v2.Instance expectedResponse = com.google.bigtable.admin.v2.Instance + .newBuilder() + .setName(INSTANCE_NAME.toString()) + .build(); + + mockOperationResult(mockCreateInstanceCallable, expectedRequest, expectedResponse); + + // Execute + com.google.cloud.bigtable.admin.v2.models.Instance actualResult = adminClient.createInstance( + CreateInstanceRequest.of(INSTANCE_NAME.getInstance()) + .setType(Type.DEVELOPMENT) + .addCluster("cluster1", "us-east1-c", 1, StorageType.SSD) + ); + + // Verify + assertThat(actualResult).isEqualTo(Instance.fromProto(expectedResponse)); + } + + @Test + public void testUpdateInstance() { + // Setup + com.google.bigtable.admin.v2.PartialUpdateInstanceRequest expectedRequest = + PartialUpdateInstanceRequest.newBuilder() + .setUpdateMask( + FieldMask.newBuilder() + .addPaths("display_name") + ) + .setInstance( + com.google.bigtable.admin.v2.Instance.newBuilder() + .setName(INSTANCE_NAME.toString()) + .setDisplayName("new display name") + ) + .build(); + + com.google.bigtable.admin.v2.Instance expectedResponse = com.google.bigtable.admin.v2.Instance + .newBuilder() + .setName(INSTANCE_NAME.toString()) + .build(); + + mockOperationResult(mockUpdateInstanceCallable, expectedRequest, expectedResponse); + + // Execute + com.google.cloud.bigtable.admin.v2.models.Instance actualResult = adminClient.updateInstance( + UpdateInstanceRequest.of(INSTANCE_NAME.getInstance()) + .setDisplayName("new display name") + ); + + // Verify + assertThat(actualResult).isEqualTo(Instance.fromProto(expectedResponse)); + } + + @Test + public void testGetInstance() { + // Setup + com.google.bigtable.admin.v2.GetInstanceRequest expectedRequest = + com.google.bigtable.admin.v2.GetInstanceRequest.newBuilder() + .setName(INSTANCE_NAME.toString()) + .build(); + + com.google.bigtable.admin.v2.Instance expectedResponse = com.google.bigtable.admin.v2.Instance + .newBuilder() + .setName(INSTANCE_NAME.toString()) + .build(); + + Mockito.when(mockGetInstanceCallable.futureCall(expectedRequest)) + .thenReturn(ApiFutures.immediateFuture(expectedResponse)); + + // Execute + Instance actualResult = adminClient.getInstance(INSTANCE_NAME.getInstance()); + + // Verify + assertThat(actualResult).isEqualTo(Instance.fromProto(expectedResponse)); + } + + @Test + public void testListInstances() { + // Setup + com.google.bigtable.admin.v2.ListInstancesRequest expectedRequest = + com.google.bigtable.admin.v2.ListInstancesRequest.newBuilder() + .setParent(PROJECT_NAME.toString()) + .build(); + + com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse = ListInstancesResponse + .newBuilder() + .addInstances( + com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1").build() + ) + .addInstances( + com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "2").build() + ) + .build(); + + Mockito.when(mockListInstancesCallable.futureCall(expectedRequest)) + .thenReturn(ApiFutures.immediateFuture(expectedResponse)); + + // Execute + List actualResult = adminClient.listInstances(); + + // Verify + assertThat(actualResult).containsExactly( + Instance.fromProto(expectedResponse.getInstances(0)), + Instance.fromProto(expectedResponse.getInstances(1)) + ); + } + + @Test + public void testListInstancesFailedZone() { + // Setup + com.google.bigtable.admin.v2.ListInstancesRequest expectedRequest = + com.google.bigtable.admin.v2.ListInstancesRequest.newBuilder() + .setParent(PROJECT_NAME.toString()) + .build(); + + com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse = ListInstancesResponse + .newBuilder() + .addInstances( + com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1").build() + ) + .addFailedLocations( + LocationName.of(PROJECT_NAME.getProject(), "us-east1-d").toString() + ) + .build(); + + Mockito.when(mockListInstancesCallable.futureCall(expectedRequest)) + .thenReturn(ApiFutures.immediateFuture(expectedResponse)); + + // Execute + Exception actualError = null; + + try { + adminClient.listInstances(); + } catch (Exception e) { + actualError = e; + } + + // Verify + assertThat(actualError).isInstanceOf(PartialListInstancesException.class); + assert actualError != null; + PartialListInstancesException partialListError = (PartialListInstancesException) actualError; + + assertThat(partialListError.getInstances()) + .containsExactly(Instance.fromProto(expectedResponse.getInstances(0))); + assertThat(partialListError.getFailedZones()).containsExactly("us-east1-d"); + } + + @Test + public void testDeleteInstance() { + // Setup + com.google.bigtable.admin.v2.DeleteInstanceRequest expectedRequest = + com.google.bigtable.admin.v2.DeleteInstanceRequest.newBuilder() + .setName(INSTANCE_NAME.toString()) + .build(); + + final AtomicBoolean wasCalled = new AtomicBoolean(false); + + Mockito.when(mockDeleteInstanceCallable.futureCall(expectedRequest)) + .thenAnswer(new Answer>() { + @Override + public ApiFuture answer(InvocationOnMock invocationOnMock) { + wasCalled.set(true); + return ApiFutures.immediateFuture(Empty.getDefaultInstance()); + } + }); + + // Execute + adminClient.deleteInstance(INSTANCE_NAME.getInstance()); + + // Verify + assertThat(wasCalled.get()).isTrue(); + } + + + private void mockOperationResult( + OperationCallable callable, ReqT request, RespT response) { + OperationSnapshot operationSnapshot = FakeOperationSnapshot.newBuilder() + .setDone(true) + .setErrorCode(GrpcStatusCode.of(Code.OK)) + .setName("fake-name") + .setResponse(response) + .build(); + + OperationFuture operationFuture = OperationFutures + .immediateOperationFuture(operationSnapshot); + + Mockito.when(callable.futureCall(request)).thenReturn(operationFuture); + } } diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java new file mode 100644 index 000000000000..cf4c4319a0b1 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java @@ -0,0 +1,163 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.bigtable.admin.v2.Cluster; +import com.google.bigtable.admin.v2.Instance; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.ProjectName; +import com.google.bigtable.admin.v2.StorageType; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CreateClusterRequestTest { + @Test + public void testProductionSingle() { + CreateInstanceRequest input = CreateInstanceRequest.of("my-instance") + .setType(Type.PRODUCTION) + .addCluster("cluster1", "us-east1-c", 3, StorageType.SSD); + + com.google.bigtable.admin.v2.CreateInstanceRequest actual = input + .toProto(ProjectName.of("my-project")); + + com.google.bigtable.admin.v2.CreateInstanceRequest expected = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder() + .setParent(ProjectName.of("my-project").toString()) + .setInstanceId("my-instance") + .setInstance( + Instance.newBuilder() + .setDisplayName("my-instance") + .setType(Type.PRODUCTION) + ) + .putClusters("cluster1", + Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-c") + .setServeNodes(3) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void testProductionReplication() { + CreateInstanceRequest input = CreateInstanceRequest.of("my-instance") + .setType(Type.PRODUCTION) + .addCluster("cluster1", "us-east1-c", 3, StorageType.SSD) + .addCluster("cluster2", "us-east1-a", 3, StorageType.SSD); + + com.google.bigtable.admin.v2.CreateInstanceRequest actual = input + .toProto(ProjectName.of("my-project")); + + com.google.bigtable.admin.v2.CreateInstanceRequest expected = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder() + .setParent(ProjectName.of("my-project").toString()) + .setInstanceId("my-instance") + .setInstance( + Instance.newBuilder() + .setDisplayName("my-instance") + .setType(Type.PRODUCTION) + ) + .putClusters("cluster1", + Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-c") + .setServeNodes(3) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .putClusters("cluster2", + Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-a") + .setServeNodes(3) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void testDevelopment() { + CreateInstanceRequest input = CreateInstanceRequest.of("my-instance") + .setType(Type.DEVELOPMENT) + .addCluster("cluster1", "us-east1-c", 1, StorageType.SSD); + + com.google.bigtable.admin.v2.CreateInstanceRequest actual = input + .toProto(ProjectName.of("my-project")); + + com.google.bigtable.admin.v2.CreateInstanceRequest expected = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder() + .setParent(ProjectName.of("my-project").toString()) + .setInstanceId("my-instance") + .setInstance( + Instance.newBuilder() + .setDisplayName("my-instance") + .setType(Type.DEVELOPMENT) + ) + .putClusters("cluster1", + Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-c") + .setServeNodes(1) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void testOptionalFields() { + CreateInstanceRequest input = CreateInstanceRequest.of("my-instance") + .setDisplayName("custom display name") + .addLabel("my label", "with some value") + .addLabel("my other label", "with some value") + .setType(Type.DEVELOPMENT) + .addCluster("cluster1", "us-east1-c", 1, StorageType.SSD); + + com.google.bigtable.admin.v2.CreateInstanceRequest actual = input + .toProto(ProjectName.of("my-project")); + + com.google.bigtable.admin.v2.CreateInstanceRequest expected = + com.google.bigtable.admin.v2.CreateInstanceRequest.newBuilder() + .setParent(ProjectName.of("my-project").toString()) + .setInstanceId("my-instance") + .setInstance( + Instance.newBuilder() + .setDisplayName("custom display name") + .putLabels("my label", "with some value") + .putLabels("my other label", "with some value") + .setType(Type.DEVELOPMENT) + ) + .putClusters("cluster1", + Cluster.newBuilder() + .setLocation("projects/my-project/locations/us-east1-c") + .setServeNodes(1) + .setDefaultStorageType(StorageType.SSD) + .build() + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } +} \ No newline at end of file diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java new file mode 100644 index 000000000000..1904028744ab --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.bigtable.admin.v2.Instance.State; +import com.google.bigtable.admin.v2.Instance.Type; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class InstanceTest { + @Test + public void testFromProto() { + com.google.bigtable.admin.v2.Instance proto = com.google.bigtable.admin.v2.Instance.newBuilder() + .setName("projects/my-project/instances/my-instance") + .setDisplayName("my display name") + .setType(Type.PRODUCTION) + .setState(State.READY) + .putLabels("label1", "value1") + .putLabels("label2", "value2") + .build(); + + Instance result = Instance.PROTO_TRANSFORMER.apply(proto); + + assertThat(result.getId()).isEqualTo("my-instance"); + assertThat(result.getDisplayName()).isEqualTo("my display name"); + assertThat(result.getType()).isEqualTo(Type.PRODUCTION); + assertThat(result.getState()).isEqualTo(State.READY); + assertThat(result.getLabels()).containsExactly( + "label1", "value1", + "label2", "value2" + ); + } + + @Test + public void testRequiresName() { + com.google.bigtable.admin.v2.Instance proto = com.google.bigtable.admin.v2.Instance.newBuilder() + .setDisplayName("my display name") + .setType(Type.PRODUCTION) + .setState(State.READY) + .putLabels("label1", "value1") + .putLabels("label2", "value2") + .build(); + + Exception actualException = null; + + try { + Instance.PROTO_TRANSFORMER.apply(proto); + } catch (Exception e) { + actualException = e; + } + + assertThat(actualException).isInstanceOf(IllegalArgumentException.class); + } +} \ No newline at end of file diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java new file mode 100644 index 000000000000..b5d7b3902853 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.admin.v2.models; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.bigtable.admin.v2.Instance; +import com.google.bigtable.admin.v2.Instance.Type; +import com.google.bigtable.admin.v2.PartialUpdateInstanceRequest; +import com.google.bigtable.admin.v2.ProjectName; +import com.google.common.collect.ImmutableMap; +import com.google.protobuf.FieldMask; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class UpdateInstanceRequestTest { + @Test + public void testEmpty() { + UpdateInstanceRequest input = UpdateInstanceRequest.of("my-instance"); + + Exception actualError = null; + + try { + input.toProto(ProjectName.of("my-project")); + } catch (Exception e) { + actualError = e; + } + + assertThat(actualError).isInstanceOf(IllegalStateException.class); + } + + @Test + public void testDisplayName() { + UpdateInstanceRequest input = UpdateInstanceRequest.of("my-instance") + .setDisplayName("my display name"); + + PartialUpdateInstanceRequest actual = input.toProto(ProjectName.of("my-project")); + + PartialUpdateInstanceRequest expected = PartialUpdateInstanceRequest.newBuilder() + .setUpdateMask( + FieldMask.newBuilder() + .addPaths("display_name") + ) + .setInstance( + Instance.newBuilder() + .setName("projects/my-project/instances/my-instance") + .setDisplayName("my display name") + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void testLabels() { + UpdateInstanceRequest input = UpdateInstanceRequest.of("my-instance") + .setLabels(ImmutableMap.of( + "label1", "value1", + "label2", "value2" + )); + + PartialUpdateInstanceRequest actual = input.toProto(ProjectName.of("my-project")); + + PartialUpdateInstanceRequest expected = PartialUpdateInstanceRequest.newBuilder() + .setUpdateMask( + FieldMask.newBuilder() + .addPaths("labels") + ) + .setInstance( + Instance.newBuilder() + .setName("projects/my-project/instances/my-instance") + .putLabels("label1", "value1") + .putLabels("label2", "value2") + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void testType() { + UpdateInstanceRequest input = UpdateInstanceRequest.of("my-instance") + .setProductionType(); + + PartialUpdateInstanceRequest actual = input.toProto(ProjectName.of("my-project")); + + PartialUpdateInstanceRequest expected = PartialUpdateInstanceRequest.newBuilder() + .setUpdateMask( + FieldMask.newBuilder() + .addPaths("type") + ) + .setInstance( + Instance.newBuilder() + .setName("projects/my-project/instances/my-instance") + .setType(Type.PRODUCTION) + ) + .build(); + + assertThat(actual).isEqualTo(expected); + } +} From 08592c7c16118e4d4ff872ca389fe515d55580f6 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Fri, 17 Aug 2018 17:14:22 -0400 Subject: [PATCH 3/7] Add sample code --- .../admin/v2/BigtableInstanceAdminClient.java | 135 ++++++++++++++++-- .../models/PartialListInstancesException.java | 10 +- .../v2/BigtableInstanceAdminClientTest.java | 2 +- .../v2/models/CreateClusterRequestTest.java | 2 +- 4 files changed, 132 insertions(+), 17 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java index 8e49ced04ece..9fde4e0dff31 100644 --- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java +++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java @@ -47,7 +47,7 @@ *

See the individual methods for example code. * * 

{@code
- * try(BigtableInstanceAdminClient client =  BigtableInstanceAdminClient.create(ProjectName.of("[PROJECT]"))) {
+ * try(BigtableInstanceAdminClient client =  BigtableInstanceAdminClient.create(ProjectName.of("my-project"))) {
  *   CreateInstanceRequest request = CreateInstanceRequest.of("my-instance")
  *     .addCluster("my-cluster", "us-east1-c", 3, StorageType.SSD);
  *
@@ -65,7 +65,7 @@
  *
  * 
{@code
  * BigtableInstanceAdminSettings settings = BigtableInstanceAdminSettings.newBuilder()
- *   .setProjectName(ProjectName.of("[PROJECT]"))
+ *   .setProjectName(ProjectName.of("my-project"))
  *   .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
  *   .build();
  *
@@ -76,7 +76,7 @@
  *
  * 
{@code
  * BigtableInstanceAdminSettings settings = BigtableInstanceAdminSettings.newBuilder()
- *   .setProjectName(ProjectName.of("[PROJECT]"))
+ *   .setProjectName(ProjectName.of("my-project"))
  *   .setEndpoint(myEndpoint)
  *   .build();
  *
@@ -127,6 +127,15 @@ public void close() {
   /**
    * Creates a new instance and returns its representation.
    *
+   * 
Sample code:
+   *
+   * 
{@code
+   * Instance instance = client.createInstance(
+   *   CreateInstanceRequest.of("my-instance")
+   *     .addCluster("my-cluster", "us-east1-c", 3, StorageType.SSD)
+   * );
+   * }

+   *
    * @see CreateInstanceRequest for details.
    */
   @SuppressWarnings("WeakerAccess")
@@ -137,6 +146,17 @@ public Instance createInstance(CreateInstanceRequest request) {
   /**
    * Asynchronously creates a new instance and returns its representation wrapped in a future.
    *
+   * 
Sample code:
+   *
+   * 
{@code
+   * ApiFuture instanceFuture = client.createInstanceAsync(
+   *   CreateInstanceRequest.of("my-instance")
+   *     .addCluster("my-cluster", "us-east1-c", 3, StorageType.SSD)
+   * );
+   *
+   * Instance instance = instanceFuture.get();
+   * }

+   *
    * @see CreateInstanceRequest for details.
    */
   @SuppressWarnings("WeakerAccess")
@@ -155,6 +175,15 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
   /**
    * Updates a new instance and returns its representation.
    *
+   * 
Sample code:
+   *
+   * 
{@code
+   * Instance instance = client.updateInstance(
+   *   UpdateInstanceRequest.of("my-instance")
+   *     .setProductionType()
+   * )
+   * }

+   *
    * @see UpdateInstanceRequest for details.
    */
   @SuppressWarnings("WeakerAccess")
@@ -165,6 +194,17 @@ public Instance updateInstance(UpdateInstanceRequest request) {
   /**
    * Asynchronously updates a new instance and returns its representation wrapped in a future.
    *
+   * 
Sample code:
+   *
+   * 
{@code
+   * ApiFuture instanceFuture = client.updateInstanceAsync(
+   *   UpdateInstanceRequest.of("my-instance")
+   *     .setProductionType()
+   * )
+   *
+   * Instance instance = instanceFuture.get();
+   * }

+   *
    * @see UpdateInstanceRequest for details.
    */
   @SuppressWarnings("WeakerAccess")
@@ -180,13 +220,30 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
         MoreExecutors.directExecutor());
   }
 
-  /** Get the instance representation. */
+  /**
+   * Get the instance representation.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * Instance instance = client.getInstance("my-instance");
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public Instance getInstance(String id) {
     return awaitFuture(getInstanceAsync(id));
   }
 
-  /** Asynchronously gets the instance representation wrapped in a future. */
+  /**
+   * Asynchronously gets the instance representation wrapped in a future.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * ApiFuture instanceFuture = client.getInstanceAsync("my-instance");
+   * Instance instance = instanceFuture.get();
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public ApiFuture getInstanceAsync(String instanceId) {
     InstanceName name = InstanceName.of(projectName.getProject(), instanceId);
@@ -206,13 +263,55 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
         MoreExecutors.directExecutor());
   }
 
-  /** Lists all of the instances in the current project. */
+  /**
+   * Lists all of the instances in the current project.
+   *
+   * 
This method will throw a {@link PartialListInstancesException} when any zone is
+   * unavailable. If partial listing are ok, the exception can be caught and inspected.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * try {
+   *   List instances = client.listInstances();
+   * } catch (PartialListInstancesException e) {
+   *   System.out.println("The following zones are unavailable: " + e.getUnavailableZones());
+   *   System.out.println("But the following instances are reachable: " + e.getInstances());
+   * }
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public List listInstances() {
     return awaitFuture(listInstancesAsync());
   }
 
-  /** Asynchronously lists all of the instances in the current project. */
+  /**
+   * Asynchronously lists all of the instances in the current project.
+   *
+   * 
This method will throw a {@link PartialListInstancesException} when any zone is
+   * unavailable.
+   * If partial listing are ok, the exception can be caught and inspected.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * ApiFutures.addCallback(instancesFuture, new ApiFutureCallback>() {
+   *   @Override
+   *   public void onFailure(Throwable t) {
+   *     if (t instanceof PartialListInstancesException) {
+   *       PartialListInstancesException partialError = (PartialListInstancesException)t;
+   *       System.out.println("The following zones are unavailable: " + partialError.getUnavailableZones());
+   *       System.out.println("But the following instances are reachable: " + partialError.getInstances());
+   *     }
+   *   }
+   *
+   *   @Override
+   *   public void onSuccess(List result) {
+   *     System.out.println("Found a complete set of instances: " + result);
+   *   }
+   * }, MoreExecutors.directExecutor());
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public ApiFuture> listInstancesAsync() {
     ListInstancesRequest request = ListInstancesRequest.newBuilder()
@@ -254,13 +353,30 @@ public List apply(ListInstancesResponse proto) {
         }, MoreExecutors.directExecutor());
   }
 
-  /** Deletes the specified instance. */
+  /**
+   * Deletes the specified instance.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * client.deleteInstance("my-instance");
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public void deleteInstance(String instanceId) {
     awaitFuture(deleteInstanceAsync(instanceId));
   }
 
-  /** Asynchronously deletes the specified instance. */
+  /**
+   * Asynchronously deletes the specified instance.
+   *
+   * 
Sample code:
+   *
+   * 
{@code
+   * ApiFuture deleteFuture = client.deleteInstance("my-instance");
+   * deleteFuture.get();
+   * }

+   */
   @SuppressWarnings("WeakerAccess")
   public ApiFuture deleteInstanceAsync(String instanceId) {
     InstanceName instanceName = InstanceName.of(projectName.getProject(), instanceId);
@@ -307,5 +423,4 @@ private  T awaitFuture(ApiFuture future) {
 
     throw error;
   }
-
 }
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
index 54bb98b98979..ad5269d28a18 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
@@ -22,19 +22,19 @@
  * instance list. This exception can be inspected to get a partial list.
  */
 public class PartialListInstancesException extends RuntimeException {
-  private final List failedZones;
+  private final List unavailableZones;
   private final List instances;
 
-  public PartialListInstancesException(List failedZones, List instances) {
+  public PartialListInstancesException(List unavailableZones, List instances) {
     super("Failed to list all instances, some zones where unavailable");
 
-    this.failedZones = failedZones;
+    this.unavailableZones = unavailableZones;
     this.instances = instances;
   }
 
   /** A list of zones, whose unavailability caused this error. */
-  public List getFailedZones() {
-    return failedZones;
+  public List getUnavailableZones() {
+    return unavailableZones;
   }
 
   /** A partial list of instances that were found in the available zones. */
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
index 82c6a78dbacf..1993b01162b0 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
@@ -269,7 +269,7 @@ public void testListInstancesFailedZone() {
 
     assertThat(partialListError.getInstances())
         .containsExactly(Instance.fromProto(expectedResponse.getInstances(0)));
-    assertThat(partialListError.getFailedZones()).containsExactly("us-east1-d");
+    assertThat(partialListError.getUnavailableZones()).containsExactly("us-east1-d");
   }
 
   @Test
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java
index cf4c4319a0b1..dd0c318bd559 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequestTest.java
@@ -160,4 +160,4 @@ public void testOptionalFields() {
 
     assertThat(actual).isEqualTo(expected);
   }
-}
\ No newline at end of file
+}

From d7ecfd2a71896d1b9cf9401d6e75e80168cac4cd Mon Sep 17 00:00:00 2001
From: Igor Bernstein 
Date: Fri, 17 Aug 2018 17:22:01 -0400
Subject: [PATCH 4/7] tweaks

---
 .../v2/BigtableInstanceAdminClientTest.java   | 49 ++++++++++---------
 .../admin/v2/models/InstanceTest.java         |  4 +-
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
index 1993b01162b0..1adbf9be0641 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTest.java
@@ -30,9 +30,7 @@
 import com.google.bigtable.admin.v2.CreateInstanceMetadata;
 import com.google.bigtable.admin.v2.Instance.Type;
 import com.google.bigtable.admin.v2.InstanceName;
-import com.google.bigtable.admin.v2.ListInstancesResponse;
 import com.google.bigtable.admin.v2.LocationName;
-import com.google.bigtable.admin.v2.PartialUpdateInstanceRequest;
 import com.google.bigtable.admin.v2.ProjectName;
 import com.google.bigtable.admin.v2.StorageType;
 import com.google.bigtable.admin.v2.UpdateInstanceMetadata;
@@ -135,7 +133,7 @@ public void testCreateInstance() {
     mockOperationResult(mockCreateInstanceCallable, expectedRequest, expectedResponse);
 
     // Execute
-    com.google.cloud.bigtable.admin.v2.models.Instance actualResult = adminClient.createInstance(
+    Instance actualResult = adminClient.createInstance(
         CreateInstanceRequest.of(INSTANCE_NAME.getInstance())
             .setType(Type.DEVELOPMENT)
             .addCluster("cluster1", "us-east1-c", 1, StorageType.SSD)
@@ -149,7 +147,7 @@ public void testCreateInstance() {
   public void testUpdateInstance() {
     // Setup
     com.google.bigtable.admin.v2.PartialUpdateInstanceRequest expectedRequest =
-        PartialUpdateInstanceRequest.newBuilder()
+        com.google.bigtable.admin.v2.PartialUpdateInstanceRequest.newBuilder()
             .setUpdateMask(
                 FieldMask.newBuilder()
                     .addPaths("display_name")
@@ -169,7 +167,7 @@ public void testUpdateInstance() {
     mockOperationResult(mockUpdateInstanceCallable, expectedRequest, expectedResponse);
 
     // Execute
-    com.google.cloud.bigtable.admin.v2.models.Instance actualResult = adminClient.updateInstance(
+    Instance actualResult = adminClient.updateInstance(
         UpdateInstanceRequest.of(INSTANCE_NAME.getInstance())
             .setDisplayName("new display name")
     );
@@ -209,15 +207,18 @@ public void testListInstances() {
             .setParent(PROJECT_NAME.toString())
             .build();
 
-    com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse = ListInstancesResponse
-        .newBuilder()
-        .addInstances(
-            com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1").build()
-        )
-        .addInstances(
-            com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "2").build()
-        )
-        .build();
+    com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse =
+        com.google.bigtable.admin.v2.ListInstancesResponse
+            .newBuilder()
+            .addInstances(
+                com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1")
+                    .build()
+            )
+            .addInstances(
+                com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "2")
+                    .build()
+            )
+            .build();
 
     Mockito.when(mockListInstancesCallable.futureCall(expectedRequest))
         .thenReturn(ApiFutures.immediateFuture(expectedResponse));
@@ -240,15 +241,17 @@ public void testListInstancesFailedZone() {
             .setParent(PROJECT_NAME.toString())
             .build();
 
-    com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse = ListInstancesResponse
-        .newBuilder()
-        .addInstances(
-            com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1").build()
-        )
-        .addFailedLocations(
-            LocationName.of(PROJECT_NAME.getProject(), "us-east1-d").toString()
-        )
-        .build();
+    com.google.bigtable.admin.v2.ListInstancesResponse expectedResponse =
+        com.google.bigtable.admin.v2.ListInstancesResponse
+            .newBuilder()
+            .addInstances(
+                com.google.bigtable.admin.v2.Instance.newBuilder().setName(INSTANCE_NAME + "1")
+                    .build()
+            )
+            .addFailedLocations(
+                LocationName.of(PROJECT_NAME.getProject(), "us-east1-d").toString()
+            )
+            .build();
 
     Mockito.when(mockListInstancesCallable.futureCall(expectedRequest))
         .thenReturn(ApiFutures.immediateFuture(expectedResponse));
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
index 1904028744ab..38199bb5bf23 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
@@ -36,7 +36,7 @@ public void testFromProto() {
         .putLabels("label2", "value2")
         .build();
 
-    Instance result = Instance.PROTO_TRANSFORMER.apply(proto);
+    Instance result = Instance.fromProto(proto);
 
     assertThat(result.getId()).isEqualTo("my-instance");
     assertThat(result.getDisplayName()).isEqualTo("my display name");
@@ -61,7 +61,7 @@ public void testRequiresName() {
     Exception actualException = null;
 
     try {
-      Instance.PROTO_TRANSFORMER.apply(proto);
+      Instance.fromProto(proto);
     } catch (Exception e) {
       actualException = e;
     }

From 265c0fc36d774a5df0d166eeffb8fe6e398181e5 Mon Sep 17 00:00:00 2001
From: Igor Bernstein 
Date: Fri, 17 Aug 2018 17:31:45 -0400
Subject: [PATCH 5/7] fix javadocs

---
 .../cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java    | 2 --
 1 file changed, 2 deletions(-)

diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
index 9fde4e0dff31..8b7300056e25 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
@@ -296,7 +296,6 @@ public List listInstances() {
    *
    * 
{@code
    * ApiFutures.addCallback(instancesFuture, new ApiFutureCallback>() {
-   *   @Override
    *   public void onFailure(Throwable t) {
    *     if (t instanceof PartialListInstancesException) {
    *       PartialListInstancesException partialError = (PartialListInstancesException)t;
@@ -305,7 +304,6 @@ public List listInstances() {
    *     }
    *   }
    *
-   *   @Override
    *   public void onSuccess(List result) {
    *     System.out.println("Found a complete set of instances: " + result);
    *   }

From 9d8e1a5c347cba921ca2bc2d5fa4ed2479a5377c Mon Sep 17 00:00:00 2001
From: Igor Bernstein 
Date: Fri, 17 Aug 2018 17:54:21 -0400
Subject: [PATCH 6/7] tweaks

---
 .../bigtable/admin/v2/models/CreateInstanceRequest.java   | 8 ++++++--
 .../google/cloud/bigtable/admin/v2/models/Instance.java   | 2 +-
 .../admin/v2/models/PartialListInstancesException.java    | 5 ++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java
index 26b41c76218e..3def1a20ca8e 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateInstanceRequest.java
@@ -65,7 +65,7 @@ public final class CreateInstanceRequest {
   private final List clusterRequests = Lists.newArrayList();
 
   /** Builds a new request to create a new instance with the specified id. */
-  public static CreateInstanceRequest of(String instanceId) {
+  public static CreateInstanceRequest of(@Nonnull String instanceId) {
     return new CreateInstanceRequest(instanceId);
   }
 
@@ -80,6 +80,7 @@ private CreateInstanceRequest(@Nonnull String instanceId) {
   /**
    * Sets the friendly display name of the instance. If left unspecified, it will default to the id
    */
+  @SuppressWarnings("WeakerAccess")
   public CreateInstanceRequest setDisplayName(@Nonnull String displayName) {
     Preconditions.checkNotNull(displayName);
     builder.getInstanceBuilder().setDisplayName(displayName);
@@ -93,6 +94,7 @@ public CreateInstanceRequest setDisplayName(@Nonnull String displayName) {
    * Please see class javadoc for details.
    */
   // TODO(igorbernstein2): try to avoid leaking protobuf generated enums
+  @SuppressWarnings("WeakerAccess")
   public CreateInstanceRequest setType(@Nonnull Type type) {
     Preconditions.checkNotNull(type);
     builder.getInstanceBuilder().setType(type);
@@ -107,6 +109,7 @@ public CreateInstanceRequest setType(@Nonnull Type type) {
    *
    * @see For more details
    */
+  @SuppressWarnings("WeakerAccess")
   public CreateInstanceRequest addLabel(@Nonnull String key, @Nonnull String value) {
     Preconditions.checkNotNull(key, "Key can't be null");
     Preconditions.checkNotNull(value, "Value can't be null");
@@ -126,7 +129,8 @@ public CreateInstanceRequest addLabel(@Nonnull String key, @Nonnull String value
    * @param storageType The type of storage used by this cluster to serve its parent instance's tables.
    */
   // TODO(igorbernstein2): try to avoid leaking protobuf generated enums
-  public CreateInstanceRequest addCluster(String clusterId, String zone, int serveNodes, StorageType storageType) {
+  @SuppressWarnings("WeakerAccess")
+  public CreateInstanceRequest addCluster(@Nonnull String clusterId, @Nonnull String zone, int serveNodes, @Nonnull StorageType storageType) {
     CreateClusterRequest clusterRequest = CreateClusterRequest
         .of("ignored-instance-id", clusterId)
         .setZone(zone)
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java
index b6cde0128f5b..5d067aaaa24e 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Instance.java
@@ -40,7 +40,7 @@ public final class Instance {
    * to be used by applications.
    */
   @InternalApi
-  public static Instance fromProto(com.google.bigtable.admin.v2.Instance proto) {
+  public static Instance fromProto(@Nonnull com.google.bigtable.admin.v2.Instance proto) {
     return new Instance(proto);
   }
 
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
index ad5269d28a18..894b2931b562 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
@@ -15,7 +15,9 @@
  */
 package com.google.cloud.bigtable.admin.v2.models;
 
+import com.google.api.core.InternalApi;
 import java.util.List;
+import javax.annotation.Nonnull;
 
 /**
  * Exception thrown when some zones are unavailable and listInstances is unable to return a full
@@ -25,7 +27,8 @@ public class PartialListInstancesException extends RuntimeException {
   private final List unavailableZones;
   private final List instances;
 
-  public PartialListInstancesException(List unavailableZones, List instances) {
+  @InternalApi
+  public PartialListInstancesException(@Nonnull List unavailableZones, @Nonnull List instances) {
     super("Failed to list all instances, some zones where unavailable");
 
     this.unavailableZones = unavailableZones;

From 73ea373a0b2d29e97e3dc78aac487a390762f128 Mon Sep 17 00:00:00 2001
From: Igor Bernstein 
Date: Mon, 27 Aug 2018 11:29:51 -0400
Subject: [PATCH 7/7] Address feedback

---
 .../admin/v2/BigtableInstanceAdminClient.java | 21 +++++++++++--------
 .../admin/v2/models/CreateClusterRequest.java |  6 ++++--
 .../models/PartialListInstancesException.java |  5 +++--
 .../v2/models/UpdateInstanceRequest.java      |  2 +-
 .../admin/v2/models/InstanceTest.java         |  2 +-
 .../v2/models/UpdateInstanceRequestTest.java  |  2 +-
 6 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
index 8b7300056e25..d0d690ad9fe2 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java
@@ -38,6 +38,7 @@
 import com.google.protobuf.Empty;
 import java.io.IOException;
 import java.util.List;
+import java.util.Objects;
 import javax.annotation.Nonnull;
 
 /**
@@ -181,7 +182,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
    * Instance instance = client.updateInstance(
    *   UpdateInstanceRequest.of("my-instance")
    *     .setProductionType()
-   * )
+   * );
    * }

    *
    * @see UpdateInstanceRequest for details.
@@ -200,7 +201,7 @@ public Instance updateInstance(UpdateInstanceRequest request) {
    * ApiFuture instanceFuture = client.updateInstanceAsync(
    *   UpdateInstanceRequest.of("my-instance")
    *     .setProductionType()
-   * )
+   * );
    *
    * Instance instance = instanceFuture.get();
    * }

@@ -221,7 +222,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
   }
 
   /**
-   * Get the instance representation.
+   * Get the instance representation by ID.
    *
    * 
Sample code:
    *
@@ -235,7 +236,7 @@ public Instance getInstance(String id) {
   }
 
   /**
-   * Asynchronously gets the instance representation wrapped in a future.
+   * Asynchronously gets the instance representation by ID wrapped in a future.
    *
    * 
Sample code:
    *
@@ -295,12 +296,16 @@ public List listInstances() {
    * 
Sample code:
    *
    * 
{@code
+   * ApiFuture instancesFuture = client.listInstancesAsync();
+   *
    * ApiFutures.addCallback(instancesFuture, new ApiFutureCallback>() {
    *   public void onFailure(Throwable t) {
    *     if (t instanceof PartialListInstancesException) {
    *       PartialListInstancesException partialError = (PartialListInstancesException)t;
    *       System.out.println("The following zones are unavailable: " + partialError.getUnavailableZones());
    *       System.out.println("But the following instances are reachable: " + partialError.getInstances());
+   *     } else {
+   *       t.printStackTrace();
    *     }
    *   }
    *
@@ -323,7 +328,8 @@ public ApiFuture> listInstancesAsync() {
         .transform(responseFuture, new ApiFunction>() {
           @Override
           public List apply(ListInstancesResponse proto) {
-            // NOTE: pagination is intentionally ignored. The server does not implement it.
+            // NOTE: pagination is intentionally ignored. The server does not implement it and never
+            // will.
             Verify.verify(proto.getNextPageToken().isEmpty(),
                 "Server returned an unexpected paginated response");
 
@@ -335,10 +341,7 @@ public List apply(ListInstancesResponse proto) {
 
             ImmutableList.Builder failedZones = ImmutableList.builder();
             for (String locationStr : proto.getFailedLocationsList()) {
-              LocationName fullLocation = LocationName.parse(locationStr);
-              if (fullLocation == null) {
-                continue;
-              }
+              LocationName fullLocation = Objects.requireNonNull(LocationName.parse(locationStr));
               failedZones.add(fullLocation.getLocation());
             }
 
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java
index 130cf46d5dfb..d653be2ac99b 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateClusterRequest.java
@@ -49,7 +49,8 @@
 public final class CreateClusterRequest {
   private final com.google.bigtable.admin.v2.CreateClusterRequest.Builder proto = com.google.bigtable.admin.v2.CreateClusterRequest
       .newBuilder();
-  // Partial ids will be set when the project is passed to toProto
+  // instanceId and zone are short ids, which will be expanded to full names when the project name
+  // is passed to toProto
   private final String instanceId;
   private String zone;
 
@@ -77,7 +78,8 @@ public CreateClusterRequest setZone(String zone) {
     return this;
   }
 
-  /** Sets the type of storage used by this cluster to serve its parent instance's tables. */
+  /** Sets the number of nodes allocated to this cluster. More nodes enable higher throughput and
+   * more consistent performance. */
   @SuppressWarnings("WeakerAccess")
   public CreateClusterRequest setServeNodes(int numNodes) {
     proto.getClusterBuilder().setServeNodes(numNodes);
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
index 894b2931b562..81bbc796afb8 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/PartialListInstancesException.java
@@ -16,6 +16,7 @@
 package com.google.cloud.bigtable.admin.v2.models;
 
 import com.google.api.core.InternalApi;
+import com.google.common.collect.ImmutableList;
 import java.util.List;
 import javax.annotation.Nonnull;
 
@@ -31,8 +32,8 @@ public class PartialListInstancesException extends RuntimeException {
   public PartialListInstancesException(@Nonnull List unavailableZones, @Nonnull List instances) {
     super("Failed to list all instances, some zones where unavailable");
 
-    this.unavailableZones = unavailableZones;
-    this.instances = instances;
+    this.unavailableZones = ImmutableList.copyOf(unavailableZones);
+    this.instances = ImmutableList.copyOf(instances);
   }
 
   /** A list of zones, whose unavailability caused this error. */
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java
index 3203971d9695..ebfda5eb576f 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequest.java
@@ -78,7 +78,7 @@ public UpdateInstanceRequest setProductionType() {
    * details
    */
   @SuppressWarnings("WeakerAccess")
-  public UpdateInstanceRequest setLabels(@Nonnull Map labels) {
+  public UpdateInstanceRequest setAllLabels(@Nonnull Map labels) {
     Preconditions.checkNotNull(labels, "labels can't be null");
     builder.getInstanceBuilder().clearLabels();
     builder.getInstanceBuilder().putAllLabels(labels);
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
index 38199bb5bf23..3d63bab16d0c 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/InstanceTest.java
@@ -68,4 +68,4 @@ public void testRequiresName() {
 
     assertThat(actualException).isInstanceOf(IllegalArgumentException.class);
   }
-}
\ No newline at end of file
+}
diff --git a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java
index b5d7b3902853..b44464fa0918 100644
--- a/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java
+++ b/google-cloud-clients/google-cloud-bigtable-admin/src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateInstanceRequestTest.java
@@ -69,7 +69,7 @@ public void testDisplayName() {
   @Test
   public void testLabels() {
     UpdateInstanceRequest input = UpdateInstanceRequest.of("my-instance")
-        .setLabels(ImmutableMap.of(
+        .setAllLabels(ImmutableMap.of(
             "label1", "value1",
             "label2", "value2"
         ));