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

Rules sets with normalization rules fail #283

Closed
dkellner opened this Issue Nov 12, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@dkellner
Copy link
Contributor

dkellner commented Nov 12, 2016

Used Cerberus version / latest commit: current master (3f673c9)

  • I consulted these documentations:

  • I consulted these sections of the docs (add more lines as necessary):

    • /api.html#rules-set-schema-registry
    • /normalization-rules.html
  • I found nothing relevant to my problem in the docs.

  • I found the documentation not helpful to my problem.

  • I have the capacity to improve the docs when my problem is solved.

  • I have the capacity to submit a patch when a bug is identified.


Bug report

Normalization rules like default cannot be part of a rules set. Consider these failings tests:

def test_rules_set_simple_with_default():
    rules_set_registry.add('foo', {'default': 42})
    assert_normalized({}, {'bar': 42}, {'bar': 'foo'})

def test_rules_set_simple_with_default_setter():
    rules_set_registry.add('foo', {'default_setter': lambda _: 42})
    assert_normalized({}, {'bar': 42}, {'bar': 'foo'})

The reason is - in my opinion - that the normalization code uses schema[field] directly without calling self._resolve_rules_set where appropriate. This causes other misbehaviour, too:

def test_rules_set_simple_with_none_value():
    rules_set_registry.add('foo', {'type': 'integer', 'nullable': True})
    assert_success({'bar': None}, {'bar': 'foo'})

This will fail because code in __normalize_default_fields will do schema['bar'].get('nullable', False) - which will fail because schema['bar'] will be the string 'foo'.

Is this actually a bug or am I missing something? Are rules sets just meant to be used for validation rules?

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Nov 12, 2016

it's a bug, the same is probably true for schema_registry.

@nicolaiarocci nicolaiarocci added the bug label Nov 14, 2016

@nicolaiarocci nicolaiarocci added this to the Unreleased milestone Nov 14, 2016

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Dec 6, 2016

i'm itchy for a bug-fixing release, @dkellner are you on this issue?

@dkellner

This comment has been minimized.

Copy link
Contributor

dkellner commented Dec 7, 2016

No, I'm not currently working on this and I'm afraid I will not be able to spend time on this the next couple of weeks.

funkyfuture added a commit to funkyfuture/cerberus that referenced this issue Jan 4, 2017

funkyfuture added a commit to funkyfuture/cerberus that referenced this issue Jan 4, 2017

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Jan 5, 2017

Solved with #294

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