evite / nudge Goto Github PK

Currently the URL decoding of the query string is performed before splitting into name-value pairs. This causes failures when a parameter value contains a properly encoded ampersand or equals sign. To alter this behavior the decoding of the name and value should take place when iterating over the arguments after splitting.

I am making this issue so we can discuss major upcoming changes to Nudge via a 1.0 release. Please see the google doc for details.

https://docs.google.com/document/d/1xEIfqFU-5dAcwe8_4x4mbdUic6GzaciOrt5edFItj4E/edit

It would be best practice for nudge to pass all strings as unicode strings to apps that use nudge.

From what I can tell, the only remaining place this doesn't happen is url encoded posts or puts. Our json decode converts to unicode. And args from the query string now end up as unicode.

I'm curious as to the purpose of the additional logging added by wrapping the ServicePublisher with the LoggingMiddleware class. When testing my services I see two log messages to the console for each request:

ERROR:root:200 GET http/localhost:4567//test_path (127.0.0.1) 0.00ms 127.0.0.1 - - [26/Jul/2011 16:24:40] "GET /test_path?q=test&restr=4 HTTP/1.1" 200 3512 0.031228

The first log message is the one added by the LoggingMiddleware instance and is always logged as an error. Is there a reason for the apparently redundant request logging?

If an endpoint's regular expression matches the URI, but the method is not supported, nudge should return 405. It will currently skip that endpoint, which can cause a lower regex to artificially match.

Josh's good idea:
Consider changing Nudge's default exception handling based on accept headers. Currently the last resort is always a json response, and in many cases we want to select HTML based on the client' accept headers.

Nudge should decode the full query string from the WSGI request before messing with the args.

IE:

'QUERY_STRING': 'f=%CE%8F',

Also, the query string really should become unicode.

The arg validator nudge.arg.Integer fails if it is defined as default=0, optional=True to to the logic in Arg.init.func. Currently the logic is:

if not data:
    if self.default:
        return self.default
    elif self.optional:
        return None

Shouldn't the conditional be on the optional parameter?

if not data:
    if self.optional:
        return self.default

This is definitely not pressing, and the usefulness of this feature is certainly questionable. However, I think there could be situations where you have 2 or more endpoints that need to do the exact same thing with the only difference being their uri regular expression.

In my initial use case for this, I was able to alter the regular expression to eliminate the need for this feature.

Therefore we probably shouldn't bother doing this unless someone can provide a concrete example where a regular expression isn't enough.

It could be easily implemented by adding an optional uris member to Endpoint, and allowing that param in the constructor. Then Publisher would need to check against all uris in all endpoints.

Possible problem with this:
If the endpoint's uris are very different, this could have adverse effects on the order in which publisher checks them. Typically endpoints need to be a list that starts at the very specific uris, and ends with more general ones. Having multiple uris in this fashion could cause problems.

The rationale for making such a feature is to avoid a lot of copy and pasting, which results in having almost identical items in more than one place.

ERROR: test_encode (tests.test_json.JsonTest)

Traceback (most recent call last):
File "/nudge/tests/test_json.py", line 80, in test_encode
data2 = json.json_encode(result)
File "/nudge/nudge/json.py", line 58, in json_encode
return _encoder.encode(o)
File "/python2.7/site-packages/simplejson/encoder.py", line 226, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/python2.7/site-packages/simplejson/encoder.py", line 296, in iterencode
return _iterencode(o, 0)
TypeError: attribute of type 'NoneType' is not callable

The cookie implementation was rushed on my part. We should enhance this to use the stdlib cookie stuff for python.

The beauty of nudge is we can just say i want a string arg, and nudge will find it where ever it may be (json post body, qstring params, etc). Why then do we ask nudge to only look in a specific place for cookies and headers?

I propose we combine all locations.

I thought of this today when trying to set a cookie. It is way too hard! This is definitely one thing common request aware webapps have on nudge.

I wonder if we can make some sort of renderer mixins so we can avoid having to copy the whole Json renderer just to add a header or cookie

If an argument of type string is not optional it should be validated.