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

Jump to bottom

fix(service-spec): the spec treats any nested anyOf or oneOf as a union type #1269

Merged
merged 24 commits into from
Sep 12, 2024
Merged

fix(service-spec): the spec treats any nested anyOf or oneOf as a union type #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
05ca207
added changes from colin's PR
comcalvi Aug 26, 2024
c561b66
chore: self mutation
invalid-email-address Aug 27, 2024
b9f7f99
tests
comcalvi Aug 27, 2024
72a22c1
t pushMerge branch 'comcalvi/union-errors' of github.com:cdklabs/awsc…
comcalvi Aug 27, 2024
77689d2
chore: self mutation
invalid-email-address Aug 27, 2024
a763a2b
remove the changes...see if jsii-diff still picks up braeking changes
comcalvi Aug 27, 2024
e4b630b
Merge t push
comcalvi Aug 27, 2024
98703a9
weird, but it fixes datasourceconfiguation and multiplexpgoram
comcalvi Aug 28, 2024
f0c1740
it thickens yet...
comcalvi Aug 28, 2024
f564baf
Merge branch 'main' of github.com:cdklabs/awscdk-service-spec into co…
comcalvi Aug 28, 2024
3832627
testing adjusting the required(oneOf) logic, it doesn't look right...
comcalvi Aug 29, 2024
6ba969f
that made it way worse
comcalvi Aug 30, 2024
ac0bcb4
works
comcalvi Aug 30, 2024
d27048e
restore it
comcalvi Aug 30, 2024
e186e7f
one working test
comcalvi Sep 5, 2024
ae616f7
reset point
comcalvi Sep 9, 2024
88ccd08
remove
comcalvi Sep 9, 2024
4ddaec0
comments
comcalvi Sep 9, 2024
555f27d
updates
comcalvi Sep 9, 2024
e4d18db
updates
comcalvi Sep 9, 2024
d3895b5
diff
comcalvi Sep 9, 2024
adfc156
projen stuff
comcalvi Sep 9, 2024
e80ff64
git pulll
comcalvi Sep 9, 2024
efa5655
more projen stuff
comcalvi Sep 9, 2024
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
147 changes: 49 additions & 98 deletions packages/@aws-cdk/service-spec-importers/schemas/CloudFormationRegistryResource.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,38 +29,7 @@
},
"anyOf": {
"items": {
"additionalProperties": false,
"description": "Type combinator fields we commonly see at the resource level\n\n(They can be nested)",
"properties": {
"allOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"anyOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"oneOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"required": {
"items": {
"type": "string"
},
"type": "array"
},
"type": {
"type": "string"
}
},
"type": "object"
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
Expand Down Expand Up @@ -110,38 +79,7 @@
"oneOf": {
"description": "Technically this should have been canonicalized out, but that will complicate a lot of the rest of the code.\n\nSo we allow embedded oneOfs/anyOfs, only at the top level.",
"items": {
"additionalProperties": false,
"description": "Type combinator fields we commonly see at the resource level\n\n(They can be nested)",
"properties": {
"allOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"anyOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"oneOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"required": {
"items": {
"type": "string"
},
"type": "array"
},
"type": {
"type": "string"
}
},
"type": "object"
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
Expand Down Expand Up @@ -216,6 +154,40 @@
],
"type": "object"
},
"CommonTypeCombinatorFields": {
"additionalProperties": false,
"description": "Type combinator fields we commonly see at the resource level\n\n(They can be nested)",
"properties": {
"allOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"anyOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"oneOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"required": {
"items": {
"type": "string"
},
"type": "array"
},
"type": {
"type": "string"
}
},
"type": "object"
},
"Handler": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -310,40 +282,6 @@
},
"type": "object"
},
"interface-403906428-3451-3797-403906428-0-57631380716174": {
"additionalProperties": false,
"description": "Type combinator fields we commonly see at the resource level\n\n(They can be nested)",
"properties": {
"allOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"anyOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"oneOf": {
"items": {
"$ref": "#/definitions/interface-403906428-3451-3797-403906428-0-57631380716174"
},
"type": "array"
},
"required": {
"items": {
"type": "string"
},
"type": "array"
},
"type": {
"type": "string"
}
},
"type": "object"
},
"jsonschema.AllOf<Schema>": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -646,6 +584,19 @@
"description": "FIXME: these are weird but oh hey?",
"type": "number"
},
"oneOf": {
"items": {
"anyOf": [
{
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
{
"$ref": "#/definitions/jsonschema.RecordLikeObject"
}
]
},
"type": "array"
},
"properties": {
"$ref": "#/definitions/jsonschema.ObjectProperties"
},
Expand Down
60 changes: 60 additions & 0 deletions packages/@aws-cdk/service-spec-importers/schemas/SamTemplateSchema.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,40 @@
"$ref": "#/definitions/SamTemplateSchema",
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"CommonTypeCombinatorFields": {
"additionalProperties": false,
"description": "Type combinator fields we commonly see at the resource level\n\n(They can be nested)",
"properties": {
"allOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"anyOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"oneOf": {
"items": {
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
"type": "array"
},
"required": {
"items": {
"type": "string"
},
"type": "array"
},
"type": {
"type": "string"
}
},
"type": "object"
},
"SamTemplateSchema": {
"$ref": "#/definitions/jsonschema.SchemaFile",
"description": "The SAM schema is just JSON schema with no frills"
Expand Down Expand Up @@ -308,6 +342,19 @@
"description": "FIXME: these are weird but oh hey?",
"type": "number"
},
"oneOf": {
"items": {
"anyOf": [
{
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
{
"$ref": "#/definitions/jsonschema.RecordLikeObject"
}
]
},
"type": "array"
},
"properties": {
"$ref": "#/definitions/jsonschema.ObjectProperties"
},
Expand Down Expand Up @@ -453,6 +500,19 @@
"description": "FIXME: these are weird but oh hey?",
"type": "number"
},
"oneOf": {
"items": {
"anyOf": [
{
"$ref": "#/definitions/CommonTypeCombinatorFields"
},
{
"$ref": "#/definitions/jsonschema.RecordLikeObject"
}
]
},
"type": "array"
},
"properties": {
"$ref": "#/definitions/jsonschema.ObjectProperties"
},
Expand Down
2 changes: 1 addition & 1 deletion ...@aws-cdk/service-spec-importers/src/types/registry-schema/CloudFormationRegistrySchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export interface CloudFormationRegistryResource extends ImplicitJsonSchemaRecord
*
* (They can be nested)
*/
interface CommonTypeCombinatorFields {
export interface CommonTypeCombinatorFields {
readonly required?: string[];
readonly type?: string;
readonly oneOf?: CommonTypeCombinatorFields[];
Expand Down
29 changes: 26 additions & 3 deletions packages/@aws-cdk/service-spec-importers/src/types/registry-schema/JsonSchema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { CommonTypeCombinatorFields } from './CloudFormationRegistrySchema';

export namespace jsonschema {
export type Schema = SingletonSchema | OneOf<Schema> | AnyOf<Schema> | AllOf<Schema>;

Expand All @@ -15,14 +17,18 @@ export namespace jsonschema {

export type EmptyObject = Record<string, never>;

export function isAnyType(x: Schema): x is AnyType {
export function isAnyType(x: Schema | CommonTypeCombinatorFields): x is AnyType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is CommonTypeCombinatorFields needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the entries in a oneOf list can be a CommonTypeCombinatorFields, and we check those entries with isAnyType below.

return x === true || isEmptyObject(x);
}

function isEmptyObject(x: any) {
return x && typeof x === 'object' && !Array.isArray(x) && Object.keys(x).length === 0;
}

function isTypeDefined(x: any) {
return 'type' in x;
}

export interface Annotatable {
readonly $comment?: string;
readonly description?: string;
Expand Down Expand Up @@ -78,6 +84,9 @@ export namespace jsonschema {
readonly anyOf: Array<S>;
}

/**
* Determines whether or not the provided schema represents an `anyOf` type operator.
*/
export function isAnyOf(x: Schema): x is AnyOf<any> {
return !isAnyType(x) && 'anyOf' in x;
}
Expand All @@ -96,14 +105,27 @@ export namespace jsonschema {
}
}

/**
* Determines whether or not the provided schema represents a `oneOf` type operator.
*/
export function isOneOf(x: Schema): x is OneOf<any> {
return !isAnyType(x) && 'oneOf' in x;
if ('oneOf' in (x as any) && !isAnyType(x)) {
for (const elem of (x as RecordLikeObject).oneOf!) {
if (!isAnyType(elem) && (isTypeDefined(elem) || isReference(elem))) {
return true;
}
}
}
return false;
}

export interface AllOf<S> extends Annotatable {
readonly allOf: Array<S>;
}

/**
* Determines whether or not the provided schema represents an `allOf` type operator.
*/
export function isAllOf(x: Schema): x is AllOf<any> {
return !isAnyType(x) && 'allOf' in x;
}
Expand Down Expand Up @@ -138,6 +160,7 @@ export namespace jsonschema {
readonly type: 'object';
readonly properties: ObjectProperties;
readonly required?: string[];
readonly oneOf?: (CommonTypeCombinatorFields | RecordLikeObject)[];
/**
* FIXME: should be required but some service teams have omitted it.
*/
Expand Down Expand Up @@ -349,7 +372,7 @@ export namespace jsonschema {
*/
export type Resolver = ReturnType<typeof makeResolver>;

export function isReference(x: Schema): x is Reference {
export function isReference(x: Schema | CommonTypeCombinatorFields): x is Reference {
if (x === undefined) {
debugger;
}
Expand Down
Loading
Loading