asyncapi / jasyncapi

/jay-sync-api/ is a Java code-first tool for AsyncAPI specification
Apache License 2.0
67 stars 23 forks source link

fix: additionalProperties must be a Boolean #131

Closed guillaumelamirand closed 1 year ago

guillaumelamirand commented 1 year ago

Description This PR fixes the type of additionalProperties which could be a Boolean or Schema.

Related issue(s)

jonaslagoni commented 1 year ago

additionalProperties can be both :)

guillaumelamirand commented 1 year ago

Indeed... I have read too fast this: https://json-schema.org/understanding-json-schema/reference/object.html?highlight=additionalproperties#additional-properties

Pakisan commented 1 year ago

@jonaslagoni hi!

Glad to see your in this repo)

I thought we still use Draft 07 specification. Am I wrong?

The Schema Object allows the definition of input and output data types. These types can be objects, but also primitives and arrays. This object is a superset of the JSON Schema Specification Draft 07. The empty schema (which allows any instance to validate) MAY be represented by the boolean value true and a schema which allows no instance to validate MAY be represented by the boolean value false.

Further information about the properties can be found in JSON Schema Core and JSON Schema Validation. Unless stated otherwise, the property definitions follow the JSON Schema specification as referenced here.

https://www.asyncapi.com/docs/reference/specification/v2.6.0#schemaObject

Based on these specification - https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-validation-01#page-14 additionalProperties must be null or valid JsonSchema object

jonaslagoni commented 1 year ago

I thought we still use Draft 7 schema. Am I wrong?

We use the AsyncAPI Schema object as default, but yea superset of JSON Schema draft 7 🙂

But a valid JSON Schema is both the object and a boolean: https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-4.3.1

Pakisan commented 1 year ago

@guillaumelamirand hi!

Please switch to develop branch. There is a whole bunch of changes:

guillaumelamirand commented 1 year ago

@jonaslagoni hi!

Glad to see your in this repo)

I thought we still use Draft 07 specification. Am I wrong?

The Schema Object allows the definition of input and output data types. These types can be objects, but also primitives and arrays. This object is a superset of the JSON Schema Specification Draft 07. The empty schema (which allows any instance to validate) MAY be represented by the boolean value true and a schema which allows no instance to validate MAY be represented by the boolean value false. Further information about the properties can be found in JSON Schema Core and JSON Schema Validation. Unless stated otherwise, the property definitions follow the JSON Schema specification as referenced here.

https://www.asyncapi.com/docs/reference/specification/v2.6.0#schemaObject

Based on these specification - https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-validation-01#page-14 additionalProperties must be null or valid JsonSchema object

I thought the same but jackson complains when the additionalProperties is false. Should we use a custom deserializer in order to set it to null in case of a boolean value ?

guillaumelamirand commented 1 year ago

@guillaumelamirand hi!

Please switch to develop branch. There is a whole bunch of changes:

  • new packages structure
  • fixes from other folks
  • 2.6.0 version of AsyncAPI

I will update the PR to manage boolean or schema.

BTW The issue regarding ObjectMapper being duplicated is present in the 2.6 code. We should follow the update done in my previous PR. I will update 2.6 package accordingly.

guillaumelamirand commented 1 year ago

PR has been updated to handle both Schema or Boolean and also by removing the use of new ObjectMapper.

BTW the new extensionFields added here and there, make invalid the asyncapi generating from the model. See bellow :

image

Pakisan commented 1 year ago

BTW The issue regarding ObjectMapper being duplicated is present in the 2.6 code. We should follow the update done in my previous PR. I will update 2.6 package accordingly.

Thanks! I Planned to update today, but you already did it)

Pakisan commented 1 year ago

@guillaumelamirand

Please remove next dirs:

Pakisan commented 1 year ago

PR has been updated to handle both Schema or Boolean and also by removing the use of new ObjectMapper.

BTW the new extensionFields added here and there, make invalid the asyncapi generating from the model. See bellow :

image

Will implement next logic:

guillaumelamirand commented 1 year ago

@guillaumelamirand

Please remove next dirs:

  • .idea/sonarlint/issuestore/index.pb
  • asyncapi-plugin/*

Done, I clean up the code.

Pakisan commented 1 year ago

https://github.com/asyncapi/jasyncapi/issues/126