[Patch] Split up @validate

[sluggo]

@validate does several different things which all work together only in a certain form scenario. Sometimes you need to do this in the middle of the action, or to skip parts of it, or to do parts of it intermixed with other stuff. One example is when you're validating against a database record ('state'), but the record isn't known or available until the action has started. Users can copy the @validate code into their actions, but it would be better to provide something more modular. Otherwise people feel bad about bypassing @validate because it's the "standard" way, and they're afraid of missing out on features they may want later or that Pylons might add someday.

[jonathan]

I've started work on a patch for this.

I put it on the cookbook:

 http://wiki.pylonshq.com/display/pylonscookbook/Hacking+validate+into+a+split+functionality

It needs quite a bit of work still, but this turns @validate into a wrapper for separate validate_form and validation_error actions.

[sluggo]

See what would be needed to make @validate compatible with FormAlchemy, or to make a parallel @fa_validate.

[sluggo]

jonathan writes: "its very buggy code that was written before i understood how form_errors works i think on my next pass, i'd make sure that there's alawys a form_errors dict in c.... it would just be empty if unset."

[sluggo]

Mike Bayer has an alternative suggestion integrating Mako:  http://techspot.zzzeek.org/?p=28

[sluggo]

Tycon has a third approach in  http://groups.google.com/group/pylons-discuss/browse_thread/thread/14add6ac027cb43c

[Mike Orr]

The 'post_only' and 'on_get' arguments are two of the other problems with @validate. They seem intended to address a security situation which is not fully described in the documentation (to prevent people from adding query parameters that override form values, which may or may not be worth worrying about), but simultaneously introduces a bigger security hole (GET requests are not validated at all -- why would you ever want that? Maybe if you're doing form display and form validation in the same method, but the standard Pylons pattern does not.)

[jonathan]

I redid my approach recently...

the code is in the pastebin:

http://pylonshq.com/pasties/4fb8ce88c6d42b2ee048cf0ef0e678c1

the usage is in pylons-discuss list:

 http://groups.google.com/group/pylons-discuss/browse_thread/thread/4269ca745e31793

this is fully backwards compatible, and also lets you easily 'cause' new errors and retrigger the form.

[Ben Bangert]

I'm rather tempted to go with Marcus's idea:  http://marcuscavanaugh.com/posts/pylons-django-forms/

Though of course it kind of sucks to need Django as a dependency just to do forms. I do like the idiom it uses (I've used the same idiom in the past with FormEncode?, but its messier), and FormEncode? does seem to be getting a bit crufty. Course, I like that FormEncode? sticks to one thing, and does it... mostly well (is there any docs anywhere on how the hell to write fancy validators vs partial validators, etc???).

Maybe get the django.forms stuff forked off of Django? Would Marcus be up for maintaining just a django.forms fork? ;)

[Ben Bangert]

Right, so I think changing @validate pre-1.0 will screw up too much. I think a cleaner idiom alltogether is needed, since validation requires so many options we're weighed down with a massive @validate with a dozen keyword options that's just horrid.

@validate should be deprecated post-1.0 when a cleaner way to utilize FormEncode? in a controller is completed.

[Mike Burrows]

I've a refactored @validate at [1] that adds JSON support and also supports this decorator-free idiom discussed with Mike Orr on pylons-discuss [2] (in fact the new decorator body looks quite similar to this):

    def update(self):
        try:
            self._parse(request, schema=LoginForm())
            # Here self.form_result is populated as normal
            # as is self.params, the decoded but unconverted form params or
            # JSON request body.  You could do more stuff here that raises
            # formencode.Invalid, validated model updates for example.
        except Invalid as e:
            return self._render_invalid(e, form='edit')
        # Do more stuff with self.form_result, e.g.
        if accepts_json():
            return render_json(json_serialisable_thing_made_from_form_result)
        else:
            return render('template')

The JSON support works nicely with my {.format} syntax for optional format extensions [3] in route paths but equally it should work with map.resource()-style routes too (in fact anything that may or may not have a "format" parameter).

Broken up like this, the use of other form validator components as proposed by others is not precluded. And it offers backward compatibility.

* [1]  http://gist.github.com/302617 * [2]  http://groups.google.com/group/pylons-discuss/browse_thread/thread/927f4367d5367fc2 * [3]  http://bitbucket.org/asplake/routes/changeset/b7fac4c806e6/ and  http://bitbucket.org/asplake/routes/changeset/be380cd90cd7/

[Ian Wilson]

I'm just going to put my 2 cents in here. I think validate should be launched into space. People always try it first and it never works and they leave confused.

My experience with validation has lead me to believe that the complexity of validation can only be solved with a class rather than a function or a function decorator. A bunch of things Mike Burrows points out are probably right but I don't think validate is the right place to solve them. Although our solutions are looking more and more similar. Also validate or a form handling solution should not speak for an entire controller since more than 1 form could be handled by the methods of a single controller. A validation solution should not involve any of the following explicitly: 1. routes, 2. htmlfill, 3. json. All these should be solved with hooks either over top of a core solution for reuse or by the end user(developer) themselves.

These all could be added by extending a minimal core.

Core Solution:  http://bitbucket.org/ianjosephwilson/formprocess/src/tip/formprocess/handler.py

Extended for pylons  http://bitbucket.org/ianjosephwilson/formprocess/src/tip/formprocess/pylonshandler.py

Extended for use in controller:  http://bitbucket.org/ianjosephwilson/demoformprocess/src/tip/demoformprocess/controllers/login.py

Granular usage of htmlfill in the template:  http://bitbucket.org/ianjosephwilson/demoformprocess/src/tip/demoformprocess/templates/login.mako#cl-65

Note that json support for obtaining params for the input could be added in fetch_form_params(). Also since no actual filling is going on json support does not need to be addressed for the output.

The usage of htmlfill in the template could be pulled back into the controller to parse the entire template or to return json and not use htmlfill at all. I just see the above usage as the best practice.

A weakness in this solution right now is that end_process might want to jump back if an error occurs and redisplay the form. This could be handled with an exception generated in a method of the handler itself. So that within end_process one could do something like raise self.error_during_processing(errors) which would be caught in process. There already is a hook to raise custom errors after formencode has finished but it still is possible that format based errors could occur even later.

[Mike Burrows]

Ian, I think we are in close agreement. As per that thread on pylons-discuss, validation needs to be broken up in to pieces that have well-defined extension points. That almost certainly means that they are methods of classes; the next question is "which classes?". I just demonstrated that it is possible with a straightforward refactoring - which may only go to reinforce the idea that the decorator was never a great idea in the first place!

A validation solution should not involve any of the following explicitly 1. routes, 2. htmlfill, 3. json

Yes (and in my own projects I hide htmlfill behind helpers), but succinct idioms (and even explicit framework support) that support multiple formats (html and json being the obvious initial candidates) would benefit the Pylons community and potential adopters significantly. This is too common a problem for it to be left to every project to work out a unique solution - that's just wasteful. As it is, I'm quite sure I'm replicating work already done by others.