Bigtable: deprecate the use of typesafe names (#4257)

* Bigtable: deprecate the use of typesafe names

Unfortunately typesafe names create more issues for bigtable than they solve. A lot of the time users need to use both bigtable data & admin apis. Unfortunately, those apis define the same names in different namespaces. This forces users to use fully qualified names which is ugly. Also, absolute names don't benefit the bigtable client because the client is anchored at the instance and all names end up being relative

* fix docs

* format code

.kokoro/continuous/bigtable-it.cfg

@@ -8,7 +8,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
+    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
 }
 
 env_vars: {

.kokoro/nightly/bigtable-it.cfg

@@ -8,7 +8,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
+    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
 }
 
 env_vars: {

.kokoro/presubmit/bigtable-it.cfg

@@ -8,7 +8,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.table=projects/gcloud-devel/instances/google-cloud-bigtable/tables/integration-tests"
+    value: "google-cloud-clients/google-cloud-bigtable -Dbigtable.env=prod -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests"
 }
 
 env_vars: {

TESTING.md

@@ -61,7 +61,9 @@ To use the `prod` environment:
     ```shell
     mvn verify -am -pl google-cloud-bigtable \
       -Dbigtable.env=prod \
-      -Dbigtable.table=projects/my-project/instances/my-instance/tables/my-table
+      -Dbigtable.project=my-project
+      -Dbigtable.instance=my-instance
+      -Dbigtable.table=my-table
     ```
 
 ### Testing code that uses Bigtable Admin

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java

@@ -20,7 +20,6 @@
 import com.google.api.gax.rpc.ServerStreamingCallSettings;
 import com.google.api.gax.rpc.UnaryCallSettings;
 import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation;
-import com.google.cloud.bigtable.data.v2.models.InstanceName;
 import com.google.cloud.bigtable.data.v2.models.KeyOffset;
 import com.google.cloud.bigtable.data.v2.models.Query;
 import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow;
@@ -49,7 +48,8 @@
  *
  * <pre>{@code
  * BigtableDataSettings.Builder settingsBuilder = BigtableDataSettings.newBuilder()
- *   .setInstanceName(InstanceName.of("my-project", "my-instance-id"))
+ *   .setProjectId("my-project")
+ *   .setInstanceId("my-instance-id")
  *   .setAppProfileId("default");
  *
  * settingsBuilder.readRowsSettings().setRetryableCodes(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE);
@@ -67,12 +67,27 @@ public static Builder newBuilder() {
     return new Builder();
   }
 
-  /** Returns the target instance */
-  public InstanceName getInstanceName() {
+  /**
+   * Returns the target instance.
+   *
+   * @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
+   */
+  @Deprecated()
+  public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
     return getTypedStubSettings().getInstanceName();
   }
 
-  /** Returns the configured AppProfile to use */
+  /** Returns the target project id. */
+  public String getProjectId() {
+    return getTypedStubSettings().getProjectId();
+  }
+
+  /** Returns the target instance id. */
+  public String getInstanceId() {
+    return getTypedStubSettings().getInstanceId();
+  }
+
+  /** Returns the configured AppProfile id to use. */
   public String getAppProfileId() {
     return getTypedStubSettings().getAppProfileId();
   }
@@ -142,17 +157,55 @@ private Builder(BigtableDataSettings settings) {
     /**
      * Sets the target instance. This setting is required. All RPCs will be made in the context of
      * this setting.
+     *
+     * @deprecated Please use {@link #setProjectId(String)} and {@link #setInstanceId(String)}.
      */
-    public Builder setInstanceName(@Nonnull InstanceName instanceName) {
+    @Deprecated
+    public Builder setInstanceName(
+        @Nonnull com.google.cloud.bigtable.data.v2.models.InstanceName instanceName) {
       getTypedStubSettings().setInstanceName(instanceName);
       return this;
     }
 
-    /** Gets the {@link InstanceName} that was previously set on this Builder. */
-    public InstanceName getInstanceName() {
+    /**
+     * Gets the {@link com.google.cloud.bigtable.data.v2.models.InstanceName} that was previously
+     * set on this Builder.
+     *
+     * @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
+     */
+    @Deprecated
+    public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
       return getTypedStubSettings().getInstanceName();
     }
 
+    /**
+     * Sets the target project. This setting is required. All RPCs will be made in the context of
+     * this setting.
+     */
+    public Builder setProjectId(@Nonnull String projectId) {
+      getTypedStubSettings().setProjectId(projectId);
+      return this;
+    }
+
+    /** Gets the project id that was previously set on this Builder. */
+    public String getProjectId() {
+      return getTypedStubSettings().getProjectId();
+    }
+
+    /**
+     * Sets the target instance. This setting is required. All RPCs will be made in the context of
+     * this setting.
+     */
+    public Builder setInstanceId(@Nonnull String instanceId) {
+      getTypedStubSettings().setInstanceId(instanceId);
+      return this;
+    }
+
+    /** Gets the instance id that was previously set on this Builder. */
+    public String getInstanceId() {
+      return getTypedStubSettings().getInstanceId();
+    }
+
     /**
      * Sets the AppProfile to use. An application profile (sometimes also shortened to "app
      * profile") is a group of configuration parameters for an individual use case. A client will

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/NameUtil.java

@@ -0,0 +1,37 @@
+/*
+ * 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.data.v2.internal;
+
+import com.google.api.core.InternalApi;
+import javax.annotation.Nonnull;
+
+/**
+ * Internal helper to compose full resource names.
+ *
+ * <p>This class is considered an internal implementation detail and not meant to be used by
+ * applications.
+ */
+@InternalApi
+public class NameUtil {
+  public static String formatInstanceName(@Nonnull String projectId, @Nonnull String instanceId) {
+    return "projects/" + projectId + "/instances/" + instanceId;
+  }
+
+  public static String formatTableName(
+      @Nonnull String projectId, @Nonnull String instanceId, @Nonnull String tableId) {
+    return formatInstanceName(projectId, instanceId) + "/tables/" + tableId;
+  }
+}

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RequestContext.java

@@ -17,7 +17,6 @@
 
 import com.google.api.core.InternalApi;
 import com.google.auto.value.AutoValue;
-import com.google.cloud.bigtable.data.v2.models.InstanceName;
 
 /**
  * Contains information necessary to construct Bigtable protobuf requests from user facing models.
@@ -32,12 +31,36 @@
 @InternalApi
 @AutoValue
 public abstract class RequestContext {
-  public static RequestContext create(InstanceName instanceName, String appProfileId) {
-    return new AutoValue_RequestContext(instanceName, appProfileId);
+
+  /** Creates a new instance of the {@link RequestContext}. */
+  public static RequestContext create(String projectId, String instanceId, String appProfileId) {
+    return new AutoValue_RequestContext(projectId, instanceId, appProfileId);
+  }
+
+  /** @deprecated Please use {@link #create(String, String, String)}. */
+  @Deprecated
+  public static RequestContext create(
+      com.google.cloud.bigtable.data.v2.models.InstanceName instanceName, String appProfileId) {
+    return new AutoValue_RequestContext(
+        instanceName.getProject(), instanceName.getInstance(), appProfileId);
   }
 
-  /** The instance that the client is configured to target */
-  public abstract InstanceName getInstanceName();
+  /** The project id that the client is configured to target. */
+  public abstract String getProjectId();
+
+  /** The instance id that the client is configured to target. */
+  public abstract String getInstanceId();
+
+  /**
+   * The instance that the client is configured to target.
+   *
+   * @deprecated Please use {@link #getProjectId()} and {@link #getInstanceId()}.
+   */
+  @Deprecated
+  public com.google.cloud.bigtable.data.v2.models.InstanceName getInstanceName() {
+    return com.google.cloud.bigtable.data.v2.models.InstanceName.of(
+        getProjectId(), getInstanceId());
+  }
 
   /** The App Profile to use when processing the current request */
   public abstract String getAppProfileId();

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/BulkMutation.java

@@ -17,7 +17,7 @@
 
 import com.google.api.core.InternalApi;
 import com.google.bigtable.v2.MutateRowsRequest;
-import com.google.bigtable.v2.TableName;
+import com.google.cloud.bigtable.data.v2.internal.NameUtil;
 import com.google.cloud.bigtable.data.v2.internal.RequestContext;
 import com.google.common.base.Preconditions;
 import com.google.protobuf.ByteString;
@@ -91,14 +91,12 @@ public BulkMutation add(@Nonnull ByteString rowKey, @Nonnull Mutation mutation)
 
   @InternalApi
   public MutateRowsRequest toProto(RequestContext requestContext) {
-    TableName tableName =
-        TableName.of(
-            requestContext.getInstanceName().getProject(),
-            requestContext.getInstanceName().getInstance(),
-            tableId);
+    String tableName =
+        NameUtil.formatTableName(
+            requestContext.getProjectId(), requestContext.getInstanceId(), tableId);
 
     return builder
-        .setTableName(tableName.toString())
+        .setTableName(tableName)
         .setAppProfileId(requestContext.getAppProfileId())
         .build();
   }

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/ConditionalRowMutation.java

@@ -17,7 +17,7 @@
 
 import com.google.api.core.InternalApi;
 import com.google.bigtable.v2.CheckAndMutateRowRequest;
-import com.google.bigtable.v2.TableName;
+import com.google.cloud.bigtable.data.v2.internal.NameUtil;
 import com.google.cloud.bigtable.data.v2.internal.RequestContext;
 import com.google.cloud.bigtable.data.v2.models.Filters.Filter;
 import com.google.common.base.Preconditions;
@@ -128,11 +128,9 @@ public CheckAndMutateRowRequest toProto(RequestContext requestContext) {
     Preconditions.checkState(
         !builder.getTrueMutationsList().isEmpty() || !builder.getFalseMutationsList().isEmpty(),
         "ConditionalRowMutations must have `then` or `otherwise` mutations.");
-    TableName tableName =
-        TableName.of(
-            requestContext.getInstanceName().getProject(),
-            requestContext.getInstanceName().getInstance(),
-            tableId);
+    String tableName =
+        NameUtil.formatTableName(
+            requestContext.getProjectId(), requestContext.getInstanceId(), tableId);
     return builder
         .setTableName(tableName.toString())
         .setAppProfileId(requestContext.getAppProfileId())

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/InstanceName.java

@@ -24,8 +24,9 @@
 import java.util.Map;
 
 // Copied from com.google.bigtable.admin.v2
-// TODO: figure out how to unify admin & data resource names
-/** Typesafe representation of a fully qualified Bigtable instance name */
+
+/** @deprecated Please use project id and instance id strings instead. */
+@Deprecated
 public class InstanceName implements ResourceName {
   private static final PathTemplate PATH_TEMPLATE =
       PathTemplate.createWithoutUrlEncoding("projects/{project}/instances/{instance}");

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java

@@ -19,8 +19,8 @@
 import com.google.bigtable.v2.ReadRowsRequest;
 import com.google.bigtable.v2.RowRange;
 import com.google.bigtable.v2.RowSet;
-import com.google.bigtable.v2.TableName;
 import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator;
+import com.google.cloud.bigtable.data.v2.internal.NameUtil;
 import com.google.cloud.bigtable.data.v2.internal.RequestContext;
 import com.google.cloud.bigtable.data.v2.internal.RowSetUtil;
 import com.google.cloud.bigtable.data.v2.models.Range.ByteStringRange;
@@ -248,14 +248,12 @@ public ByteStringRange getBound() {
    */
   @InternalApi
   public ReadRowsRequest toProto(RequestContext requestContext) {
-    TableName tableName =
-        TableName.of(
-            requestContext.getInstanceName().getProject(),
-            requestContext.getInstanceName().getInstance(),
-            tableId);
+    String tableName =
+        NameUtil.formatTableName(
+            requestContext.getProjectId(), requestContext.getInstanceId(), tableId);
 
     return builder
-        .setTableName(tableName.toString())
+        .setTableName(tableName)
         .setAppProfileId(requestContext.getAppProfileId())
         .build();
   }

google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/ReadModifyWriteRow.java

@@ -18,7 +18,7 @@
 import com.google.api.core.InternalApi;
 import com.google.bigtable.v2.ReadModifyWriteRowRequest;
 import com.google.bigtable.v2.ReadModifyWriteRule;
-import com.google.bigtable.v2.TableName;
+import com.google.cloud.bigtable.data.v2.internal.NameUtil;
 import com.google.cloud.bigtable.data.v2.internal.RequestContext;
 import com.google.common.base.Preconditions;
 import com.google.protobuf.ByteString;
@@ -128,13 +128,12 @@ public ReadModifyWriteRow increment(
 
   @InternalApi
   public ReadModifyWriteRowRequest toProto(RequestContext requestContext) {
-    TableName tableName =
-        TableName.of(
-            requestContext.getInstanceName().getProject(),
-            requestContext.getInstanceName().getInstance(),
-            tableId);
+    String tableName =
+        NameUtil.formatTableName(
+            requestContext.getProjectId(), requestContext.getInstanceId(), tableId);
+
     return builder
-        .setTableName(tableName.toString())
+        .setTableName(tableName)
         .setAppProfileId(requestContext.getAppProfileId())
         .build();
   }