From 816bf9b283217208debd979e893a6daf29f1f739 Mon Sep 17 00:00:00 2001
From: Alexander Fenster <fenster@google.com>
Date: Thu, 5 Mar 2020 13:48:59 -0800
Subject: [PATCH] feat!: stop accepting Promise constructor (#737)

BREAKING CHANGE

It won't be possible to pass a third-party Promise constructor (e.g. bluebird promise) to gax-managed client libraries.
---
 src/apiCaller.ts                              |  9 +--
 src/bundlingCalls/bundleApiCaller.ts          |  9 +--
 src/call.ts                                   |  6 +-
 src/createApiCall.ts                          |  2 +-
 src/fallback.ts                               |  9 +--
 src/gax.ts                                    | 21 +----
 src/grpc.ts                                   |  9 +--
 src/longRunningCalls/longRunningApiCaller.ts  |  9 +--
 src/longRunningCalls/longrunning.ts           |  6 +-
 src/normalCalls/normalApiCaller.ts            |  9 +--
 src/paginationCalls/pagedApiCaller.ts         |  6 +-
 src/streamingCalls/streamingApiCaller.ts      |  4 +-
 .../src/v1beta1/echo_client.js                |  2 -
 test/unit/apiCallable.ts                      | 28 -------
 test/unit/longrunning.ts                      | 80 -------------------
 15 files changed, 23 insertions(+), 186 deletions(-)

diff --git a/src/apiCaller.ts b/src/apiCaller.ts
index a742ef828..5c9590f1d 100644
--- a/src/apiCaller.ts
+++ b/src/apiCaller.ts
@@ -28,19 +28,12 @@ import {GoogleError} from './googleError';
 import {NormalApiCaller} from './normalCalls/normalApiCaller';
 import {StreamProxy} from './streamingCalls/streaming';
 
-export interface ApiCallerSettings {
-  promise: PromiseConstructor;
-}
-
 /**
  * An interface for all kinds of API callers (normal, that just calls API, and
  * all special ones: long-running, paginated, bundled, streaming).
  */
 export interface APICaller {
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall | StreamProxy;
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall | StreamProxy;
   wrap(func: GRPCCall): GRPCCall;
   call(
     apiCall: SimpleCallbackFunction,
diff --git a/src/bundlingCalls/bundleApiCaller.ts b/src/bundlingCalls/bundleApiCaller.ts
index 82888f6a9..78952a238 100644
--- a/src/bundlingCalls/bundleApiCaller.ts
+++ b/src/bundlingCalls/bundleApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {CallSettings} from '../gax';
@@ -34,14 +34,11 @@ export class BundleApiCaller implements APICaller {
     this.bundler = bundler;
   }
 
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/call.ts b/src/call.ts
index 08bf6531c..ac541a817 100644
--- a/src/call.ts
+++ b/src/call.ts
@@ -96,13 +96,11 @@ export class OngoingCallPromise extends OngoingCall {
   /**
    * GaxPromise is GRPCCallbackWrapper, but it holds a promise when
    * the API call finishes.
-   * @param {Function} PromiseCtor - A constructor for a promise that implements
-   * the ES6 specification of promise.
    * @constructor
    * @private
    */
   // tslint:disable-next-line variable-name
-  constructor(PromiseCtor: PromiseConstructor) {
+  constructor() {
     let resolveCallback: (
       result: [ResponseType, NextPageRequestType, RawResponseType]
     ) => void;
@@ -121,7 +119,7 @@ export class OngoingCallPromise extends OngoingCall {
         throw new GoogleError('Neither error nor response are defined');
       }
     };
-    const promise = new PromiseCtor((resolve, reject) => {
+    const promise = new Promise((resolve, reject) => {
       resolveCallback = resolve;
       rejectCallback = reject;
     }) as CancellablePromise<ResultTuple>;
diff --git a/src/createApiCall.ts b/src/createApiCall.ts
index c1bf98c7f..9b785960a 100644
--- a/src/createApiCall.ts
+++ b/src/createApiCall.ts
@@ -78,7 +78,7 @@ export function createApiCall(
       currentApiCaller = createAPICaller(settings, undefined);
     }
 
-    const ongoingCall = currentApiCaller.init(thisSettings, callback);
+    const ongoingCall = currentApiCaller.init(callback);
     funcPromise
       .then((func: GRPCCall) => {
         // Initially, the function is just what gRPC server stub contains.
diff --git a/src/fallback.ts b/src/fallback.ts
index b2b96f81a..f75c83dce 100644
--- a/src/fallback.ts
+++ b/src/fallback.ts
@@ -64,7 +64,6 @@ interface FallbackServiceStub {
 export class GrpcClient {
   auth?: OAuth2Client | GoogleAuth;
   authClient?: OAuth2Client | Compute | JWT | UserRefreshClient;
-  promise?: PromiseConstructor;
   fallback: boolean;
   grpcVersion: string;
 
@@ -74,8 +73,6 @@ export class GrpcClient {
    *
    * @param {Object=} options.auth - An instance of OAuth2Client to use in browser, or an instance of GoogleAuth from google-auth-library
    *  to use in Node.js. Required for browser, optional for Node.js.
-   * @param {Function=} options.promise - A constructor for a promise that
-   * implements the ES6 specification of promise.
    * @constructor
    */
 
@@ -93,7 +90,6 @@ export class GrpcClient {
         (options.auth as GoogleAuth) ||
         new GoogleAuth(options as GoogleAuthOptions);
     }
-    this.promise = 'promise' in options ? options.promise! : Promise;
     this.fallback = true;
     this.grpcVersion = 'fallback'; // won't be used anywhere but we need it to exist in the class
   }
@@ -202,8 +198,7 @@ export class GrpcClient {
       clientConfig,
       configOverrides,
       Status,
-      {metadataBuilder: buildMetadata},
-      this.promise
+      {metadataBuilder: buildMetadata}
     );
   }
 
@@ -374,8 +369,6 @@ export class GrpcClient {
  * gRPC-fallback version of lro
  *
  * @param {Object=} options.auth - An instance of google-auth-library.
- * @param {Function=} options.promise - A constructor for a promise that
- * implements the ES6 specification of promise.
  * @return {Object} A OperationsClientBuilder that will return a OperationsClient
  */
 export function lro(options: GrpcClientOptions) {
diff --git a/src/gax.ts b/src/gax.ts
index 1548b1b5c..f8261a222 100644
--- a/src/gax.ts
+++ b/src/gax.ts
@@ -48,9 +48,6 @@ import {BundleOptions} from './bundlingCalls/bundleExecutor';
  * @property {boolean=} isBundling - If set to false and the call is configured
  *   for bundling, bundling is not performed.
  * @property {BackoffSettings=} longrunning - BackoffSettings used for polling.
- * @property {Function=} promise - A constructor for a promise that implements the ES6
- * specification of promise which will be used to create promises. If not
- * provided, native promises will be used.
  * @example
  * // suppress bundling for bundled method.
  * api.bundlingMethod(
@@ -127,7 +124,6 @@ export interface CallOptions {
   bundleOptions?: BundleOptions | null;
   isBundling?: boolean;
   longrunning?: BackoffSettings;
-  promise?: PromiseConstructor;
 }
 
 export class CallSettings {
@@ -142,7 +138,6 @@ export class CallSettings {
   bundleOptions?: BundleOptions | null;
   isBundling: boolean;
   longrunning?: BackoffSettings;
-  promise: PromiseConstructor;
 
   /**
    * @param {Object} settings - An object containing parameters of this settings.
@@ -159,9 +154,6 @@ export class CallSettings {
    * in the page streaming request.
    * @param {Object} settings.otherArgs - Additional arguments to be passed to
    *   the API calls.
-   * @param {Function=} settings.promise - A constructor for a promise that
-   * implements the ES6 specification of promise. If not provided, native
-   * promises will be used.
    *
    * @constructor
    */
@@ -178,7 +170,6 @@ export class CallSettings {
     this.isBundling = 'isBundling' in settings ? settings.isBundling! : true;
     this.longrunning =
       'longrunning' in settings ? settings.longrunning : undefined;
-    this.promise = 'promise' in settings ? settings.promise! : Promise;
   }
 
   /**
@@ -202,7 +193,6 @@ export class CallSettings {
     let otherArgs = this.otherArgs;
     let isBundling = this.isBundling;
     let longrunning = this.longrunning;
-    let promise = this.promise;
     if ('timeout' in options) {
       timeout = options.timeout!;
     }
@@ -252,10 +242,6 @@ export class CallSettings {
       longrunning = options.longrunning;
     }
 
-    if ('promise' in options) {
-      promise = options.promise!;
-    }
-
     return new CallSettings({
       timeout,
       retry,
@@ -267,7 +253,6 @@ export class CallSettings {
       maxResults,
       otherArgs,
       isBundling,
-      promise,
     });
   }
 }
@@ -613,8 +598,6 @@ export interface ClientConfig {
  *   those codes.
  * @param {Object} otherArgs - the non-request arguments to be passed to the API
  *   calls.
- * @param {Function=} promise - A constructor for a promise that implements the
- * ES6 specification of promise. If not provided, native promises will be used.
  * @return {Object} A mapping from method name to CallSettings, or null if the
  *   service is not found in the config.
  */
@@ -623,8 +606,7 @@ export function constructSettings(
   clientConfig: ClientConfig,
   configOverrides: ClientConfig,
   retryNames: {},
-  otherArgs?: {},
-  promise?: PromiseConstructor
+  otherArgs?: {}
 ) {
   otherArgs = otherArgs || {};
   // tslint:disable-next-line no-any
@@ -679,7 +661,6 @@ export function constructSettings(
         ? createBundleOptions(bundlingConfig)
         : null,
       otherArgs,
-      promise: promise || Promise,
     });
   }
 
diff --git a/src/grpc.ts b/src/grpc.ts
index 009b326c7..5ce5a5c44 100644
--- a/src/grpc.ts
+++ b/src/grpc.ts
@@ -42,7 +42,6 @@ const COMMON_PROTO_FILES = walk
 
 export interface GrpcClientOptions extends GoogleAuthOptions {
   auth?: GoogleAuth;
-  promise?: PromiseConstructor;
   grpc?: GrpcModule;
 }
 
@@ -76,7 +75,6 @@ export class ClientStub extends grpc.Client {
 
 export class GrpcClient {
   auth: GoogleAuth;
-  promise: PromiseConstructor;
   grpc: GrpcModule;
   grpcVersion: string;
   fallback: boolean;
@@ -93,14 +91,10 @@ export class GrpcClient {
    * @param {Object=} options.grpc - When specified, this will be used
    *   for the 'grpc' module in this context. By default, it will load the grpc
    *   module in the standard way.
-   * @param {Function=} options.promise - A constructor for a promise that
-   * implements the ES6 specification of promise. If not provided, native
-   * promises will be used.
    * @constructor
    */
   constructor(options: GrpcClientOptions = {}) {
     this.auth = options.auth || new GoogleAuth(options);
-    this.promise = options.promise || Promise;
     this.fallback = false;
 
     if ('grpc' in options) {
@@ -258,8 +252,7 @@ export class GrpcClient {
       clientConfig,
       configOverrides,
       this.grpc.status,
-      {metadataBuilder: this.metadataBuilder(headers)},
-      this.promise
+      {metadataBuilder: this.metadataBuilder(headers)}
     );
   }
 
diff --git a/src/longRunningCalls/longRunningApiCaller.ts b/src/longRunningCalls/longRunningApiCaller.ts
index 4c3b97edc..931a3a973 100644
--- a/src/longRunningCalls/longRunningApiCaller.ts
+++ b/src/longRunningCalls/longRunningApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {
@@ -45,14 +45,11 @@ export class LongrunningApiCaller implements APICaller {
     this.longrunningDescriptor = longrunningDescriptor;
   }
 
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/longRunningCalls/longrunning.ts b/src/longRunningCalls/longrunning.ts
index 894241a39..275c2c19e 100644
--- a/src/longRunningCalls/longrunning.ts
+++ b/src/longRunningCalls/longrunning.ts
@@ -163,8 +163,7 @@ export class Operation extends EventEmitter {
     function promisifyResponse() {
       if (!callback) {
         // tslint:disable-next-line variable-name
-        const PromiseCtor = self._callOptions!.promise!;
-        return new PromiseCtor((resolve, reject) => {
+        return new Promise((resolve, reject) => {
           if (self.latestResponse.error) {
             const error = new GoogleError(self.latestResponse.error.message!);
             error.code = self.latestResponse.error.code!;
@@ -339,8 +338,7 @@ export class Operation extends EventEmitter {
    */
   promise() {
     // tslint:disable-next-line variable-name
-    const PromiseCtor = this._callOptions!.promise!;
-    return new PromiseCtor((resolve, reject) => {
+    return new Promise((resolve, reject) => {
       this.on('error', reject).on(
         'complete',
         (result, metadata, rawResponse) => {
diff --git a/src/normalCalls/normalApiCaller.ts b/src/normalCalls/normalApiCaller.ts
index aca09dfb3..50c424efb 100644
--- a/src/normalCalls/normalApiCaller.ts
+++ b/src/normalCalls/normalApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {GoogleError} from '../googleError';
@@ -23,14 +23,11 @@ import {GoogleError} from '../googleError';
  * Creates an API caller for regular unary methods.
  */
 export class NormalApiCaller implements APICaller {
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/paginationCalls/pagedApiCaller.ts b/src/paginationCalls/pagedApiCaller.ts
index 3d79f5680..30d765353 100644
--- a/src/paginationCalls/pagedApiCaller.ts
+++ b/src/paginationCalls/pagedApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {
   GRPCCall,
   NextPageRequestType,
@@ -120,11 +120,11 @@ export class PagedApiCaller implements APICaller {
    * @param settings Call settings. Can only be used to replace Promise with another promise implementation.
    * @param [callback] Callback to be called, if any.
    */
-  init(settings: ApiCallerSettings, callback?: APICallback) {
+  init(callback?: APICallback) {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   /**
diff --git a/src/streamingCalls/streamingApiCaller.ts b/src/streamingCalls/streamingApiCaller.ts
index ad72a207c..ce265aa97 100644
--- a/src/streamingCalls/streamingApiCaller.ts
+++ b/src/streamingCalls/streamingApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {
   APICallback,
   BiDiStreamingCall,
@@ -42,7 +42,7 @@ export class StreamingApiCaller implements APICaller {
     this.descriptor = descriptor;
   }
 
-  init(settings: ApiCallerSettings, callback: APICallback): StreamProxy {
+  init(callback: APICallback): StreamProxy {
     return new StreamProxy(this.descriptor.type, callback);
   }
 
diff --git a/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js b/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js
index 0ad1b13ff..c1725ec4b 100644
--- a/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js
+++ b/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js
@@ -54,8 +54,6 @@ class EchoClient {
    *     app is running in an environment which supports
    *     {@link https://developers.google.com/identity/protocols/application-default-credentials Application Default Credentials},
    *     your project ID will be detected automatically.
-   * @param {function} [options.promise] - Custom promise module to use instead
-   *     of native Promises.
    * @param {string} [options.apiEndpoint] - The domain name of the
    *     API remote host.
    */
diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts
index 9e8919b74..867b0f220 100644
--- a/test/unit/apiCallable.ts
+++ b/test/unit/apiCallable.ts
@@ -221,34 +221,6 @@ describe('Promise', () => {
       })
     ).to.be.undefined;
   });
-
-  it('uses a provided promise module.', done => {
-    let called = false;
-    function MockPromise(
-      resolver: (
-        resolve: (value?: unknown) => void,
-        reject: (reason?: {}) => void
-      ) => void
-    ) {
-      called = true;
-      return new Promise(resolver);
-    }
-
-    function func(argument: {}, metadata: {}, options: {}, callback: Function) {
-      callback(null, 42);
-    }
-    const apiCall = createApiCall(func);
-    // @ts-ignore incomplete options
-    apiCall({}, {promise: MockPromise})
-      .then((response: number[]) => {
-        expect(response).to.be.an('array');
-        expect(response[0]).to.eq(42);
-        // tslint:disable-next-line no-unused-expression
-        expect(called).to.be.true;
-        done();
-      })
-      .catch(done);
-  });
 });
 
 describe('retryable', () => {
diff --git a/test/unit/longrunning.ts b/test/unit/longrunning.ts
index 463a7dd58..5aabd901e 100644
--- a/test/unit/longrunning.ts
+++ b/test/unit/longrunning.ts
@@ -378,40 +378,6 @@ describe('longrunning', () => {
             done();
           });
       });
-
-      it('uses provided promise constructor.', done => {
-        const func = (
-          argument: {},
-          metadata: {},
-          options: {},
-          callback: Function
-        ) => {
-          callback(null, PENDING_OP);
-        };
-
-        let called = false;
-        function MockPromise(
-          executor: (
-            resolve: (value?: unknown) => void,
-            reject: (reason?: {}) => void
-          ) => void
-        ) {
-          const promise = new Promise(executor);
-          called = true;
-          return promise;
-        }
-
-        const client = mockOperationsClient();
-        const apiCall = createApiCall(func, client);
-        // @ts-ignore incomplete options
-        apiCall({}, {promise: MockPromise}).then(responses => {
-          const operation = responses[0];
-          operation.getOperation();
-          // tslint:disable-next-line no-unused-expression
-          expect(called).to.be.true;
-          done();
-        });
-      });
     });
 
     describe('promise', () => {
@@ -525,52 +491,6 @@ describe('longrunning', () => {
             done();
           });
       });
-
-      it('uses provided promise constructor', done => {
-        const client = mockOperationsClient();
-        const desc = new LongrunningDescriptor(
-          client,
-          (mockDecoder as unknown) as AnyDecoder,
-          (mockDecoder as unknown) as AnyDecoder
-        );
-        const initialRetryDelayMillis = 1;
-        const retryDelayMultiplier = 2;
-        const maxRetryDelayMillis = 3;
-        const totalTimeoutMillis = 4;
-        const unusedRpcValue = 0;
-        const backoff = gax.createBackoffSettings(
-          initialRetryDelayMillis,
-          retryDelayMultiplier,
-          maxRetryDelayMillis,
-          unusedRpcValue,
-          unusedRpcValue,
-          unusedRpcValue,
-          totalTimeoutMillis
-        );
-        let called = false;
-        function MockPromise(
-          executor: (
-            resolve: (value?: unknown) => void,
-            reject: (reason?: {}) => void
-          ) => void
-        ) {
-          const promise = new Promise(executor);
-          called = true;
-          return promise;
-        }
-        const operation = longrunning.operation(
-          (SUCCESSFUL_OP as {}) as operationProtos.google.longrunning.Operation,
-          desc,
-          backoff,
-          {
-            promise: (MockPromise as {}) as PromiseConstructor,
-          }
-        );
-        operation.promise();
-        // tslint:disable-next-line no-unused-expression
-        expect(called).to.be.true;
-        done();
-      });
     });
 
     describe('cancel', () => {