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

Add Avro Schema Validation #37

Closed
JQrdan opened this issue Feb 8, 2021 · 10 comments · Fixed by #96
Closed

Add Avro Schema Validation #37

JQrdan opened this issue Feb 8, 2021 · 10 comments · Fixed by #96
Labels
help wanted Extra attention is needed released

Comments

@JQrdan
Copy link

JQrdan commented Feb 8, 2021

Reason/Context

Please try answering few of those questions

  • Why we need this improvement?
    • This improvement would keep the behaviour of the parsers consistent with each other
  • How will this change help?
    • It means that consumers of the parser will not have to rely on their own Avro validation
  • What is the motivation?
    • All the over parsers in the AsyncApi library validate json schemas, it would be nice if it validated Avro too. This way, parser is responsible for all validation and then can be used straight away

Description

Please try answering few of those questions

  • Will this be a breaking change?
    • Hopefully not
  • How could it be implemented/designed?
    • Similar to how the json validation is done with other parsers
@JQrdan JQrdan added the enhancement New feature or request label Feb 8, 2021
@github-actions
Copy link

github-actions bot commented Feb 8, 2021

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
Copy link

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 ❤️

@github-actions github-actions bot added the stale label Apr 10, 2021
@derberg
Copy link
Member

derberg commented Apr 12, 2021

@JQrdan did you plan to work on this. I'm not sure from the description how would you see it works. Are there some js avro validators you know that could be integrated?

@JQrdan
Copy link
Author

JQrdan commented Apr 12, 2021

@derberg Unfortunately not. I'm not aware of any reputable Avro validators out there for JS but wanted to raise the issue to see if there were any plans/thoughts about this kind of thing given that everything else is validated 😄

@derberg derberg removed enhancement New feature or request stale labels Jun 15, 2021
@derberg derberg reopened this Jun 15, 2021
@github-actions
Copy link

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 ❤️

@github-actions github-actions bot added the stale label Aug 15, 2021
@derberg derberg added help wanted Extra attention is needed and removed stale labels Aug 16, 2021
@dalelane
Copy link
Collaborator

dalelane commented Nov 3, 2021

https://github.com/mtth/avsc looks pretty good

@JQrdan - think this would cover what you had in mind?

const assert = require('assert');

// npm install avsc
const avro = require('avsc');

// -------- TEST SCHEMAS -------------------------------------------

const VALID_SCHEMA = {
    type : 'record',
    fields : [
        { name : 'test', type : 'string' }
    ]
};

const INVALID_SCHEMA = {
    type : 'record',
    fields : [
        { name : 'test', type : 'fish' }
    ]
};

const NOT_EVEN_CLOSE = {
    hello : 'world'
};

// -------- PARSING THE TEST SCHEMAS -------------------------------------------

const type = avro.Type.forSchema(VALID_SCHEMA);
assert(type);

try {
    const type2 = avro.Type.forSchema(INVALID_SCHEMA);
    assert.fail();
}
catch (err) {
    assert(err);
}

try {
    const type3 = avro.Type.forSchema(NOT_EVEN_CLOSE);
    assert.fail();
}
catch (err) {
    assert(err);
}

@M3lkior
Copy link
Collaborator

M3lkior commented Nov 9, 2021

your library seems to be the work @dalelane. Have you tried to define an avro schema with additional attributes to test if the validation passed ?
This is required to keep this feature (#82) already works =)

@dalelane
Copy link
Collaborator

dalelane commented Nov 9, 2021

@M3lkior yeah - seems to be okay with https://github.com/M3lkior/avro-schema-parser/blob/c36357e8ec5b64c5a971eeb263c7f89046f27ce1/tests/schemas/Person-1.9.0-additional-attributes.avsc

dalelane added a commit that referenced this issue Nov 13, 2021
This commit adds validation to the avro-schema-parser so that
invalid Avro schemas will be rejected.

I had to fix a few invalid Avro schemas in the tests (which were
using the type 'uuid' which isn't in the Avro spec) so this should
potentially be considered a major breaking change to the parser.

Closes: #37

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
@dalelane
Copy link
Collaborator

@JQrdan - would you like to review #96 and check it was what you were expecting?

dalelane added a commit that referenced this issue Nov 19, 2021
This commit adds validation to the avro-schema-parser so that
invalid Avro schemas will be rejected.

I had to fix a few invalid Avro schemas in the tests (which were
using the type 'uuid' which isn't in the Avro spec) so this should
potentially be considered a major breaking change to the parser.

Closes: #37

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
dalelane added a commit that referenced this issue Nov 22, 2021
This commit adds validation to the avro-schema-parser so that
invalid Avro schemas will be rejected.

I had to fix a few invalid Avro schemas in the tests (which were
using the type 'uuid' which isn't in the Avro spec) so this should
potentially be considered a major breaking change to the parser.

Closes: #37

Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants