Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1105 (closed enhancement: wontfix)

Opened 13 years ago

Last modified 10 years ago

[patch] API change suggestions for @validate

Reported by: bjourne Owned by: anonymous
Priority: high Milestone: 1.x
Component: TurboGears Version: 0.9a6
Severity: normal Keywords:
Cc:

Description

I have some API change suggestions for the @validate decorator:

  1. Rename the validators parameter to schema. The code currently allows a dictionary to be passed. But that feature seem to not be used except for unit testing. It should be removed. This simplified implementation and documentation.
  2. Rename the Form class' validate() method to to_python(). The method has the same signature and behaviour as the to_python() method in FormEncode schemas and with this change duck typing becomes easier.
  3. Do not assign to cherrypy.request.validated_form, instead use cherrypy.request.validated_entity. With this change, you can assign to that attribute unconditionally after validation, not just after validation of a form. Once again, this simplified implementation and documentation.

With these proposed changes, the signature of the @validate decorator can change to:

def validate(schema, state_factory=None)

Where schema is either a TurboGears widget or a FormEncode Schema. This is much simpler and removes the special-casing for widgets.

Attachments

improved-api-for-validate-dec.patch Download (26.2 KB) - added by bjourne 13 years ago.
Big patch implementing most of the suggestions in this bug.

Change History

comment:1 Changed 13 years ago by bjourne

Also, I forgot:

  1. Do not assign to cherrypy.request.validation_exception, client code doesn't need to know about the Invalid exception.

Here is the @validate decorator with the suggestions implemented:

def validate2(validator, state_factory=None):
    """
    Decorates a function with validation.

    A decorator that validates parameters to the decorated function
    and converts them to Python types. Only the parameters that have
    the same names as fields in the schema are validated. Other
    parameters are left untouched.

    Validation results in four attributes of cherrypy.request being
    set.

    validation_errors: A dictionary containing names of the parameters
    that failed validation as keys and error messages for each failing
    field as values.

    input_values: A dictionary containing names of the parameters that
    failed validation as keys and the fields values as values.

    validated_entity: The validator that was passed to the validate
    decorator.

    validation_state: State of validation.

    @param validator Usually a subclass of formencode.Schema, but any
                     object that has a to_python() method which raises
                     formencode.api.Invalid exceptions work. A factory
                     function or class that creates an instance with a
                     to_python() method also works.
                     
    @param state_factory Callable which returns the initial state
                         instance for validation
    """
    def entangle(func):
        recursion_guard = dict(func=func)
        if callable(validator) and not hasattr(validator, "to_python"):
            init_validator = lambda self: validate(self)
        else:
            init_validator = lambda self: validate

        def validate(func, *args, **kw):
            if tg_util.call_on_stack("validate", recursion_guard, 4):
                return func(*args, **kw)
            validate = init_validator(args and args[0] or kw["self"])
            args, kw = tg_util.to_kw(func, args, kw)
            
            if state_factory is not None:
                state = state_factory()
            else:
                state = None

            errors = {}
            value = kw.copy()
            try:
                kw.update(validator.to_python(value, state))
            except Invalid, e:
                errors = e.unpack_errors()
                
            cherrypy.request.validation_errors = errors
            cherrypy.request.validated_entity = validator
            cherrypy.request.input_values = kw.copy()
            cherrypy.request.validation_state = state

            args, kw = tg_util.from_kw(func, args, kw)
            return errorhandling.run_with_errors(errors, func, *args, **kw)

        return validate
    return weak_signature_decorator(entangle)

Note that the code won't work for widgets because they don't have a to_python() method.

I also think that the @validate decorator should be changed further in the future so that only those parameters that exist in the widget/schema definition are validated. This change would fix the freakishly annoying "The input field 'self' was not expected" problem. It can be accomplished by changing the fields attribute of form widgets so that it works like FormEncodes? schemas. fields would become a dictionary where the keys are names of the contained widgets.

comment:2 Changed 13 years ago by alberto

  • Type changed from defect to enhancement

Some comments:

  • My client code *does* use request.validation_exception ;)
  • I agree that forcing validator to be a Schema really simplifies things and makes it easier to document. Though TG's glue code doesn't depend on it, I'm sure how many users depend on this in their apps...
  • If we use to_python to validate then validators which perform validation in the validate_python method will silently fail (If my memory isn't failing me). Duck typing is equally easy as we only need to check for 'validate' instead of 'to_python' in the validator's attrs.
  • Why wipe away the support for widgets? I mean, supporting them is as easy as keeping the form parameter and doing validator = form.validator before validating.
  • I'll be glad to hear of a way to remove the 'self' key from kw (plus other variables errorhandling introduces like 'tg_errors', 'recursion_guard', etc... before validating (somethiing prettier than hand-popping them out, please ;) ) and keep all the errorhandling tests passing.
  • I would rename request.validated_entity to request.validating_entity or even {{request.tg_validator}}} to make semantics clearer.
  • Patches are more easily merged than code snippets at a ticket ;)

Thanks, Alberto

comment:3 Changed 13 years ago by bjourne

Hi! I'm glad to hear you liked some of my suggestions!

  • Can you show some example code in which request.validation_exception is used? I can't see why you need it and the more attributes you assing to cherrypy.request the more rigid the function becomes.
  • I don't understand. validate_python isn't used at all by TG and shouldn't be affected by the name change validate -> to_python?
  • The suggested changes doesn't wipe away support for widgets. They just becomes more similar to FormEncode schemas which removes lots of special casing in the code. @validate(SomeWidget()) will still work. But @validate doesn't know it is a widget, it treats it just like any other FormEncode schema.
  • The idea is simple. You only validate those parameters from the function signature whos names exist as fields in the schema.
class MySchema(formencode.Schema):
    foo = validators.Int()
    bar = validators.Int()

@validate(MySchema())
def blah(self, foo, bar, moo = 33, goo = "o"):
    ...

Only the foo and bar parameters should be validated. This requires the validator to be able to tell you which fields it contain. FormEncodes? schemas have the fields attribute which is a dictonary where the keys are the names of the fields. TurboGears widgets doesn't work like that - their fields attribute is a list of widgets. But IMHO the widgets should be changed to match FormEncode's way of doing things.

Changed 13 years ago by bjourne

Big patch implementing most of the suggestions in this bug.

comment:4 Changed 12 years ago by jorge.vargas

  • Priority changed from normal to high

comment:5 Changed 12 years ago by alberto

  • Milestone changed from 1.1 to __unclassified__

Batch moved into unclassified from 1.1 to properly track progress on the later

comment:6 Changed 10 years ago by jorge.vargas

  • Milestone changed from __unclassified__ to 1.x
  • Summary changed from API change suggestions for @validate to [patch] API change suggestions for @validate

comment:7 Changed 10 years ago by Chris Arndt

  • Status changed from new to closed
  • Resolution set to wontfix

This won't be applied in TG 1.x. Using a dict for the validators argument to @validate is common usage now.

Closing ticket as wontfix.

Note: See TracTickets for help on using tickets.