refactor: Pull code to compute location into one function (#1094)

* snapshot tests for create cluster * change request to config * More snapshot tests * Refactor constants out * Add generated testcases for partial cluster update * Add header to constants file * Remove need to pass location along * Remove comment * Use new function to calculate location * Add space to stay consistent * Use getClusterBaseConfigWithFullLocation again * Fix test * linting fix * eliminate compile error * lint fix * remove the stubs that are not used anymore * Add an assert statement for location
googleapis · Jul 18, 2022 · 81ea5f7 · 81ea5f7
1 parent 0dfb960
commit 81ea5f7
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 35 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -612,15 +612,9 @@ export class Bigtable {
         );
       }
       ClusterUtils.validateClusterMetadata(cluster);
-      const clusterClone = Object.assign({}, cluster);
-      if (clusterClone.location) {
-        clusterClone.location = Cluster.getLocation_(
-          this.projectId,
-          clusterClone.location
-        );
-      }
-      clusters[cluster.id!] = ClusterUtils.getClusterBaseConfig(
-        clusterClone,
+      clusters[cluster.id!] = ClusterUtils.getClusterBaseConfigWithFullLocation(
+        cluster,
+        this.projectId,
         undefined
       );
       Object.assign(clusters[cluster.id!], {

diff --git a/src/instance.ts b/src/instance.ts
@@ -394,15 +394,9 @@ Please use the format 'my-instance' or '${bigtable.projectName}/instances/my-ins
     } as google.bigtable.admin.v2.CreateClusterRequest;
     ClusterUtils.validateClusterMetadata(options);
     if (!is.empty(options)) {
-      const optionsClone = Object.assign({}, options);
-      if (optionsClone.location) {
-        optionsClone.location = Cluster.getLocation_(
-          this.bigtable.projectId,
-          optionsClone.location
-        );
-      }
-      reqOpts.cluster = ClusterUtils.getClusterBaseConfig(
-        optionsClone,
+      reqOpts.cluster = ClusterUtils.getClusterBaseConfigWithFullLocation(
+        options,
+        this.bigtable.projectId,
         undefined
       );
     }

diff --git a/src/utils/cluster.ts b/src/utils/cluster.ts
@@ -13,7 +13,12 @@
 // limitations under the License.
 
 import * as protos from '../../protos/protos';
-import {ICluster, SetClusterMetadataOptions} from '../cluster';
+import {
+  BasicClusterConfig,
+  Cluster,
+  ICluster,
+  SetClusterMetadataOptions,
+} from '../cluster';
 import {google} from '../../protos/protos';
 
 export class ClusterUtils {
@@ -53,6 +58,7 @@ export class ClusterUtils {
       }
     }
   }
+
   static getUpdateMask(metadata: SetClusterMetadataOptions): string[] {
     const updateMask: string[] = [];
     if (metadata.nodes) {
@@ -85,6 +91,21 @@ export class ClusterUtils {
     return updateMask;
   }
 
+  static getClusterBaseConfigWithFullLocation(
+    metadata: BasicClusterConfig,
+    projectId: string,
+    name: string | undefined
+  ): google.bigtable.admin.v2.ICluster {
+    const metadataClone = Object.assign({}, metadata);
+    if (metadataClone.location) {
+      metadataClone.location = Cluster.getLocation_(
+        projectId,
+        metadataClone.location
+      );
+    }
+    return ClusterUtils.getClusterBaseConfig(metadataClone, name);
+  }
+
   static getClusterBaseConfig(
     metadata: SetClusterMetadataOptions,
     name: string | undefined

diff --git a/test/index.ts b/test/index.ts
@@ -20,7 +20,7 @@ import * as gax from 'google-gax';
 import * as proxyquire from 'proxyquire';
 import * as sn from 'sinon';
 
-import {Cluster, CreateClusterOptions} from '../src/cluster.js';
+import {Cluster} from '../src/cluster.js';
 import {Instance, InstanceOptions} from '../src/instance.js';
 import {PassThrough} from 'stream';
 import {RequestOptions} from '../src';
@@ -494,12 +494,10 @@ describe('Bigtable', () => {
     });
 
     it('should respect the clusters option', done => {
-      const fakeLocation = 'a/b/c/d';
-      FakeCluster.getLocation_ = (project: string, location: string) => {
-        assert.strictEqual(project, PROJECT_ID);
-        assert.strictEqual(location, OPTIONS.clusters[0].location);
-        return fakeLocation;
-      };
+      const fakeLocation = Cluster.getLocation_(
+        PROJECT_ID,
+        OPTIONS.clusters[0].location
+      );
       const fakeStorage = 20;
       FakeCluster.getStorageType_ = (storage: {}) => {
         assert.strictEqual(storage, OPTIONS.clusters[0].storage);

diff --git a/test/instance.ts b/test/instance.ts
@@ -22,7 +22,7 @@ import * as snapshot from 'snap-shot-it';
 
 import * as inst from '../src/instance';
 import {AppProfile, AppProfileOptions} from '../src/app-profile';
-import {CreateClusterOptions} from '../src/cluster';
+import {Cluster, CreateClusterOptions} from '../src/cluster';
 import {Family} from '../src/family';
 import {
   Policy,
@@ -356,6 +356,10 @@ describe('Bigtable/Instance', () => {
         assert.strictEqual(config.method, 'createCluster');
         assert.strictEqual(config.reqOpts.parent, INSTANCE_NAME);
         assert.strictEqual(config.reqOpts.clusterId, CLUSTER_ID);
+        assert.strictEqual(
+          config.reqOpts.cluster.location,
+          'projects/my-project/locations/us-central1-b'
+        );
         assert.strictEqual(config.gaxOpts, undefined);
         done();
       };
@@ -404,14 +408,10 @@ describe('Bigtable/Instance', () => {
         location: 'us-central1-b',
         nodes: 2,
       } as CreateClusterOptions;
-      const fakeLocation = 'a/b/c/d';
-      sandbox
-        .stub(FakeCluster, 'getLocation_')
-        .callsFake((project, location) => {
-          assert.strictEqual(project, BIGTABLE.projectId);
-          assert.strictEqual(location, options.location);
-          return fakeLocation;
-        });
+      const fakeLocation = Cluster.getLocation_(
+        BIGTABLE.projectId,
+        options.location
+      );
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       (instance.bigtable.request as Function) = (config: any) => {
         assert.strictEqual(config.reqOpts.cluster.location, fakeLocation);