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 #389 (closed defect: fixed)

Opened 13 years ago

Last modified 12 years ago

always run defined validators in the controller

Reported by: ischenko@… Owned by: anonymous
Priority: normal Milestone: 0.9
Component: CherryPy Version:
Severity: normal Keywords:
Cc:

Description

Current code that runs validators first check whether given field exists in request parameter map and if it doesn't exist the validator is skipped. This may lead to counter-intuitive results.

Example:

    @turbogears.expose(validators = {
                'page':validators.Int(if_empty=0),
            })
    def index(self, **kw):
           # here I'd assume kw[page] always exists 
           # and is 0 unless specified by the user

The following patch fixes this behavior:

Index: turbogears/controllers.py
========================================================
--- turbogears/controllers.py	(revision 506)
+++ turbogears/controllers.py	(working copy)
@@ -119,10 +119,8 @@
             if validators:
                 if isinstance(validators, dict):
                     for field, validator in validators.items():
-                        if field not in kw:
-                            continue
                         try:
-                            kw[field] = validator.to_python(kw[field])
+                            kw[field] = validator.to_python(kw.get(field))
                         except turbogearsvalid.Invalid, error:
                             errors[field] = error
                 else:

Change History

comment:1 Changed 13 years ago by anonymous

  • Milestone set to 0.9

comment:2 Changed 13 years ago by kevin

I agree that this case can be counter intuitive. However, this is here to avoid other counter intuitive behavior:

def myfunc(self, age=10):
    pass

Having age not come in is still valid. Perhaps you're right, though, that the value should go on the validator.

comment:3 Changed 13 years ago by kevin

  • Summary changed from [PATCH]: always run defined validators in the controller to always run defined validators in the controller

As part of another ticket, I'm suggesting that we make sure the original function is available to additional decorators. The upshot of this is that we can introspect the function to see if there's a default value. If there is, I think we should allow the method's default to kick in if the request did not include a value. If there isn't a default value on the function, then all validators should be run as suggested in this ticket. Doing that would, I think, eliminate the confusion. (The only confusion might be if you have a default in both places. What I'm describing would give the method signature preference.)

Removing the patch tag for now.

comment:4 Changed 13 years ago by anonymous

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

I see the code has been commited in r617 changeset.

Note: See TracTickets for help on using tickets.