Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

feat: add message validation based on schema #21

Merged
merged 4 commits into from
Jul 26, 2021

Conversation

smoya
Copy link
Collaborator

@smoya smoya commented Jul 22, 2021

Description

This PR adds the required code to validate Kafka message against a JSON Schema document, which is grabbed from the message payload declared in the AsyncAPI document.

It introduces a new error validation notification mechanism, that allows configuring a Go channel where all validation errors will flow. Now the behavior is just logging those validation errors.

You can see a demo of this feature working here

Related issue(s)
Fixes #10

@smoya smoya added the enhancement New feature or request label Jul 22, 2021
@smoya smoya requested review from fmvilas and magicmatatjahu July 22, 2021 17:56
Copy link
Collaborator

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

I have only concerns to the way of message validation. Great job! :)

asyncapi/v2/v2.go Outdated Show resolved Hide resolved
asyncapi/v2/v2.go Outdated Show resolved Hide resolved
asyncapi/v2/validation.go Outdated Show resolved Hide resolved
proxy/validation.go Show resolved Hide resolved
@smoya smoya requested a review from magicmatatjahu July 23, 2021 16:51
Copy link
Collaborator

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

Two more comments 😅 But overall good!

asyncapi/document.go Show resolved Hide resolved
asyncapi/v2/validation.go Show resolved Hide resolved
Comment on lines 330 to 342
if len(o.MessageField.Payload().OneOf()) == 0 {
return []asyncapi.Message{o.MessageField}
}

var msgs []asyncapi.Message
for _, payload := range o.MessageField.Payload().OneOf() {
p := payload.(*Schema)
msgs = append(msgs, Message{
PayloadField: p,
})
}

return msgs
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not the true. MessageField should have OneOf property and you should use it, not Payload.OneOf(). Payload.OneOF is related to single message, not to array of messages :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wow, this is not explained at all in the documentation :O Well, only through an example but not clear at all: https://www.asyncapi.com/docs/specifications/v2.0.0#messageObject

Is there any other exception or hidden gem that you know about that I should add here?

Copy link
Collaborator

@magicmatatjahu magicmatatjahu Jul 26, 2021

Choose a reason for hiding this comment

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

You have information about oneOf in the https://www.asyncapi.com/docs/specifications/v2.0.0#operationObject

for the message field, you can read:

A definition of the message that will be published or received on this channel. oneOf is allowed here to specify multiple messages, however, a message MUST be valid only against one of the referenced message objects.

so you can have shape:

publish:
  message:
    payload: ...

or

publish:
  message:
    oneOf:
      - payload:  ... # message 1
      - payload:  ... # message 2

But I agree that should be cleared in the doc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, This oneOf field should be either part of the message object or a new type that holds that field.
Ok, gotta work on it! thks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is done

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For reference asyncapi/spec#588

@smoya smoya requested a review from magicmatatjahu July 26, 2021 13:28
Copy link
Collaborator

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@smoya smoya merged commit 5d5b36a into asyncapi-archived-repos:master Jul 26, 2021
@smoya smoya deleted the feat-message-validation branch July 26, 2021 13:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[kafka] Match messages received on Produce requests with the described messages in the AsyncAPI doc
3 participants