Bigtable: table model improvements (#3640)

* Flatten cluster
* expose column families as list only
* use relative ids for tables & families

Author: igorbernstein2

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/ClusterState.java

@@ -26,7 +26,6 @@
 
 /** Wrapper for {@link ColumnFamily} protocol buffer object */
 public final class ColumnFamily {
-  // TODO(igorbernstein2): rename this to `name`
   private final String id;
   private final GCRule rule;
 
@@ -44,21 +43,21 @@ private ColumnFamily(String id, GCRule rule) {
   }
 
   /**
-   * Gets the columnfamily name
+   * Gets the column family's id.
    */
   public String getId() {
     return id;
   }
 
   /**
-   * Get's the GCRule configured for the columnfamily
+   * Get's the GCRule configured for the column family.
    */
   public GCRule getGCRule() {
     return rule;
   }
 
   /**
-   * Returns true if a GCRule has been configured for the family
+   * Returns true if a GCRule has been configured for the family.
    */
   public boolean hasGCRule() {
     return !RuleCase.RULE_NOT_SET.equals(rule.toProto().getRuleCase());

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/ColumnFamily.java

@@ -277,7 +277,7 @@ public boolean equals(Object o) {
         return false;
       }
       VersionRule that = (VersionRule) o;
-      return Objects.equal(builder, that.builder);
+      return Objects.equal(builder.build(), that.builder.build());
     }
 
     @Override
@@ -324,7 +324,7 @@ public boolean equals(Object o) {
         return false;
       }
       DurationRule that = (DurationRule) o;
-      return Objects.equal(builder, that.builder);
+      return Objects.equal(builder.build(), that.builder.build());
     }
 
     @Override

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/GCRules.java

@@ -16,86 +16,68 @@
 package com.google.cloud.bigtable.admin.v2.models;
 
 import com.google.api.core.InternalApi;
+import com.google.bigtable.admin.v2.Table.ClusterState.ReplicationState;
 import com.google.bigtable.admin.v2.TableName;
-import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import javax.annotation.Nonnull;
 
 /** Wrapper for {@link Table} protocol buffer object */
 public final class Table {
-  private final TableName tableName;
-  //  TODO(igorbernstein2): avoid duplication of id as keys and embedded in the models
-  private final Map<String, ClusterState> clusterStates;
-  private final Map<String, ColumnFamily> columnFamilies;
+  private final String id;
+  private final String instanceId;
+  private final Map<String, ReplicationState> replicationStatesByClusterId;
+  private final List<ColumnFamily> columnFamilies;
 
   @InternalApi
   public static Table fromProto(@Nonnull com.google.bigtable.admin.v2.Table proto) {
-    ImmutableMap.Builder<String, ClusterState> clusterStates = ImmutableMap.builder();
+    ImmutableMap.Builder<String, ReplicationState> replicationStates = ImmutableMap.builder();
 
     for (Entry<String, com.google.bigtable.admin.v2.Table.ClusterState> entry : proto.getClusterStatesMap().entrySet()) {
-      clusterStates.put(entry.getKey(), ClusterState.fromProto(entry.getKey(), entry.getValue()));
+      replicationStates.put(entry.getKey(), entry.getValue().getReplicationState());
     }
 
-    ImmutableMap.Builder<String, ColumnFamily> columnFamilies = ImmutableMap.builder();
+    ImmutableList.Builder<ColumnFamily> columnFamilies = ImmutableList.builder();
 
     for (Entry<String, com.google.bigtable.admin.v2.ColumnFamily> entry : proto.getColumnFamiliesMap().entrySet()) {
-      columnFamilies.put(entry.getKey(), ColumnFamily.fromProto(entry.getKey(), entry.getValue()));
+      columnFamilies.add(ColumnFamily.fromProto(entry.getKey(), entry.getValue()));
     }
 
     return new Table(
         TableName.parse(proto.getName()),
-        clusterStates.build(),
+        replicationStates.build(),
         columnFamilies.build());
   }
 
   private Table(TableName tableName,
-      Map<String, ClusterState> clusterStates,
-      Map<String, ColumnFamily> columnFamilies) {
-    this.tableName = tableName;
-    this.clusterStates = clusterStates;
+      Map<String, ReplicationState> replicationStatesByClusterId,
+      List<ColumnFamily> columnFamilies) {
+    this.instanceId = tableName.getInstance();
+    this.id = tableName.getTable();
+    this.replicationStatesByClusterId = replicationStatesByClusterId;
     this.columnFamilies = columnFamilies;
   }
 
-  /**
-   * Gets the unique name of the table in the format:
-   * projects/{project}/instances/{instance}/tables/{tableId}
-   */
-  public TableName getTableName() {
-    return tableName;
+  /** Gets the table's id. */
+  public String getId() {
+    return id;
   }
 
-  // TODO(igorbernstein2): don't expose the objects as both maps & lists
-  /**
-   * Returns state of the table by clusters in the instance as map of clusterId and {@link
-   * ClusterState}
-   */
-  public Map<String, ClusterState> getClusterStatesMap() {
-    return clusterStates;
+  /** Gets the id of the instance that owns this Table. */
+  public String getInstanceId() {
+    return instanceId;
   }
 
-  /**
-   * Returns a map of columfamilies in the table keyed by columnfamily and name
-   */
-  public Map<String, ColumnFamily> getColumnFamiliesMap() {
-    return columnFamilies;
-  }
-
-  /**
-   * Returns state of the table by clusters in the instance as a Collection
-   */
-  public Collection<ClusterState> getClusterStates() {
-    return clusterStates.values();
+  public Map<String, ReplicationState> getReplicationStatesByClusterId() {
+    return replicationStatesByClusterId;
   }
 
-  /**
-   * Returns all columnfamilies in the table as a Collection
-   */
-  public Collection<ColumnFamily> getColumnFamiles() {
-    return columnFamilies.values();
+  public List<ColumnFamily> getColumnFamilies() {
+    return columnFamilies;
   }
 
   @Override
@@ -107,22 +89,15 @@ public boolean equals(Object o) {
       return false;
     }
     Table table = (Table) o;
-    return Objects.equal(tableName, table.tableName) &&
-        Objects.equal(clusterStates, table.clusterStates) &&
+    return Objects.equal(id, table.id) &&
+        Objects.equal(instanceId, table.instanceId) &&
+        Objects
+            .equal(replicationStatesByClusterId, table.replicationStatesByClusterId) &&
         Objects.equal(columnFamilies, table.columnFamilies);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hashCode(tableName, clusterStates, columnFamilies);
-  }
-
-  @Override
-  public String toString() {
-    return MoreObjects.toStringHelper(this)
-        .add("tableName", tableName)
-        .add("clusterStates", getClusterStates())
-        .add("columnFamiles", getColumnFamiles())
-        .toString();
+    return Objects.hashCode(id, instanceId, replicationStatesByClusterId, columnFamilies);
   }
 }

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Table.java

@@ -24,6 +24,7 @@
 import com.google.bigtable.admin.v2.InstanceName;
 import com.google.bigtable.admin.v2.TableName;
 import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
+import com.google.cloud.bigtable.admin.v2.models.ColumnFamily;
 import com.google.cloud.bigtable.admin.v2.models.GCRules.DurationRule;
 import com.google.cloud.bigtable.admin.v2.models.GCRules.IntersectionRule;
 import com.google.cloud.bigtable.admin.v2.models.GCRules.UnionRule;
@@ -32,9 +33,11 @@
 import com.google.cloud.bigtable.admin.v2.models.ModifyColumnFamiliesRequest;
 import com.google.cloud.bigtable.admin.v2.models.ConsistencyToken;
 import com.google.cloud.bigtable.admin.v2.models.Table;
+import com.google.common.collect.Maps;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
 import java.util.List;
+import java.util.Map;
 import org.junit.AfterClass;
 import org.junit.AssumptionViolatedException;
 import org.junit.Before;
@@ -88,13 +91,18 @@ public void createTable() {
     try {
       Table tableResponse = tableAdmin.createTable(createTableReq);
       assertNotNull(tableResponse);
-      assertEquals(tableId, tableResponse.getTableName().getTable());
-      assertEquals(2, tableResponse.getColumnFamiles().size());
-      assertFalse(tableResponse.getColumnFamiliesMap().get("cf1").hasGCRule());
-      assertTrue(tableResponse.getColumnFamiliesMap().get("cf2").hasGCRule());
+      assertEquals(tableId, tableResponse.getId());
+
+      Map<String, ColumnFamily> columnFamilyById = Maps.newHashMap();
+      for (ColumnFamily columnFamily : tableResponse.getColumnFamilies()) {
+        columnFamilyById.put(columnFamily.getId(), columnFamily);
+      }
+      assertEquals(2, tableResponse.getColumnFamilies().size());
+      assertFalse(columnFamilyById.get("cf1").hasGCRule());
+      assertTrue(columnFamilyById.get("cf2").hasGCRule());
       assertEquals(
           10,
-          ((VersionRule) tableResponse.getColumnFamiliesMap().get("cf2").getGCRule())
+          ((VersionRule) columnFamilyById.get("cf2").getGCRule())
               .getMaxVersions());
     } finally {
       tableAdmin.deleteTable(tableId);
@@ -131,35 +139,40 @@ public void modifyFamilies() {
     try {
       tableAdmin.createTable(CreateTableRequest.of(tableId));
       Table tableResponse = tableAdmin.modifyFamilies(modifyFamiliesReq);
-      assertEquals(5, tableResponse.getColumnFamiles().size());
-      assertNotNull(tableResponse.getColumnFamiliesMap().get("mf1"));
-      assertNotNull(tableResponse.getColumnFamiliesMap().get("mf2"));
+
+      Map<String, ColumnFamily> columnFamilyById = Maps.newHashMap();
+      for (ColumnFamily columnFamily : tableResponse.getColumnFamilies()) {
+        columnFamilyById.put(columnFamily.getId(), columnFamily);
+      }
+      assertEquals(5, columnFamilyById.size());
+      assertNotNull(columnFamilyById.get("mf1"));
+      assertNotNull(columnFamilyById.get("mf2"));
       assertEquals(
           2,
-          ((UnionRule) tableResponse.getColumnFamiliesMap().get("mf1").getGCRule())
+          ((UnionRule) columnFamilyById.get("mf1").getGCRule())
               .getRulesList()
               .size());
       assertEquals(
           1000,
-          ((DurationRule) tableResponse.getColumnFamiliesMap().get("mf2").getGCRule())
+          ((DurationRule) columnFamilyById.get("mf2").getGCRule())
               .getMaxAge()
               .getSeconds());
       assertEquals(
           20000,
-          ((DurationRule) tableResponse.getColumnFamiliesMap().get("mf2").getGCRule())
+          ((DurationRule) columnFamilyById.get("mf2").getGCRule())
               .getMaxAge()
               .getNano());
       assertEquals(
           2,
-          ((IntersectionRule) tableResponse.getColumnFamiliesMap().get("mf3").getGCRule())
+          ((IntersectionRule) columnFamilyById.get("mf3").getGCRule())
               .getRulesList()
               .size());
       assertEquals(
           360,
-          ((DurationRule) tableResponse.getColumnFamiliesMap().get("mf4").getGCRule())
+          ((DurationRule) columnFamilyById.get("mf4").getGCRule())
               .getMaxAge()
               .getSeconds());
-      assertNotNull(tableResponse.getColumnFamiliesMap().get("mf7"));
+      assertNotNull(columnFamilyById.get("mf7"));
     } finally {
       tableAdmin.deleteTable(tableId);
     }
@@ -180,7 +193,7 @@ public void getTable() {
       tableAdmin.createTable(CreateTableRequest.of(tableId));
       Table tableResponse = tableAdmin.getTable(tableId);
       assertNotNull(tableResponse);
-      assertEquals(tableId, tableResponse.getTableName().getTable());
+      assertEquals(tableId, tableResponse.getId());
     } finally {
       tableAdmin.deleteTable(tableId);
     }