Skip to content

Commit

Permalink
fix(service-spec): the spec treats any nested anyOf or oneOf as a…
Browse files Browse the repository at this point in the history 
… union type (#1269)

Improves handling of `oneOf` clauses. `oneOf` is able to define entire
types:
```
properties: {
  foo: {
    oneOf: [
      {  type: 'string' },
      { type: 'number' },
  ],
 }
}
```

They can also contain references:

```
properties: {
  foo: {
    oneOf: [
      { Foo: { Ref: 'something' } },
      { Bar: { Ref: 'something' } },
  ],
 }
}
```

Previously, we were not processing these correctly.

---------

Signed-off-by: github-actions <github-actions@github.com>
Co-authored-by: github-actions <github-actions@github.com>
  • Loading branch information
@comcalvi
comcalvi and github-actions authored Sep 12, 2024
1 parent 222b4cc commit e9f585a
Show file tree
Hide file tree
Showing 6 changed files with 331 additions and 126 deletions.
147 changes: 49 additions & 98 deletions packages/@aws-cdk/service-spec-importers/schemas/CloudFormationRegistryResource.schema.json
View file
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
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
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
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 {
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

0 comments on commit e9f585a

Please sign in to comment.