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

Validate protocol definition upon ingestion #332

Closed
diehuxx opened this issue Apr 29, 2023 · 3 comments
Closed

Validate protocol definition upon ingestion #332

diehuxx opened this issue Apr 29, 2023 · 3 comments
Assignees
Labels
feature New feature or request

Comments

@diehuxx
Copy link

diehuxx commented Apr 29, 2023

Currently we don't do any validation of a protocol definition within ProtocolsConfigure before we store the definition. There are a few things we can check to validate that the protocol definition isn't broken before we try to write records to it.

  1. Validate that the protocolPath in a given rule set is not longer than the ancestor path to reach that rule set. Currently we check this upon Records* handling here
  2. As we refactor towards Revisit protocol definition schema style and consider adopting a style with predefined property names #326, we will need to validate that
    a. no two recordTypes have the same id
    b. no two descendants in a given ruleset have the same recordType
@diehuxx diehuxx self-assigned this Apr 29, 2023
@thehenrytsai thehenrytsai added the feature New feature or request label Apr 29, 2023
@thehenrytsai
Copy link
Contributor

thehenrytsai commented Apr 29, 2023

@diehuxx, just want to sanity check so I understanding the issue fully: I am pretty sure we do validation for the protocol definition within ProtocolsConfigure during parse() before we store the definition as well as during create(). You were modifying the schema for it, so I don't fully understand, can you clarify?

Agree that we can and should do more validation beyond basic JSON Schema, especially if we revamp the protocol definition schema, but this is true in general to all DWN message types.

@diehuxx
Copy link
Author

diehuxx commented Apr 30, 2023

Agree that we can and should do more validation beyond basic JSON Schema, especially if we revamp the protocol definition schema, but this is true in general to all DWN message types.

Yep this is what I meant. Sorry, should have been more clear in the description! We definitely do JSON schema validation, which covers a lot of what we need. But there's a few more validations (like those listed in the description) that we need to add to the code, or I need to figure out how to do those checks within JSON schema.

@thehenrytsai
Copy link
Contributor

thehenrytsai commented May 30, 2023

We have revamped the protocol definition its code quite drastically, this is no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants