Skip to content

Commit

Permalink
fix: ensure MIS model name mismatch cannot cause unexpected behavior (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
iartemiev authored Nov 21, 2024
1 parent 89ad85a commit ba19bb4
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/stale-apricots-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aws-amplify/data-schema': patch
---

fix prevent mis model mismatch
52 changes: 32 additions & 20 deletions packages/data-schema/__tests__/runtime/APIClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -809,27 +809,39 @@ describe('generateGraphQLDocument()', () => {
};

test.each([
['GET', 'User', '$userId: ID!'],
['GET', userSchemaModel, '$userId: ID!'],
[
'GET',
'Product',
productSchemaModel,
'$sku: String!,$factoryId: String!,$warehouseId: String!',
],
['GET', 'person', 'getPerson'],
['LIST', 'person', 'listPeople'],
['LIST', 'person', '$filter: ModelPersonFilterInput'],
['CREATE', 'person', '$input: CreatePersonInput!'],
['UPDATE', 'person', '$input: UpdatePersonInput!'],
['DELETE', 'person', '$input: DeletePersonInput!'],
['ONCREATE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
['ONUPDATE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
['ONDELETE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
['GET', personSchemaModel, 'getPerson'],
['LIST', personSchemaModel, 'listPeople'],
['LIST', personSchemaModel, '$filter: ModelPersonFilterInput'],
['CREATE', personSchemaModel, '$input: CreatePersonInput!'],
['UPDATE', personSchemaModel, '$input: UpdatePersonInput!'],
['DELETE', personSchemaModel, '$input: DeletePersonInput!'],
[
'ONCREATE',
personSchemaModel,
'$filter: ModelSubscriptionPersonFilterInput',
],
[
'ONUPDATE',
personSchemaModel,
'$filter: ModelSubscriptionPersonFilterInput',
],
[
'ONDELETE',
personSchemaModel,
'$filter: ModelSubscriptionPersonFilterInput',
],
])(
'%s generates operation name or arguments for model %s to contain %s',
(modelOperation, modelName, expectedArgs) => {
(modelOperation, model, expectedArgs) => {
const document = generateGraphQLDocument(
mockModelDefinitions,
modelName,
model,
modelOperation as ModelOperation,
);

Expand Down Expand Up @@ -873,31 +885,31 @@ describe('generateGraphQLDocument()', () => {

test.each([
[
'EnumAsIndexPartitionKey',
modelIntroSchema.models.EnumAsIndexPartitionKey,
'$status: EnumAsIndexPartitionKeyStatus!',
getIndexMeta('EnumAsIndexPartitionKey'),
{ status: 'yes' },
],
[
'EnumAsIndexSortKey',
modelIntroSchema.models.EnumAsIndexSortKey,
'$status: ModelStringKeyConditionInput',
getIndexMeta('EnumAsIndexSortKey'),
{ title: 'test' },
],
[
'RefEnumAsIndexPartitionKey',
modelIntroSchema.models.RefEnumAsIndexPartitionKey,
'$status: EnumIndexStatus!',
getIndexMeta('RefEnumAsIndexPartitionKey'),
{ status: 'yes' },
],
[
'RefEnumAsIndexSortKey',
modelIntroSchema.models.RefEnumAsIndexSortKey,
'$status: ModelStringKeyConditionInput',
getIndexMeta('RefEnumAsIndexSortKey'),
{ title: 'test' },
],
[
'SecondaryIndexModel',
modelIntroSchema.models.SecondaryIndexModel,
'$timestamp: ModelIntKeyConditionInput',
getIndexMeta(
'SecondaryIndexModel',
Expand All @@ -907,10 +919,10 @@ describe('generateGraphQLDocument()', () => {
],
])(
'generates document for model %p containing input type: %p',
(modelName, expectedSortKeyInputString, indexMeta, args) => {
(model, expectedSortKeyInputString, indexMeta, args) => {
const document = generateGraphQLDocument(
modelIntroSchema,
modelName,
model,
modelOperation,
args,
indexMeta,
Expand Down
13 changes: 5 additions & 8 deletions packages/data-schema/src/runtime/internals/APIClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

// SPDX-License-Identifier: Apache-2.0

import {
AmplifyServer,
AssociationBelongsTo,
Expand Down Expand Up @@ -245,7 +244,7 @@ export function initializeModel(
if (record[curVal]) {
acc[curVal] = record[curVal];
}
return acc;
return acc;
},
{},
);
Expand Down Expand Up @@ -843,13 +842,11 @@ export function generateSelectionSet(

export function generateGraphQLDocument(
modelIntrospection: ModelIntrospectionSchema,
modelName: string,
modelDefinition: SchemaModel,
modelOperation: ModelOperation,
listArgs?: ListArgs | QueryArgs,
indexMeta?: IndexMeta,
): string {
const modelDefinition = modelIntrospection.models[modelName];

const {
name,
pluralName,
Expand Down Expand Up @@ -917,7 +914,7 @@ export function generateGraphQLDocument(
)?.properties?.name;

skQueryArgs = {
[compositeSkArgName]: `Model${capitalize(modelName)}${capitalize(keyName)}CompositeKeyConditionInput`,
[compositeSkArgName]: `Model${capitalize(name)}${capitalize(keyName)}CompositeKeyConditionInput`,
};
}

Expand All @@ -941,7 +938,7 @@ export function generateGraphQLDocument(

const selectionSetFields = generateSelectionSet(
modelIntrospection,
modelName,
name,
selectionSet as ListArgs['selectionSet'],
);

Expand Down Expand Up @@ -1011,7 +1008,7 @@ export function generateGraphQLDocument(
const compositeSkArgName = resolvedSkName(sortKeyFieldNames);

return {
[compositeSkArgName]: `Model${capitalize(modelName)}PrimaryCompositeKeyConditionInput`,
[compositeSkArgName]: `Model${capitalize(name)}PrimaryCompositeKeyConditionInput`,
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function _get(

const query = generateGraphQLDocument(
modelIntrospection,
name,
model,
operation,
options,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function _indexQuery(

const query = generateGraphQLDocument(
modelIntrospection,
name,
model,
'INDEX_QUERY',
args,
indexMeta,
Expand Down Expand Up @@ -189,7 +189,12 @@ function _indexQuery(
const { data, errors } = error;

// `data` is not `null`, and is not an empty object:
if (data !== undefined && data !== null && Object.keys(data).length !== 0 && errors) {
if (
data !== undefined &&
data !== null &&
Object.keys(data).length !== 0 &&
errors
) {
const [key] = Object.keys(data);

if (data[key]?.items) {
Expand Down
4 changes: 2 additions & 2 deletions packages/data-schema/src/runtime/internals/operations/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function listFactory(
getInternals,
args,
undefined,
customUserAgentDetails
customUserAgentDetails,
);
};

Expand All @@ -81,7 +81,7 @@ function _list(

const query = generateGraphQLDocument(
modelIntrospection,
name,
model,
'LIST',
args,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function subscriptionFactory(
const subscription = (args?: Record<string, any>) => {
const query = generateGraphQLDocument(
modelIntrospection,
name,
model,
operation,
args,
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { a, ClientSchema } from '@aws-amplify/data-schema';
import { Amplify } from 'aws-amplify';
import {
buildAmplifyConfig,
mockedGenerateClient,
optionsAndHeaders,
expectGraphqlMatches,
} from '../../utils';

const sampleTodo = {
__typename: 'Todo',
id: 'some-id',
content: 'some content',
description: 'something something',
owner: 'some-body',
done: false,
updatedAt: '2024-03-01T19:05:44.536Z',
createdAt: '2024-03-01T18:05:44.536Z',
};

function manipulateMis(config: Awaited<ReturnType<typeof buildAmplifyConfig>>) {
const todoDef = { ...config.modelIntrospection.models['Todo'] };
const notTodoDef = { ...config.modelIntrospection.models['NotTodo'] };

// swap definitions
config.modelIntrospection.models['Todo'] = notTodoDef;
config.modelIntrospection.models['NotTodo'] = todoDef;

return config;
}

describe('Manipulated MIS cannot be used to call the wrong model', () => {
const schema = a
.schema({
Todo: a.model({
content: a.string(),
}),
NotTodo: a.model({
content: a.string(),
}),
})
.authorization((allow) => [allow.publicApiKey()]);
type Schema = ClientSchema<typeof schema>;

afterEach(() => {
jest.clearAllMocks();
});

test('create performs operation against intended model', async () => {
// #region mocking
const { spy, generateClient } = mockedGenerateClient([
{
data: {
createTodo: sampleTodo,
},
},
]);

// simulated amplifyconfiguration.json
const original = await buildAmplifyConfig(schema);
const config = manipulateMis(original);
// #endregion mocking

// App.tsx
Amplify.configure(config);
const client = generateClient<Schema>();
const { errors, data: newTodo } = await client.models.Todo.create({
content: 'My new todo',
});
// #endregion

// #region assertions
expectGraphqlMatches(
optionsAndHeaders(spy)[0][0].query,
`mutation($input: CreateTodoInput!) {
createTodo(input: $input) { id content createdAt updatedAt }
}`,
);
expect(errors).toBeUndefined();
expect(newTodo).toEqual(sampleTodo);
// #endregion assertions
});

test('update performs operation against intended model', async () => {
// #region mocking
const { spy, generateClient } = mockedGenerateClient([
{
data: {
updateTodo: sampleTodo,
},
},
]);

// simulated amplifyconfiguration.json
const original = await buildAmplifyConfig(schema);
const config = manipulateMis(original);
// #endregion mocking

Amplify.configure(config);
const client = generateClient<Schema>();
const { data: updatedTodo, errors } = await client.models.Todo.update({
id: 'some_id',
content: 'Updated content',
});
// #endregion

// #region assertions
expectGraphqlMatches(
optionsAndHeaders(spy)[0][0].query,
`mutation($input: UpdateTodoInput!) {
updateTodo(input: $input) { id content createdAt updatedAt }
}`,
);
expect(errors).toBeUndefined();
expect(updatedTodo).toEqual(sampleTodo);
// #endregion assertions
});

test('delete performs operation against intended model', async () => {
// #region mocking
const { spy, generateClient } = mockedGenerateClient([
{
data: {
deleteTodo: sampleTodo,
},
},
]);

// simulated amplifyconfiguration.json
const original = await buildAmplifyConfig(schema);
const config = manipulateMis(original);
// #endregion mocking

// #region
Amplify.configure(config);
const client = generateClient<Schema>();
const toBeDeletedTodo = {
id: '123123213',
};
const { data: deletedTodo, errors } =
await client.models.Todo.delete(toBeDeletedTodo);
// #endregion

// #region assertions
expectGraphqlMatches(
optionsAndHeaders(spy)[0][0].query,
`mutation($input: DeleteTodoInput!) {
deleteTodo(input: $input) { id content createdAt updatedAt }
}`,
);
expect(errors).toBeUndefined();
expect(deletedTodo).toEqual(sampleTodo);
// #endregion assertions
});
});

0 comments on commit ba19bb4

Please sign in to comment.