interface name cleanups: IFileNode, IImmutableFileNode, IMutableFileNode

The proper hierarchy is:
 IFilesystemNode
 +IFileNode
 ++IMutableFileNode
 ++IImmutableFileNode
 +IDirectoryNode

Also expand test_client.py (NodeMaker) to hit all IFilesystemNode types.

src/allmydata/dirnode.py

@@ -8,8 +8,8 @@ import simplejson
 from allmydata.mutable.common import NotMutableError
 from allmydata.mutable.filenode import MutableFileNode
 from allmydata.unknown import UnknownNode
-from allmydata.interfaces import IMutableFileNode, IDirectoryNode,\
+from allmydata.interfaces import IFilesystemNode, IDirectoryNode, IFileNode, \
-     IFileNode, IFilesystemNode, \
+     IImmutableFileNode, IMutableFileNode, \
      ExistingChildError, NoSuchChildError, ICheckable, IDeepCheckable, \
      CannotPackUnknownNodeError
 from allmydata.check_results import DeepCheckResults, \
@@ -687,7 +687,7 @@ class DeepStats:
             self.add("count-mutable-files")
             # TODO: update the servermap, compute a size, add it to
             # size-mutable-files, max it into "largest-mutable-file"
-        elif IFileNode.providedBy(node): # CHK and LIT
+        elif IImmutableFileNode.providedBy(node): # CHK and LIT
             self.add("count-files")
             size = node.get_size()
             self.histogram("size-files-histogram", size)

src/allmydata/immutable/filenode.py

@@ -5,7 +5,7 @@ from twisted.internet import defer
 from twisted.internet.interfaces import IPushProducer, IConsumer
 from twisted.protocols import basic
 from foolscap.api import eventually
-from allmydata.interfaces import IFileNode, ICheckable, \
+from allmydata.interfaces import IImmutableFileNode, ICheckable, \
      IDownloadTarget, IUploadResults
 from allmydata.util import dictutil, log, base32
 from allmydata.uri import CHKFileURI, LiteralFileURI
@@ -15,7 +15,7 @@ from allmydata.immutable.repairer import Repairer
 from allmydata.immutable import download
 
 class _ImmutableFileNodeBase(object):
-    implements(IFileNode, ICheckable)
+    implements(IImmutableFileNode, ICheckable)
 
     def get_readonly_uri(self):
         return self.get_uri()
@@ -29,12 +29,12 @@ class _ImmutableFileNodeBase(object):
     def __hash__(self):
         return self.u.__hash__()
     def __eq__(self, other):
-        if IFileNode.providedBy(other):
+        if isinstance(other, _ImmutableFileNodeBase):
             return self.u.__eq__(other.u)
         else:
             return False
     def __ne__(self, other):
-        if IFileNode.providedBy(other):
+        if isinstance(other, _ImmutableFileNodeBase):
             return self.u.__eq__(other.u)
         else:
             return True

src/allmydata/interfaces.py

@@ -483,6 +483,13 @@ class UnhandledCapTypeError(Exception):
 class NotDeepImmutableError(Exception):
     """Deep-immutable directories can only contain deep-immutable children"""
 
+# The hierarchy looks like this:
+#  IFilesystemNode
+#   IFileNode
+#    IMutableFileNode
+#    IImmutableFileNode
+#   IDirectoryNode
+
 class IFilesystemNode(Interface):
     def get_cap():
         """Return the strongest 'cap instance' associated with this node.
@@ -563,10 +570,11 @@ class IFilesystemNode(Interface):
         data this node represents.
         """
 
-class IMutableFilesystemNode(IFilesystemNode):
-    pass
-
 class IFileNode(IFilesystemNode):
+    """I am a node which represents a file: a sequence of bytes. I am not a
+    container, like IDirectoryNode."""
+
+class IImmutableFileNode(IFileNode):
     def download(target):
         """Download the file's contents to a given IDownloadTarget"""
 
@@ -626,7 +634,7 @@ class IFileNode(IFilesystemNode):
 
         """
 
-class IMutableFileNode(IFileNode, IMutableFilesystemNode):
+class IMutableFileNode(IFileNode):
     """I provide access to a 'mutable file', which retains its identity
     regardless of what contents are put in it.
 
@@ -819,10 +827,11 @@ class ExistingChildError(Exception):
 class NoSuchChildError(Exception):
     """A directory node was asked to fetch a child which does not exist."""
 
-class IDirectoryNode(IMutableFilesystemNode):
+class IDirectoryNode(IFilesystemNode):
-    """I represent a name-to-child mapping, holding the tahoe equivalent of a
+    """I represent a filesystem node that is a container, with a
-    directory. All child names are unicode strings, and all children are some
+    name-to-child mapping, holding the tahoe equivalent of a directory. All
-    sort of IFilesystemNode (either files or subdirectories).
+    child names are unicode strings, and all children are some sort of
+    IFilesystemNode (either files or subdirectories).
     """
 
     def get_uri():
@@ -846,8 +855,8 @@ class IDirectoryNode(IMutableFilesystemNode):
     def list():
         """I return a Deferred that fires with a dictionary mapping child
         name (a unicode string) to (node, metadata_dict) tuples, in which
-        'node' is either an IFileNode or IDirectoryNode, and 'metadata_dict'
+        'node' is an IFilesystemNode (either IFileNode or IDirectoryNode),
-        is a dictionary of metadata."""
+        and 'metadata_dict' is a dictionary of metadata."""
 
     def has_child(name):
         """I return a Deferred that fires with a boolean, True if there
@@ -877,7 +886,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         name."""
 
     def get_child_at_path(path):
-        """Transform a child path into an IDirectoryNode or IFileNode.
+        """Transform a child path into an IFilesystemNode.
 
         I perform a recursive series of 'get' operations to find the named
         descendant node. I return a Deferred that fires with the node, or
@@ -888,8 +897,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         """
 
     def get_child_and_metadata_at_path(path):
-        """Transform a child path into an IDirectoryNode/IFileNode and
+        """Transform a child path into an IFilesystemNode and metadata.
-        metadata.
 
         I am like get_child_at_path(), but my Deferred fires with a tuple of
         (node, metadata). The metadata comes from the last edge. If the path
@@ -934,7 +942,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         when the operation finishes. This Deferred will fire with the child
         node that was just added. I will replace any existing child of the
         same name. The child name must be a unicode string. The 'child'
-        instance must be an instance providing IDirectoryNode or IFileNode.
+        instance must be an instance providing IFilesystemNode.
 
         If metadata= is provided, I will use it as the metadata for the named
         edge. This will replace any existing metadata. If metadata= is left
@@ -2068,10 +2076,10 @@ class IClient(Interface):
 
         @return: an instance that provides IFilesystemNode (or more usefully
                  one of its subclasses). File-specifying URIs will result in
-                 IFileNode or IMutableFileNode -providing instances, like
+                 IFileNode-providing instances, like ImmutableFileNode,
-                 ImmutableFileNode, LiteralFileNode, or MutableFileNode.
+                 LiteralFileNode, or MutableFileNode. Directory-specifying
-                 Directory-specifying URIs will result in
+                 URIs will result in IDirectoryNode-providing instances, like
-                 IDirectoryNode-providing instances, like DirectoryNode.
+                 DirectoryNode.
         """
 
 class INodeMaker(Interface):
@@ -2083,7 +2091,7 @@ class INodeMaker(Interface):
     or modify its contents.
 
     The NodeMaker encapsulates all the authorities that these
-    IfilesystemNodes require (like references to the StorageFarmBroker). Each
+    IFilesystemNodes require (like references to the StorageFarmBroker). Each
     Tahoe process will typically have a single NodeMaker, but unit tests may
     create simplified/mocked forms for testing purposes.
     """

src/allmydata/test/common.py

@@ -8,7 +8,7 @@ from twisted.web.error import Error as WebError
 from foolscap.api import flushEventualQueue, fireEventually
 from allmydata import uri, dirnode, client
 from allmydata.introducer.server import IntroducerNode
-from allmydata.interfaces import IMutableFileNode, IFileNode, \
+from allmydata.interfaces import IMutableFileNode, IImmutableFileNode, \
      FileTooLargeError, NotEnoughSharesError, ICheckable
 from allmydata.check_results import CheckResults, CheckAndRepairResults, \
      DeepCheckResults, DeepCheckAndRepairResults
@@ -32,9 +32,9 @@ def flush_but_dont_ignore(res):
     return d
 
 class FakeCHKFileNode:
-    """I provide IFileNode, but all of my data is stored in a class-level
+    """I provide IImmutableFileNode, but all of my data is stored in a
-    dictionary."""
+    class-level dictionary."""
-    implements(IFileNode)
+    implements(IImmutableFileNode)
     all_contents = {}
     bad_shares = {}
 

src/allmydata/test/test_client.py

@@ -9,7 +9,8 @@ from allmydata import client
 from allmydata.storage_client import StorageFarmBroker
 from allmydata.introducer.client import IntroducerClient
 from allmydata.util import base32, fileutil
-from allmydata.interfaces import IFilesystemNode, IFileNode, IDirectoryNode
+from allmydata.interfaces import IFilesystemNode, IFileNode, \
+     IImmutableFileNode, IMutableFileNode, IDirectoryNode
 from foolscap.api import flushEventualQueue
 import common_util as testutil
 
@@ -247,13 +248,44 @@ class NodeMaker(unittest.TestCase):
         n = c.create_node_from_uri("URI:CHK:6nmrpsubgbe57udnexlkiwzmlu:bjt7j6hshrlmadjyr7otq3dc24end5meo5xcr5xe5r663po6itmq:3:10:7277")
         self.failUnless(IFilesystemNode.providedBy(n))
         self.failUnless(IFileNode.providedBy(n))
+        self.failUnless(IImmutableFileNode.providedBy(n))
+        self.failIf(IMutableFileNode.providedBy(n))
         self.failIf(IDirectoryNode.providedBy(n))
         self.failUnless(n.is_readonly())
         self.failIf(n.is_mutable())
 
+        n = c.create_node_from_uri("URI:LIT:n5xgk")
+        self.failUnless(IFilesystemNode.providedBy(n))
+        self.failUnless(IFileNode.providedBy(n))
+        self.failUnless(IImmutableFileNode.providedBy(n))
+        self.failIf(IMutableFileNode.providedBy(n))
+        self.failIf(IDirectoryNode.providedBy(n))
+        self.failUnless(n.is_readonly())
+        self.failIf(n.is_mutable())
+
+        n = c.create_node_from_uri("URI:SSK:n6x24zd3seu725yluj75q5boaa:mm6yoqjhl6ueh7iereldqxue4nene4wl7rqfjfybqrehdqmqskvq")
+        self.failUnless(IFilesystemNode.providedBy(n))
+        self.failUnless(IFileNode.providedBy(n))
+        self.failIf(IImmutableFileNode.providedBy(n))
+        self.failUnless(IMutableFileNode.providedBy(n))
+        self.failIf(IDirectoryNode.providedBy(n))
+        self.failIf(n.is_readonly())
+        self.failUnless(n.is_mutable())
+
+        n = c.create_node_from_uri("URI:SSK-RO:b7sr5qsifnicca7cbk3rhrhbvq:mm6yoqjhl6ueh7iereldqxue4nene4wl7rqfjfybqrehdqmqskvq")
+        self.failUnless(IFilesystemNode.providedBy(n))
+        self.failUnless(IFileNode.providedBy(n))
+        self.failIf(IImmutableFileNode.providedBy(n))
+        self.failUnless(IMutableFileNode.providedBy(n))
+        self.failIf(IDirectoryNode.providedBy(n))
+        self.failUnless(n.is_readonly())
+        self.failUnless(n.is_mutable())
+
         n = c.create_node_from_uri("URI:DIR2:n6x24zd3seu725yluj75q5boaa:mm6yoqjhl6ueh7iereldqxue4nene4wl7rqfjfybqrehdqmqskvq")
         self.failUnless(IFilesystemNode.providedBy(n))
         self.failIf(IFileNode.providedBy(n))
+        self.failIf(IImmutableFileNode.providedBy(n))
+        self.failIf(IMutableFileNode.providedBy(n))
         self.failUnless(IDirectoryNode.providedBy(n))
         self.failIf(n.is_readonly())
         self.failUnless(n.is_mutable())
@@ -261,6 +293,8 @@ class NodeMaker(unittest.TestCase):
         n = c.create_node_from_uri("URI:DIR2-RO:b7sr5qsifnicca7cbk3rhrhbvq:mm6yoqjhl6ueh7iereldqxue4nene4wl7rqfjfybqrehdqmqskvq")
         self.failUnless(IFilesystemNode.providedBy(n))
         self.failIf(IFileNode.providedBy(n))
+        self.failIf(IImmutableFileNode.providedBy(n))
+        self.failIf(IMutableFileNode.providedBy(n))
         self.failUnless(IDirectoryNode.providedBy(n))
         self.failUnless(n.is_readonly())
         self.failUnless(n.is_mutable())
@@ -269,5 +303,7 @@ class NodeMaker(unittest.TestCase):
         n = c.create_node_from_uri(future)
         self.failUnless(IFilesystemNode.providedBy(n))
         self.failIf(IFileNode.providedBy(n))
+        self.failIf(IImmutableFileNode.providedBy(n))
+        self.failIf(IMutableFileNode.providedBy(n))
         self.failIf(IDirectoryNode.providedBy(n))
         self.failUnlessEqual(n.get_uri(), future)

src/allmydata/test/test_dirnode.py

@@ -6,7 +6,7 @@ from twisted.internet import defer
 from allmydata import uri, dirnode
 from allmydata.client import Client
 from allmydata.immutable import upload
-from allmydata.interfaces import IFileNode, IMutableFileNode, \
+from allmydata.interfaces import IImmutableFileNode, IMutableFileNode, \
      ExistingChildError, NoSuchChildError, NotDeepImmutableError, \
      IDeepCheckResults, IDeepCheckAndRepairResults, CannotPackUnknownNodeError
 from allmydata.mutable.filenode import MutableFileNode
@@ -762,7 +762,7 @@ class Dirnode(GridTestMixin, unittest.TestCase,
             uploadable1 = upload.Data("some data", convergence="converge")
             d.addCallback(lambda res: n.add_file(u"newfile", uploadable1))
             d.addCallback(lambda newnode:
-                          self.failUnless(IFileNode.providedBy(newnode)))
+                          self.failUnless(IImmutableFileNode.providedBy(newnode)))
             uploadable2 = upload.Data("some data", convergence="stuff")
             d.addCallback(lambda res:
                           self.shouldFail(ExistingChildError, "add_file-no",
@@ -784,7 +784,7 @@ class Dirnode(GridTestMixin, unittest.TestCase,
                                                  uploadable3,
                                                  {"key": "value"}))
             d.addCallback(lambda newnode:
-                          self.failUnless(IFileNode.providedBy(newnode)))
+                          self.failUnless(IImmutableFileNode.providedBy(newnode)))
             d.addCallback(lambda res: n.get_metadata_for(u"newfile-metadata"))
             d.addCallback(lambda metadata:
                               self.failUnless((set(metadata.keys()) == set(["key", "tahoe"])) and

src/allmydata/web/directory.py

@@ -14,8 +14,8 @@ from foolscap.api import fireEventually
 
 from allmydata.util import base32, time_format
 from allmydata.uri import from_string_dirnode
-from allmydata.interfaces import IDirectoryNode, IFileNode, IMutableFileNode, \
+from allmydata.interfaces import IDirectoryNode, IFileNode, IFilesystemNode, \
-     IFilesystemNode, ExistingChildError, NoSuchChildError
+     IImmutableFileNode, IMutableFileNode, ExistingChildError, NoSuchChildError
 from allmydata.monitor import Monitor, OperationCancelledError
 from allmydata import dirnode
 from allmydata.web.common import text_plain, WebError, \
@@ -40,8 +40,6 @@ class BlockingFileError(Exception):
 def make_handler_for(node, client, parentnode=None, name=None):
     if parentnode:
         assert IDirectoryNode.providedBy(parentnode)
-    if IMutableFileNode.providedBy(node):
-        return FileNodeHandler(client, node, parentnode, name)
     if IFileNode.providedBy(node):
         return FileNodeHandler(client, node, parentnode, name)
     if IDirectoryNode.providedBy(node):
@@ -687,7 +685,7 @@ class DirectoryAsHTML(rend.Page):
 
             info_link = "%s/uri/%s?t=info" % (root, quoted_uri)
 
-        elif IFileNode.providedBy(target):
+        elif IImmutableFileNode.providedBy(target):
             dlurl = "%s/file/%s/@@named=/%s" % (root, quoted_uri, nameurl)
 
             ctx.fillSlots("filename",
@@ -789,17 +787,16 @@ def DirectoryJSONMetadata(ctx, dirnode):
             assert IFilesystemNode.providedBy(childnode), childnode
             rw_uri = childnode.get_uri()
             ro_uri = childnode.get_readonly_uri()
-            if (IDirectoryNode.providedBy(childnode)
+            if IFileNode.providedBy(childnode):
-                or IFileNode.providedBy(childnode)):
                 if childnode.is_readonly():
                     rw_uri = None
-            if IFileNode.providedBy(childnode):
                 kiddata = ("filenode", {'size': childnode.get_size(),
                                         'mutable': childnode.is_mutable(),
                                         })
             elif IDirectoryNode.providedBy(childnode):
-                kiddata = ("dirnode", {'mutable': childnode.is_mutable(),
+                if childnode.is_readonly():
-                                       })
+                    rw_uri = None
+                kiddata = ("dirnode", {'mutable': childnode.is_mutable()})
             else:
                 kiddata = ("unknown", {})
             kiddata[1]["metadata"] = metadata