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.

Sign up for GitHub

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: allow customtypes ref in arguments/main #497

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

feat: allow customtypes ref in arguments/main #497

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e7f97fa
feat(custom-operations): allow CustomType and RefType in arguments
vaisshnavi7 Nov 8, 2024
0e0561e
Refactor type tests for CustomType and RefType
vaisshnavi7 Nov 8, 2024
1a54cde
Merge branch 'main' into feat/allow-customtype-ref-in-arguments
vaisshnavi7 Nov 13, 2024
ecf2ec2
Merge branch 'main' into feat/allow-customtype-ref-in-arguments
vaisshnavi7 Nov 21, 2024
a0872b9
feat: add support for custom, nested, and ref type arguments (#402)
vaisshnavi7 Nov 26, 2024
2f7e948
Merge branch 'main' into feat/allow-customtype-ref-in-arguments
vaisshnavi7 Nov 26, 2024
45074a3
Merge branch 'main' into feat/allow-customtype-ref-in-arguments
vaisshnavi7 Dec 11, 2024
3517643
feat: Custom type and ref arg handling in SchemaProcessor with tests …
vaisshnavi7 Jan 9, 2025
2e81a9d
Merge remote-tracking branch 'origin/main' into feat/allow-customtype…
vaisshnavi7 Jan 9, 2025
c54ed9b
Fix: Generate nested CustomTypes in schema output
vaisshnavi7 Jan 15, 2025
a7dd990
Merge branch 'main' into feat/allow-customtypes-ref-in-arguments/main
vaisshnavi7 Jan 15, 2025
cc50e20
refactor: Add tests for nested custom types and mixed refs
vaisshnavi7 Jan 15, 2025
fff0b5d
Add test for RefType with nested CustomType in complex structures
vaisshnavi7 Jan 16, 2025
f4b9171
Merge branch 'main' into feat/allow-customtypes-ref-in-arguments/main
stocaaro Jan 16, 2025
949e6ee
fix: Adjust schema component ordering to minimize enum reordering
vaisshnavi7 Jan 17, 2025
cdb02d1
Revert unnecessary changes for custom types and refs
vaisshnavi7 Jan 17, 2025
2f8c281
Merge branch 'main' into feat/allow-customtypes-ref-in-arguments/main
vaisshnavi7 Jan 21, 2025
6b82830
Add unit tests for custom operations with custom types and refs in Cu…
vaisshnavi7 Jan 22, 2025
f1b8e0e
Refactor schemaPreprocessor Consolidate schema components into single…
vaisshnavi7 Jan 23, 2025
acaa7d4
Improve tests for CustomType and RefType arguments in custom operations
vaisshnavi7 Jan 23, 2025
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
5 changes: 5 additions & 0 deletions .changeset/polite-crabs-rush.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@aws-amplify/data-schema": minor
---

Allow CustomType and RefType in arguments for custom operations
112 changes: 106 additions & 6 deletions packages/data-schema/__tests__/CustomOperations.test-d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
import { configure } from '../src/ModelSchema';
import { Nullable } from '../src/ModelField';
import { defineFunctionStub } from './utils';
import type { CustomOperation } from '../src/CustomOperation';

describe('custom operations return types', () => {
describe('when .ref() a basic custom type', () => {
Expand Down Expand Up @@ -84,15 +85,15 @@ describe('custom operations return types', () => {
});

it('produces async function handler types', () => {
const handler = defineFunctionStub({})
const handler = defineFunctionStub({});
const schema = a.schema({
aQuery: a
.query()
.handler(a.handler.function(handler).async())
.arguments({
input: a.string(),
content: a.string().required(),
})
}),
});

type Schema = ClientSchema<typeof schema>;
Expand All @@ -109,10 +110,7 @@ describe('custom operations return types', () => {
type ExpectedResult = {
success: boolean;
} | null;
type ExpectedFunctionHandler = AppSyncResolverHandler<
ActualArgs,
void
>;
type ExpectedFunctionHandler = AppSyncResolverHandler<ActualArgs, void>;
type _T1 = Expect<Equal<ActualArgs, ExpectedArgs>>;
type _T2 = Expect<Equal<ActualResult, ExpectedResult>>;
type _T3 = Expect<Equal<ActualHandler, ExpectedFunctionHandler>>;
Expand Down Expand Up @@ -858,3 +856,105 @@ describe('.for() modifier', () => {
a.subscription().for(a.ref('Model'));
});
});

describe('.arguments() modifier', () => {
// Test to verify that CustomType can be used as an argument in custom operations
it('accepts CustomType in arguments', () => {
const operation: CustomOperation<
any,
'arguments' | 'for',
'queryCustomOperation'
> = a.query().arguments({
customArg: a.customType({
field1: a.string(),
field2: a.integer(),
}),
});
});
Copy link
Member

Choose a reason for hiding this comment

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

I might expect Equal<A,B> to show up here. Could you use type equality instead of setting the type?

type A = CustomOperation<
      any,
      'arguments' | 'for',
      'queryCustomOperation'
    >;

const example = a.query().arguments({
      customArg: a.customType({
        field1: a.string(),
        field2: a.integer(),
      }),
    });
type test = Expect<Equal<A, typeof example>>;

Copy link
Contributor Author

@vaisshnavi7 vaisshnavi7 Jan 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Implemented as suggested.


// Test to verify that RefType can be used as an argument in custom operations
it('accepts RefType in arguments', () => {
const operation: CustomOperation<
any,
'arguments' | 'for',
'queryCustomOperation'
> = a.query().arguments({
refArg: a.ref('SomeType'),
});
});

it('handles deeply nested custom types', () => {
const schema = a.schema({
DeepNested: a.customType({
level1: a.customType({
level2: a.customType({
level3: a.string(),
}),
}),
}),
deepQuery: a
.query()
.arguments({
input: a.ref('DeepNested'),
})
.returns(a.string()),
});

type Schema = ClientSchema<typeof schema>;

type ExpectedArgs = {
input?: {
level1?: {
level2?: {
level3?: string | null;
} | null;
} | null;
} | null;
};

type ActualArgs = Schema['deepQuery']['args'];

type Test = Expect<Equal<ActualArgs, ExpectedArgs>>;
});

it('handles mixed custom types and refs', () => {
const schema = a.schema({
RefType: a.customType({
field: a.string(),
}),
stocaaro marked this conversation as resolved.
Show resolved Hide resolved
MixedType: a.customType({
nested: a.customType({
refField: a.ref('RefType'),
customField: a.customType({
deepField: a.integer(),
}),
}),
}),
mixedQuery: a
.query()
.arguments({
input: a.ref('MixedType'),
})
.returns(a.string()),
});

type Schema = ClientSchema<typeof schema>;

type ExpectedArgs = {
input?: {
nested?: {
refField?: {
field?: string | null;
} | null;
customField?: {
deepField?: number | null;
} | null;
} | null;
} | null;
};

type ActualArgs = Schema['mixedQuery']['args'];

type Test = Expect<Equal<ActualArgs, ExpectedArgs>>;
});
});
80 changes: 50 additions & 30 deletions packages/data-schema/__tests__/CustomOperations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,15 +907,15 @@ describe('CustomOperation transform', () => {
.query()
.arguments({})
.handler(a.handler.function(fn1).async())
.authorization((allow) => allow.authenticated())
.authorization((allow) => allow.authenticated()),
});

const { schema, lambdaFunctions } = s.transform();
expect(schema).toMatchSnapshot();
expect(lambdaFunctions).toMatchObject({
FnGetPostDetails: fn1,
});
})
});

test('defineFunction sync - async', () => {
const fn1 = defineFunctionStub({});
Expand All @@ -927,15 +927,15 @@ describe('CustomOperation transform', () => {
a.handler.function(fn1),
a.handler.function(fn1).async(),
])
.authorization((allow) => allow.authenticated())
.authorization((allow) => allow.authenticated()),
});

const { schema, lambdaFunctions } = s.transform();
expect(schema).toMatchSnapshot();
expect(lambdaFunctions).toMatchObject({
FnGetPostDetails: fn1,
});
})
});

test('defineFunction sync - async with returns generates type errors', () => {
const fn1 = defineFunctionStub({});
Expand All @@ -949,9 +949,9 @@ describe('CustomOperation transform', () => {
])
.authorization((allow) => allow.authenticated())
// @ts-expect-error
.returns({ })
.returns({}),
});
})
});

test('defineFunction async - async', () => {
const fn1 = defineFunctionStub({});
Expand All @@ -965,7 +965,7 @@ describe('CustomOperation transform', () => {
a.handler.function(fn1).async(),
a.handler.function(fn2).async(),
])
.authorization((allow) => allow.authenticated())
.authorization((allow) => allow.authenticated()),
});

const { schema, lambdaFunctions } = s.transform();
Expand All @@ -974,7 +974,7 @@ describe('CustomOperation transform', () => {
FnGetPostDetails: fn1,
FnGetPostDetails2: fn2,
});
})
});

test('defineFunction async - sync', () => {
const fn1 = defineFunctionStub({});
Expand All @@ -987,12 +987,12 @@ describe('CustomOperation transform', () => {
a.handler.function(fn1),
])
.returns(a.customType({}))
.authorization((allow) => allow.authenticated())
.authorization((allow) => allow.authenticated()),
});

const { schema, lambdaFunctions } = s.transform();
expect(schema).toMatchSnapshot();
})
});

test('pipeline / mix', () => {
const fn1 = defineFunctionStub({});
Expand Down Expand Up @@ -1341,15 +1341,14 @@ describe('custom operations + custom type auth inheritance', () => {

test('implicit custom type inherits auth rules from referencing op', () => {
const s = a.schema({
MyQueryReturnType: a.customType({
fieldA: a.string(),
fieldB: a.integer(),
}),
myQuery: a
.query()
.handler(a.handler.function('myFn'))
.returns(
a.customType({
fieldA: a.string(),
fieldB: a.integer(),
}),
)
.returns(a.ref('MyQueryReturnType'))
.authorization((allow) => allow.publicApiKey()),
});

Expand All @@ -1363,23 +1362,22 @@ describe('custom operations + custom type auth inheritance', () => {

test('nested custom types inherit auth rules from top-level referencing op', () => {
const s = a.schema({
MyQueryReturnType: a.customType({
fieldA: a.string(),
fieldB: a.integer(),
nestedCustomType: a.customType({
nestedA: a.string(),
nestedB: a.string(),
grandChild: a.customType({
grandA: a.string(),
grandB: a.string(),
}),
}),
}),
myQuery: a
.query()
.handler(a.handler.function('myFn'))
.returns(
a.customType({
fieldA: a.string(),
fieldB: a.integer(),
nestedCustomType: a.customType({
nestedA: a.string(),
nestedB: a.string(),
grandChild: a.customType({
grandA: a.string(),
grandB: a.string(),
}),
}),
}),
)
.returns(a.ref('MyQueryReturnType'))
.authorization((allow) => allow.publicApiKey()),
});

Expand All @@ -1401,6 +1399,28 @@ describe('custom operations + custom type auth inheritance', () => {
);
});

test('inline custom type inherits auth rules from referencing op', () => {
Copy link
Member

Choose a reason for hiding this comment

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

The change description doesn't talk about inherited auth rules. I'm puzzled by the how the test changes in this file related back to the broader change.

Took a closer look here expecting to find arguments snapshot tests for similar cases to the client type tests in the .t.test file. It looks like we have some related tests in the e2e define_bahavior testing. Why are they there vs here?

My thinking is that we would have unit tests covering a many cases and basic behavior testing in the e2e, but I'm open to this being misaligned with guidance from others. Interested to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have overlooked the connection between the test changes and the broader change in the description. The current tests, including those for inherited auth rules, are covering the functionality for custom types and refs in arguments. The e2e define_behavior tests complement the unit tests here by covering end-to-end scenarios.

const s = a.schema({
myQuery: a
.query()
.handler(a.handler.function('myFn'))
.returns(
a.customType({
fieldA: a.string(),
fieldB: a.integer(),
}),
)
.authorization((allow) => allow.publicApiKey()),
});

const result = s.transform().schema;

expect(result).toMatchSnapshot();
expect(result).toEqual(
expect.stringContaining('type MyQueryReturnType @aws_api_key\n{'),
);
});

test('top-level custom type with nested top-level custom types inherits combined auth rules from referencing ops', () => {
const s = a.schema({
myQuery: a
Expand Down
26 changes: 13 additions & 13 deletions packages/data-schema/__tests__/__snapshots__/ClientSchema.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,20 @@ type Query {
`;

exports[`custom operations Add entities to SQL schema add custom type, enum, and custom query to generated SQL schema 1`] = `
"type post @model(timestamps: null) @auth(rules: [{allow: private}])
{
title: String!
description: String
author: String
}

enum PostStatus {
"enum PostStatus {
draft
pending
approved
published
}

type post @model(timestamps: null) @auth(rules: [{allow: private}])
{
title: String!
description: String
author: String
}

type PostMeta @aws_cognito_user_pools
{
viewCount: Int
Expand Down Expand Up @@ -595,7 +595,11 @@ exports[`schema auth rules global public auth - multiple models 1`] = `
"functionSlots": [],
"jsFunctions": [],
"lambdaFunctions": {},
"schema": "type A @model @auth(rules: [{allow: public, provider: apiKey}])
"schema": "enum DTired {
?
}

type A @model @auth(rules: [{allow: public, provider: apiKey}])
{
field: String
}
Expand All @@ -621,10 +625,6 @@ type D @model @auth(rules: [{allow: public, provider: apiKey}])
tired: DTired
cId: ID
c: C @belongsTo(references: ["cId"])
}

enum DTired {
?
}",
}
`;
Expand Down
Loading
Loading