From 87c1e8e066f5105c8044a3c6585ff5411f319cb5 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@allmydata.com>
Date: Fri, 16 May 2008 16:09:47 -0700
Subject: [PATCH] dirnode: add overwrite= to most API calls, defaulting to
 True. When False, this raises ExistingChildError rather than overwriting an
 existing child

---
 src/allmydata/dirnode.py           | 38 ++++++++------
 src/allmydata/interfaces.py        | 25 +++++----
 src/allmydata/test/test_dirnode.py | 83 +++++++++++++++++++++++++++++-
 3 files changed, 119 insertions(+), 27 deletions(-)

diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py
index 27e35c54..3033b4d6 100644
--- a/src/allmydata/dirnode.py
+++ b/src/allmydata/dirnode.py
@@ -7,7 +7,8 @@ import simplejson
 from allmydata.mutable.common import NotMutableError
 from allmydata.mutable.node import MutableFileNode
 from allmydata.interfaces import IMutableFileNode, IDirectoryNode,\
-     IURI, IFileNode, IMutableFileURI, IVerifierURI, IFilesystemNode
+     IURI, IFileNode, IMutableFileURI, IVerifierURI, IFilesystemNode, \
+     ExistingChildError
 from allmydata.util import hashutil, mathutil
 from allmydata.util.hashutil import netstring
 from allmydata.util.limiter import ConcurrencyLimiter
@@ -70,11 +71,12 @@ class MetadataSetter:
 
 
 class Adder:
-    def __init__(self, node, entries=None):
+    def __init__(self, node, entries=None, overwrite=True):
         self.node = node
         if entries is None:
             entries = []
         self.entries = entries
+        self.overwrite = overwrite
 
     def set_node(self, name, node, metadata):
         self.entries.append( [name, node, metadata] )
@@ -91,6 +93,8 @@ class Adder:
                 name, child, new_metadata = e
             assert isinstance(name, unicode)
             if name in children:
+                if not self.overwrite:
+                    raise ExistingChildError("child '%s' already exists" % name)
                 metadata = children[name][1].copy()
             else:
                 metadata = {"ctime": now,
@@ -313,7 +317,7 @@ class NewDirectoryNode:
             d.addCallback(_got)
         return d
 
-    def set_uri(self, name, child_uri, metadata=None):
+    def set_uri(self, name, child_uri, metadata=None, overwrite=True):
         """I add a child (by URI) at the specific name. I return a Deferred
         that fires with the child node when the operation finishes. I will
         replace any existing child of the same name.
@@ -325,13 +329,13 @@ class NewDirectoryNode:
         NotMutableError."""
         assert isinstance(name, unicode)
         child_node = self._create_node(child_uri)
-        d = self.set_node(name, child_node, metadata)
+        d = self.set_node(name, child_node, metadata, overwrite)
         d.addCallback(lambda res: child_node)
         return d
 
-    def set_children(self, entries):
+    def set_children(self, entries, overwrite=True):
         # this takes URIs
-        a = Adder(self)
+        a = Adder(self, overwrite=overwrite)
         node_entries = []
         for e in entries:
             if len(e) == 2:
@@ -344,7 +348,7 @@ class NewDirectoryNode:
             a.set_node(name, self._create_node(child_uri), metadata)
         return self._node.modify(a.modify)
 
-    def set_node(self, name, child, metadata=None):
+    def set_node(self, name, child, metadata=None, overwrite=True):
         """I add a child at the specific name. I return a Deferred that fires
         when the operation finishes. This Deferred will fire with the child
         node that was just added. I will replace any existing child of the
@@ -357,22 +361,22 @@ class NewDirectoryNode:
             return defer.fail(NotMutableError())
         assert isinstance(name, unicode)
         assert IFilesystemNode.providedBy(child), child
-        a = Adder(self)
+        a = Adder(self, overwrite=overwrite)
         a.set_node(name, child, metadata)
         d = self._node.modify(a.modify)
         d.addCallback(lambda res: child)
         return d
 
-    def set_nodes(self, entries):
+    def set_nodes(self, entries, overwrite=True):
         if self.is_readonly():
             return defer.fail(NotMutableError())
-        a = Adder(self, entries)
+        a = Adder(self, entries, overwrite=overwrite)
         d = self._node.modify(a.modify)
         d.addCallback(lambda res: None)
         return d
 
 
-    def add_file(self, name, uploadable, metadata=None):
+    def add_file(self, name, uploadable, metadata=None, overwrite=True):
         """I upload a file (using the given IUploadable), then attach the
         resulting FileNode to the directory at the given name. I return a
         Deferred that fires (with the IFileNode of the uploaded file) when
@@ -383,7 +387,8 @@ class NewDirectoryNode:
         d = self._client.upload(uploadable)
         d.addCallback(lambda results: results.uri)
         d.addCallback(self._client.create_node_from_uri)
-        d.addCallback(lambda node: self.set_node(name, node, metadata))
+        d.addCallback(lambda node:
+                      self.set_node(name, node, metadata, overwrite))
         return d
 
     def delete(self, name):
@@ -397,7 +402,7 @@ class NewDirectoryNode:
         d.addCallback(lambda res: deleter.old_child)
         return d
 
-    def create_empty_directory(self, name):
+    def create_empty_directory(self, name, overwrite=True):
         """I create and attach an empty directory at the given name. I return
         a Deferred that fires (with the new directory node) when the
         operation finishes."""
@@ -407,7 +412,7 @@ class NewDirectoryNode:
         d = self._client.create_empty_dirnode()
         def _created(child):
             entries = [(name, child, None)]
-            a = Adder(self, entries)
+            a = Adder(self, entries, overwrite=overwrite)
             d = self._node.modify(a.modify)
             d.addCallback(lambda res: child)
             return d
@@ -415,7 +420,7 @@ class NewDirectoryNode:
         return d
 
     def move_child_to(self, current_child_name, new_parent,
-                      new_child_name=None):
+                      new_child_name=None, overwrite=True):
         """I take one of my children and move them to a new parent. The child
         is referenced by name. On the new parent, the child will live under
         'new_child_name', which defaults to 'current_child_name'. I return a
@@ -428,7 +433,8 @@ class NewDirectoryNode:
         assert isinstance(new_child_name, unicode)
         d = self.get(current_child_name)
         def sn(child):
-            return new_parent.set_node(new_child_name, child)
+            return new_parent.set_node(new_child_name, child,
+                                       overwrite=overwrite)
         d.addCallback(sn)
         d.addCallback(lambda child: self.delete(current_child_name))
         return d
diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 789ae71a..5bcb1c74 100644
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -725,6 +725,10 @@ class IMutableFileNode(IFileNode, IMutableFilesystemNode):
         writer-visible data using this writekey.
         """
 
+class ExistingChildError(Exception):
+    """A directory node was asked to add or replace a child that already
+    exists, and overwrite= was set to False."""
+
 class IDirectoryNode(IMutableFilesystemNode):
     """I represent a name-to-child mapping, holding the tahoe equivalent of a
     directory. All child names are unicode strings, and all children are some
@@ -790,10 +794,12 @@ class IDirectoryNode(IMutableFilesystemNode):
         path-name elements. All elements must be unicode strings.
         """
 
-    def set_uri(name, child_uri, metadata=None):
+    def set_uri(name, child_uri, metadata=None, overwrite=True):
         """I add a child (by URI) at the specific name. I return a Deferred
-        that fires when the operation finishes. I will replace any existing
-        child of the same name. The child name must be a unicode string.
+        that fires when the operation finishes. If overwrite= is True, I will
+        replace any existing child of the same name, otherwise an existing
+        child will cause me to return ExistingChildError. The child name must
+        be a unicode string.
 
         The child_uri could be for a file, or for a directory (either
         read-write or read-only, using a URI that came from get_uri() ).
@@ -808,7 +814,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         If this directory node is read-only, the Deferred will errback with a
         NotMutableError."""
 
-    def set_children(entries):
+    def set_children(entries, overwrite=True):
         """Add multiple (name, child_uri) pairs (or (name, child_uri,
         metadata) triples) to a directory node. Returns a Deferred that fires
         (with None) when the operation finishes. This is equivalent to
@@ -816,7 +822,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         child names must be unicode strings.
         """
 
-    def set_node(name, child, metadata=None):
+    def set_node(name, child, metadata=None, overwrite=True):
         """I add a child at the specific name. I return a Deferred that fires
         when the operation finishes. This Deferred will fire with the child
         node that was just added. I will replace any existing child of the
@@ -832,7 +838,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         If this directory node is read-only, the Deferred will errback with a
         NotMutableError."""
 
-    def set_nodes(entries):
+    def set_nodes(entries, overwrite=True):
         """Add multiple (name, child_node) pairs (or (name, child_node,
         metadata) triples) to a directory node. Returns a Deferred that fires
         (with None) when the operation finishes. This is equivalent to
@@ -840,7 +846,7 @@ class IDirectoryNode(IMutableFilesystemNode):
         child names must be unicode strings."""
 
 
-    def add_file(name, uploadable, metadata=None):
+    def add_file(name, uploadable, metadata=None, overwrite=True):
         """I upload a file (using the given IUploadable), then attach the
         resulting FileNode to the directory at the given name. I set metadata
         the same way as set_uri and set_node. The child name must be a
@@ -854,12 +860,13 @@ class IDirectoryNode(IMutableFilesystemNode):
         fires when the operation finishes. The child name must be a unicode
         string."""
 
-    def create_empty_directory(name):
+    def create_empty_directory(name, overwrite=True):
         """I create and attach an empty directory at the given name. The
         child name must be a unicode string. I return a Deferred that fires
         when the operation finishes."""
 
-    def move_child_to(current_child_name, new_parent, new_child_name=None):
+    def move_child_to(current_child_name, new_parent, new_child_name=None,
+                      overwrite=True):
         """I take one of my children and move them to a new parent. The child
         is referenced by name. On the new parent, the child will live under
         'new_child_name', which defaults to 'current_child_name'. TODO: what
diff --git a/src/allmydata/test/test_dirnode.py b/src/allmydata/test/test_dirnode.py
index 62f69398..d0d5bb32 100644
--- a/src/allmydata/test/test_dirnode.py
+++ b/src/allmydata/test/test_dirnode.py
@@ -4,7 +4,7 @@ from zope.interface import implements
 from twisted.trial import unittest
 from allmydata import uri, dirnode, upload
 from allmydata.interfaces import IURI, IClient, IMutableFileNode, \
-     INewDirectoryURI, IReadonlyNewDirectoryURI, IFileNode
+     INewDirectoryURI, IReadonlyNewDirectoryURI, IFileNode, ExistingChildError
 from allmydata.util import hashutil, testutil
 from allmydata.test.common import make_chk_file_uri, make_mutable_file_uri, \
      FakeDirectoryNode, create_chk_filenode
@@ -186,15 +186,22 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
             d.addCallback(lambda res: n.has_child(u"missing"))
             d.addCallback(lambda res: self.failIf(res))
             fake_file_uri = make_mutable_file_uri()
+            other_file_uri = make_mutable_file_uri()
             m = Marker(fake_file_uri)
             ffu_v = m.get_verifier()
             assert isinstance(ffu_v, str)
             self.expected_manifest.append(ffu_v)
             d.addCallback(lambda res: n.set_uri(u"child", fake_file_uri))
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "set_uri-no",
+                                          "child 'child' already exists",
+                                          n.set_uri, u"child", other_file_uri,
+                                          overwrite=False))
             # /
             # /child = mutable
 
             d.addCallback(lambda res: n.create_empty_directory(u"subdir"))
+
             # /
             # /child = mutable
             # /subdir = directory
@@ -206,6 +213,12 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                 self.expected_manifest.append(new_v)
             d.addCallback(_created)
 
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "mkdir-no",
+                                          "child 'subdir' already exists",
+                                          n.create_empty_directory, u"subdir",
+                                          overwrite=False))
+
             d.addCallback(lambda res: n.list())
             d.addCallback(lambda children:
                           self.failUnlessEqual(sorted(children.keys()),
@@ -287,6 +300,12 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
             # set_node + metadata
             # it should be possible to add a child without any metadata
             d.addCallback(lambda res: n.set_node(u"d2", n, {}))
+            d.addCallback(lambda res: self.client.create_empty_dirnode())
+            d.addCallback(lambda n2:
+                          self.shouldFail(ExistingChildError, "set_node-no",
+                                          "child 'd2' already exists",
+                                          n.set_node, u"d2", n2,
+                                          overwrite=False))
             d.addCallback(lambda res: n.get_metadata_for(u"d2"))
             d.addCallback(lambda metadata: self.failUnlessEqual(metadata, {}))
 
@@ -315,6 +334,16 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                                                    (u"e3", fake_file_uri,
                                                     {"key": "value"}),
                                                    ]))
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "set_children-no",
+                                          "child 'e1' already exists",
+                                          n.set_children,
+                                          [ (u"e1", other_file_uri),
+                                            (u"new", other_file_uri), ],
+                                          overwrite=False))
+            # and 'new' should not have been created
+            d.addCallback(lambda res: n.list())
+            d.addCallback(lambda children: self.failIf(u"new" in children))
             d.addCallback(lambda res: n.get_metadata_for(u"e1"))
             d.addCallback(lambda metadata:
                           self.failUnlessEqual(sorted(metadata.keys()),
@@ -335,6 +364,16 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                                                     (u"f3", n,
                                                      {"key": "value"}),
                                                     ]))
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "set_nodes-no",
+                                          "child 'f1' already exists",
+                                          n.set_nodes,
+                                          [ (u"f1", n),
+                                            (u"new", n), ],
+                                          overwrite=False))
+            # and 'new' should not have been created
+            d.addCallback(lambda res: n.list())
+            d.addCallback(lambda children: self.failIf(u"new" in children))
             d.addCallback(lambda res: n.get_metadata_for(u"f1"))
             d.addCallback(lambda metadata:
                           self.failUnlessEqual(sorted(metadata.keys()),
@@ -427,6 +466,13 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
             d.addCallback(lambda res: n.add_file(u"newfile", uploadable))
             d.addCallback(lambda newnode:
                           self.failUnless(IFileNode.providedBy(newnode)))
+            other_uploadable = upload.Data("some data", convergence="stuff")
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "add_file-no",
+                                          "child 'newfile' already exists",
+                                          n.add_file, u"newfile",
+                                          other_uploadable,
+                                          overwrite=False))
             d.addCallback(lambda res: n.list())
             d.addCallback(lambda children:
                           self.failUnlessEqual(sorted(children.keys()),
@@ -436,7 +482,6 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                           self.failUnlessEqual(sorted(metadata.keys()),
                                                ["ctime", "mtime"]))
 
-            uploadable = upload.Data("some data", convergence="some convergence string")
             d.addCallback(lambda res: n.add_file(u"newfile-metadata",
                                                  uploadable,
                                                  {"key": "value"}))
@@ -450,6 +495,9 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
             d.addCallback(lambda res: n.create_empty_directory(u"subdir2"))
             def _created2(subdir2):
                 self.subdir2 = subdir2
+                # put something in the way, to make sure it gets overwritten
+                return subdir2.add_file(u"child", upload.Data("overwrite me",
+                                                              "converge"))
             d.addCallback(_created2)
 
             d.addCallback(lambda res:
@@ -462,6 +510,37 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
             d.addCallback(lambda children:
                           self.failUnlessEqual(sorted(children.keys()),
                                                sorted([u"child"])))
+            d.addCallback(lambda res: self.subdir2.get(u"child"))
+            d.addCallback(lambda child:
+                          self.failUnlessEqual(child.get_uri(),
+                                               fake_file_uri.to_string()))
+
+            # move it back, using new_child_name=
+            d.addCallback(lambda res:
+                          self.subdir2.move_child_to(u"child", n, u"newchild"))
+            d.addCallback(lambda res: n.list())
+            d.addCallback(lambda children:
+                          self.failUnlessEqual(sorted(children.keys()),
+                                               sorted([u"newchild", u"newfile",
+                                                       u"subdir2"])))
+            d.addCallback(lambda res: self.subdir2.list())
+            d.addCallback(lambda children:
+                          self.failUnlessEqual(sorted(children.keys()), []))
+
+            # now make sure that we honor overwrite=False
+            d.addCallback(lambda res:
+                          self.subdir2.set_uri(u"newchild", other_file_uri))
+
+            d.addCallback(lambda res:
+                          self.shouldFail(ExistingChildError, "move_child_to-no",
+                                          "child 'newchild' already exists",
+                                          n.move_child_to, u"newchild",
+                                          self.subdir2,
+                                          overwrite=False))
+            d.addCallback(lambda res: self.subdir2.get(u"newchild"))
+            d.addCallback(lambda child:
+                          self.failUnlessEqual(child.get_uri(),
+                                               other_file_uri.to_string()))
 
             return d
 
-- 
2.45.2