Skip to content

Commit

Permalink
feat: add fieldNames option in instance#getMetadata() (#760)
Browse files Browse the repository at this point in the history
* feat: add fieldNames option in instance#getMetadata()

* docs: add a list of eligible values and a usage sample

* refactor: simplify and clean up

* refactor: use single quote

Co-authored-by: Benjamin E. Coe <[email protected]>
Co-authored-by: skuruppu <[email protected]>
  • Loading branch information
3 people committed Jan 16, 2020
1 parent 5f2c59a commit fa3154e
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 12 deletions.
61 changes: 57 additions & 4 deletions src/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export type DeleteInstanceResponse = [instanceAdmin.protobuf.IEmpty];
export type ExistsInstanceResponse = [boolean];
export type GetInstanceResponse = [Instance, IInstance];
export type GetInstanceMetadataResponse = [IInstance];
export interface GetInstanceMetadataOptions {
fieldNames?: string | string[];
}
export type GetDatabasesResponse = PagedResponse<
Database,
databaseAdmin.spanner.admin.database.v1.IListDatabasesResponse
Expand Down Expand Up @@ -83,7 +86,9 @@ export type SetInstanceMetadataCallback = ResourceCallback<
GaxOperation,
IOperation
>;
export interface GetInstanceConfig extends GetConfig {}
export interface GetInstanceConfig
extends GetConfig,
GetInstanceMetadataOptions {}

interface InstanceRequest {
(
Expand Down Expand Up @@ -510,6 +515,9 @@ class Instance extends common.GrpcServiceObject {
* @param {options} [options] Configuration object.
* @param {boolean} [options.autoCreate=false] Automatically create the
* object if it does not exist.
* @param {string | string[]} [options.fieldNames] A list of `Instance` field
* names to be requested. Eligible values are: `name`, `displayName`,
* `endpointUris`, `labels`, `config`, `nodeCount`, `state`.
* @param {GetInstanceCallback} [callback] Callback function.
* @returns {Promise<GetInstanceResponse>}
*
Expand Down Expand Up @@ -542,7 +550,13 @@ class Instance extends common.GrpcServiceObject {
? optionsOrCallback
: ({} as GetInstanceConfig);

this.getMetadata((err, metadata) => {
const getMetadataOptions: GetInstanceMetadataOptions = new Object(null);
if (options.fieldNames) {
getMetadataOptions.fieldNames = options['fieldNames'];
delete options.fieldNames;
}

this.getMetadata(getMetadataOptions, (err, metadata) => {
if (err) {
if (err.code === 5 && options.autoCreate) {
this.create(
Expand Down Expand Up @@ -681,8 +695,14 @@ class Instance extends common.GrpcServiceObject {
}
);
}
getMetadata(): Promise<GetInstanceMetadataResponse>;
getMetadata(
options?: GetInstanceMetadataOptions
): Promise<GetInstanceMetadataResponse>;
getMetadata(callback: GetInstanceMetadataCallback): void;
getMetadata(
options: GetInstanceMetadataOptions,
callback: GetInstanceMetadataCallback
): void;
/**
* @typedef {array} GetInstanceMetadataResponse
* @property {object} 0 The {@link Instance} metadata.
Expand All @@ -702,6 +722,10 @@ class Instance extends common.GrpcServiceObject {
* @see {@link v1.InstanceAdminClient#getInstance}
* @see [GetInstance API Documentation](https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.admin.instance.v1#google.spanner.admin.instance.v1.InstanceAdmin.GetInstance)
*
* @param {GetInstanceMetadataOptions} [options] Configuration object
* @param {string | string[]} [options.fieldNames] A list of `Instance` field
* names to be requested. Eligible values are: `name`, `displayName`,
* `endpointUris`, `labels`, `config`, `nodeCount`, `state`.
* @param {GetInstanceMetadataCallback} [callback] Callback function.
* @returns {Promise<GetInstanceMetadataResponse>}
*
Expand All @@ -714,6 +738,23 @@ class Instance extends common.GrpcServiceObject {
* instance.getMetadata(function(err, metadata, apiResponse) {});
*
* //-
* // Request only `displayName`.
* //-
* instance.getMetadata({fieldNames: 'displayName'}, (err, metadata, apiResponse) => {
* // metadata will only contain value for `displayName`
* const displayName = metadata['displayName'];
* })
*
* //-
* // Request multiple specific field names.
* //-
* instance.getMetadata({fieldNames: ['displayName', 'nodeCount']}, (err, metadata, apiResponse) => {
* // metadata will only contain value for `displayName` and 'nodeCount'
* const displayName = metadata['displayName'];
* const nodeCount = metadata['nodeCount'];
* });
*
* //-
* // If the callback is omitted, we'll return a Promise.
* //-
* instance.getMetadata().then(function(data) {
Expand All @@ -722,11 +763,23 @@ class Instance extends common.GrpcServiceObject {
* });
*/
getMetadata(
callback?: GetInstanceMetadataCallback
optionsOrCallback?:
| GetInstanceMetadataOptions
| GetInstanceMetadataCallback,
cb?: GetInstanceMetadataCallback
): Promise<GetInstanceMetadataResponse> | void {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!;
const options =
typeof optionsOrCallback === 'object' ? optionsOrCallback : {};
const reqOpts = {
name: this.formattedName_,
};
if (options.fieldNames) {
reqOpts['fieldMask'] = {
paths: arrify(options['fieldNames']!).map(snakeCase),
};
}
return this.request<IInstance>(
{
client: 'InstanceAdminClient',
Expand Down
16 changes: 16 additions & 0 deletions system-test/spanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,22 @@ describe('Spanner', () => {
});
});

it('should respect the FieldMask', async () => {
const fieldNames = ['name', 'displayName'];

const [metadata] = await instance.getMetadata({fieldNames});
assert.deepStrictEqual(metadata['endpointUris'], []);
assert.deepStrictEqual(metadata['labels'], {});
assert.strictEqual(metadata.name, instance.formattedName_);
assert.ok(!metadata['config']);
assert.strictEqual(
metadata['displayName'],
instance.formattedName_.split('/').pop()
);
assert.strictEqual(metadata['nodeCount'], 0);
assert.strictEqual(metadata['state'], 'STATE_UNSPECIFIED');
});

it('should auto create an instance', done => {
const instance = spanner.instance(generateName('instance'));

Expand Down
99 changes: 91 additions & 8 deletions test/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import * as proxyquire from 'proxyquire';
import * as pfy from '@google-cloud/promisify';
import {ServiceError} from 'grpc';
import * as sinon from 'sinon';
import snakeCase = require('lodash.snakecase');

import * as inst from '../src/instance';
import {Spanner, Database} from '../src';
import arrify = require('arrify');
import {SessionPoolOptions} from '../src/session-pool';

const fakePaginator = {
Expand Down Expand Up @@ -499,7 +501,17 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(cb => cb(error as ServiceError));
.callsFake(
(
opts_:
| inst.GetInstanceMetadataOptions
| inst.GetInstanceMetadataCallback,
cb
) => {
cb = typeof opts_ === 'function' ? opts_ : cb;
cb(error as ServiceError);
}
);

instance.exists((err, exists) => {
assert.strictEqual(err, error);
Expand All @@ -509,7 +521,19 @@ describe('Instance', () => {
});

it('should return true if error is absent', done => {
sandbox.stub(instance, 'getMetadata').callsFake(cb => cb(null));
sandbox
.stub(instance, 'getMetadata')
.callsFake(
(
opts_:
| inst.GetInstanceMetadataOptions
| inst.GetInstanceMetadataCallback,
cb
) => {
cb = typeof opts_ === 'function' ? opts_ : cb;
cb(null);
}
);

instance.exists((err, exists) => {
assert.ifError(err);
Expand All @@ -523,7 +547,18 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(error as ServiceError));
.callsFake(
(
opts_:
| inst.GetInstanceMetadataOptions
| inst.GetInstanceMetadataCallback,
callback
) => {
callback = typeof opts_ === 'function' ? opts_ : callback;

callback(error as ServiceError);
}
);

instance.exists((err, exists) => {
assert.ifError(err);
Expand All @@ -548,6 +583,24 @@ describe('Instance', () => {
instance.get(assert.ifError);
});

it('should accept and pass `fields` string as is', () => {
const fieldNames = 'nodeCount';
const spyMetadata = sandbox.spy(instance, 'getMetadata');

instance.get({fieldNames}, assert.ifError);

assert.ok(spyMetadata.calledWith({fieldNames}));
});

it('should accept and pass `fields` array as is', () => {
const fieldNames = ['name', 'labels', 'nodeCount'];
const spyMetadata = sandbox.stub(instance, 'getMetadata');

instance.get({fieldNames}, assert.ifError);

assert.ok(spyMetadata.calledWith({fieldNames}));
});

describe('autoCreate', () => {
const error = new ApiError('Error.') as ServiceError;
error.code = 5;
Expand All @@ -569,7 +622,7 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(error));
.callsFake((opts_: {}, callback) => callback!(error));

instance.create = (options, callback) => {
callback(null, null, OPERATION);
Expand Down Expand Up @@ -639,7 +692,7 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(error));
.callsFake((opts_: {}, callback) => callback!(error));

instance.create = () => {
throw new Error('Should not create.');
Expand All @@ -657,7 +710,7 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(error));
.callsFake((opts_: {}, callback) => callback!(error));

instance.create = () => {
throw new Error('Should not create.');
Expand All @@ -674,7 +727,7 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(error));
.callsFake((opts_: {}, callback) => callback!(error));

instance.get(err => {
assert.strictEqual(err, error);
Expand All @@ -687,7 +740,7 @@ describe('Instance', () => {

sandbox
.stub(instance, 'getMetadata')
.callsFake(callback => callback!(null, apiResponse));
.callsFake((opts_: {}, callback) => callback!(null, apiResponse));

instance.get((err, instance_, apiResponse_) => {
assert.ifError(err);
Expand Down Expand Up @@ -812,6 +865,36 @@ describe('Instance', () => {
const returnValue = instance.getMetadata(callback);
assert.strictEqual(returnValue, requestReturnValue);
});

it('should accept `fieldNames` as string', done => {
const fieldNames = 'nodeCount';

instance.request = config => {
assert.deepStrictEqual(config.reqOpts, {
fieldMask: {
paths: arrify(fieldNames).map(snakeCase),
},
name: instance.formattedName_,
});
done();
};
instance.getMetadata({fieldNames}, assert.ifError);
});

it('should accept `fieldNames` as string array', done => {
const fieldNames = ['name', 'labels', 'nodeCount'];

instance.request = config => {
assert.deepStrictEqual(config.reqOpts, {
fieldMask: {
paths: fieldNames.map(snakeCase),
},
name: instance.formattedName_,
});
done();
};
instance.getMetadata({fieldNames}, assert.ifError);
});
});

describe('setMetadata', () => {
Expand Down

0 comments on commit fa3154e

Please sign in to comment.