DataAccessException for null fields submitted

[prologic]

Based on the code in my fork of yota_examples (https://github.com/therealprologic/yota_examples/tree/server/src/circuits_examples) an empty form submission crashes with:

Internal Server Error

ERROR: (<class 'yota.exceptions.DataAccessException'>) Node first cannot find name first in submission data.
  File "/home/prologic/work/circuits-dev/circuits/core/manager.py", line 547, in _dispatcher
    value = handler(event, *eargs, **ekwargs)
  File "/home/prologic/work/circuits-dev/circuits/web/controllers.py", line 36, in wrapper
    return f(self, *args, **kwargs)
  File "./example.py", line 181, in basic
    success, out = form.validate_render(kwargs)
  File "/home/prologic/work/yota/src/yota/__init__.py", line 522, in validate_render
    block, invalid = self._gen_validate(data)
  File "/home/prologic/work/yota/src/yota/__init__.py", line 373, in _gen_validate
    node.resolve_data(data)
  File "/home/prologic/work/yota/src/yota/nodes.py", line 151, in resolve_data
    "submission data.".format(self._attr_name, self.name))

Fields should probably default to None or "" (empty string) if not found in the submitted form data?

cheers James

[icook]

This was actually a design decision, however I'm not opposed to changing the default. At first glance this appears to occur because circuits does not populate keys for empty POST data? Admittedly I'm not particularly familiar with circuits.

[prologic]

Circuits.wrb uses some pretty standard libs. Yes this is correct. Ill investigate further and see eho needs to change,

Sent from my iPad

On 28/08/2013, at 5:55, Isaac Cook notifications@github.com wrote:

This was actually a design decision, however I'm not opposed to changing the default. At first glance this appears to occur because circuits does not populate keys for empty POST data? Admittedly I'm not particularly familiar with circuits.

— Reply to this email directly or view it on GitHubhttps://github.com/icook/yota/issues/74#issuecomment-23365545 .

[ericecook]

imo, probably the default should be changed. Although designed to catch user errors, various things will cause the form not to submit POST data for a node, such as disabled form elements

[prologic]

I agree. As i said circuits.web uses some std. libs to parse x-www-url-encoded data and by design i believe the std. libs leave out keys that contain no data. Ill double check when I'm in front of my desktop.

Sent from my iPad

On 28/08/2013, at 7:51, ericecook notifications@github.com wrote:

imo, probably the default should be changed. Although designed to catch user errors, various things will cause the form not to submit POST data for a node, such as disabled form elements

— Reply to this email directly or view it on GitHubhttps://github.com/icook/yota/issues/74#issuecomment-23373966 .

[icook]

I'll plan to try and patch it soon then. As eric said initially it was intended to catch user error and avoid confusing bugs, but there's just too many cases where the key won't be present in submission data for it to make sense.

[prologic]

Agreed.

Sent from my iPad

On 28/08/2013, at 8:17, Isaac Cook notifications@github.com wrote:

I'll plan to try and patch it soon then. As eric said initially it was intended to catch user error and avoid confusing bugs, but there's just too many cases where the key won't be present in submission data for it to make sense.

— Reply to this email directly or view it on GitHubhttps://github.com/icook/yota/issues/74#issuecomment-23375788 .

[prologic]

This is confirmed. parse_qsl which is what circuits.web's QueryStringParser uses defaults to the beheavior where null values are ommitted from the resulting dictionary. See:

~/circuits-dev
$ python
Python 2.7.5 (default, Jul 31 2013, 14:32:58) 
[GCC 4.2.1 Compatible Apple LLVM 4.2 (clang-425.0.28)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from urlparse import parse_qsl
>>> data = "a=&b=&c=1"
>>> parse_qsl(data)
[('c', '1')]
>>> 

For reference to circuits.web's code that performt this:

https://bitbucket.org/circuits/circuits-dev/src/tip/circuits/web/parsers/querystring.py#cl-32

Thanks for fixing this in yota! :)

cheers James

[prologic]

Btw... I could se keep_blank_values=True See:

parse_qsl(qs, keep_blank_values=0, strict_parsing=0)
    Parse a query given as a string argument.

    Arguments:

    qs: percent-encoded query string to be parsed

    keep_blank_values: flag indicating whether blank values in
        percent-encoded queries should be treated as blank strings.  A
        true value indicates that blanks should be retained as blank
        strings.  The default false value indicates that blank values
        are to be ignored and treated as if they were  not included.

    strict_parsing: flag indicating what to do with parsing errors. If
        false (the default), errors are silently ignored. If true,
        errors raise a ValueError exception.

    Returns a list, as G-d intended.

But again the default is not to. I'd have to change https://bitbucket.org/circuits/circuits-dev/src/tip/circuits/web/parsers/querystring.py#cl-32 just to suit yota's design assumption.

Thoughts?

cheers James

[prologic]

It's been recommended to my by a colleague that I change the behavior of circuits.web and set keep_blank_values=True in it's QueryStringParser.

diff --git a/circuits/web/parsers/querystring.py b/circuits/web/parsers/querystring.py
--- a/circuits/web/parsers/querystring.py
+++ b/circuits/web/parsers/querystring.py
@@ -30,7 +30,7 @@
         [self.process(x) for x in sorted_pairs]

     def _sorted_from_string(self, data):
-        stage1 = parse_qsl(data)
+        stage1 = parse_qsl(data, keep_blank_values=True)
         stage2 = [(x[0].strip(), x[1].strip()) for x in stage1]
         return sorted(stage2, key=lambda p: p[0])

I'm running unit tests now to ensure nothing breaks. I'll post back later if this is a suitable solution.

cheers James

[prologic]

  py27: commands succeeded
  py32: commands succeeded
  py33: commands succeeded
  congratulations :)

https://bitbucket.org/circuits/circuits-dev/commits/6ea458c3c2cb7c3858e25e2cf04093a6c1b6a03d

I think this issue can be safely closed.

cheers James

[prologic]

I've also updated my fork of yota_examples to show how yota can work with circuits.wbe

See: https://github.com/therealprologic/yota_examples

Would you guys be open to a pull request to include circuits.web examples?

cheers James

[ericecook]

Glad to hear you arrived at an acceptable solution! I'll leave this issue open for now because it is a default we will be patching.

Please feel free to make a pull request, examples for more frameworks would be great.

[prologic]

No worries! And Thanks!

cheers James

James Mills / prologic

E: prologic@shortcircuit.net.au W: prologic.shortcircuit.net.au

On Wed, Aug 28, 2013 at 11:19 AM, ericecook notifications@github.comwrote:

Glad to hear you arrived at an acceptable solution! I'll leave this issue open for now because it is a default we will be patching.

Please feel free to make a pull request, examples for more frameworks would be great.

— Reply to this email directly or view it on GitHubhttps://github.com/icook/yota/issues/74#issuecomment-23383700 .

[prologic]

I think this approach should work well :)