[Twisted-Python] Refactoring GetChildWithDefault (removal is ideal)

[Clark C. Evans]

First, before I get started, let me just say that I think that
the resource delegation mechanism in this library is just 
brilliant in its simplicity and operation.  

However, after much musing, I've decided that getChildWithDefault
isn't very useful and kinda mucks up the waters:

    # public interface
    def getChild(self, path, request):
        return error.NoResource("No such child resource.")
    
    # private interface
    def getChildWithDefault(self, path, request):
        if self.children.has_key(path):
           return self.children[path]
        return self.getChild(path, request)
    def getChildForRequest(self, request):
        res = self
        while request.postpath and not res.isLeaf:
            pathElement = request.postpath.pop(0)
            request.acqpath.append(pathElement)
            request.prepath.append(pathElement)
            res = res.getChildWithDefault(pathElement, request)
        return res

Suggested refactor:

    # module variables
    resourceNotFound = error.NoResource('No such child resource.')

        # public interface
    def getChild(self, path, request):
        if self.children.has_key(path):
           return self.children[path]
        return None

        # private interface (called on root only)
    def getChildForRequest(self, request):
        res = self
        while request.postpath and not res.isLeaf:
            pathElement = request.postpath.pop(0)
            request.acqpath.append(pathElement)
            request.prepath.append(pathElement)
            res = res.getChild(pathElement, request)
            if res is None: return resourceNotFound
        return res

Rationale:

   1.  It is very useful to have a *public* interface function 
       which is _always_ called for every request.  In this manner,
       an application can implement request modifiers/filters.
       Currently the function that satisfies this need, 
       getChildWithDefault is private.

   2.  Unless you break the public interface, the current mechanism
       always searches children first without a hook for the 
       application.  This isn't always desireable.

       For example, a 'security' FilterResource may want to check 
       user access before descending down a given resource sub-tree.  
       Yes, you could implement this security as part of each 
       resource (by inheriting); but I feel that this is inferior
       to haveing a more "component" based solution where the
       security filter is injected into the resource tree.

   3.  From a object-oriented perspective, getChildWithDefault 
       actually does the 'default' behavior that people may want
       to inherit and discard, and thus this default searching
       code should go into getChild instead; the user can then
       decide how to best use this default behavior.

   4.  getChild's current interface, always returning a resource,
       albeit a not-very-useful resource limits possible innovative
       combinations of intra-resource delegation and cooperation.
       It should intead return a None value which can be tested for...

Impact on change:

    Anyone who wrote a previous resource who dependend on the
    set of children being searched *before* getChild is called
    would break.    I think that this is probably a pretty
    rare event; but it is a clean break, and the fix is simple...

    class MyResource(Resource):
        def getChild(self,path,request):
            res = Resource.getChild(self,path,request)
            if res is None:
                // try to create a dynamic resource
            return None

    Alternatively, if they wanted to search the dynamic 
    resources first, they could code it this way:

    class MyResource(Resource):
        def getChild(self,path,resource):
            res = None
            // try to create dynamic resource
            if res is not None: return res
            return Resource.getChild(self,path,request)

    Perhaps a few examples would have to be changed, but most
    likely the above impact is in only a few select resources.

Alternative refactor:

    The simplest alternative is to add getChildWithDefault to the 
    public interface and document the mechanism.  It think that this,
    in the long run is not as good as the proposed refactor since 
    it adds extra complexity for the "search children first or last"
    behavior choice.  It's just clunky the way it is, IMHO.

In any case, the Resource finding mechanism in Twisted is very
clever, and I'm using my PathArgs *alot* so I'd like a solution
so that my requirements don't require a breaking the public
interface.  Ohh, and to answer:

On Thu, Feb 27, 2003 at 05:51:02PM +0100, Mario Ruggier wrote:
| A source of confusion for me is knowing which,
| and when, specific methods are called automatically.
| Particularly, it would be nice to have a clarification (in the
| API docs) of when the methods getChildForRequest() and
| getChildWithDefault() are called -- they seem not be called in
| a non-siteroot resource. Things worked well with PathArgs, it
| being set as root resource, but for an arbitrary resource,
| like the example I previoulsy included, the game seems to
| change .

getChildWithDefault is infact called on non-siteroot resources,
so the current code for PathArgs will work at any level... albeit
a violation of the private/public encapsulation.  But yes, the
overall mechanism (by having a 3rd wheel) is less than ideal.

Best,

Clark