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

Support for Cerberus 1.0 (work in progress) #917

Closed
wants to merge 1 commit into from

Conversation

dkellner
Copy link
Contributor

Integrating Cerberus 1.0 was pretty straightforward and fun :). Most information is already in the commit message:

"Work in progress" because we still have to wait for the Cerberus issue
"`readonly` conflicts with `default`" to be resolved.
(see https://github.com/nicolaiarocci/cerberus/issues/268)

This is a rather big change. I still decided to do a single commit, as
intermediate commits would be in a non-working state anyway.

Breaking changes:

- `keyschema` was renamed to `valueschema` and `propertyschema` to
  `keyschema` (following changes in Cerberus).
- A PATCH on a document which misses a field having a default value will
  now result in setting this value, even if the field was not provided
  in the PATCH's payload.
- Error messages for `keyschema` are now returned as dictionary.
  Before: {'propertyschema_dict': 'propertyschema_dict'}
  Now: {'keyschema_dict': {'AAA': "value does not match regex '[a-z]+'"}}
- Error messages for `type` validations are different now (following
  changes in Cerberus).
- It is no longer valid to have a field with `default` = None and
  `nullable` = False.
  (see patch.py:test_patch_nested_document_nullable_missing)

In a nutshell, changes to the codebase are as follows:

- Add data layer independent subclass of `cerberus.Validator`
  * Support new signature of `__init__` and `validate`
  * Use `_config`-aware properties instead of bare member attributes to
    pass the `resource`, `document_id` and `persisted_document` to make
    them available to child validators
  * Add schema-docstrings to all `_validate_*` methods

- Adjust Mongo-specific `Validator` subclass
  * Adjust `_validate_type_*` methods (following changes in Cerberus)
  * Add schema-docstrings to all `_validate_*` methods

- Add custom ErrorHandler to support `VALIDATION_ERROR_AS_LIST`

- A few renames:
  * `ValidationError` -> `DocumentError`
  * `propertyschema` -> `keyschema` and `keyschema` -> `valueschema`

- Adjust tests due to different validation error messages
  (mostly for `type`)

- Remove `transparent_schema_rules` without replacement
- Remove `default`-handling, as Cerberus takes care of this now

I have two questions for the "final touch":

  1. Am I supposed to add ..versionchanged (etc.) comments myself? If yes, which version should I use? 0.7?
  2. I removed the docstrings of _validate_*-methods because Cerberus now uses the docstring for schema validation. Should I re-insert them as comments? Most of them were pretty meaningless, though... (e.g. "Enable validation for media data type")

"Work in progress" because we still have to wait for the Cerberus issue
"`readonly` conflicts with `default`" to be resolved.
(see pyeve/cerberus#268)

This is a rather big change. I still decided to do a single commit, as
intermediate commits would be in a non-working state anyway.

Breaking changes:

- `keyschema` was renamed to `valueschema` and `propertyschema` to
  `keyschema` (following changes in Cerberus).
- A PATCH on a document which misses a field having a default value will
  now result in setting this value, even if the field was not provided
  in the PATCH's payload.
- Error messages for `keyschema` are now returned as dictionary.
  Before: {'propertyschema_dict': 'propertyschema_dict'}
  Now: {'keyschema_dict': {'AAA': "value does not match regex '[a-z]+'"}}
- Error messages for `type` validations are different now (following
  changes in Cerberus).
- It is no longer valid to have a field with `default` = None and
  `nullable` = False.
  (see patch.py:test_patch_nested_document_nullable_missing)

In a nutshell, changes to the codebase are as follows:

- Add data layer independent subclass of `cerberus.Validator`
  * Support new signature of `__init__` and `validate`
  * Use `_config`-aware properties instead of bare member attributes to
    pass the `resource`, `document_id` and `persisted_document` to make
    them available to child validators
  * Add schema-docstrings to all `_validate_*` methods

- Adjust Mongo-specific `Validator` subclass
  * Adjust `_validate_type_*` methods (following changes in Cerberus)
  * Add schema-docstrings to all `_validate_*` methods

- Add custom ErrorHandler to support `VALIDATION_ERROR_AS_LIST`

- A few renames:
  * `ValidationError` -> `DocumentError`
  * `propertyschema` -> `keyschema` and `keyschema` -> `valueschema`

- Adjust tests due to different validation error messages
  (mostly for `type`)

- Remove `transparent_schema_rules` without replacement
- Remove `default`-handling, as Cerberus takes care of this now
@dkellner dkellner mentioned this pull request Sep 26, 2016
@nicolaiarocci
Copy link
Member

nicolaiarocci commented Sep 26, 2016

I'll review this later but at quick glance this looks like stellar work, thank you!

  1. My plan is to release v0.7 with old cerberus. This is to let people take advantage of Mongo Aggregation right away, with no need to deal with breaking changes. v0.8 would then be released, (relatively) shortly after v0.7. Of course v0.8 big thing would be Cerberus 1.0 support.
  2. Docstrings for validate methods we can loose them.
  3. Would be great if you could also update the documentation (validation.rst comes to mind, maybe other files too).

Again, great job.

@dkellner
Copy link
Contributor Author

Would be great if you could also update the documentation (validation.rst comes to mind, maybe other files too).

Ah yes, of course :). I will take care of the Cerberus issue pyeve/cerberus#268 first and then update documentation on both projects.

@nicolaiarocci
Copy link
Member

I deleted the develop branch so the PR got auto-closed. Could you resubmit against master? - Thanks!

@dkellner
Copy link
Contributor Author

dkellner commented Feb 7, 2017

Of course. I will do that later. There's still some work to do on this, but I have not forgot it. And now that we have Cerberus 1.1 (including the fix for the default & readonly issue) I can continue to work based on that.

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

Successfully merging this pull request may close these issues.

2 participants