feat: parse Avro name field to custom x-parser-schema-id #69
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. 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.
damaru-inc
changed the title
… in x-parser-schema-id as recommended in the comments.
derberg
changed the title
|
Okay, I've addressed all the comments. |
| if (avroDefinition.doc) jsonSchema.description = avroDefinition.doc; | ||
| if (avroDefinition.default !== undefined) jsonSchema.default = avroDefinition.default; | ||
|
|
||
| const fullyQualifiedName = getFullyQualifiedName(avroDefinition); | ||
| if (isTopLevel && fullyQualifiedName !== undefined) { |
There was a problem hiding this comment.
| if (isTopLevel && fullyQualifiedName !== undefined) { | |
| if (isTopLevel && !fullyQualifiedName) { |
There was a problem hiding this comment.
I'll change that if you like, but I was just trying to be consistent with the style. Just above on line 26 we have:
if (avroDefinition.default !== undefined) jsonSchema.default = avroDefinition.default;
There was a problem hiding this comment.
oh, I think now I remember why it was avroDefinition.default !== undefined, because probably avroDefinition.default could be null which should be fine 🤔 sorry but can you change it back and add a proper code comment?
README.md
Outdated
|
|
||
| If, at the top level of the Avro schema, the 'name' attribute is defined, it will be copied to the corresponding JSON schema's 'x-parser-schema-id' attribute. If the Avro schema also has the 'namespace' attribute defined, then that schema's fully qualified name will be put into that attribute. The fully qualified name is defined by the namespace, followed by a dot, followed by the name. | ||
|
|
||
| If there are two schemas that resolve to the same fully qualified name, only the second schema will be returned by the parser, the first will be discarded. |
There was a problem hiding this comment.
I have to say this one doesn't let me sleep 😄
what part of the code is responsible for it? why 2nd is returned, then what if 3 schemas resolve to the same?
There was a problem hiding this comment.
I assume that parser-js keeps a map where the key is x-parser-schema-id. So if there are several schemas that share the same x-parser-schema-id, then only the last one parsed will end up in that map. But if their document has several schemas with the same fully qualified name, I'd hope that they're identical, they repeat them because we force them to put Avro schemas into the message/payload section instead of components/schemas.
Maybe I should just not mention that at all? Or maybe that's an argument for using the title field after all, and adding x-namespace or something?
There was a problem hiding this comment.
ah, now it makes sense, you are referring to 👇🏼
/**
* @returns {Map<string, Schema>}
*/
allSchemas() {
const schemas = new Map();
const allSchemasCallback = (schema) => {
if (schema.uid()) {
schemas.set(schema.uid(), schema);
}
};
traverseAsyncApiDocument(this, allSchemasCallback);
return schemas;
}I think the sentence is ok, but just maybe write
If there are two schemas that resolve to the same fully qualified name, only the last one will be returned by the parser. Make sure names of your schemas are unique ?
There was a problem hiding this comment.
sorry @MichaelDavisSolace for the late review, one small readme stuff and I think we can merge and observe how the earth stands still 😄
Thanks, I assume you mean that bit about the parser discarding schemas that have the same qualified name - should I just remove that? |
|
@damaru-inc oh, you here? what time is it there man!
no just change to something similar I suggested there |
I'll make that change right now. It's 5:00 AM and I can't sleep. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
🎉 This PR is included in version 0.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
The 'name' field is required in Avro schemas, but this parser does not make that available in the converted JSON schema. With this change, the Avro name is copied to the JSON schema's title field.
Related issue(s)
Resolves #68