Add an *effective* flavor for AsyncAPI documents where traits have been applied

[c-pius]

User story.
As a developer, I want to write AsyncAPI rules working on the "effective" AsyncAPI document. Effective meaning that Operation, and Message traits (see fields) have been applied to the document, similar as done with $ref resolving.

Is your feature request related to a problem?
We want to validate that some of our common header attributes are always defined for certain messages. However, some define those headers directly on each message, others define a common trait that is mixing those headers into the messages. This makes it very hard to validate the message for such common headers as we need to consider multiple places where the info may be, including varying naming of the traits and even the algorithm for applying traits.

Example:

Based on the AsyncAPI Streetlights. We have the LightMeasured message where the myCommonHeader is not directly defined on the message, but mixed in via the CommonHeaders trait. Checking that myCommonHeader is defined on the message is hard as I have to a) check if it is directly on the message, b) check what traits are defined on the message and further check each of those if the header is defined there (even considering that traits are applied in-order using json-merge-patch algorithm).

asyncapi: '2.4.0'
info:
  title: Streetlights API
  version: '1.0.0'
  description: |
    The Smartylighting Streetlights API allows you
    to remotely manage the city lights.
channels:
  light/measured:
    publish:
      summary: Inform about environmental lighting conditions for a particular streetlight.
      operationId: onLightMeasured
      message:
        name: LightMeasured
        payload:
          type: object
          properties:
            id:
              type: integer
              minimum: 0
              description: Id of the streetlight.
            lumens:
              type: integer
              minimum: 0
              description: Light intensity measured in lumens.
            sentAt:
              type: string
              format: date-time
              description: Date and time when the message was sent.
        traits:
          - $ref: CommonHeaders
components:
  messageTraits:
    CommonHeaders:
      headers:
        properties:
          myCommonHeader:
            type: string
            description: Common header used by many messages.

Resulting effective message schema:

name: LightMeasured
payload:
  type: object
  properties:
    id:
      type: integer
      minimum: 0
      description: Id of the streetlight.
    lumens:
      type: integer
      minimum: 0
      description: Light intensity measured in lumens.
    sentAt:
      type: string
      format: date-time
      description: Date and time when the message was sent.
headers:
  properties:
    myCommonHeader:
      type: string
      description: Common header used by many messages.

Describe the solution you'd like
Similar to the$ref resolving, provide a possibility for rules to work on either the "plain" AsyncAPI without applied traits, or the "effective" AsyncAPI where traits have been applied.

Additional context
Note that traits in AsyncAPI also have their flaws and potential for undesired behavior. But actually, I think that providing validations rules based on the effective AsyncAPI would be a means to reduce errors made because of wrongly applying traits.

[magicmatatjahu]

@c-pius Hello! Thanks for the issue!

I understand the problem because writing a new rule to check uniqueness of messageId myself, I had to write functionality to check uniqueness in traits as well - you can see it here https://github.com/stoplightio/spectral/pull/2224/files#diff-6d40e65a1708ce8482632cdfba579951d2b6d3983c99640c8c38f5bd6f7637bcR9

However, I have a serious objection to merging traits by spectral - consider that after merging you will be operating on a single object (without looking at traits). Let's take an example:

      asyncapi: '2.4.0',
      channels: {
        someChannel1: {
          subscribe: {
            message: {
              messageId: 'id1',
            },
          },
        },
        someChannel2: {
          subscribe: {
            message: {
              messageId: 'id2',
            },
          },
          publish: {
            message: {
              oneOf: [
                {
                  messageId: 'id3',
                  traits: [
                    {
                      messageId: 'id4',
                    },
                    {
                      messageId: 'id2',
                    },
                  ],
                },
                {
                  messageId: 'id3',
                  traits: [
                    {
                      messageId: 'id1',
                    },
                  ],
                },
              ],
            },
          },
        },
      },

When we would have merged all the traits then the error would show the path

['channels', 'someChannel2', 'publish', 'message', 'oneOf', '0', 'messageId']

instead of:

['channels', 'someChannel2', 'publish', 'message', 'oneOf', '0', 'traits', '1', 'messageId']

In this case, the user would have a problem in understanding where the error occurs - would have to know that a certain trait can override something and would have to check the trait "visually" without spectral. Using the above example, we can see that the problem is not as simple to solve as enabling/disabling $ref resolving.

What do you think?

cc @smoya @jonaslagoni

[c-pius]

@magicmatatjahu yeah I see where you are coming from. Maybe that could be addressed by configurable plain vs effective flavor. Using effective means you validate the end result of the message, regardless what "syntactic sugar" was used to define it. Consequently, also a path pointing to the message is somehow correct (and the only thing you can really do then). If you want to validate down into the single traits, you can do it with the plain flavor but that might cause some pain (pun intended 😄).

At least I would consider a path pointing to the message not as completely wrong, and an acceptable trade-off for the a lot easier validation on message level that you could do. Please correct me if I am wrong but using the core functions like defined is virtually impossible on AsyncAPI messages right now because you can never know where an attribute is coming from?

[magicmatatjahu]

@c-pius

At least I would consider a path pointing to the message not as completely wrong, and an acceptable trade-off for the a lot easier validation on message level that you could do. Please correct me if I am wrong but using the core functions like defined is virtually impossible on AsyncAPI messages right now because you can never know where an attribute is coming from?

Yeah, atm it's problematic, you would only check "root" of message without merged traits, so defined function would only say truth in the cases without traits.

We'll see, maybe Spectral Team will consider adding a new option to the rule shape you proposed :)

[jonaslagoni]

In this case, the user would have a problem in understanding where the error occurs - would have to know that a certain trait can override something and would have to check the trait "visually" without spectral. Using the above example, we can see that the problem is not as simple to solve as enabling/disabling $ref resolving.

Definitely a problem, but not one I see as critical, i.e. spectral already handles this behavior with the refs right? Cause the same "problem" must be present there. I definitely see this as only an implementation issue that can easily be handed within spectral itself 🙂

I am definitely for finding a way to support this yes 👍

[magicmatatjahu]

@jonaslagoni

Definitely a problem, but not one I see as critical, i.e. spectral already handles this behavior with the refs right? Cause the same "problem" must is present there. I definitely see this as only an implementation issue that can easily be handed within spectral itself 🙂

But Spectral points to the $ref when you have errors in your spec, so it means that you will have path like:

['channels', 'someChannel2', 'publish', 'message', 'oneOf', '0', '$ref']

and if given $ref points to external file you will have also errors related to that file - as results you will have diagnostics for your main document and referenced document, so you can easily check in which place you have invalid value/syntax (exactly in which place in external document). In case of traits you have to know that traits can override values in your object and error/warning/info/hint will points to the place from final object perspective, not from traits perspective, so it can may be difficult to verify. Here we needed the logic for the rule to check each trait and conclude which override would cause problems relative to the final object.

Also I think that I have idea how to enable that behaviour. Rather to have implemented in source code functionality like resolved, unresolved we can write function to merge traits and then perform additional logic, so rule should look like:

{
      given: [ ... ],
      then: {
        function: mergeTraits,
        functionOptions: {
          field: 'operatioId' // example
          function: truthy
        },
      },
}

so we will "extend" then behaviour to wrap native then behaviour in options of mergeTraits function. WDYT?

cc @jonaslagoni @c-pius

[jonaslagoni]

Sounds like a good plan 🤔 👍