Skip to content

Commit

Permalink
Only require additionalProperties when required fields are present.
Browse files Browse the repository at this point in the history
Signed-off-by: dblock <[email protected]>
  • Loading branch information
dblock committed Jun 18, 2024
1 parent 748a505 commit 132e580
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 14 deletions.
8 changes: 4 additions & 4 deletions tools/src/tester/MergedOpenApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ export default class MergedOpenApiSpec {
}

private inject_additional_properties(ctx: SpecificationContext, spec: OpenAPIV3.Document): void {
const visitor = new SchemaVisitor((_ctx, schema) => {
if ('properties' in schema && schema.additionalProperties === undefined) {
const visitor = new SchemaVisitor((_ctx, schema: any) => {
if (('required' in schema) && ('properties' in schema) && !('additionalProperties' in schema)) {
// causes any undeclared field in the response to produce an error
(schema as any).additionalProperties = {
schema.additionalProperties = {
not: true,
errorMessage: "property is not defined in the spec"
}
}
});
})

visitor.visit_specification(ctx, spec)
}
Expand Down
31 changes: 21 additions & 10 deletions tools/tests/tester/MergedOpenApiSpec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,27 @@
import { Logger } from 'Logger'
import MergedOpenApiSpec from "tester/MergedOpenApiSpec"

const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger())

test('adds additionalProperties when not present', () => {
describe('additionalProperties', () => {
const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger())
const responses: any = spec.spec().components?.responses
const schema = responses['info@200'].content['application/json'].schema
expect(schema.additionalProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' })
})

test('does not add additionalProperties when present', () => {
const responses: any = spec.spec().components?.responses
const schema = responses['info@201'].content['application/json'].schema
expect(schema.additionalProperties).toEqual(true)
test('is added with required fields', () => {
const schema = responses['info@200'].content['application/json'].schema
expect(schema.additionalProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' })
})

test('is not added when true', () => {
const schema = responses['info@201'].content['application/json'].schema
expect(schema.additionalProperties).toEqual(true)
})

test('is not added when object', () => {
const schema = responses['info@404'].content['application/json'].schema
expect(schema.additionalProperties).toEqual({ type: 'object' })
})

test('is not added unless required is present', () => {
const schema = responses['info@500'].content['application/json'].schema
expect(schema.additionalProperties).toBeUndefined()
})
})
26 changes: 26 additions & 0 deletions tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ paths:
$ref: '#/components/responses/info@200'
'201':
$ref: '#/components/responses/info@201'
'404':
$ref: '#/components/responses/info@404'
'500':
$ref: '#/components/responses/info@500'
components:
responses:
info@200:
Expand All @@ -37,3 +41,25 @@ components:
required:
- tagline
additionalProperties: true
info@404:
description: ''
content:
application/json:
schema:
type: object
properties:
tagline:
type: string
required:
- tagline
additionalProperties:
type: object
info@500:
description: ''
content:
application/json:
schema:
type: object
properties:
tagline:
type: string

0 comments on commit 132e580

Please sign in to comment.