Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions auth/utils/get-authenticator-from-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 23 additions & 8 deletions lib/base_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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.');
}
Expand All @@ -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.
*
Expand All @@ -131,7 +146,7 @@ export class BaseService {
const { url, disableSsl } = properties;

if (url) {
results.url = url;
results.serviceUrl = url;
}
if (disableSsl === true) {
results.disableSslVerification = disableSsl;
Expand Down
14 changes: 11 additions & 3 deletions lib/requestwrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -189,7 +197,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;
Expand Down
3 changes: 3 additions & 0 deletions migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
56 changes: 38 additions & 18 deletions test/unit/base-service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -120,7 +140,7 @@ describe('Base Service', () => {
authenticator: AUTHENTICATOR,
disableSslVerification: false,
proxy: false,
url: DEFAULT_URL,
serviceUrl: DEFAULT_URL,
qs: EMPTY_OBJECT,
});
});
Expand All @@ -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,
}));

Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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/);
});
Expand Down
15 changes: 11 additions & 4 deletions test/unit/get-authenticator-from-environment.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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',
}));
}

Expand Down
Loading