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 JSON after decode #623

Merged
merged 2 commits into from
Jan 25, 2021
Merged

Validate JSON after decode #623

merged 2 commits into from
Jan 25, 2021

Conversation

gregbeech
Copy link
Contributor

Validates JSON retrieved from the database after decode rather than before.

Description

Changes the order of operations so JSON is decoded before validation is run, along with a test for each of the three supported input types for the field.

Motivation and Context

Custom JSON validations are currently run before the value stored in the database is converted to the Python type. This creates an asymmetry in the validation function as it runs against the Python type on create/update, but against the raw JSON string on fetch.

Closes #621

How Has This Been Tested?

All unit tests run.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Closes #621

Custom JSON validations are currently run before the value stored in the database is converted to the Python type. This creates an asymmetry in the validation function as it runs against the Python type on create/update, but against the raw JSON string on fetch.

This changes the order of operations so the JSON is decoded before validation is run, along with a test for each of the three supported input types for the field.
id = fields.IntField(pk=True)
data = fields.JSONField()
data_null = fields.JSONField(null=True)
data_default = fields.JSONField(default={"a": 1})
data_validate = fields.JSONField(null=True, validators=[lambda v: JSONFields.dict_or_list(v)])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Putting the field on this model means the 'describe' tests need to be updated, but it feels more appropriate than putting it on the validation model as this is testing the JSONField itself, not packaged validations. I've used a static method on the object rather than a module method as the validator to keep things self-contained, but happy to change if you find it too clumsy.

@long2ice
Copy link
Member

Looks good, could you update changelog and version? After CI pass, I will merge it.

@coveralls
Copy link

coveralls commented Jan 25, 2021

Pull Request Test Coverage Report for Build 509686588

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.154%

Totals Coverage Status
Change from base Build 506712016: 0.0%
Covered Lines: 4366
Relevant Lines: 4488

💛 - Coveralls

Copy link
Member

@long2ice long2ice left a comment

Choose a reason for hiding this comment

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

Good!

@long2ice long2ice merged commit 1a0cceb into tortoise:develop Jan 25, 2021
@gregbeech gregbeech deleted the validate-json-after-decode branch January 25, 2021 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSONField calls validate before decode
3 participants