nodemaker.create_new_mutable_directory: pack_children() in initial_contents=

instead of creating an empty file and then adding the children later.

This should speed up mkdir(initial_children) considerably, removing two
roundtrips and an entire read-modify-write cycle, probably bringing it down
to a single roundtrip. A quick test (against the volunteergrid) suggests a
30% speedup.

test_dirnode: add new tests to enforce the restrictions that interfaces.py
claims for create_new_mutable_directory(): no UnknownNodes, metadata dicts

diff --git a/src/allmydata/nodemaker.py b/src/allmydata/nodemaker.py
index 70a6b8a5c47f99e1d1351d109265fae801031417..bf65e5c72cce881006eedbddb3a6017e8c83595d 100644 (file)
--- a/src/allmydata/nodemaker.py
+++ b/src/allmydata/nodemaker.py
@@ -1,9 +1,10 @@
 import weakref
 from zope.interface import implements
+from allmydata.util.assertutil import precondition
 from allmydata.interfaces import INodeMaker
 from allmydata.immutable.filenode import FileNode, LiteralFileNode
 from allmydata.mutable.filenode import MutableFileNode
-from allmydata.dirnode import DirectoryNode
+from allmydata.dirnode import DirectoryNode, pack_children
 from allmydata.unknown import UnknownNode
 from allmydata.uri import DirectoryURI, ReadonlyDirectoryURI
 
@@ -79,8 +80,14 @@ class NodeMaker:
         return d
 
     def create_new_mutable_directory(self, initial_children={}):
-        d = self.create_mutable_file()
+        # initial_children must have metadata (i.e. {} instead of None), and
+        # should not contain UnknownNodes
+        for (name, (node, metadata)) in initial_children.iteritems():
+            precondition(not isinstance(node, UnknownNode),
+                         "create_new_mutable_directory does not accept UnknownNode", node)
+            precondition(isinstance(metadata, dict),
+                         "create_new_mutable_directory requires metadata to be a dict, not None", metadata)
+        d = self.create_mutable_file(lambda n:
+                                     pack_children(n, initial_children))
         d.addCallback(self._create_dirnode)
-        if initial_children:
-            d.addCallback(lambda n: n.set_nodes(initial_children))
         return d

diff --git a/src/allmydata/test/test_dirnode.py b/src/allmydata/test/test_dirnode.py
index aed7e9d76cad834f0dd5eed671b9655f0469980e..e4116567fc08f090647e7467a9e596f657792d15 100644 (file)
--- a/src/allmydata/test/test_dirnode.py
+++ b/src/allmydata/test/test_dirnode.py
@@ -40,10 +40,11 @@ class Dirnode(GridTestMixin, unittest.TestCase,
         self.basedir = "dirnode/Dirnode/test_initial_children"
         self.set_up_grid()
         c = self.g.clients[0]
+        nm = c.nodemaker
         setup_py_uri = "URI:CHK:n7r3m6wmomelk4sep3kw5cvduq:os7ijw5c3maek7pg65e5254k2fzjflavtpejjyhshpsxuqzhcwwq:3:20:14861"
         one_uri = "URI:LIT:n5xgk" # LIT for "one"
-        kids = {u"one": (c.nodemaker.create_from_cap(one_uri), {}),
-                u"two": (c.nodemaker.create_from_cap(setup_py_uri),
+        kids = {u"one": (nm.create_from_cap(one_uri), {}),
+                u"two": (nm.create_from_cap(setup_py_uri),
                          {"metakey": "metavalue"}),
                 }
         d = c.create_dirnode(kids)
@@ -62,10 +63,24 @@ class Dirnode(GridTestMixin, unittest.TestCase,
             self.failUnless(isinstance(one_metadata, dict), one_metadata)
             self.failUnlessEqual(two_metadata["metakey"], "metavalue")
         d.addCallback(_check_kids)
-        d.addCallback(lambda ign:
-                      c.nodemaker.create_new_mutable_directory(kids))
+        d.addCallback(lambda ign: nm.create_new_mutable_directory(kids))
         d.addCallback(lambda dn: dn.list())
         d.addCallback(_check_kids)
+        future_writecap = "x-tahoe-crazy://I_am_from_the_future."
+        future_readcap = "x-tahoe-crazy-readonly://I_am_from_the_future."
+        future_node = UnknownNode(future_writecap, future_readcap)
+        bad_kids1 = {u"one": (future_node, {})}
+        d.addCallback(lambda ign:
+                      self.shouldFail(AssertionError, "bad_kids1",
+                                      "does not accept UnknownNode",
+                                      nm.create_new_mutable_directory,
+                                      bad_kids1))
+        bad_kids2 = {u"one": (nm.create_from_cap(one_uri), None)}
+        d.addCallback(lambda ign:
+                      self.shouldFail(AssertionError, "bad_kids2",
+                                      "requires metadata to be a dict",
+                                      nm.create_new_mutable_directory,
+                                      bad_kids2))
         return d
 
     def test_check(self):
@@ -726,7 +741,7 @@ class Dirnode(GridTestMixin, unittest.TestCase,
             fake_file_uri = make_mutable_file_uri()
             other_file_uri = make_mutable_file_uri()
             md = {"metakey": "metavalue"}
-            kids = {u"kid1": (nm.create_from_cap(fake_file_uri), None),
+            kids = {u"kid1": (nm.create_from_cap(fake_file_uri), {}),
                     u"kid2": (nm.create_from_cap(other_file_uri), md),
                     }
             d = n.create_subdirectory(u"subdir", kids)
@@ -811,12 +826,22 @@ class Packing(unittest.TestCase):
 class FakeMutableFile:
     counter = 0
     def __init__(self, initial_contents=""):
-        self.data = initial_contents
+        self.data = self._get_initial_contents(initial_contents)
         counter = FakeMutableFile.counter
         FakeMutableFile.counter += 1
         writekey = hashutil.ssk_writekey_hash(str(counter))
         fingerprint = hashutil.ssk_pubkey_fingerprint_hash(str(counter))
         self.uri = uri.WriteableSSKFileURI(writekey, fingerprint)
+
+    def _get_initial_contents(self, contents):
+        if isinstance(contents, str):
+            return contents
+        if contents is None:
+            return ""
+        assert callable(contents), "%s should be callable, not %s" % \
+               (contents, type(contents))
+        return contents(self)
+
     def get_uri(self):
         return self.uri.to_string()
     def download_best_version(self):