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

Datastore/feat user agent suffix #10086

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ded3b75
Adds suffix to user agent for calls to API initiated by DataStore
erinleigh90 Jul 6, 2022
98a019d
Attempts to fix first half of user agent not being sent
erinleigh90 Jul 7, 2022
500fecc
Merges updated upstream into local changes
erinleigh90 Jul 8, 2022
2e77003
Makes setting of user agent header more concise
erinleigh90 Jul 8, 2022
78ec7db
Moves appending of suffix to user agent to core library
erinleigh90 Jul 12, 2022
b2a4488
Moves user agent suffix constant to common util in datastore
erinleigh90 Jul 12, 2022
8128e23
Removes unused import
erinleigh90 Jul 12, 2022
4d206aa
Unit test for api-graphql
erinleigh90 Jul 12, 2022
259ee25
Pulls in user agent suffix from datastore utils class
erinleigh90 Jul 12, 2022
416d18f
Adds unit test for getAmplifyUserAgent with and without content
erinleigh90 Jul 13, 2022
8deb5d2
Fixes issue found while testing, line too long.
erinleigh90 Jul 14, 2022
7481800
Adds test for DataStore mutation.ts
erinleigh90 Jul 14, 2022
68af8fb
Tests user agent suffix in datastore sync
erinleigh90 Jul 15, 2022
1c0fc6f
Adds user agent suffix assertion to subscription test
erinleigh90 Jul 15, 2022
e7ff5f9
Merge branch 'aws-amplify:main' into user-agent-suffix-datastore
erinleigh90 Jul 15, 2022
7b47744
Fixes variable declaration: const instead of let
erinleigh90 Jul 15, 2022
ede6f59
Removes leftover lines of code from testing objectContains
erinleigh90 Jul 15, 2022
502415a
Removes code style changes unrelated to user agent suffix
erinleigh90 Jul 15, 2022
23a9798
Removes code style changes unrelated to user agent suffix
erinleigh90 Jul 15, 2022
a1f3bb4
Removes unnecessary null value option for userAgentSuffix - undefined…
erinleigh90 Jul 15, 2022
73e01eb
Replaces imports from lib-esm
erinleigh90 Jul 18, 2022
f8fcf69
Replaces hard-coded string in assertion with constant from util file
erinleigh90 Jul 18, 2022
acd5c1d
Moves var declaration under import
erinleigh90 Jul 18, 2022
987971f
Makes test method names more descriptive
erinleigh90 Jul 18, 2022
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
58 changes: 57 additions & 1 deletion packages/api-graphql/__tests__/GraphQLAPI-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,63 @@ describe('API test', () => {
},
});
});
});

test('happy-case-query with userAgentSuffix', async () => {
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
const spyonAuth = jest
.spyOn(Credentials, 'get')
.mockImplementationOnce(() => {
return new Promise((res, rej) => {
res('cred');
});
});

const spyon = jest
.spyOn(RestClient.prototype, 'post')
.mockImplementationOnce((url, init) => {
return new Promise((res, rej) => {
res({});
});
});

const api = new API(config);
const url = 'https://appsync.amazonaws.com',
region = 'us-east-2',
apiKey = 'secret_api_key',
variables = { id: '809392da-ec91-4ef0-b219-5238a8f942b2' },
userAgentSuffix = '/DataStore';
api.configure({
aws_appsync_graphqlEndpoint: url,
aws_appsync_region: region,
aws_appsync_authenticationType: 'API_KEY',
aws_appsync_apiKey: apiKey,
});

const headers = {
Authorization: null,
'X-Api-Key': apiKey,
'x-amz-user-agent': `${Constants.userAgent}/DataStore`,
};

const body = {
query: getEventQuery,
variables,
};

const init = {
headers,
body,
signerServiceInfo: {
service: 'appsync',
region,
},
cancellableToken: mockCancellableToken,
};
let authToken: undefined;

await api.graphql(graphqlOperation(GetEvent, variables, authToken, userAgentSuffix));

expect(spyon).toBeCalledWith(url, init);
});

describe('configure test', () => {
test('without aws_project_region', () => {
Expand Down
26 changes: 17 additions & 9 deletions packages/api-graphql/src/GraphQLAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import Observable from 'zen-observable-ts';
import {
Amplify,
ConsoleLogger as Logger,
Constants,
Credentials,
getAmplifyUserAgent,
INTERNAL_AWS_APPSYNC_REALTIME_PUBSUB_PROVIDER,
} from '@aws-amplify/core';
import PubSub from '@aws-amplify/pubsub';
Expand All @@ -43,11 +43,13 @@ const logger = new Logger('GraphQLAPI');
export const graphqlOperation = (
query,
variables = {},
authToken?: string
authToken?: string,
userAgentSuffix?: string
) => ({
query,
variables,
authToken,
userAgentSuffix,
});

/**
Expand Down Expand Up @@ -224,7 +226,13 @@ export class GraphQLAPIClass {
* @returns An Observable if the query is a subscription query, else a promise of the graphql result.
*/
graphql<T = any>(
{ query: paramQuery, variables = {}, authMode, authToken }: GraphQLOptions,
{
query: paramQuery,
variables = {},
authMode,
authToken,
userAgentSuffix,
}: GraphQLOptions,
additionalHeaders?: { [key: string]: string }
): Observable<GraphQLResult<T>> | Promise<GraphQLResult<T>> {
const query =
Expand All @@ -233,7 +241,7 @@ export class GraphQLAPIClass {
: parse(print(paramQuery));

const [operationDef = {}] = query.definitions.filter(
(def) => def.kind === 'OperationDefinition'
def => def.kind === 'OperationDefinition'
);
const { operation: operationType } =
operationDef as OperationDefinitionNode;
Expand All @@ -251,7 +259,7 @@ export class GraphQLAPIClass {
const cancellableToken = this._api.getCancellableToken();
const initParams = { cancellableToken };
const responsePromise = this._graphql<T>(
{ query, variables, authMode },
{ query, variables, authMode, userAgentSuffix },
headers,
initParams
);
Expand All @@ -268,7 +276,7 @@ export class GraphQLAPIClass {
}

private async _graphql<T = any>(
{ query, variables, authMode }: GraphQLOptions,
{ query, variables, authMode, userAgentSuffix }: GraphQLOptions,
additionalHeaders = {},
initParams = {}
): Promise<GraphQLResult<T>> {
Expand All @@ -294,7 +302,7 @@ export class GraphQLAPIClass {
...(await graphql_headers({ query, variables })),
...additionalHeaders,
...(!customGraphqlEndpoint && {
[USER_AGENT_HEADER]: Constants.userAgent,
[USER_AGENT_HEADER]: getAmplifyUserAgent(userAgentSuffix),
}),
};

Expand Down Expand Up @@ -421,14 +429,14 @@ export class GraphQLAPIClass {
*/
_ensureCredentials() {
return this.Credentials.get()
.then((credentials) => {
.then(credentials => {
if (!credentials) return false;
const cred = this.Credentials.shear(credentials);
logger.debug('set credentials for api', cred);

return true;
})
.catch((err) => {
.catch(err => {
logger.warn('ensure credentials error', err);
return false;
});
Expand Down
1 change: 1 addition & 0 deletions packages/api-graphql/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface GraphQLOptions {
variables?: object;
authMode?: keyof typeof GRAPHQL_AUTH_MODE;
authToken?: string;
userAgentSuffix?: string;
}

export interface GraphQLResult<T = object> {
Expand Down
13 changes: 13 additions & 0 deletions packages/core/__tests__/Platform-test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getAmplifyUserAgent } from '../lib-esm';
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
import Platform from '../src/Platform';

describe('Platform test', () => {
Expand All @@ -6,4 +7,16 @@ describe('Platform test', () => {
expect(Platform.isReactNative).toBe(false);
});
});

describe('getAmplifyUserAgent test', () => {
test('without content', () => {
expect(getAmplifyUserAgent()).toBe(Platform.userAgent);
});

test('with content', () => {
expect(getAmplifyUserAgent('/DataStore')).toBe(
`${Platform.userAgent}/DataStore`
);
});
});
});
4 changes: 2 additions & 2 deletions packages/core/src/Platform/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ if (typeof navigator !== 'undefined' && navigator.product) {
}
}

export const getAmplifyUserAgent = () => {
return Platform.userAgent;
export const getAmplifyUserAgent = (content?: string) => {
david-mcafee marked this conversation as resolved.
Show resolved Hide resolved
return `${Platform.userAgent}${content ? content : ''}`;
};

/**
Expand Down
31 changes: 24 additions & 7 deletions packages/datastore/__tests__/mutation.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const mockRestPost = jest.fn();
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
import { RestClient } from '@aws-amplify/api-rest';
import {
MutationProcessor,
Expand All @@ -17,6 +18,8 @@ import {
} from '../src/types';
import { createMutationInstanceFromModelOperation } from '../src/sync/utils';
import { MutationEvent } from '../src/sync/';
import { Constants } from '@aws-amplify/core';
import { USER_AGENT_SUFFIX_DATASTORE } from '../src/util';

let syncClasses: any;
let modelInstanceCreator: any;
Expand All @@ -25,7 +28,11 @@ let PostCustomPK: PersistentModelConstructor<PostCustomPKType>;
let PostCustomPKSort: PersistentModelConstructor<PostCustomPKSortType>;
let axiosError;

describe('Jittered retry', () => {
beforeEach(() => {
axiosError = timeoutError;
});

describe('Jittered backoff', () => {
it('should progress exponentially until some limit', () => {
const COUNT = 13;

Expand Down Expand Up @@ -143,6 +150,21 @@ describe('MutationProcessor', () => {
expect(input.postId).toEqual(100);
});
});
describe('Call to rest api', () => {
it('Should send a Datastore User Agent to the rest api', async () => {
jest.spyOn(mutationProcessor, 'resume');
await mutationProcessor.resume();

expect(mockRestPost).toBeCalledWith(
expect.anything(),
expect.objectContaining({
headers: expect.objectContaining({
'x-amz-user-agent': `${Constants.userAgent}${USER_AGENT_SUFFIX_DATASTORE}`,
}),
})
);
});
});
afterAll(() => {
jest.restoreAllMocks();
});
Expand Down Expand Up @@ -225,15 +247,14 @@ describe('error handler', () => {
);
});
});

// Mocking restClient.post to throw the error we expect
// when experiencing poor network conditions
jest.mock('@aws-amplify/api-rest', () => {
return {
...jest.requireActual('@aws-amplify/api-rest'),
RestClient() {
return {
post: jest.fn().mockImplementation(() => {
post: mockRestPost.mockImplementation(() => {
return Promise.reject(axiosError);
}),
getCancellableToken: () => {},
Expand Down Expand Up @@ -429,7 +450,3 @@ const timeoutError = {
},
code: 'ECONNABORTED',
};

beforeEach(() => {
axiosError = timeoutError;
});
16 changes: 12 additions & 4 deletions packages/datastore/__tests__/subscription.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import Observable from 'zen-observable-ts';
let mockObservable = new Observable(() => {});
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
const mockGraphQL = jest.fn(() => mockObservable);

import Amplify from 'aws-amplify';
import { GRAPHQL_AUTH_MODE } from '@aws-amplify/api';
import { CONTROL_MSG as PUBSUB_CONTROL_MSG } from '@aws-amplify/pubsub';
import Observable from 'zen-observable-ts';
import {
SubscriptionProcessor,
USER_CREDENTIALS,
Expand All @@ -16,14 +19,13 @@ import {
InternalSchema,
PersistentModelConstructor,
} from '../src/types';

let mockObservable = new Observable(() => {});
import { USER_AGENT_SUFFIX_DATASTORE } from '../lib-esm/util';
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved

// mock graphql to return a mockable observable
jest.mock('@aws-amplify/api', () => {
return {
...jest.requireActual('@aws-amplify/api'),
graphql: jest.fn(() => mockObservable),
graphql: mockGraphQL,
};
});

Expand Down Expand Up @@ -650,6 +652,12 @@ describe('error handler', () => {
)
)
);

expect(mockGraphQL).toHaveBeenCalledWith(
expect.objectContaining({
userAgentSuffix: USER_AGENT_SUFFIX_DATASTORE,
})
);
});

done();
Expand Down
48 changes: 43 additions & 5 deletions packages/datastore/__tests__/sync.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// These tests should be replaced once SyncEngine.partialDataFeatureFlagEnabled is removed.

let mockGraphQl;
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
import { GRAPHQL_AUTH_MODE } from '@aws-amplify/api-graphql';
import { defaultAuthStrategy } from '../src/authModeStrategies';

Expand Down Expand Up @@ -281,6 +281,41 @@ describe('Sync', () => {
}
});
});

it('should send user agent suffix', async () => {
window.sessionStorage.setItem('datastorePartialData', 'true');
const resolveResponse = {
data: {
syncPosts: {
items: [
{
id: '1',
title: 'Item 1',
},
{
id: '2',
title: 'Item 2',
},
],
},
},
};

const SyncProcessor = jitteredRetrySyncProcessorSetup({
resolveResponse,
});

await SyncProcessor.jitteredRetry({
query: defaultQuery,
variables: defaultVariables,
opName: defaultOpName,
modelDefinition: defaultModelDefinition,
});

expect(mockGraphQl).toHaveBeenCalledWith(
expect.objectContaining({ userAgentSuffix: '/DataStore' })
erinleigh90 marked this conversation as resolved.
Show resolved Hide resolved
);
});
});

describe('error handler', () => {
Expand Down Expand Up @@ -409,16 +444,19 @@ function jitteredRetrySyncProcessorSetup({
coreMocks?: object;
errorHandler?: () => null;
}) {
jest.mock('@aws-amplify/api', () => ({
...jest.requireActual('@aws-amplify/api'),
graphql: () =>
mockGraphQl = jest.fn(
() =>
new Promise((res, rej) => {
if (resolveResponse) {
res(resolveResponse);
} else if (rejectResponse) {
rej(rejectResponse);
}
}),
})
);
jest.mock('@aws-amplify/api', () => ({
...jest.requireActual('@aws-amplify/api'),
graphql: mockGraphQl,
}));

jest.mock('@aws-amplify/core', () => ({
Expand Down
Loading