Usages of allOf within message payload could be flattened

[jamescrowley]

Describe the bug

I'm not sure if this is a bug or not, but for me, allOf is rendered in an unexpected way when used in the message payload. Can you confirm if this is deliberate or not? If not, I'll look to fix.

How to Reproduce

asyncapi: 2.0.0
info:
  title: MQTT API
  version: '2.0.0'

defaultContentType: application/json

channels:
  /v2/abc/data/pack_data:
    description: Publish logging data from devices
    publish:
      operationId: publishPackData
      message:
        name: pack_data
        payload:
          $ref: "#/components/schemas/pack_data"

components:
  schemas:
    pack_data:
      allOf:
      - $ref: "#/components/schemas/common_payload"
      - type: object
        properties:
          cycle_id:
            type: string
    common_payload:
      type: object
      properties:
        device_id:
            type: string
        site_id:
            type: string

Expected behavior

I'd expect the use of 'allOf' to be transparent in the documentation (ie as if everything had just been defined explicitly in-line).

ie

[github-actions]

Welcome to AsyncAPI. Thanks a lot for reporting your first issue.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

[derberg]

Makes sense, this is the part of the template responsible for schema -> https://github.com/asyncapi/html-template/blob/master/partials/schema-prop.html

[jamescrowley]

As allOf requires the message to validate the schema of each (as opposed to extend them), I'm starting to think that 'flattening' these may only make sense when 'additionalProperties' is set to true or is undefined for every element? @derberg thoughts?

[InTheCloudDan]

Just adding a 👍 here from conversation on Slack. I'm looking for this feature also. My current usage is pretty basic though and I haven't done any interacting with allOf and additionalProperties so not sure on the best approach to handling them.

[derberg]

@jamescrowley yeap, looking at https://json-schema.org/understanding-json-schema/reference/combining.html#id5 looks like there is no other option but flatten only if "additionalProperties": false is not present. Pretty complicated case as maybe there are other combinations too.

I checked quickly with OpenAPI and the swagger UI is flattening always

[InTheCloudDan]

Any update on this one? It would be useful feature to me.

[derberg]

@InTheCloudDan have a look on the thread in this PR asyncapi/html-template#16 from @jamescrowley. We are pretty much stuck there, appriciate your input into discussion.

[WaleedAshraf]

I also have this problem. But for now, we are okay with two schemas in the view. In our case, first schema is always the same for all the messages, So, it's better hidden.
Mostly we are only interested in seeing the second part.

I think best solution will be if on top somewhere, we can provide checkboxes

• merge allOf schemas
• expand all schemas
• collapse all schemas

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[FabienSailliet]

Has there been any progress on this issue? Would be really useful for me too

[derberg]

@FabienSailliet Hi, as you can see there was a PR for that asyncapi/generator#115 but because of concerns mentioned in this issue and the PR, we did not come with an final conclusion how this shoudl be solved. What is your opinion on the topic?

[FabienSailliet]

It is to be noted that in the JSON Schema specification, they are stating that allOf is not to be used to extend schemas:

While it might be surprising, allOf can not be used to “extend” a schema to add more details to it in the sense of object-oriented inheritance.

They later say:

There are some proposals to address this in the next version of the JSON schema specification.

But they do not give more details about it, so maybe it could be better to wait for something on their side, I did not check if there were some discussions about it.

In my opinion, I think that if a property is described in multiple objects to be merged, the resulting merged object should contain the most specific given definition, and we should be allowed to do this only if the definitions are compatible (we cannot merge a property that should be an array on one side and an integer on another). By doing it this way, we can have objects allowing additional properties or not, as long as they are not incompatible.

[WaleedAshraf]

"unevaluatedProperties" in draft-2019 solves the problem of using "additionalProperties" with "allOf".

Example,

{
  "$schema": "http://json-schema.org/draft/2019-09/schema",
  "allOf": [
    {
      "type": "object",
      "properties": {
        "veggieName": {
          "type": "string",
        }
      }
    },
    {
      "type": "object",
      "properties": {
        "data": {
          "type": "object",
          "additionalProperties": false,
          "properties": {
            "veggieLike": {
              "type": "boolean",
            }
          }
        }
      }
    }
  ],
  "unevaluatedProperties": false,
}

Adding any additional prop to any of allOf schema will fail the validation.
Test it here: https://www.jsonschemavalidator.net/s/MxFv67dv

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[elnipa]

Any update on this one? A solution would be highly appreciated to have the same behavior as in Swagger/Redoc for our openAPI rest API.

[derberg]

@elnipa could you show some examples? I'm not very familiar with how these tools handle it and what you refer to exactly. Thanks 🙇🏼

[elnipa]

@elnipa could you show some examples? I'm not very familiar with how these tools handle it and what you refer to exactly. Thanks 🙇🏼

Sure, in Redoc the attributes within allOf are flattened and only oneOf attributes are splitted.
Compare for the following example:

  exampleChannel:
    description: example channel
    publish:
      message:
        payload:
          allOf:
            - type: object
              properties:
                foo:
                  type: string
            - type: object
              properties:
                bar:
                  type: string
            - oneOf:
                - type: object
                  properties:
                    option1:
                      type: boolean
                - type: object
                  properties:
                    option2:
                      type: integer

asyncAPI

openAPI (Redoc)

[derberg]

@elnipa thanks a lot, it helps!

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

[derberg]

I'm supporting merging only if this is optional as I stated above because there might be people that designed their schemas in a way that it will always be valid when we merge in one.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[derberg]

I'm wondering if we should not have it solved on parser side, have a helper that allows to return merged schema, so in react template we just make it optional by props.

anyway moving to react as it should be solved there

[github-actions]

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[magicmatatjahu]

Still valid.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[fmvilas]

Still relevant.

[ChrisEdwards]

Why does this feature in AsyncAPI behave differently than it does in OpenAPI? Those two specs should behave the same. Users expect this, and it only causes confusion when they work differently. From the user's perspective, the allOf feature in AsyncAPI is inconsistent with OpenAPI, and thus is perceived as broken.

This also causes a lot of copy-paste for common event properties across all AsyncAPI specs.

[fmvilas]

They don't work differently. It's the same but the way this component handles it is different from the way other existing components for OpenAPI handle it. In all fairness, the way it's shown here is the correct way, even though I don't like it. This is not OpenAPI or AsyncAPI stuff, it's JSON Schema, and this is how JSON Schema defines it. JSON Schema is not a model or type definition language. It's a set of constraints and validations and, therefore, allOf can't be understood as merging all the objects. Again, I don't like it, but then we should probably escalate this to JSON Schema itself instead of this or other components.

@jonaslagoni started this initiative to clarify this on the JSON Schema side: https://github.com/json-schema-org/vocab-idl. It didn't have much traction so far so if you're really impacted by this issue it would be awesome if you could contribute there. If we get an IDL vocabulary, we'll for sure change AsyncAPI to use it and, as a result, this component would be able to merge all the allOf items.

[fmvilas]

In the meantime, we could add a flag to this repo to let the user decide how they want to treat allOf. Seems like a plausible medium-term solution.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[fmvilas]

Dear bot, close it not, por favor.

[henhal]

Any progress on this?
I just started creating specs for all my APIs today with AsyncAPI, and my initial excitement unfortunately faded quickly when I realized that the resulting HTML was pretty much unreadable. I structure my schemas heavily, relying on inheritance of base schemas etc, and to have documentation that does not merge allOfs in such a way that e.g. Redoc does it unfortunately makes it unusable to me.

I see that the PR has been open for several years. Is there a plan forward? Is there any debate of whether HTML should merge allOfs into the most restrictive combined schema? 🤔 For example, the highly used https://www.npmjs.com/package/json-schema-merge-allof handles the logic for allOf merging.

const mergeAllof = require('json-schema-merge-allof');

mergeAllOf({
  allOf: [
    {type: 'object', properties: {foo: {type: 'string'}}}, 
    {type: 'object', properties: {bar: {type: 'string'}, foo: {enum: ['a', 'b']}}}
  ]
});

Output:

{
  "type":"object",
  "properties":{
    "foo":{"type":"string","enum":["a","b"]},
    "bar":{"type":"string"}
  }
}

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[fmvilas]

@henhal we're looking for contributors to implement it. If you're still interested, we would welcome a PR 🙏

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️