feat: add setServiceUrl method as a setter for the serviceUrl property (#41)

* Change `url` to `serviceUrl`
* Validate that `serviceUrl` is set in the `sendRequest` method
* Make "auth type" case-insensitive
* Throw an error for unsupported "auth types" - only default to 'iam' when no "auth type" is provided

BREAKING CHANGE: The internal property `url` no longer exists on the `baseOptions` object, it has been renamed to `serviceUrl`

Author: dpopp07

auth/utils/get-authenticator-from-environment.ts

@@ -54,24 +54,34 @@ export function getAuthenticatorFromEnvironment(serviceName: string) {
     delete credentials.authDisableSsl; 
   }
 
+  // default the auth type to `iam` if authType is undefined, or not a string
+  let { authType } = credentials;
+  if (!authType || typeof authType !== 'string') {
+    authType = 'iam';
+  }
+
   // create and return the appropriate authenticator
   let authenticator;
 
-  switch (credentials.authType) {
-    case 'noAuth':
+  // fold authType to lower case for case insensitivity
+  switch (authType.toLowerCase()) {
+    case 'noauth':
       authenticator = new NoAuthAuthenticator();
       break;
     case 'basic':
       authenticator = new BasicAuthenticator(credentials);
       break;
-    case 'bearerToken':
+    case 'bearertoken':
       authenticator = new BearerTokenAuthenticator(credentials);
       break;
     case 'cp4d':
       authenticator = new CloudPakForDataAuthenticator(credentials);
       break;
-    default: // default the authentication type to iam
+    case 'iam':
       authenticator = new IamAuthenticator(credentials);
+      break;
+    default:
+      throw new Error('Invalid value for AUTH_TYPE: ' + authType);
   }
 
   return authenticator;

lib/base_service.ts

@@ -23,7 +23,8 @@ import { stripTrailingSlash } from './helper';
 import { RequestWrapper } from './requestwrapper';
 
 export interface UserOptions {
-  url?: string;
+  url?: string; // deprecated
+  serviceUrl?: string;
   version?: string;
   headers?: OutgoingHttpHeaders;
   disableSslVerification?: boolean;
@@ -66,12 +67,17 @@ export class BaseService {
     const _options = {} as BaseServiceOptions;
     const options = extend({}, userOptions);
 
-    if (options.url) {
-      _options.url = stripTrailingSlash(options.url);
+    // for compatibility
+    if (options.url && !options.serviceUrl) {
+      options.serviceUrl = options.url;
     }
 
-    // check url for common user errors
-    const credentialProblems = checkCredentials(options, ['url']);
+    if (options.serviceUrl) {
+      _options.serviceUrl = stripTrailingSlash(options.serviceUrl);
+    }
+
+    // check serviceUrl for common user errors
+    const credentialProblems = checkCredentials(options, ['serviceUrl']);
     if (credentialProblems) {
       throw new Error(credentialProblems);
     }
@@ -84,15 +90,15 @@ export class BaseService {
 
     const serviceClass = this.constructor as typeof BaseService;
     this.baseOptions = extend(
-      { qs: {}, url: serviceClass.URL },
+      { qs: {}, serviceUrl: serviceClass.URL },
       options,
       this.readOptionsFromExternalConfig(),
       _options
     );
 
     this.requestWrapperInstance = new RequestWrapper(this.baseOptions);
 
-    // set authenticator
+    // enforce that an authenticator is set
     if (!options.authenticator) {
       throw new Error('Authenticator must be set.');
     }
@@ -109,6 +115,15 @@ export class BaseService {
     return this.authenticator;
   }
 
+  /**
+   * Set the service URL to send requests to.
+   *
+   * @param {string} the base URL for the service
+   */
+  public setServiceUrl(url: string): void {
+    this.baseOptions.serviceUrl = url;
+  }
+
   /**
    * Wrapper around `sendRequest` that enforces the request will be authenticated.
    *
@@ -144,7 +159,7 @@ export class BaseService {
       const { url, disableSsl } = properties;
 
       if (url) {
-        results.url = url;
+        results.serviceUrl = url;
       }
       if (disableSsl === true) {
         results.disableSslVerification = disableSsl;

lib/requestwrapper.ts

@@ -122,8 +122,16 @@ export class RequestWrapper {
    */
   public sendRequest(parameters, _callback) {
     const options = extend(true, {}, parameters.defaultOptions, parameters.options);
-    const { path, body, form, formData, qs, method } = options;
-    let { url, headers } = options;
+    const { path, body, form, formData, qs, method, serviceUrl } = options;
+    let { headers, url } = options;
+
+    // validate service url parameter has been set
+    if (!serviceUrl || typeof serviceUrl !== 'string') {
+      return _callback(new Error('The service URL is required'), null);
+    }
+
+    // use default service url if `url` parameter is not specified in generated method
+    url = url || serviceUrl;
 
     const multipartForm = new FormData();
 
@@ -171,7 +179,7 @@ export class RequestWrapper {
 
     // Add service default endpoint if options.url start with /
     if (url && url.charAt(0) === '/') {
-      url = parameters.defaultOptions.url + url;
+      url = serviceUrl + url;
     }
 
     let data = body;

migration-guide.md

@@ -31,3 +31,6 @@ const service = new MyService({
 - `JwtTokenManagerV1` renamed to `JwtTokenManager`
 - Token managers no longer support the `accessToken` parameter. There is no need for a token manager when a user is managing their own token. This behavior is replaced by the `BearerTokenAuthenticator` class.
 - In the IAM Token Manager: the method `setAuthorizationInfo` is renamed to `setClientIdAndSecret`
+
+#### URL parameter name changed
+The variable name for the stored, URL parameter has been changed from `url` to `serviceUrl`. Note that `url` can still be compatibility passed into the constructor as an alias for `serviceUrl`. However, if you try to access the `url` property directly in your code, this is a breaking change.

test/unit/base-service.test.js

@@ -70,14 +70,34 @@ describe('Base Service', () => {
     }).toThrow();
   });
 
-  it('should strip trailing slash of url during instantiation', () => {
+  it('should strip trailing slash of serviceUrl during instantiation', () => {
+    const testService = new TestService({
+      authenticator: AUTHENTICATOR,
+      serviceUrl: 'https://example.ibm.com/',
+    });
+
+    expect(testService.baseOptions.serviceUrl).toBe('https://example.ibm.com');
+  });
+
+  it('should accept `url` instead of `serviceUrl` for compaitiblity', () => {
     const testService = new TestService({
       authenticator: AUTHENTICATOR,
-      version: 'v1',
       url: 'https://example.ibm.com/',
     });
 
-    expect(testService.baseOptions.url).toBe('https://example.ibm.com');
+    expect(testService.baseOptions.serviceUrl).toBe('https://example.ibm.com');
+  });
+
+  it('should support setting the service url after instantiation', () => {
+    const testService = new TestService({
+      authenticator: AUTHENTICATOR,
+    });
+
+    expect(testService.baseOptions.serviceUrl).toBe(DEFAULT_URL);
+
+    const newUrl = 'new.com';
+    testService.setServiceUrl(newUrl);
+    expect(testService.baseOptions.serviceUrl).toBe(newUrl);
   });
 
   it('should throw an error if an authenticator is not passed in', () => {
@@ -120,7 +140,7 @@ describe('Base Service', () => {
       authenticator: AUTHENTICATOR,
       disableSslVerification: false,
       proxy: false,
-      url: DEFAULT_URL,
+      serviceUrl: DEFAULT_URL,
       qs: EMPTY_OBJECT,
     });
   });
@@ -134,20 +154,20 @@ describe('Base Service', () => {
     expect(testService.baseOptions.qs).toEqual(EMPTY_OBJECT);
   });
 
-  it('should use the default service url', () => {
+  it('should use the default service serviceUrl', () => {
     const testService = new TestService({
       authenticator: AUTHENTICATOR,
     });
 
-    expect(testService.baseOptions.url).toBe(DEFAULT_URL);
+    expect(testService.baseOptions.serviceUrl).toBe(DEFAULT_URL);
   });
 
-  it('should read url and disableSslVerification from env', () => {
-    const url = 'abc123.com';
+  it('should read serviceUrl and disableSslVerification from env', () => {
+    const serviceUrl = 'abc123.com';
     const disableSsl = true;
 
     readExternalSourcesMock.mockImplementation(() => ({
-      url,
+      url: serviceUrl,
       disableSsl,
     }));
 
@@ -157,7 +177,7 @@ describe('Base Service', () => {
 
     const fromCredsFile = testService.readOptionsFromExternalConfig();
 
-    expect(fromCredsFile.url).toBe(url);
+    expect(fromCredsFile.serviceUrl).toBe(serviceUrl);
     expect(fromCredsFile.disableSslVerification).toBe(disableSsl);
     expect(readExternalSourcesMock).toHaveBeenCalled();
     expect(readExternalSourcesMock.mock.calls[0][0]).toBe(DEFAULT_NAME);
@@ -170,12 +190,12 @@ describe('Base Service', () => {
 
     const testService = new TestService({
       authenticator: AUTHENTICATOR,
-      url: 'withtrailingslash.com/api/',
+      serviceUrl: 'withtrailingslash.com/api/',
       proxy: false,
     });
 
     expect(testService.baseOptions).toEqual({
-      url: 'withtrailingslash.com/api',
+      serviceUrl: 'withtrailingslash.com/api',
       disableSslVerification: true,
       proxy: false,
       qs: EMPTY_OBJECT,
@@ -192,11 +212,11 @@ describe('Base Service', () => {
 
     const parameters = {
       defaultOptions: {
-        url: DEFAULT_URL,
+        serviceUrl: DEFAULT_URL,
         Accept: 'application/json',
       },
       options: {
-        url: '/v2/assistants/{assistant_id}/sessions',
+        serviceUrl: '/v2/assistants/{assistant_id}/sessions',
         method: 'POST',
         path: {
           id: '123',
@@ -219,11 +239,11 @@ describe('Base Service', () => {
 
     const parameters = {
       defaultOptions: {
-        url: DEFAULT_URL,
+        serviceUrl: DEFAULT_URL,
         Accept: 'application/json',
       },
       options: {
-        url: '/v2/assistants/{assistant_id}/sessions',
+        serviceUrl: '/v2/assistants/{assistant_id}/sessions',
         method: 'POST',
         path: {
           id: '123',
@@ -268,11 +288,11 @@ describe('Base Service', () => {
     expect(testService.readOptionsFromExternalConfig()).toEqual(EMPTY_OBJECT);
   });
 
-  it('should check url for common problems', () => {
+  it('should check serviceUrl for common problems', () => {
     expect(() => {
       new TestService({
         authenticator: AUTHENTICATOR,
-        url: 'myapi.com/{instanceId}',
+        serviceUrl: 'myapi.com/{instanceId}',
       });
     }).toThrow(/Revise these credentials/);
   });

test/unit/get-authenticator-from-environment.test.js

@@ -31,8 +31,8 @@ describe('Get Authenticator From Environment Module', () => {
     expect(() => getAuthenticatorFromEnvironment(SERVICE_NAME)).toThrow();
   });
 
-  it('should get no auth authenticator', () => {
-    setUpNoauthPayload();
+  it('should get no auth authenticator - tests case insentivity of auth type', () => {
+    setUpNoAuthPayload();
     const authenticator = getAuthenticatorFromEnvironment(SERVICE_NAME);
     expect(authenticator).toBeInstanceOf(NoAuthAuthenticator);
     expect(readExternalSourcesMock).toHaveBeenCalled();
@@ -82,12 +82,19 @@ describe('Get Authenticator From Environment Module', () => {
     expect(authenticator).toBeInstanceOf(IamAuthenticator);
     expect(authenticator.apikey).toBe(APIKEY);
   });
+
+  it('should throw an error when an unsupported auth type is provided', () => {
+    readExternalSourcesMock.mockImplementation(() => ({ apikey: APIKEY, authType: 'unsupported' }));
+    expect(() => {
+      getAuthenticatorFromEnvironment(SERVICE_NAME);
+    }).toThrow();
+  });
 });
 
 // mock payloads for the read-external-sources module
-function setUpNoauthPayload() {
+function setUpNoAuthPayload() {
   readExternalSourcesMock.mockImplementation(() => ({
-    authType: 'noAuth',
+    authType: 'noauth',
   }));
 }