From 9cca1e11d12685810e84797eb53a5c6442f3239c Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Thu, 1 Nov 2018 15:01:38 -0700 Subject: [PATCH 1/2] chore: improve typescript config and types --- package.json | 5 +- src/batch-transaction.ts | 1 - src/codec.ts | 124 +++++++++++++++++++-------------------- src/database.ts | 68 +++++++++++++-------- src/index.ts | 8 ++- src/session.ts | 22 ++++--- src/transaction.ts | 1 + test/codec.ts | 17 ++---- test/database.ts | 2 +- tsconfig.json | 3 - 10 files changed, 130 insertions(+), 121 deletions(-) diff --git a/package.json b/package.json index b1e2cd1b4..ff387ad20 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,6 @@ "docs": "jsdoc -c .jsdoc.js", "generate-scaffolding": "repo-tools generate all && repo-tools generate lib_samples_readme -l samples/ --config ../.cloud-repo-tools.json", "lint": "echo disabled for now", - "prettier": "prettier --write benchmark/*.js src/*.js src/*/*.js src/admin/database/*/*.js src/admin/instance/*/*.js samples/*.js samples/*/*.js samples/*/*/*.js test/*.js test/*/*.js system-test/*.js system-test/*/*.js", "samples-test": "cd samples/ && npm link ../ && npm test && cd ../", "system-test": "mocha build/system-test --timeout 600000", "cleanup": "mocha scripts/cleanup.js --timeout 30000", @@ -63,13 +62,13 @@ "ycsb": "node ./benchmark/ycsb.js run -P ./benchmark/workloada -p table=usertable -p cloudspanner.instance=ycsb-instance -p operationcount=100 -p cloudspanner.database=ycsb", "fix": "eslint --fix '**/*.js'", "clean": "gts clean", - "compile": "tsc -p . && cp src/v1/*.json build/src/v1 && cp -r protos build", + "compile": "tsc -p . && cp -r src/v1 build/src && cp -r protos build", "prepare": "npm run compile", "pretest": "npm run compile", "presystem-test": "npm run compile" }, "dependencies": { - "@google-cloud/common-grpc": "^0.9.0", + "@google-cloud/common-grpc": "^0.9.2", "@google-cloud/paginator": "^0.1.0", "@google-cloud/projectify": "^0.3.0", "@google-cloud/promisify": "^0.3.0", diff --git a/src/batch-transaction.ts b/src/batch-transaction.ts index 6bf8caea3..fad504900 100644 --- a/src/batch-transaction.ts +++ b/src/batch-transaction.ts @@ -30,7 +30,6 @@ import {Transaction} from './transaction'; * @param {TransactionOptions} [options] [Transaction options](https://cloud.google.com/spanner/docs/timestamp-bounds). */ class BatchTransaction extends Transaction { - session: Session; readTimestamp?: {}; constructor(session) { diff --git a/src/codec.ts b/src/codec.ts index e47d751da..2ea8d8432 100644 --- a/src/codec.ts +++ b/src/codec.ts @@ -21,7 +21,7 @@ import {Service} from '@google-cloud/common-grpc'; import * as extend from 'extend'; import * as is from 'is'; -class SpannerDate { +export class SpannerDate { value; constructor(value) { if (arguments.length > 1) { @@ -39,7 +39,7 @@ class SpannerDate { } } -class Float { +export class Float { value; constructor(value) { this.value = value; @@ -49,7 +49,7 @@ class Float { } } -class Int { +export class Int { value; constructor(value) { this.value = value.toString(); @@ -84,73 +84,67 @@ const TYPE = Symbol(); * * @returns {array} */ -function Struct() { - const struct = []; +export class Struct extends Array { + constructor() { + super(); + this[TYPE] = Struct.TYPE; + Object.defineProperty(this, 'toJSON', { + enumerable: false, + value: codec.generateToJSONFromRow(this) + }); + } - struct[TYPE] = Struct.TYPE; + /** + * Use this to assign/check the type when dealing with structs. + * + * @private + */ + static TYPE = 'struct'; + + /** + * Converts an array of objects to a struct array. + * + * @private + * + * @param {object[]} arr Struct array. + * @return {Struct} + */ + static fromArray(arr) { + const struct = new Struct(); + struct.push.apply(struct, arr); + return struct; + } - Object.defineProperty(struct, 'toJSON', { - enumerable: false, - value: codec.generateToJSONFromRow(struct), - }); + /** + * Converts a JSON object to a struct array. + * + * @private + * + * @param {object} json Struct JSON. + * @return {Struct} + */ + static fromJSON(json: {}) { + const struct = new Struct(); + Object.keys(json || {}).forEach(name => { + const value = json[name]; + struct.push({name, value}); + }); + return struct; + } - return struct; + /** + * Checks to see if the provided object is a Struct. + * + * @private + * + * @param {*} thing The object to check. + * @returns {boolean} + */ + static isStruct(thing: {}) { + return !!(thing && thing[TYPE] === Struct.TYPE); + } } -/** - * Use this to assign/check the type when dealing with structs. - * - * @private - */ -Struct.TYPE = 'struct'; - -/** - * Converts an array of objects to a struct array. - * - * @private - * - * @param {object[]} arr Struct array. - * @return {Struct} - */ -Struct.fromArray = (arr) => { - // tslint:disable-next-line no-any - const struct = new (Struct as any)(); - struct.push.apply(struct, arr); - return struct; -}; - -/** - * Converts a JSON object to a struct array. - * - * @private - * - * @param {object} json Struct JSON. - * @return {Struct} - */ -Struct.fromJSON = (json) => { - // tslint:disable-next-line no-any - const struct = new (Struct as any)(); - - Object.keys(json || {}).forEach(name => { - const value = json[name]; - struct.push({name, value}); - }); - - return struct; -}; - -/** - * Checks to see if the provided object is a Struct. - * - * @private - * - * @param {*} thing The object to check. - * @returns {boolean} - */ -Struct.isStruct = (thing) => { - return !!(thing && thing[TYPE] === Struct.TYPE); -}; - /** * Wherever a row object is returned, it is assigned a "toJSON" function. This * function will create that function in a consistent format. diff --git a/src/database.ts b/src/database.ts index 1576007cf..85aa2cf4c 100644 --- a/src/database.ts +++ b/src/database.ts @@ -18,7 +18,7 @@ import * as arrify from 'arrify'; import {promisifyAll} from '@google-cloud/promisify'; -const {ServiceObject} = require('@google-cloud/common-grpc'); +import {ServiceObject} from '@google-cloud/common-grpc'; import * as extend from 'extend'; import * as is from 'is'; import * as retry from 'p-retry'; @@ -32,6 +32,17 @@ import {SessionPool} from './session-pool'; import {Table} from './table'; import {Transaction} from './transaction'; import {TransactionRequest} from './transaction-request'; +import { ServiceObjectConfig, DeleteCallback, ExistsCallback, GetMetadataCallback, ApiError, Metadata } from '@google-cloud/common'; +import * as r from 'request'; + +export interface GetDatabaseOptions { + autoCreate?: boolean; +} +export type DatabaseResponse = [Database, r.Response]; +export interface DatabaseCallback { + (err: Error|null, database?: Database, apiResponse?: r.Response): void; +} + /** * Interface for implementing custom session pooling logic, it should extend the * {@link https://nodejs.org/api/events.html|EventEmitter} class and emit any @@ -156,7 +167,7 @@ class Database extends ServiceObject { createMethod: (_, options, callback) => { return instance.createDatabase(formattedName_, options, callback); }, - }); + } as {} as ServiceObjectConfig); this.formattedName_ = formattedName_; this.request = instance.request; @@ -240,8 +251,9 @@ class Database extends ServiceObject { * }); */ close(callback) { - const key = this.id.split('/').pop(); - this.parent.databases_.delete(key); + const key = this.id!.split('/').pop(); + // tslint:disable-next-line no-any + (this.parent as any).databases_.delete(key); this.pool_.close(callback); } /** @@ -503,7 +515,9 @@ class Database extends ServiceObject { * const apiResponse = data[0]; * }); */ - delete(callback) { + delete(): Promise<[r.Response]>; + delete(callback: DeleteCallback): void; + delete(callback?: DeleteCallback): void|Promise<[r.Response]> { const reqOpts = { database: this.formattedName_, }; @@ -514,7 +528,7 @@ class Database extends ServiceObject { method: 'dropDatabase', reqOpts, }, - callback + callback! ); }); } @@ -550,17 +564,18 @@ class Database extends ServiceObject { * const exists = data[0]; * }); */ - exists(callback) { + exists(): Promise<[boolean]>; + exists(callback: ExistsCallback): void; + exists(callback?: ExistsCallback): void|Promise<[boolean]> { const NOT_FOUND = 5; this.getMetadata(err => { - if (err && err.code !== NOT_FOUND) { - callback(err, null); + if (err && (err as ApiError).code !== NOT_FOUND) { + callback!(err); return; } - - const exists = !err || err.code !== NOT_FOUND; - callback(null, exists); + const exists = !err || (err as ApiError).code !== NOT_FOUND; + callback!(null, exists); }); } /** @@ -607,30 +622,31 @@ class Database extends ServiceObject { * const apiResponse = data[0]; * }); */ - get(options, callback) { - if (is.fn(options)) { - callback = options; - options = {}; - } + get(options?: GetDatabaseOptions): Promise; + get(options: GetDatabaseOptions, callback: DatabaseCallback): void; + get(callback: DatabaseCallback): void; + get(optionsOrCallback?: GetDatabaseOptions|DatabaseCallback, cb?: DatabaseCallback): void|Promise { + const options = typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + const callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; this.getMetadata((err, metadata) => { if (err) { - if (options.autoCreate && err.code === 5) { + if (options.autoCreate && (err as ApiError).code === 5) { this.create(options, (err, database, operation) => { if (err) { - callback(err); + callback!(err); return; } - operation.on('error', callback).on('complete', metadata => { + operation!.on('error', callback!).on('complete', metadata => { this.metadata = metadata; - callback(null, this, metadata); + callback!(null, this, metadata); }); }); return; } - callback(err); + callback!(err); return; } - callback(null, this, metadata); + callback!(null, this, metadata); }); } /** @@ -678,7 +694,9 @@ class Database extends ServiceObject { * const apiResponse = data[1]; * }); */ - getMetadata(callback) { + getMetadata(): Promise; + getMetadata(callback: GetMetadataCallback): void; + getMetadata(callback?: GetMetadataCallback): void|Promise { const reqOpts = { name: this.formattedName_, }; @@ -688,7 +706,7 @@ class Database extends ServiceObject { method: 'getDatabase', reqOpts, }, - callback + callback! ); } /** diff --git a/src/index.ts b/src/index.ts index 257c76737..3e4e71c40 100644 --- a/src/index.ts +++ b/src/index.ts @@ -662,7 +662,8 @@ this.getInstancesStream = paginator.streamify('getInstances'); if (!name) { throw new Error('A name is required to access an Operation object.'); } - return new Operation(this, name); + // tslint:disable-next-line no-any + return new Operation(this as any, name); } /** @@ -707,7 +708,7 @@ this.getInstancesStream = paginator.streamify('getInstances'); * @returns {Promise} */ // tslint:disable-next-line no-any - request(config, callback): void|Promise { + request(config: any, callback?: any): any { if (is.fn(callback)) { this.prepareGapicRequest_(config, (err, requestFn) => { if (err) { @@ -740,7 +741,8 @@ this.getInstancesStream = paginator.streamify('getInstances'); * @param {function} [callback] Callback function. * @returns {Stream} */ - requestStream(config) { + // tslint:disable-next-line no-any + requestStream(config): any { const stream = streamEvents(through.obj()); stream.once('reading', () => { this.prepareGapicRequest_(config, (err, requestFn) => { diff --git a/src/session.ts b/src/session.ts index ce749ae67..81db3990f 100644 --- a/src/session.ts +++ b/src/session.ts @@ -20,11 +20,14 @@ 'use strict'; -const {ServiceObject} = require('@google-cloud/common-grpc'); +import {ServiceObject} from '@google-cloud/common-grpc'; import {promisifyAll} from '@google-cloud/promisify'; import * as extend from 'extend'; import * as is from 'is'; +import * as r from 'request'; import {Transaction} from './transaction'; +import { Database } from './database'; +import { ServiceObjectConfig, DeleteCallback, Metadata, GetMetadataCallback } from '@google-cloud/common'; /** * Create a Session object to interact with a Cloud Spanner session. @@ -65,7 +68,7 @@ import {Transaction} from './transaction'; * const session = database.session_('session-name'); */ class Session extends ServiceObject { - constructor(database, name) { + constructor(database: Database, name: string) { const methods = { /** * Create a session. @@ -177,7 +180,6 @@ class Session extends ServiceObject { callback = options; options = {}; } - return database.createSession(options, (err, session, apiResponse) => { if (err) { callback(err, null, apiResponse); @@ -188,7 +190,7 @@ class Session extends ServiceObject { callback(null, this, apiResponse); }); }, - }); + } as {} as ServiceObjectConfig); this.request = database.request; this.requestStream = database.requestStream; @@ -261,7 +263,9 @@ class Session extends ServiceObject { * const apiResponse = data[0]; * }); */ - delete(callback) { + delete(): Promise<[r.Response]>; + delete(callback: DeleteCallback): void; + delete(callback?: DeleteCallback): void|Promise<[r.Response]> { const reqOpts = { name: this.formattedName_, }; @@ -271,7 +275,7 @@ class Session extends ServiceObject { method: 'deleteSession', reqOpts, }, - callback + callback! ); } /** @@ -307,7 +311,9 @@ class Session extends ServiceObject { * const apiResponse = data[1]; * }); */ - getMetadata(callback) { + getMetadata(): Promise<[Metadata]>; + getMetadata(callback: GetMetadataCallback): void; + getMetadata(callback?: GetMetadataCallback): void|Promise<[Metadata]> { const reqOpts = { name: this.formattedName_, }; @@ -317,7 +323,7 @@ class Session extends ServiceObject { method: 'getSession', reqOpts, }, - callback + callback! ); } /** diff --git a/src/transaction.ts b/src/transaction.ts index bdd9cd9d8..6e5f376d5 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -28,6 +28,7 @@ import {codec} from './codec'; import {partialResultStream} from './partial-result-stream'; import {TransactionRequest} from './transaction-request'; import { Metadata } from '@google-cloud/common'; +import { Session } from './session'; const config = require('./v1/spanner_client_config.json').interfaces[ 'google.spanner.v1.Spanner' diff --git a/test/codec.ts b/test/codec.ts index 4f52f503b..2aeb9156c 100644 --- a/test/codec.ts +++ b/test/codec.ts @@ -450,9 +450,7 @@ describe('codec', () => { describe('encode', () => { beforeEach(() => { - FakeGrpcService.encodeValue_ = function(value) { - return value; - }; + FakeGrpcService.encodeValue_ = value => value; }); it('should return the value from the common encoder', () => { @@ -479,14 +477,12 @@ describe('codec', () => { it('should encode structs', () => { const value = codec.Struct.fromJSON({a: 'b', c: 'd'}); const encoded = codec.encode(value); - assert.deepStrictEqual(encoded, ['b', 'd']); + assert.deepStrictEqual([].concat(encoded.slice()), ['b', 'd']); }); it('should stringify Infinity', () => { const value = Infinity; - const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString()); }); @@ -617,16 +613,13 @@ describe('codec', () => { it('should determine if the value is a struct', () => { const struct = codec.Struct.fromJSON({a: 'b'}); const type = codec.getType(struct); - - assert.deepStrictEqual(type, { - type: 'struct', - fields: [ + assert.strictEqual(type.type, 'struct'); + assert.deepStrictEqual([].concat(type.fields.slice()), [ { name: 'a', type: 'string', }, - ], - }); + ]); }); it('should attempt to determine arrays and their values', () => { diff --git a/test/database.ts b/test/database.ts index 991b70c75..caf9830b9 100644 --- a/test/database.ts +++ b/test/database.ts @@ -574,7 +574,7 @@ describe('Database', () => { database.exists((err, exists) => { assert.strictEqual(err, error); - assert.strictEqual(exists, null); + assert.strictEqual(exists, undefined); done(); }); }); diff --git a/tsconfig.json b/tsconfig.json index 6727ca0f3..afd556e07 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,9 +3,6 @@ "compilerOptions": { "rootDir": ".", "outDir": "build", - "allowJs": true, - "declaration": false, - "skipLibCheck": true, "noImplicitAny": false }, "include": [ From ad2d71516157b27d87f5f09ba6b12370ee3a6f2a Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Fri, 2 Nov 2018 00:26:28 -0700 Subject: [PATCH 2/2] Update src/codec.ts Co-Authored-By: JustinBeckwith --- src/codec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codec.ts b/src/codec.ts index 2ea8d8432..680603d90 100644 --- a/src/codec.ts +++ b/src/codec.ts @@ -140,7 +140,7 @@ export class Struct extends Array { * @param {*} thing The object to check. * @returns {boolean} */ - static isStruct(thing: {}) { + static isStruct(thing: {}) : thing is Struct { return !!(thing && thing[TYPE] === Struct.TYPE); } }