[Zope-Checkins] SVN: Zope/branches/2.13/src/ micro optimizations

Hanno Schlichting hanno at hannosch.eu
Sat Jul 30 08:14:24 EDT 2011


Hi Nikolay,

some small comments in-line:

On Sat, Jul 30, 2011 at 4:36 AM, Nikolay Kim <fafhrd91 at gmail.com> wrote:
> Log message for revision 122429:
>  micro optimizations
>
> Changed:
>  U   Zope/branches/2.13/src/OFS/PropertyManager.py
>  U   Zope/branches/2.13/src/OFS/Traversable.py
>  U   Zope/branches/2.13/src/ZPublisher/HTTPRequest.py
>
> Modified: Zope/branches/2.13/src/OFS/Traversable.py
> ===================================================================
> --- Zope/branches/2.13/src/OFS/Traversable.py   2011-07-29 19:11:59 UTC (rev 122428)
> +++ Zope/branches/2.13/src/OFS/Traversable.py   2011-07-30 02:36:10 UTC (rev 122429)
> @@ -39,6 +39,7 @@
>  from zope.traversing.namespace import nsParse
>
>  _marker = object()
> +NullResource = None

wouldn't it be simpler to do a normal module scope import of NullResource?

>  class Traversable:
> @@ -142,30 +143,29 @@
>         If true, then all of the objects along the path are validated with
>         the security machinery. Usually invoked using restrictedTraverse().
>         """
> -        from webdav.NullResource import NullResource
>         if not path:
>             return self
>
> -        if isinstance(path, str):
> +        if type(path) is str:
>             # Unicode paths are not allowed
>             path = path.split('/')
>         else:
>             path = list(path)
>
>         REQUEST = {'TraversalRequestNameStack': path}
> -        path.reverse()
> +        #path.reverse()

Please don't leave commented out code, just remove it - we have
version control to look at older versions ;)

>         path_pop = path.pop
>
> -        if len(path) > 1 and not path[0]:
> +        if len(path) > 1 and not path[-1]:
>             # Remove trailing slash
> -            path_pop(0)
> +            path_pop()
>
>         if restricted:
>             validate = getSecurityManager().validate
>
> -        if not path[-1]:
> +        if not path[0]:
>             # If the path starts with an empty string, go to the root first.
> -            path_pop()
> +            path_pop(0)
>             obj = self.getPhysicalRoot()
>             if restricted:
>                 validate(None, None, None, obj) # may raise Unauthorized
> @@ -256,6 +256,10 @@
>                                     # The item lookup may return a NullResource,
>                                     # if this is the case we save it and return it
>                                     # if all other lookups fail.
> +                                    global NullResource
> +                                    if NullResource is None:
> +                                        from webdav.NullResource import NullResource
> +
>                                     if isinstance(next, NullResource):
>                                         resource = next
>                                         raise KeyError(name)
>
> Modified: Zope/branches/2.13/src/ZPublisher/HTTPRequest.py
> ===================================================================
> --- Zope/branches/2.13/src/ZPublisher/HTTPRequest.py    2011-07-29 19:11:59 UTC (rev 122428)
> +++ Zope/branches/2.13/src/ZPublisher/HTTPRequest.py    2011-07-30 02:36:10 UTC (rev 122429)
> @@ -1525,7 +1517,7 @@
>                     base64.decodestring(auth.split()[-1]).split(':', 1)
>                 return name, password
>
> -    def taintWrapper(self, enabled=TAINTING_ENABLED):
> +    def taintWrapper(self, enabled=False):
>         return enabled and TaintRequestWrapper(self) or self
>
>     def shiftNameToApplication(self):

On the 2.13 branch we cannot change the tainting behavior - I believe
that method should still use the constant.

Hanno


More information about the Zope-Checkins mailing list