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

Keep axios instance between requests. Update work with headers #213

Merged
merged 9 commits into from
Sep 17, 2024
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
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
node_modules
build
spec/**/*.json
__tests__/**/*.json
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
### Changed
- **Breaking change** Drop support of Node.js 12. The version [5.1.4](https://github.com/reportportal/client-javascript/releases/tag/v5.1.4) is the latest that supports it.
- The client now creates an instance of the `axios` HTTP client in the constructor.
- The `HOST` HTTP header is added to all requests as it was skipped by the HTTP client.
### Fixed
- Proxy support on HTTPS requests. Resolves [#30](https://github.com/reportportal/client-javascript/issues/30), related to [axios#4531](https://github.com/axios/axios/issues/4531).
- Allow using `restClientConfig` in `checkConnect()` method. Thanks to [stevez](https://github.com/stevez).
### Security
- Updated versions of vulnerable packages (braces).
Expand Down
File renamed without changes.
File renamed without changes.
29 changes: 0 additions & 29 deletions spec/helpers.spec.js → __tests__/helpers.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const os = require('os');
const fs = require('fs');
const glob = require('glob');
const helpers = require('../lib/helpers');
const RestClient = require('../lib/rest');
const pjson = require('../package.json');

describe('Helpers', () => {
Expand All @@ -27,34 +26,6 @@ describe('Helpers', () => {
});
});

describe('getServerResults', () => {
it('calls RestClient#request', () => {
jest.spyOn(RestClient, 'request').mockImplementation();

helpers.getServerResult(
'http://localhost:80/api/v1',
{ userId: 1 },
{
headers: {
'X-Custom-Header': 'WOW',
},
},
'POST',
);

expect(RestClient.request).toHaveBeenCalledWith(
'POST',
'http://localhost:80/api/v1',
{ userId: 1 },
{
headers: {
'X-Custom-Header': 'WOW',
},
},
);
});
});

describe('readLaunchesFromFile', () => {
it('should return the right ids', () => {
jest.spyOn(glob, 'sync').mockReturnValue(['rplaunch-fileOne.tmp', 'rplaunch-fileTwo.tmp']);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const process = require('process');
const RPClient = require('../lib/report-portal-client');
const RestClient = require('../lib/rest');
const helpers = require('../lib/helpers');
const { OUTPUT_TYPES } = require('../lib/constants/outputs');

Expand Down Expand Up @@ -35,7 +34,7 @@

client.logDebug('message');

expect(console.log).toHaveBeenCalledWith('message', '');

Check warning on line 37 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (14)

Unexpected console statement

Check warning on line 37 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (16)

Unexpected console statement

Check warning on line 37 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (18)

Unexpected console statement

Check warning on line 37 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (20)

Unexpected console statement

Check warning on line 37 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (22)

Unexpected console statement
});

it('should call console.log with messages if debug is true and dataMsg provided', () => {
Expand All @@ -49,7 +48,7 @@

client.logDebug('message', { key: 1, value: 2 });

expect(console.log).toHaveBeenCalledWith('message', { key: 1, value: 2 });

Check warning on line 51 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (14)

Unexpected console statement

Check warning on line 51 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (16)

Unexpected console statement

Check warning on line 51 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (18)

Unexpected console statement

Check warning on line 51 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (20)

Unexpected console statement

Check warning on line 51 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (22)

Unexpected console statement
});

it('should not call console.log if debug is false', () => {
Expand All @@ -63,7 +62,7 @@

client.logDebug('message');

expect(console.log).not.toHaveBeenCalled();

Check warning on line 65 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (14)

Unexpected console statement

Check warning on line 65 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (16)

Unexpected console statement

Check warning on line 65 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (18)

Unexpected console statement

Check warning on line 65 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (20)

Unexpected console statement

Check warning on line 65 in __tests__/report-portal-client.spec.js

View workflow job for this annotation

GitHub Actions / test (22)

Unexpected console statement
});
});

Expand Down Expand Up @@ -134,41 +133,12 @@
project: 'test',
endpoint: 'https://abc.com',
});
jest.spyOn(RestClient, 'request').mockReturnValue(Promise.resolve('ok'));
jest.spyOn(client.restClient, 'request').mockReturnValue(Promise.resolve('ok'));

const request = client.checkConnect();

return expect(request).resolves.toBeDefined();
});

it('client should include restClientConfig', () => {
const client = new RPClient({
apiKey: 'test',
project: 'test',
endpoint: 'https://abc.com/v1',
restClientConfig: {
proxy: false,
timeout: 0,
},
});
jest.spyOn(RestClient, 'request').mockImplementation();

client.checkConnect();

expect(RestClient.request).toHaveBeenCalledWith(
'GET',
'https://abc.com/v1/user',
{},
{
headers: {
'User-Agent': 'NodeJS',
Authorization: `bearer test`,
},
proxy: false,
timeout: 0,
},
);
});
});

describe('triggerAnalyticsEvent', () => {
Expand Down Expand Up @@ -278,15 +248,11 @@
startTime: time,
});

expect(client.restClient.create).toHaveBeenCalledWith(
'launch',
{
name: 'Test launch name',
startTime: time,
attributes: fakeSystemAttr,
},
{ headers: client.headers },
);
expect(client.restClient.create).toHaveBeenCalledWith('launch', {
name: 'Test launch name',
startTime: time,
attributes: fakeSystemAttr,
});
});

it('should call restClient with suitable parameters, attributes is concatenated', () => {
Expand All @@ -312,22 +278,18 @@
attributes: [{ value: 'value' }],
});

expect(client.restClient.create).toHaveBeenCalledWith(
'launch',
{
name: 'Test launch name',
startTime: time,
attributes: [
{ value: 'value' },
{
key: 'client',
value: 'client-name|1.0',
system: true,
},
],
},
{ headers: client.headers },
);
expect(client.restClient.create).toHaveBeenCalledWith('launch', {
name: 'Test launch name',
startTime: time,
attributes: [
{ value: 'value' },
{
key: 'client',
value: 'client-name|1.0',
system: true,
},
],
});
});

it('dont start new launch if launchDataRQ.id is not empty', () => {
Expand Down Expand Up @@ -599,9 +561,7 @@

expect(promise.then).toBeDefined();
await promise;
expect(client.restClient.create).toHaveBeenCalledWith('launch/merge', fakeMergeDataRQ, {
headers: client.headers,
});
expect(client.restClient.create).toHaveBeenCalledWith('launch/merge', fakeMergeDataRQ);
});

it('should not call rest client if something went wrong', async () => {
Expand Down
18 changes: 17 additions & 1 deletion spec/rest.spec.js → __tests__/rest.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ const nock = require('nock');
const isEqual = require('lodash/isEqual');
const http = require('http');
const RestClient = require('../lib/rest');
const logger = require('../lib/logger');

describe('RestClient', () => {
const options = {
baseURL: 'http://report-portal-host:8080/api/v1',
headers: {
Authorization: 'bearer 00000000-0000-0000-0000-000000000000',
'User-Agent': 'NodeJS',
Authorization: 'Bearer 00000000-0000-0000-0000-000000000000',
},
restClientConfig: {
agent: {
Expand All @@ -34,6 +35,21 @@ describe('RestClient', () => {
expect(restClient.baseURL).toBe(options.baseURL);
expect(restClient.headers).toEqual(options.headers);
expect(restClient.restClientConfig).toEqual(options.restClientConfig);
expect(restClient.axiosInstance).toBeDefined();
});

it('adds Logger to axios instance if enabled', () => {
const spyLogger = jest.spyOn(logger, 'addLogger').mockReturnValue();
const optionsWithLoggerEnabled = {
...options,
restClientConfig: {
...options.restClientConfig,
debug: true,
},
};
const client = new RestClient(optionsWithLoggerEnabled);

expect(spyLogger).toHaveBeenCalledWith(client.axiosInstance);
});
});

Expand Down
File renamed without changes.
15 changes: 11 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
module.exports = {
moduleFileExtensions: ['js'],
"testMatch": [
"<rootDir>/spec/**/*[sS]pec.js"
],
coverageReporters: ["lcov", "text-summary"],
testRegex: '/__tests__/.*\\.(test|spec).js$',
testEnvironment: 'node',
collectCoverageFrom: ['lib/**/*.js', '!lib/logger.js'],
coverageThreshold: {
global: {
branches: 80,
functions: 75,
lines: 80,
statements: 80,
},
},
bail: false,
};
3 changes: 2 additions & 1 deletion lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ module.exports = {
return new Date().valueOf();
},

// TODO: deprecate and remove
getServerResult(url, request, options, method) {
return RestClient.request(method, url, request, options);
return new RestClient(options).request(method, url, request, options);
},

readLaunchesFromFile() {
Expand Down
41 changes: 41 additions & 0 deletions lib/logger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const addLogger = (axiosInstance) => {
axiosInstance.interceptors.request.use((config) => {
const startDate = new Date();
// eslint-disable-next-line no-param-reassign
config.startTime = startDate.valueOf();

console.log(`Request method=${config.method} url=${config.url} [${startDate.toISOString()}]`);

return config;
});

axiosInstance.interceptors.response.use(
(response) => {
const date = new Date();
const { status, config } = response;

console.log(
`Response status=${status} url=${config.url} time=${
date.valueOf() - config.startTime
}ms [${date.toISOString()}]`,
);

return response;
},
(error) => {
const date = new Date();
const { response, config } = error;
const status = response ? response.status : null;

console.log(
`Response ${status ? `status=${status}` : `message='${error.message}'`} url=${
config.url
} time=${date.valueOf() - config.startTime}ms [${date.toISOString()}]`,
);

return Promise.reject(error);
},
);
};

module.exports = { addLogger };
34 changes: 14 additions & 20 deletions lib/report-portal-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class RPClient {
this.baseURL = [this.config.endpoint, this.config.project].join('/');
this.headers = {
'User-Agent': 'NodeJS',
Authorization: `bearer ${this.apiKey}`,
'Content-Type': 'application/json; charset=UTF-8',
Authorization: `Bearer ${this.apiKey}`,
...(this.config.headers || {}),
};
this.helpers = helpers;
Expand Down Expand Up @@ -130,12 +131,7 @@ class RPClient {

checkConnect() {
const url = [this.config.endpoint.replace('/v2', '/v1'), 'user'].join('/');
return RestClient.request(
'GET',
url,
{},
{ headers: this.headers, ...this.restClient.getRestConfig() },
);
return this.restClient.request('GET', url, {});
}

async triggerStatisticsEvent() {
Expand Down Expand Up @@ -206,7 +202,7 @@ class RPClient {
this.map[tempId] = this.getNewItemObj((resolve, reject) => {
const url = 'launch';
this.logDebug(`Start launch with tempId ${tempId}`, launchDataRQ);
this.restClient.create(url, launchData, { headers: this.headers }).then(
this.restClient.create(url, launchData).then(
(response) => {
this.map[tempId].realId = response.id;
this.launchUuid = response.id;
Expand Down Expand Up @@ -265,7 +261,7 @@ class RPClient {
() => {
this.logDebug(`Finish launch with tempId ${launchTempId}`, finishExecutionData);
const url = ['launch', launchObj.realId, 'finish'].join('/');
this.restClient.update(url, finishExecutionData, { headers: this.headers }).then(
this.restClient.update(url, finishExecutionData).then(
(response) => {
this.logDebug(`Success finish launch with tempId ${launchTempId}`, response);
console.log(`\nReportPortal Launch Link: ${response.link}`);
Expand Down Expand Up @@ -337,11 +333,13 @@ class RPClient {
'filter.in.uuid': launchUUIds,
'page.size': launchUUIds.length,
});
const launchSearchUrl = this.config.mode === 'DEBUG' ?
`launch/mode?${params.toString()}` : `launch?${params.toString()}`;
const launchSearchUrl =
this.config.mode === 'DEBUG'
? `launch/mode?${params.toString()}`
: `launch?${params.toString()}`;
this.logDebug(`Find launches with UUIDs to merge: ${launchUUIds}`);
return this.restClient
.retrieveSyncAPI(launchSearchUrl, { headers: this.headers })
.retrieveSyncAPI(launchSearchUrl)
.then(
(response) => {
const launchIds = response.content.map((launch) => launch.id);
Expand All @@ -357,7 +355,7 @@ class RPClient {
const request = this.getMergeLaunchesRequest(launchIds, mergeOptions);
this.logDebug(`Merge launches with ids: ${launchIds}`, request);
const mergeURL = 'launch/merge';
return this.restClient.create(mergeURL, request, { headers: this.headers });
return this.restClient.create(mergeURL, request);
})
.then((response) => {
this.logDebug(`Launches with UUIDs: ${launchUUIds} were successfully merged!`);
Expand Down Expand Up @@ -426,7 +424,7 @@ class RPClient {
() => {
const url = ['launch', launchObj.realId, 'update'].join('/');
this.logDebug(`Update launch with tempId ${launchTempId}`, launchData);
this.restClient.update(url, launchData, { headers: this.headers }).then(
this.restClient.update(url, launchData).then(
(response) => {
this.logDebug(`Launch with tempId ${launchTempId} were successfully updated`, response);
resolvePromise(response);
Expand Down Expand Up @@ -534,7 +532,7 @@ class RPClient {
}
testItemData.launchUuid = realLaunchId;
this.logDebug(`Start test item with tempId ${tempId}`, testItemData);
this.restClient.create(url, testItemData, { headers: this.headers }).then(
this.restClient.create(url, testItemData).then(
(response) => {
this.logDebug(`Success start item with tempId ${tempId}`, response);
this.map[tempId].realId = response.id;
Expand Down Expand Up @@ -721,7 +719,6 @@ class RPClient {
return this.restClient.create(
url,
Object.assign(saveLogRQ, { launchUuid }, isItemUuid && { itemUuid }),
{ headers: this.headers },
);
};
return this.saveLog(itemObj, requestPromise);
Expand Down Expand Up @@ -777,7 +774,6 @@ class RPClient {
return this.restClient
.create(url, this.buildMultiPartStream([saveLogRQ], fileObj, MULTIPART_BOUNDARY), {
headers: {
...this.headers,
'Content-Type': `multipart/form-data; boundary=${MULTIPART_BOUNDARY}`,
},
})
Expand Down Expand Up @@ -840,9 +836,7 @@ class RPClient {
const url = ['item', itemObj.realId].join('/');
this.logDebug(`Finish test item with tempId ${itemTempId}`, itemObj);
this.restClient
.update(url, Object.assign(finishTestItemData, { launchUuid: this.launchUuid }), {
headers: this.headers,
})
.update(url, Object.assign(finishTestItemData, { launchUuid: this.launchUuid }))
.then(
(response) => {
this.logDebug(`Success finish item with tempId ${itemTempId}`, response);
Expand Down
Loading
Loading