Skip to content

Commit

Permalink
feat: refactor core to use Promises instead of callbacks (#55)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: None of the authenticators or request methods take callbacks as arguments anymore - they return Promises instead.
  • Loading branch information
dpopp07 authored Oct 2, 2019
1 parent dbd6736 commit a668ec5
Show file tree
Hide file tree
Showing 22 changed files with 424 additions and 468 deletions.
5 changes: 1 addition & 4 deletions auth/authenticators/authenticator-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ export interface AuthenticateOptions {
[propName: string]: any;
}

// callback can send one arg, error or null
export type AuthenticateCallback = (result: null | Error) => void;

export interface AuthenticatorInterface {
authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void
authenticate(options: AuthenticateOptions): Promise<void | Error>
}
7 changes: 4 additions & 3 deletions auth/authenticators/authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { OutgoingHttpHeaders } from 'http';
import { AuthenticateCallback, AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';
import { AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';

export class Authenticator implements AuthenticatorInterface {
/**
Expand All @@ -31,7 +31,8 @@ export class Authenticator implements AuthenticatorInterface {
}
}

public authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void {
throw new Error('Should be implemented by subclass!');
public authenticate(options: AuthenticateOptions): Promise<void | Error> {
const error = new Error('Should be implemented by subclass!');
return Promise.reject(error);
}
}
14 changes: 8 additions & 6 deletions auth/authenticators/basic-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import extend = require('extend');
import { computeBasicAuthHeader, validateInput } from '../utils';
import { Authenticator } from './authenticator';
import { AuthenticateCallback, AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';
import { AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';

export type Options = {
username?: string;
Expand Down Expand Up @@ -48,11 +48,13 @@ export class BasicAuthenticator extends Authenticator implements AuthenticatorIn
this.password = options.password;
}

public authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void {
const authHeaderString = computeBasicAuthHeader(this.username, this.password);
const authHeader = { Authorization: authHeaderString }
public authenticate(options: AuthenticateOptions): Promise<void> {
return new Promise((resolve, reject) => {
const authHeaderString = computeBasicAuthHeader(this.username, this.password);
const authHeader = { Authorization: authHeaderString }

options.headers = extend(true, {}, options.headers, authHeader);
callback(null);
options.headers = extend(true, {}, options.headers, authHeader);
resolve();
});
}
}
12 changes: 7 additions & 5 deletions auth/authenticators/bearer-token-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import extend = require('extend');
import { validateInput } from '../utils';
import { Authenticator } from './authenticator';
import { AuthenticateCallback, AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';
import { AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';

export type Options = {
bearerToken: string;
Expand Down Expand Up @@ -48,9 +48,11 @@ export class BearerTokenAuthenticator extends Authenticator implements Authentic
this.bearerToken = bearerToken;
}

public authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void {
const authHeader = { Authorization: `Bearer ${this.bearerToken}` };
options.headers = extend(true, {}, options.headers, authHeader);
callback(null);
public authenticate(options: AuthenticateOptions): Promise<void> {
return new Promise((resolve, reject) => {
const authHeader = { Authorization: `Bearer ${this.bearerToken}` };
options.headers = extend(true, {}, options.headers, authHeader);
resolve();
});
}
}
6 changes: 3 additions & 3 deletions auth/authenticators/no-auth-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { Authenticator } from './authenticator';
import { AuthenticateCallback, AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';
import { AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';

export class NoAuthAuthenticator extends Authenticator implements AuthenticatorInterface {
/**
Expand All @@ -29,8 +29,8 @@ export class NoAuthAuthenticator extends Authenticator implements AuthenticatorI
super();
}

public authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void {
public authenticate(options: AuthenticateOptions): Promise<void> {
// immediately proceed to request. it will probably fail
callback(null);
return Promise.resolve();
}
}
16 changes: 6 additions & 10 deletions auth/authenticators/token-request-based-authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import extend = require('extend');
import { OutgoingHttpHeaders } from 'http';
import { JwtTokenManager } from '../token-managers';
import { Authenticator } from './authenticator';
import { AuthenticateCallback, AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';
import { AuthenticateOptions, AuthenticatorInterface } from './authenticator-interface';

export type BaseOptions = {
headers?: OutgoingHttpHeaders;
Expand Down Expand Up @@ -84,15 +84,11 @@ export class TokenRequestBasedAuthenticator extends Authenticator implements Aut
this.tokenManager.setHeaders(this.headers);
}

public authenticate(options: AuthenticateOptions, callback: AuthenticateCallback): void {
this.tokenManager.getToken((err, token) => {
if (err) {
callback(err);
} else {
const authHeader = { Authorization: `Bearer ${token}` };
options.headers = extend(true, {}, options.headers, authHeader);
callback(null);
}
public authenticate(options: AuthenticateOptions): Promise<void | Error> {
return this.tokenManager.getToken().then(token => {
const authHeader = { Authorization: `Bearer ${token}` };
options.headers = extend(true, {}, options.headers, authHeader);
return;
});
}
}
14 changes: 3 additions & 11 deletions auth/token-managers/cp4d-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,12 @@ export class Cp4dTokenManager extends JwtTokenManager {
this.password = options.password;
}

/**
* Callback for handling response.
*
* @callback requestTokenCallback
* @param {Error} An error if there is one, null otherwise
* @param {Object} The response if request is successful, null otherwise
*/
/**
* Request an CP4D token using a basic auth header.
*
* @param {requestTokenCallback} callback - The callback that handles the response.
* @returns {void}
* @returns {Promise}
*/
protected requestToken(callback: Function): void {
protected requestToken(): Promise<any> {
// these cannot be overwritten
const requiredHeaders = {
Authorization: computeBasicAuthHeader(this.username, this.password),
Expand All @@ -103,6 +95,6 @@ export class Cp4dTokenManager extends JwtTokenManager {
rejectUnauthorized: !this.disableSslVerification,
}
};
this.requestWrapperInstance.sendRequest(parameters, callback);
return this.requestWrapperInstance.sendRequest(parameters);
}
}
15 changes: 4 additions & 11 deletions auth/token-managers/iam-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,12 @@ export class IamTokenManager extends JwtTokenManager {
}
}

/**
* Callback for handling response.
*
* @callback requestTokenCallback
* @param {Error} An error if there is one, null otherwise
* @param {Object} The response if request is successful, null otherwise
*/
/**
* Request an IAM token using an API key.
*
* @param {requestTokenCallback} callback - The callback that handles the response.
* @returns {void}
* @returns {Promise}
*/
protected requestToken(callback: Function): void {
protected requestToken(): Promise<any> {
// these cannot be overwritten
const requiredHeaders = {
'Content-type': 'application/x-www-form-urlencoded',
Expand All @@ -148,6 +140,7 @@ export class IamTokenManager extends JwtTokenManager {
rejectUnauthorized: !this.disableSslVerification,
}
};
this.requestWrapperInstance.sendRequest(parameters, callback);

return this.requestWrapperInstance.sendRequest(parameters);
}
}
32 changes: 11 additions & 21 deletions auth/token-managers/jwt-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class JwtTokenManager {
protected tokenName: string;
protected disableSslVerification: boolean;
protected headers: OutgoingHttpHeaders;
protected requestWrapperInstance;
protected requestWrapperInstance: RequestWrapper;
private tokenInfo: any;
private expireTime: number;

Expand Down Expand Up @@ -70,33 +70,24 @@ export class JwtTokenManager {
}

/**
* This function sends an access token back through a callback. The source of the token
* is determined by the following logic:
* This function returns a Promise that resolves with an access token, if successful.
* The source of the token is determined by the following logic:
* 1. If user provides their own managed access token, assume it is valid and send it
* 2. a) If this class is managing tokens and does not yet have one, make a request for one
* b) If this class is managing tokens and the token has expired, request a new one
* 3. If this class is managing tokens and has a valid token stored, send it
*
* @param {Function} cb - callback function that the token will be passed to
*/
public getToken(cb: Function) {
public getToken(): Promise<any> {
if (!this.tokenInfo[this.tokenName] || this.isTokenExpired()) {
// 1. request a new token
this.requestToken((err, tokenResponse) => {
if (!err) {
try {
this.saveTokenInfo(tokenResponse.result);
} catch(e) {
// send lower level error through callback for user to handle
err = e;
}
}
// return null for access_token if there is an error
return cb(err, this.tokenInfo[this.tokenName] || null);
return this.requestToken().then(tokenResponse => {
this.saveTokenInfo(tokenResponse.result);
return this.tokenInfo[this.tokenName];
});
} else {
// 2. use valid, managed token
return cb(null, this.tokenInfo[this.tokenName]);
return Promise.resolve(this.tokenInfo[this.tokenName]);
}
}

Expand Down Expand Up @@ -129,12 +120,11 @@ export class JwtTokenManager {
/**
* Request a JWT using an API key.
*
* @param {Function} cb - The callback that handles the response.
* @returns {void}
* @returns {Promise}
*/
protected requestToken(cb: Function): void {
protected requestToken(): Promise<any> {
const err = new Error('`requestToken` MUST be overridden by a subclass of JwtTokenManagerV1.');
cb(err, null);
return Promise.reject(err);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions lib/base_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,18 @@ export class BaseService {
* @param {Object} parameters.defaultOptions
* @param {string} parameters.defaultOptions.serviceUrl - the base URL of the service
* @param {OutgoingHttpHeaders} parameters.defaultOptions.headers - additional headers to be passed on the request.
* @param {Function} callback - callback function to pass the response back to
* @returns {ReadableStream|undefined}
* @returns {Promise<any>}
*/
protected createRequest(parameters, callback) {
protected createRequest(parameters): Promise<any> {
// validate serviceUrl parameter has been set
const serviceUrl = parameters.defaultOptions && parameters.defaultOptions.serviceUrl;
if (!serviceUrl || typeof serviceUrl !== 'string') {
return callback(new Error('The service URL is required'), null);
return Promise.reject(new Error('The service URL is required'));
}

this.authenticator.authenticate(parameters.defaultOptions, err => {
err ? callback(err) : this.requestWrapperInstance.sendRequest(parameters, callback);
return this.authenticator.authenticate(parameters.defaultOptions).then(() => {
// resolve() handles rejection as well, so resolving the result of sendRequest should allow for proper handling later
return this.requestWrapperInstance.sendRequest(parameters);
});
}

Expand Down
28 changes: 14 additions & 14 deletions lib/requestwrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class RequestWrapper {
* @returns {ReadableStream|undefined}
* @throws {Error}
*/
public sendRequest(parameters, _callback) {
public sendRequest(parameters): Promise<any> {
const options = extend(true, {}, parameters.defaultOptions, parameters.options);
const { path, body, form, formData, qs, method, serviceUrl } = options;
let { headers, url } = options;
Expand Down Expand Up @@ -201,14 +201,8 @@ export class RequestWrapper {
},
};

this.axiosInstance(requestParams)
// placing `catch` block first because it is for catching request errors
// if it is after the `then` block, it will also catch errors if they occur
// inside of the `then` block
.catch(error => {
_callback(this.formatError(error));
})
.then(res => {
return this.axiosInstance(requestParams).then(
res => {
// sometimes error responses will still trigger the `then` block - escape that behavior here
if (!res) { return };

Expand All @@ -221,17 +215,23 @@ export class RequestWrapper {
res.result = res.data;
delete res.data;

_callback(null, res);
});
// return another promise that resolves with 'res' to be handled in generated code
return res;
},
err => {
// return another promise that rejects with 'err' to be handled in generated code
throw this.formatError(err);
}
);
}

/**
* Format error returned by axios
* @param {Function} cb the request callback
* @param {object} the object returned by axios via rejection
* @private
* @returns {request.RequestCallback}
* @returns {Error}
*/
public formatError(axiosError: any) {
public formatError(axiosError: any): Error {
// return an actual error object,
// but make it flexible so we can add properties like 'body'
const error: any = new Error();
Expand Down
Loading

0 comments on commit a668ec5

Please sign in to comment.