feat: validate message object examples with spectral schemas #782

chrispatmore · 2023-06-07T08:46:55Z

validate message object examples against the schemas in the
object by parsing the schemas using spectral before validating
this proves that the examples are valid.

Related issue(s)
Contributes to: #781

Signed-off-by: Chris Patmore [email protected]

chrispatmore

Couple of comments / questions for others

package.json

chrispatmore · 2023-06-07T08:48:31Z

src/ruleset/v2/ruleset.ts

@@ -238,6 +239,7 @@ export const v2SchemasRuleset = (parser: Parser) => {
          },
        },
      },
+      'asyncapi2-message-examples-custom-format': asyncApi2MessageExamplesParserRule(parser),


Added the rule in but might not want to do this yet. If not, need a pointer on how to get the tests to run without being in the ruleset

chrispatmore · 2023-06-07T08:51:43Z

There is code duplication w.r.t to the old rule. I can deal with this after review / thoughts on what's there

chrispatmore · 2023-06-12T15:30:48Z

has duplication which I can sort out with other comments. and is out of date with base now I think, but any comments on the code are appreciated @jonaslagoni

jonaslagoni · 2023-06-12T16:53:49Z

I dont think it changed, I just need to find some time to review it ASAP 😅

chrispatmore · 2023-06-13T13:59:57Z

This will probably need some slight changes / merge conflict resolutions from #786

jonaslagoni · 2023-06-14T13:27:44Z

@chrispatmore fixed the conflicts, gonna review this once I get back from the gym 🙂

jonaslagoni

@chrispatmore would it make sense to add the rule ONLY for when schemaFormat has been defined? i.e. that is the difference between the two spectral rules and they can both live together without having to change that down the road?

Cause that would remove all duplicated code parts and tests and simplify the approach by a mile

chrispatmore · 2023-06-15T12:29:30Z

Let me take a look. Forcing me to play around with json path expressions again 😉

validate message object examples against the schemas in the object by parsing the schemas using spectral before validation this proves that the examples are valid Signed-off-by: Chris Patmore <[email protected]>

Signed-off-by: Chris Patmore <[email protected]>

switch the new rule to only run of the schemaFormat is set in the message. Also adjust the tests to better align with this separation. Signed-off-by: Chris Patmore <[email protected]>

chrispatmore · 2023-06-15T13:08:58Z

@jonaslagoni This block still duplicated across the two rules:

function getMessageExamples(message: v2.MessageObject): Array<{ path: JsonPath; value: v2.MessageExampleObject }> {
  if (!Array.isArray(message.examples)) {
    return [];
  }
  return (
    message.examples.map((example, index) => {
      return {
        path: ['examples', index],
        value: example,
      };
    }) ?? []
  );
}

function validate(
  value: unknown,
  path: JsonPath,
  type: 'payload' | 'headers',
  schema: unknown,
  ctx: RulesetFunctionContext,
): ReturnType<typeof schemaFn> {
  return schemaFn(
    value,
    {
      allErrors: true,
      schema: schema as JSONSchema7,
    },
    {
      ...ctx,
      path: [...ctx.path, ...path, type],
    },
  );
}

Not really sure how best to de-depe. Could export from the first one so new once can use, but they're so minor I'm not sure it's worth it

chrispatmore · 2023-06-15T13:16:13Z

That code smell is false positive, if I remove the as any it won't build

Signed-off-by: Chris Patmore <[email protected]>

chrispatmore · 2023-06-21T09:14:09Z

How did removing code increase duplication...

Edit: ah now it's finding the duplicated scenarios in the tests. I think these duplicate usages are valid, because they highlight the difference in behaviour between the two rules

chrispatmore · 2023-06-21T09:16:48Z

@jonaslagoni thoughts on duplication in the tests?

jonaslagoni · 2023-06-21T14:57:36Z

test/ruleset/rules/v2/asyncapi2-message-examples.spec.ts

+              headers: {
+                type: 'record',
+                name: 'TestHeader',
+                fields: [
+                  {name: 'someKey', type: 'string'},
+                  {name: 'someOtherKey', type: 'string'}
+                ]
+              },


What was this change for? 🤔

I think I added this when the same tests were being used for both rules, to ensure no change in behaviour, less relevant now, can remove

jonaslagoni · 2023-06-21T14:58:10Z

test/ruleset/rules/v2/asyncapi2-message-examples.spec.ts

@@ -378,19 +390,32 @@ testRule('asyncapi2-message-examples', [
                  {name: 'speed', type: 'string'}
                ]
              },
+              headers: {


Pretty sure headers cannot be avro 🤔 At least not in v2

No I added the headers to the original tests (schemaFormat not set case) so that It added confidence that header behaviour was maintained across both versions of the rule (before we decided to split the rules)
I can revert this.

Though the new info that headers cannot be v2 means I can simplify the new v2 rule!

src/ruleset/v2/functions/messageExamples-spectral-rule-v2.ts

jonaslagoni · 2023-06-21T15:00:29Z

Sorry @chrispatmore I am context-switching like crazy at the moment, might be forgetting stuff...

Do correct me if I say something completely off the chart 😄

remove logic expecting headers to be avro remove redundant test pieces Signed-off-by: Chris Patmore <[email protected]>

chrispatmore · 2023-06-21T15:44:44Z

No worries @jonaslagoni not in any particular rush. Have adapted to your comments.

The headers cant be avro was interesting, because my rule validating the headers examples against an avro header schema was perfectly happy with it!

Have removed it regardless as, as you say it's not allowed in the spec atm

I expect we will still have quite high duplication in the test spect

chrispatmore · 2023-08-01T08:29:15Z

Hey @jonaslagoni , I am back from a little break, what are we thinking here?

sonarcloud · 2023-08-01T14:25:14Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

jonaslagoni

👍

jonaslagoni · 2023-08-03T07:01:07Z

/rtm

jonaslagoni · 2023-08-03T07:01:23Z

Thanks for the patience @chrispatmore, sorry it took so long 🙏

asyncapi-bot · 2023-08-03T07:11:34Z

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

chrispatmore · 2023-08-03T08:14:05Z

Thanks @jonaslagoni no worries !

chrispatmore requested review from fmvilas, magicmatatjahu, jonaslagoni, derberg, smoya and asyncapi-bot-eve as code owners June 7, 2023 08:46

chrispatmore commented Jun 7, 2023

View reviewed changes

jonaslagoni requested changes Jun 14, 2023

View reviewed changes

Chris Patmore added 3 commits June 15, 2023 13:33

feat: validate message object examples with spectral schemas

8ec4d95

validate message object examples against the schemas in the object by parsing the schemas using spectral before validation this proves that the examples are valid Signed-off-by: Chris Patmore <[email protected]>

fix: switch names around

0ee59db

Signed-off-by: Chris Patmore <[email protected]>

refactor: only run new rule if schemaFormat is set

26eda0c

switch the new rule to only run of the schemaFormat is set in the message. Also adjust the tests to better align with this separation. Signed-off-by: Chris Patmore <[email protected]>

chrispatmore force-pushed the validate-examples-against-non-default-schemas branch from b5a47e4 to 26eda0c Compare June 15, 2023 13:07

fix: remove duplication

e8d342a

Signed-off-by: Chris Patmore <[email protected]>

jonaslagoni reviewed Jun 21, 2023

View reviewed changes

refactor: review comments

f64833b

remove logic expecting headers to be avro remove redundant test pieces Signed-off-by: Chris Patmore <[email protected]>

chrispatmore added 2 commits June 29, 2023 09:46

Merge branch 'master' into validate-examples-against-non-default-schemas

acf614e

Merge branch 'master' into validate-examples-against-non-default-schemas

d0206bc

chrispatmore added 2 commits July 21, 2023 09:08

Merge branch 'master' into validate-examples-against-non-default-schemas

225462a

Merge branch 'master' into validate-examples-against-non-default-schemas

fe2990e

Merge branch 'master' into validate-examples-against-non-default-schemas

f4cd06d

jonaslagoni approved these changes Aug 3, 2023

View reviewed changes

asyncapi-bot added the ready-to-merge label Aug 3, 2023

asyncapi-bot merged commit 298b289 into asyncapi:master Aug 3, 2023
11 checks passed

asyncapi-bot added the released label Aug 3, 2023

chrispatmore deleted the validate-examples-against-non-default-schemas branch August 3, 2023 08:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: validate message object examples with spectral schemas #782

feat: validate message object examples with spectral schemas #782

chrispatmore commented Jun 7, 2023

chrispatmore left a comment

chrispatmore Jun 7, 2023

chrispatmore commented Jun 7, 2023

chrispatmore commented Jun 12, 2023

jonaslagoni commented Jun 12, 2023

chrispatmore commented Jun 13, 2023

jonaslagoni commented Jun 14, 2023

jonaslagoni left a comment •

edited

Loading

chrispatmore commented Jun 15, 2023

chrispatmore commented Jun 15, 2023 •

edited

Loading

chrispatmore commented Jun 15, 2023

chrispatmore commented Jun 21, 2023 •

edited

Loading

chrispatmore commented Jun 21, 2023

jonaslagoni Jun 21, 2023

chrispatmore Jun 21, 2023

jonaslagoni Jun 21, 2023

chrispatmore Jun 21, 2023

jonaslagoni commented Jun 21, 2023

chrispatmore commented Jun 21, 2023

chrispatmore commented Aug 1, 2023

sonarcloud bot commented Aug 1, 2023

jonaslagoni left a comment

jonaslagoni commented Aug 3, 2023

jonaslagoni commented Aug 3, 2023

asyncapi-bot commented Aug 3, 2023

chrispatmore commented Aug 3, 2023

feat: validate message object examples with spectral schemas #782

feat: validate message object examples with spectral schemas #782

Conversation

chrispatmore commented Jun 7, 2023

chrispatmore left a comment

Choose a reason for hiding this comment

chrispatmore Jun 7, 2023

Choose a reason for hiding this comment

chrispatmore commented Jun 7, 2023

chrispatmore commented Jun 12, 2023

jonaslagoni commented Jun 12, 2023

chrispatmore commented Jun 13, 2023

jonaslagoni commented Jun 14, 2023

jonaslagoni left a comment • edited Loading

Choose a reason for hiding this comment

chrispatmore commented Jun 15, 2023

chrispatmore commented Jun 15, 2023 • edited Loading

chrispatmore commented Jun 15, 2023

chrispatmore commented Jun 21, 2023 • edited Loading

chrispatmore commented Jun 21, 2023

jonaslagoni Jun 21, 2023

Choose a reason for hiding this comment

chrispatmore Jun 21, 2023

Choose a reason for hiding this comment

jonaslagoni Jun 21, 2023

Choose a reason for hiding this comment

chrispatmore Jun 21, 2023

Choose a reason for hiding this comment

jonaslagoni commented Jun 21, 2023

chrispatmore commented Jun 21, 2023

chrispatmore commented Aug 1, 2023

sonarcloud bot commented Aug 1, 2023

jonaslagoni left a comment

Choose a reason for hiding this comment

jonaslagoni commented Aug 3, 2023

jonaslagoni commented Aug 3, 2023

asyncapi-bot commented Aug 3, 2023

chrispatmore commented Aug 3, 2023

jonaslagoni left a comment •

edited

Loading

chrispatmore commented Jun 15, 2023 •

edited

Loading

chrispatmore commented Jun 21, 2023 •

edited

Loading