filetree: put SubTreeMaker and NodeMaker in separate classes

src/allmydata/filetree/interfaces.py

@@ -11,17 +11,17 @@ class INode(Interface):
     def serialize_node():
         """Return a data structure which contains enough information to build
         this node again in the future (by calling
-        vdrive.make_node_from_serialized(). For IDirectoryNodes, this will be
+        INodeMaker.make_node_from_serialized(). For IDirectoryNodes, this
-        a list. For all other nodes this will be a string of the form
+        will be a list. For all other nodes this will be a string of the form
         'prefix:body', where 'prefix' must be the same as the class attribute
         .prefix ."""
     def populate_node(body, node_maker):
-        """vdrive.make_node_from_serialized() will first use the prefix from
+        """INodeMaker.make_node_from_serialized() will first use the prefix
-        the .prefix attribute to decide what kind of Node to create. They
+        from the .prefix attribute to decide what kind of Node to create.
-        will then call this populate_node() method with the body to populate
+        They will then call this populate_node() method with the body to
-        the new Node. 'node_maker' provides INodeMaker, which provides that
+        populate the new Node. 'node_maker' provides INodeMaker, which
-        same make_node_from_serialized function to create any internal child
+        provides that same make_node_from_serialized function to create any
-        nodes that might be necessary."""
+        internal child nodes that might be necessary."""
 
 class IFileNode(Interface):
     """This is a file which can be retrieved."""
@@ -74,14 +74,14 @@ class ISubTree(Interface):
 
     # All ISubTree-providing instances must have a class-level attribute
     # named .node_class which references the matching INode-providing class.
-    # This is used by the Opener to turn nodes into subtrees.
+    # This is used by the ISubTreeMaker to turn nodes into subtrees.
 
     def populate_from_node(node, parent_is_mutable, node_maker, downloader):
-        """Subtrees are created by opener.open() being called with an INode
+        """Subtrees are created by ISubTreeMaker.open() being called with an
-        which describes both the kind of subtree to be created and a way to
+        INode which describes both the kind of subtree to be created and a
-        obtain its contents. open() uses the node to create a new instance of
+        way to obtain its contents. open() uses the node to create a new
-        the appropriate subtree type, then calls this populate_from_node()
+        instance of the appropriate subtree type, then calls this
-        method.
+        populate_from_node() method.
 
         Each subtree's populate_from_node() method is expected to use the
         downloader to obtain a file with the subtree's serialized contents
@@ -195,80 +195,29 @@ class ISubTree(Interface):
 
 class INodeMaker(Interface):
     def make_node_from_serialized(serialized):
-        """Turn a string into an INode, which contains information about
+        """Turn a string into an INode, which contains information about the
-        the file or directory (like a URI), but does not contain the actual
+        file or directory (like a URI), but does not contain the actual
-        contents. An IOpener can be used later to retrieve the contents
+        contents. An ISubTreeMaker can be used later to retrieve the contents
-        (which means downloading the file if this is an IFileNode, or
+        (which means downloading the file if this is an IFileNode, or perhaps
-        perhaps creating a new subtree from the contents)."""
+        creating a new subtree from the contents)."""
 
 class ISubTreeMaker(Interface):
     def make_subtree_from_node(node, parent_is_mutable):
-        """Turn an INode into an ISubTree (using an internal opener to
+        """Turn an INode into an ISubTree.
-        download the data, if necessary).
+
-        This returns a Deferred that fires with the ISubTree instance.
+        I accept an INode-providing specification of a subtree, and return a
+        Deferred that fires with an ISubTree-providing instance. I will
+        perform network IO and download the serialized data that the INode
+        references, if necessary, or ask the queen (or other provider) for a
+        pointer, or read it from local disk.
         """
 
-#class IMutableSubTree(Interface):
-#    def mutation_affects_parent():
-#        """This returns True for CHK nodes where you must inform the parent
-#        of the new URI each time you change the child subtree. It returns
-#        False for SSK nodes (or other nodes which have a pointer stored in
-#        some mutable form).
-#        """
-#
-#    def add_subpath(subpath, child_spec, work_queue):
-#        """Ask this subtree to add the given child to an internal node at the
-#        given subpath. The subpath must not exit the subtree through another
-#        subtree (specifically get_subtree_for_path(subpath) must either
-#        return None or (True,node), and in the latter case, this subtree will
-#        create new internal nodes as necessary).
-#
-#        The subtree will probably serialize itself to a file and add steps to
-#        the work queue to accomplish its goals.
-#
-#        This returns a Deferred (the value of which is ignored) when
-#        everything has been added to the work queue.
-#        """
-#
-#    def serialize_to_file(f):
-#        """Write a bencoded data structure to the given filehandle that can
-#        be used to reproduce the contents of this subtree."""
-#
-#class ISubTreeSpecification(Interface):
-#    def serialize():
-#        """Return a tuple that describes this subtree. This tuple can be
-#        passed to IOpener.open() to reconstitute the subtree. It can also be
-#        bencoded and stuffed in a series of persistent bytes somewhere on the
-#        mesh or in a file."""
-
-class IOpener(Interface):
-    def open(subtree_node, parent_is_mutable, node_maker):
-        """I can take an INode-providing specification of a subtree and
-        return a Deferred which fires with an instance that provides ISubTree
-        (and maybe even IMutableSubTree). I probably do this by performing
-        network IO: reading a file from the mesh, or from local disk, or
-        asking some central-service node for the current value."""
-
 
 class IVirtualDrive(Interface):
 
     def __init__(workqueue, downloader, root_node):
         pass
 
-    # internal methods
-
-    def make_node_from_serialized(serialized):
-        """Given a string produced by original_node.serialize_node(), produce
-        an equivalent node.
-        """
-    def make_subtree_from_node(node, parent_is_mutable):
-        """Given an INode, create an ISubTree.
-
-        This returns a Deferred that fires (with the new subtree) when the
-        subtree is ready for use. This uses an IOpener to download the
-        subtree data, if necessary.
-        """
-
     # commands to manipulate files
 
     def list(path):

src/allmydata/filetree/opener.py

@@ -1,77 +0,0 @@
-
-from zope.interface import implements
-from twisted.internet import defer
-from allmydata.filetree import interfaces, directory, redirect
-#from allmydata.filetree.file import CHKFile, MutableSSKFile, ImmutableSSKFile
-from allmydata.filetree.interfaces import INode, IDirectoryNode, INodeMaker
-
-all_openable_subtree_types = [
-    directory.LocalFileSubTree,
-    directory.CHKDirectorySubTree,
-    directory.SSKDirectorySubTree,
-    redirect.LocalFileRedirection,
-    redirect.QueenRedirection,
-    redirect.QueenOrLocalFileRedirection,
-    redirect.HTTPRedirection,
-    ]
-
-# the Opener can turn an INode (which describes a subtree, like a directory
-# or a redirection) into the fully-populated subtree.
-
-class Opener(object):
-    implements(interfaces.IOpener)
-    def __init__(self, queen, downloader):
-        self._queen = queen
-        self._downloader = downloader
-        self._cache = {}
-
-    def _create(self, node, parent_is_mutable, node_maker):
-        assert INode(node)
-        assert INodeMaker(node_maker)
-        for subtree_class in all_openable_subtree_types:
-            if isinstance(node, subtree_class.node_class):
-                subtree = subtree_class()
-                d = subtree.populate_from_node(node,
-                                               parent_is_mutable,
-                                               node_maker,
-                                               self._downloader)
-                return d
-        raise RuntimeError("unable to handle subtree specification '%s'"
-                           % (node,))
-
-    def open(self, node, parent_is_mutable, node_maker):
-        assert INode(node)
-        assert not isinstance(node, IDirectoryNode)
-        assert INodeMaker(node_maker)
-
-        # is it in cache? To check this we need to use the node's serialized
-        # form, since nodes are instances and don't compare by value
-        node_s = node.serialize_node()
-        if node_s in self._cache:
-            return defer.succeed(self._cache[node_s])
-
-        d = defer.maybeDeferred(self._create,
-                                node, parent_is_mutable, node_maker)
-        d.addCallback(self._add_to_cache, node_s)
-        return d
-
-    def _add_to_cache(self, subtree, node_s):
-        self._cache[node_s] = subtree
-        # TODO: remove things from the cache eventually
-        return subtree
-
-"""
-    def _get_chk_file(self, spec):
-        subtree = CHKFile(spec.get_uri())
-        return defer.succeed(subtree)
-
-    def _get_ssk_file(self, spec):
-        if isinstance(spec, fspec.MutableSSKFileSpecification):
-            subtree = MutableSSKFile(spec.get_read_capability(),
-                                     spec.get_write_capability())
-        else:
-            assert isinstance(spec, fspec.ImmutableSSKFileSpecification)
-            subtree = ImmutableSSKFile(spec.get_read_cap())
-        return defer.succeed(subtree)
-
-"""

src/allmydata/filetree/vdrive.py

@@ -1,16 +1,18 @@
 
 from zope.interface import implements
-from allmydata.filetree import opener, directory, file, redirect
+from twisted.internet import defer
+from allmydata.filetree import directory, file, redirect
 from allmydata.filetree.interfaces import (
-    IVirtualDrive, INodeMaker, INode, ISubTree, IFileNode, IDirectoryNode,
+    IVirtualDrive, ISubTreeMaker,
+    INodeMaker, INode, ISubTree, IFileNode, IDirectoryNode,
     NoSuchDirectoryError, NoSuchChildError, PathAlreadyExistsError,
     PathDoesNotExistError,
     )
 from allmydata.upload import IUploadable
 
-# this list is used by VirtualDrive.make_node_from_serialized() to convert
+# this list is used by NodeMaker to convert node specification strings (found
-# node specification strings (found inside the serialized form of subtrees)
+# inside the serialized form of subtrees) into Nodes (which live in the
-# into Nodes (which live in the in-RAM form of subtrees).
+# in-RAM form of subtrees).
 all_node_types = [
     directory.LocalFileSubTreeNode,
     directory.CHKDirectorySubTreeNode,
@@ -23,27 +25,15 @@ all_node_types = [
     redirect.QueenOrLocalFileRedirectionNode,
 ]
 
-class VirtualDrive(object):
+class NodeMaker(object):
-    implements(IVirtualDrive, INodeMaker)
+    implements(INodeMaker)
 
-    def __init__(self, workqueue, downloader, root_node):
-        assert INode(root_node)
-        self.workqueue = workqueue
-        workqueue.set_vdrive(self)
-        # TODO: queen?
-        self.queen = None
-        self.opener = opener.Opener(self.queen, downloader)
-        self.root_node = root_node
-
-    # these are called when loading and creating nodes
-
-    # INodeMaker
     def make_node_from_serialized(self, serialized):
         # this turns a string into an INode, which contains information about
         # the file or directory (like a URI), but does not contain the actual
-        # contents. An IOpener can be used later to retrieve the contents
+        # contents. An ISubTreeMaker can be used later to retrieve the
-        # (which means downloading the file if this is an IFileNode, or
+        # contents (which means downloading the file if this is an IFileNode,
-        # perhaps creating a new subtree from the contents)
+        # or perhaps creating a new subtree from the contents)
 
         # maybe include parent_is_mutable?
         assert isinstance(serialized, str)
@@ -56,15 +46,78 @@ class VirtualDrive(object):
                 return node
         raise RuntimeError("unable to handle node type '%s'" % prefix)
 
-    # ISubTreeMaker
+all_openable_subtree_types = [
+    directory.LocalFileSubTree,
+    directory.CHKDirectorySubTree,
+    directory.SSKDirectorySubTree,
+    redirect.LocalFileRedirection,
+    redirect.QueenRedirection,
+    redirect.QueenOrLocalFileRedirection,
+    redirect.HTTPRedirection,
+    ]
+
+class SubTreeMaker(object):
+    implements(ISubTreeMaker)
+
+    def __init__(self, queen, downloader):
+        # this is created with everything it might need to download and
+        # create subtrees. That means a Downloader and a reference to the
+        # queen.
+        self._queen = queen
+        self._downloader = downloader
+        self._node_maker = NodeMaker()
+        self._cache = {}
+
+    def _create(self, node, parent_is_mutable):
+        assert INode(node)
+        assert INodeMaker(self._node_maker)
+        for subtree_class in all_openable_subtree_types:
+            if isinstance(node, subtree_class.node_class):
+                subtree = subtree_class()
+                d = subtree.populate_from_node(node,
+                                               parent_is_mutable,
+                                               self._node_maker,
+                                               self._downloader)
+                return d
+        raise RuntimeError("unable to handle subtree specification '%s'"
+                           % (node,))
+
     def make_subtree_from_node(self, node, parent_is_mutable):
         assert INode(node)
-        return self.opener.open(node, parent_is_mutable, self)
+        assert not isinstance(node, IDirectoryNode)
+
+        # is it in cache? To check this we need to use the node's serialized
+        # form, since nodes are instances and don't compare by value
+        node_s = node.serialize_node()
+        if node_s in self._cache:
+            return defer.succeed(self._cache[node_s])
+
+        d = defer.maybeDeferred(self._create, node, parent_is_mutable)
+        d.addCallback(self._add_to_cache, node_s)
+        return d
+
+    def _add_to_cache(self, subtree, node_s):
+        self._cache[node_s] = subtree
+        # TODO: remove things from the cache eventually
+        return subtree
+
+
+class VirtualDrive(object):
+    implements(IVirtualDrive)
+
+    def __init__(self, workqueue, downloader, root_node):
+        assert INode(root_node)
+        self.workqueue = workqueue
+        workqueue.set_vdrive(self)
+        # TODO: queen?
+        self.queen = None
+        self.root_node = root_node
+        self.subtree_maker = SubTreeMaker(self.queen, downloader)
 
     # these methods are used to walk through our subtrees
 
     def _get_root(self):
-        return self.make_subtree_from_node(self.root_node, False)
+        return self.subtree_maker.make_subtree_from_node(self.root_node, False)
 
     def _get_node(self, path):
         d = self._get_closest_node(path)
@@ -94,7 +147,7 @@ class VirtualDrive(object):
             # traversal done
             return (node, remaining_path)
         # otherwise, we must open and recurse into a new subtree
-        d = self.make_subtree_from_node(node, parent_is_mutable)
+        d = self.subtree_maker.make_subtree_from_node(node, parent_is_mutable)
         def _opened(next_subtree):
             next_subtree = ISubTree(next_subtree)
             return self._get_closest_node_1(next_subtree, remaining_path)

src/allmydata/test/test_filetree_new.py

@@ -339,7 +339,7 @@ class Stuff(unittest.TestCase):
         # TODO: we only need this VirtualDrive for the opener. Perhaps
         # make_subtree_from_node should move out of that class and into a
         # module-level function.
-        v = self.makeVirtualDrive("test_filetree_new/testDirectory")
+        stm = vdrive.SubTreeMaker(None, None)
 
         # create an empty directory (stored locally)
         subtree = directory.LocalFileSubTree()
@@ -399,7 +399,7 @@ class Stuff(unittest.TestCase):
         d = defer.maybeDeferred(subtree.update_now, None)
         def _updated(node):
             # now reconstruct it
-            return v.make_subtree_from_node(node, False)
+            return stm.make_subtree_from_node(node, False)
         d.addCallback(_updated)
 
         def _opened(new_subtree):