Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrong validation with AsyncAPI v3 in VS Code and JetBrains IDEs #495

Merged
merged 21 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0396524
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 10, 2024
8af32be
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 10, 2024
518e3e0
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 11, 2024
78ce54d
Merge branch 'master' into fix/494
Pakisan Mar 12, 2024
b5f2139
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 12, 2024
87a6097
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 12, 2024
981c80d
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 12, 2024
212cc82
Merge branch 'master' into fix/494
Pakisan Mar 15, 2024
e8ee4a2
Merge branch 'master' into fix/494
Pakisan Mar 19, 2024
b573b54
Update .gitignore
Pakisan Mar 19, 2024
62c5193
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 19, 2024
297e778
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 19, 2024
dddb3d3
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 22, 2024
a4c5ef0
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 25, 2024
acfb26e
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 25, 2024
469431c
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 28, 2024
b45e610
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 28, 2024
52b06c5
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Mar 29, 2024
d34130d
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Apr 2, 2024
a9a7d47
fix: AsyncAPI v3 shows warning in Studio and IntelliJ plugin when ref…
Pakisan Apr 2, 2024
eba77ed
Merge branch 'master' into fix/494
smoya Apr 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
node_modules
.nyc_output
.vscode
coverage
coverage

.DS_Store
/test/.DS_Store
/examples/.DS_Store
/bindings/.DS_Store
/.git/.DS_Store
Pakisan marked this conversation as resolved.
Show resolved Hide resolved
/definitions/.DS_Store
2 changes: 1 addition & 1 deletion definitions/3.0.0/multiFormatSchema.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"description": "Definition of the message payload. It can be of any type but defaults to Schema Object. It MUST match the schema format defined in schemaFormat, including the encoding type. E.g., Avro should be inlined as either a YAML or JSON object instead of as a string to be parsed as YAML or JSON. Non-JSON-based schemas (e.g., Protobuf or XSD) MUST be inlined as a string."
}
},
"allOf": [
"anyOf": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a deep investigation, I concluded this cannot be changed to anyOf and should stay as allOf.

The reason is that anyOf will try to match with any schema in the array. The problem is that since those schemas are if, and an if conditional fails, it is considered an empty schema, which is equivalent to true assertion of that element in the anyOf array.

See this example I made by extracting the logic in place and reducing the complexity for test purposes: https://www.jsonschemavalidator.net/s/q0RfvVqb

As you can see, the if is failing (doesn't match the schemaFormat), but the anyOf is resulting in a true assertion. Totally against what we expect.

How can we fix then the issue?

Let's read what the JSON Schema validator says:

Slack_TGf24bNa@2x

The issue comes not at this line, but at this other one.

"oneOf": [
    {
        "$ref": "#/definitions/Reference"
    },
    {
        "$ref": "#/definitions/schema"
    }
]

The point is that the keyword $ref is both defined in the Reference schema and in the schema, which is a superset of JSON Schema and it does include the $ref keyword itself.
That's why JSON Schema validation says the schema is matching with more than one schema in thisoneOf.

By changing that oneOf to anyOf, we ensure that by matching with any of those schemas will be considered a valid doc.

I spent some time trying to find edge-cases for the change I'm suggesting. For example, what if we change how our Reference Object ($ref) look like? Then, it will differ from JSON Schema provided one, and we could set it back then to oneOf if needed.

Btw, this change should be propagated to the oneOf in OpenAPI since OpenAPI schema is also a superset of JSON Schema and it includes the $ref property.

Additionally, I investigated a bit if we could get rid of those if/then and I believe we could just use allOf with all the possible cases as schemas right directly, without the if/then, including the case when the schemaFormat is just an unknown string.
But this, is out of the scope of this issue I believe. Just for the record.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoya check out new validation rules. I have removed all if/then constructions

Playground: https://www.jsonschemavalidator.net/s/JmGON4jI
Specification to test:

{
  "asyncapi": "3.0.0",
  "info": {
    "version": "",
    "title": ""
  },
  "components": {
    "messages": {
      "asyncAPISchema1": {
        "payload": {
          "schemaFormat": "application/vnd.aai.asyncapi+json;version=3.0.0",
          "schema": {
            "$ref": "#/components/schemas/asyncAPISchema1"
          }
        },
        "name": "asyncAPISchema1",
        "title": "asyncAPISchema1",
        "description": "asyncAPISchema1 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "asyncAPISchema2": {
        "payload": {
          "schemaFormat": "application/vnd.aai.asyncapi+json;version=3.0.0",
          "schema": {
            "externalDocs": {
              "description": "description field",
              "url": "https://local.url"
            },
            "$ref": "#/components/schemas/asyncAPISchema2"
          }
        },
        "name": "asyncAPISchema2",
        "title": "asyncAPISchema2",
        "description": "asyncAPISchema2 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "asyncAPIReference": {
        "payload": {
          "$ref": "#/components/schemas/asyncAPIReference"
        },
        "name": "asyncAPIReference",
        "title": "asyncAPIReference",
        "description": "asyncAPIReference payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "jsonSchema": {
        "payload": {
          "schemaFormat": "application/schema+json;version=draft-07",
          "schema": {
            "title": "AnotherPayloadDto",
            "uniqueItems": true,
            "$ref": "#/components/schemas/jsonSchema"
          }
        },
        "name": "jsonSchema",
        "title": "jsonSchema",
        "description": "jsonSchema payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "openAPISchema1": {
        "payload": {
          "schemaFormat": "application/vnd.oai.openapi+json;version=3.0.0",
          "schema": {
            "$ref": "#/components/schemas/openAPISchema1"
          }
        },
        "name": "openAPISchema1",
        "title": "openAPISchema1",
        "description": "openAPISchema1 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "openAPISchema2": {
        "payload": {
          "schemaFormat": "application/vnd.oai.openapi+json;version=3.0.0",
          "schema": {
            "writeOnly": true,
            "readOnly": true,
            "nullable": true
          }
        },
        "name": "openAPISchema2",
        "title": "openAPISchema2",
        "description": "openAPISchema2 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "avroSchema1": {
        "payload": {
          "schemaFormat": "application/vnd.apache.avro+json;version=1.9.0",
          "schema": {
            "type" : "record",
            "namespace" : "Tutorialspoint",
            "name" : "Employee",
            "fields" : [
              { "name" : "Name" , "type" : "string" },
              { "name" : "Age" , "type" : "int" }
            ]
          }
        },
        "name": "avroSchema1",
        "title": "avroSchema1",
        "description": "avroSchema-1 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      },
      "avroSchema2": {
        "payload": {
          "schemaFormat": "application/vnd.apache.avro+json;version=1.9.0",
          "schema": {
            "$ref": "#/components/schemas/avroSchema2"
          }
        },
        "name": "avroSchema2",
        "title": "avroSchema2",
        "description": "avroSchema-2 payload model",
        "bindings": {
          "amqp": {
            "bindingVersion": "0.3.0"
          }
        }
      }
    }
  }
}

{
"if": {
"not": {
Expand Down
46 changes: 23 additions & 23 deletions schemas/3.0.0-without-$id.json
Original file line number Diff line number Diff line change
Expand Up @@ -2055,7 +2055,7 @@
"properties": {
"type": "array",
"items": {
"$ref": "#/definitions/bindings-jms-0.0.1-server"
"$ref": "#/definitions/bindings-jms-0.0.1-server/definitions/property"
},
"description": "Additional properties to set on the JMS ConnectionFactory implementation for the JMS Provider."
},
Expand Down Expand Up @@ -2752,7 +2752,7 @@
"description": "Definition of the message payload. It can be of any type but defaults to Schema Object. It MUST match the schema format defined in schemaFormat, including the encoding type. E.g., Avro should be inlined as either a YAML or JSON object instead of as a string to be parsed as YAML or JSON. Non-JSON-based schemas (e.g., Protobuf or XSD) MUST be inlined as a string."
}
},
"allOf": [
"anyOf": [
{
"if": {
"not": {
Expand Down Expand Up @@ -5718,10 +5718,10 @@
"description": "The name of the topic. Can be different from the channel name to allow flexibility around AWS resource naming limitations."
},
"ordering": {
"$ref": "#/definitions/bindings-sns-0.1.0-channel"
"$ref": "#/definitions/bindings-sns-0.1.0-channel/definitions/ordering"
},
"policy": {
"$ref": "#/definitions/bindings-sns-0.1.0-channel"
"$ref": "#/definitions/bindings-sns-0.1.0-channel/definitions/policy"
},
"tags": {
"type": "object",
Expand Down Expand Up @@ -5776,7 +5776,7 @@
"type": "array",
"description": "An array of statement objects, each of which controls a permission for this topic",
"items": {
"$ref": "#/definitions/bindings-sns-0.1.0-channel"
"$ref": "#/definitions/bindings-sns-0.1.0-channel/definitions/statement"
}
}
},
Expand Down Expand Up @@ -5863,11 +5863,11 @@
"properties": {
"queue": {
"description": "A definition of the queue that will be used as the channel.",
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/queue"
},
"deadLetterQueue": {
"description": "A definition of the queue that will be used for un-processable messages.",
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/queue"
},
"bindingVersion": {
"type": "string",
Expand Down Expand Up @@ -5946,10 +5946,10 @@
"default": 345600
},
"redrivePolicy": {
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/redrivePolicy"
},
"policy": {
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/policy"
},
"tags": {
"type": "object",
Expand All @@ -5971,7 +5971,7 @@
},
"properties": {
"deadLetterQueue": {
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/identifier"
},
"maxReceiveCount": {
"type": "integer",
Expand Down Expand Up @@ -6015,7 +6015,7 @@
"type": "array",
"description": "An array of statement objects, each of which controls a permission for this queue.",
"items": {
"$ref": "#/definitions/bindings-sqs-0.2.0-channel"
"$ref": "#/definitions/bindings-sqs-0.2.0-channel/definitions/statement"
}
}
},
Expand Down Expand Up @@ -7636,19 +7636,19 @@
},
"properties": {
"topic": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation",
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/identifier",
"description": "Often we can assume that the SNS Topic is the channel name-we provide this field in case the you need to supply the ARN, or the Topic name is not the channel name in the AsyncAPI document."
},
"consumers": {
"type": "array",
"description": "The protocols that listen to this topic and their endpoints.",
"items": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation"
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/consumer"
},
"minItems": 1
},
"deliveryPolicy": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation",
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/deliveryPolicy",
"description": "Policy for retries to HTTP. The field is the default for HTTP receivers of the SNS Topic which may be overridden by a specific consumer."
},
"bindingVersion": {
Expand Down Expand Up @@ -7716,7 +7716,7 @@
},
"endpoint": {
"description": "The endpoint messages are delivered to.",
"$ref": "#/definitions/bindings-sns-0.1.0-operation"
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/identifier"
},
"filterPolicy": {
"type": "object",
Expand Down Expand Up @@ -7757,10 +7757,10 @@
"description": "If true AWS SNS attributes are removed from the body, and for SQS, SNS message attributes are copied to SQS message attributes. If false the SNS attributes are included in the body."
},
"redrivePolicy": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation"
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/redrivePolicy"
},
"deliveryPolicy": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation",
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/deliveryPolicy",
"description": "Policy for retries to HTTP. The parameter is for that SNS Subscription and overrides any policy on the SNS Topic."
},
"displayName": {
Expand Down Expand Up @@ -7832,7 +7832,7 @@
},
"properties": {
"deadLetterQueue": {
"$ref": "#/definitions/bindings-sns-0.1.0-operation",
"$ref": "#/definitions/bindings-sns-0.1.0-operation/definitions/identifier",
"description": "The SQS queue to use as a dead letter queue (DLQ)."
},
"maxReceiveCount": {
Expand Down Expand Up @@ -7910,7 +7910,7 @@
"type": "array",
"description": "Queue objects that are either the endpoint for an SNS Operation Binding Object, or the deadLetterQueue of the SQS Operation Binding Object.",
"items": {
"$ref": "#/definitions/bindings-sqs-0.2.0-operation"
"$ref": "#/definitions/bindings-sqs-0.2.0-operation/definitions/queue"
}
},
"bindingVersion": {
Expand Down Expand Up @@ -7994,10 +7994,10 @@
"default": 345600
},
"redrivePolicy": {
"$ref": "#/definitions/bindings-sqs-0.2.0-operation"
"$ref": "#/definitions/bindings-sqs-0.2.0-operation/definitions/redrivePolicy"
},
"policy": {
"$ref": "#/definitions/bindings-sqs-0.2.0-operation"
"$ref": "#/definitions/bindings-sqs-0.2.0-operation/definitions/policy"
},
"tags": {
"type": "object",
Expand All @@ -8018,7 +8018,7 @@
},
"properties": {
"deadLetterQueue": {
"$ref": "#/definitions/bindings-sqs-0.2.0-operation"
"$ref": "#/definitions/bindings-sqs-0.2.0-operation/definitions/identifier"
},
"maxReceiveCount": {
"type": "integer",
Expand Down Expand Up @@ -8062,7 +8062,7 @@
"type": "array",
"description": "An array of statement objects, each of which controls a permission for this queue.",
"items": {
"$ref": "#/definitions/bindings-sqs-0.2.0-operation"
"$ref": "#/definitions/bindings-sqs-0.2.0-operation/definitions/statement"
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion schemas/3.0.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -2803,7 +2803,7 @@
"description": "Definition of the message payload. It can be of any type but defaults to Schema Object. It MUST match the schema format defined in schemaFormat, including the encoding type. E.g., Avro should be inlined as either a YAML or JSON object instead of as a string to be parsed as YAML or JSON. Non-JSON-based schemas (e.g., Protobuf or XSD) MUST be inlined as a string."
}
},
"allOf": [
"anyOf": [
{
"if": {
"not": {
Expand Down
18 changes: 17 additions & 1 deletion tools/bundler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,23 @@ function getDefinitionName(def) {
}
if (def.startsWith('http://asyncapi.com/bindings')) {
const result = bindingsRegex.exec(def);
if (result) return `${result[1].replace('/', '-')}-${result[2]}-${result[3]}`;
if (result) {
Pakisan marked this conversation as resolved.
Show resolved Hide resolved
/*
4th element is for internal definitions like http://asyncapi.com/bindings/jms/0.0.1/server.json#/definitions/property

When is empty, we can ignore it:
convert this: http://asyncapi.com/bindings/jms/0.0.1/server.json
to this: bindings-jms-0.0.1-server
Otherwise we MUST add it to not broke Json Schema validation:
convert this: http://asyncapi.com/bindings/jms/0.0.1/server.json#/definitions/property
to this: bindings-jms-0.0.1-server/definitions/property
*/
if (result[4] === '') {
return `${result[1].replace('/', '-')}-${result[2]}-${result[3]}`;
}

return `${result[1].replace('/', '-')}-${result[2]}-${result[3]}/${result[4].replace('#/', '')}`;
}
}

return path.basename(def, '.json');
Expand Down
Loading