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

DEVEXP-256: Add timezone response plugin #8

Merged
merged 4 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
![GitHub Workflow Status](https://img.shields.io/github/actions/workflow/status/sinch/sinch-sdk-node/run_tests.yaml?branch=main)
[![Node.js LTS](https://img.shields.io/badge/Node.js-LTS%20supported-brightgreen)](https://nodejs.org/en/download/)
![Latest Release](https://img.shields.io/npm/v/@sinch/sdk-core?label=%40sinch%2Fsdk-core&labelColor=FFC658)
[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://github.com/sinch/sinch-sdk-python/blob/main/LICENSE)
[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://github.com/sinch/sinch-sdk-node/blob/main/LICENSE)

</h1>

Expand Down
1 change: 0 additions & 1 deletion examples/simple-examples/src/voice/calls/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { GetCallResultRequestData, VoiceRegion } from '@sinch/sdk-core';
const printFormat = getPrintFormat(process.argv);

if (printFormat === 'pretty') {
// TODO: Add a post process on the response to fix the missing timezone in the timestamp
console.log(`The call '${response.callId}' was from '${response.from}' to '${response.to?.endpoint}' and lasted ${response.duration} seconds
Rate: ${response.userRate?.amount} ${response.userRate?.currencyId} - Debit: ${response.debit?.amount} ${response.debit?.currencyId}`);
} else {
Expand Down
6 changes: 4 additions & 2 deletions packages/sdk-client/src/client/api-fetch-client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ResponsePlugin } from '../plugins/core/response-plugin';
import { VersionRequest } from '../plugins/version/version.request';
import { ExceptionResponse } from '../plugins/exception/exception.response';
import { VersionRequest } from '../plugins/version';
import { ExceptionResponse } from '../plugins/exception';
import { TimezoneResponse } from '../plugins/timezone';
import {
ApiClient,
ApiClientOptions,
Expand Down Expand Up @@ -34,6 +35,7 @@ export class ApiFetchClient extends ApiClient {
...options,
requestPlugins: [new VersionRequest(), ...(options.requestPlugins || [])],
responsePlugins: [
new TimezoneResponse(),
new ExceptionResponse(),
...(options.responsePlugins || []),
],
Expand Down
7 changes: 0 additions & 7 deletions packages/sdk-client/src/plugins/exception/readme.md

This file was deleted.

1 change: 1 addition & 0 deletions packages/sdk-client/src/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ export * from './basicAuthentication';
export * from './exception';
export * from './oauth2';
export * from './signing';
export * from './timezone';
export * from './version';
export * from './x-timestamp';
1 change: 1 addition & 0 deletions packages/sdk-client/src/plugins/timezone/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './timezone.response';
40 changes: 40 additions & 0 deletions packages/sdk-client/src/plugins/timezone/timezone.response.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { ResponsePlugin, ResponsePluginContext } from '../core/response-plugin';
import { PluginRunner } from '../core';

const buggyOperationIds: string[] = [
'GetCallResult',
];

const buggyFields: Record<string, string> = {
'GetCallResult': 'timestamp',
};

export class TimezoneResponse<V extends Record<string, any> | undefined = Record<string, any>>
implements ResponsePlugin<V | Record<string, any>> {

public load(
context: ResponsePluginContext,
): PluginRunner<V | Record<string, unknown>, V> {
return {
transform(res: V) {
// HACK to fix a server-side bug: the timestamp is returned without the timezone
if (res && buggyOperationIds.includes(context.operationId) ) {
for (const key in res) {
if (Object.prototype.hasOwnProperty.call(res, key)) {
const buggyKey = buggyFields[context.operationId];
if (key === buggyKey && typeof res[buggyKey] === 'string') {
const timestampValue = res[key] as string;
const timeZoneRegex = /([+-]\d{2}:?\d{2}|Z)$/;
if (!timeZoneRegex.test(timestampValue)) {
res[key] = timestampValue + 'Z';
}
}
}
}
}
return res;
},
};
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { TimezoneResponse } from '../../../src';
import { ResponsePluginContext } from '../../../src/plugins/core/response-plugin';
import { Headers } from 'node-fetch';

describe('Timezone response plugin', () => {

let context: ResponsePluginContext;
const TIMESTAMP_WITH_MISSING_TIMEZONE = '2024-01-09T15:50:24.000';
const TIMESTAMP_WITH_TIMEZONE = '2024-01-09T15:50:24.000Z';

beforeEach(() => {
context = {
operationId: 'GetCallResult',
apiName: '',
url: '',
requestOptions: {
headers: new Headers(),
basePath: '',
},
};
});

it('should update the timestamp if the timezone is missing', async () => {
const apiResponse = {
timestamp: TIMESTAMP_WITH_MISSING_TIMEZONE,
};
const plugin = new TimezoneResponse();
const runner = plugin.load(context);
const result = await runner.transform(apiResponse);

expect(result.timestamp).toBe(TIMESTAMP_WITH_TIMEZONE);
});

it('should NOT update the timestamp if the timezone is already there', async () => {
const apiResponse = {
timestamp: TIMESTAMP_WITH_TIMEZONE,
};
const plugin = new TimezoneResponse();
const runner = plugin.load(context);
const result = await runner.transform(apiResponse);

expect(result.timestamp).toBe(TIMESTAMP_WITH_TIMEZONE);
});

it('should NOT update the timestamp if the operationId if not listed', async () => {
context.operationId = 'notListedAsBuggy';
const apiResponse = {
timestamp: TIMESTAMP_WITH_MISSING_TIMEZONE,
};
const plugin = new TimezoneResponse();
const runner = plugin.load(context);
const result = await runner.transform(apiResponse);

expect(result.timestamp).toBe(TIMESTAMP_WITH_MISSING_TIMEZONE);
});
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when API is updated but SDK is not,
Can you check for case when correct timezone is returned in response?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New test added

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required but another test with a +XX timezone format could ensure regexp coverage

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
export interface GetCallResponseObj {

/** Contains the caller information. */
from?: string;
from?: GetCallResponseFrom;
/** Contains the callee information. */
to?: GetCallResponseTo;
/** Must be `pstn` for PSTN. */
Expand All @@ -27,7 +27,7 @@ export interface GetCallResponseObj {
/** The date and time of the call. */
timestamp?: Date;
/** An object that can be used to pass custom information related to the call. */
custom?: object;
custom?: string;
/** The rate per minute that was charged for the call. */
userRate?: GetCallResponseUserRate;
/** The total amount charged for the call. */
Expand All @@ -37,9 +37,17 @@ export interface GetCallResponseObj {
export interface GetCallResponseTo {
/** The type of the destination. */
type?: string;
/** The phone number, user name, or other identifier of the destination. */
/** The phone number, username, or other identifier of the destination. */
endpoint?: string;
}

export interface GetCallResponseFrom {
/** The type of the destination. */
type?: string;
/** The phone number, username, or other identifier of the destination. */
endpoint?: string;
}

export interface GetCallResponseUserRate {
/** The currency in which the call is charged. */
currencyId?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export type {
GetCallResponseObj,
GetCallResponseFrom,
GetCallResponseTo,
GetCallResponseUserRate,
GetCallResponseDebit,
Expand Down