webapi 'move'-button cleanups

test_web.py: use shouldFail2(), safer than old shouldFail()
directory.py: forbid slashes in from_name=, return BAD_REQUEST instead of
              GONE when trying to move into a non-directory

diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py
index 5103478f18bacece85097cdb612625f6fdc11472..54a30bee80336d2dac38561af64911305aed89c3 100644 (file)
--- a/src/allmydata/test/test_web.py
+++ b/src/allmydata/test/test_web.py
@@ -3286,13 +3286,12 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def test_POST_move_file_no_replace(self):
-        d = self.POST(self.public_url + "/foo", t="move", replace="false",
-                      from_name="bar.txt", to_name="baz.txt", to_dir="sub")
-        d.addBoth(self.shouldFail, error.Error,
-                  "POST_move_file_no_replace",
-                  "409 Conflict",
-                  "There was already a child by that name, and you asked me "
-                  "to not replace it")
+        d = self.shouldFail2(error.Error, "POST_move_file_no_replace",
+                             "409 Conflict",
+                             "There was already a child by that name, and you asked me to not replace it",
+                             self.POST, self.public_url + "/foo", t="move",
+                             replace="false", from_name="bar.txt",
+                             to_name="baz.txt", to_dir="sub")
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt"))
         d.addCallback(self.failUnlessIsBarDotTxt)
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json"))
@@ -3302,26 +3301,33 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def test_POST_move_file_slash_fail(self):
-        d = self.POST(self.public_url + "/foo", t="move",
-                      from_name="bar.txt", to_name="slash/fail.txt", to_dir="sub")
-        d.addBoth(self.shouldFail, error.Error,
-                  "test_POST_rename_file_slash_fail",
-                  "400 Bad Request",
-                  "to_name= may not contain a slash",
-                  )
+        d = self.shouldFail2(error.Error, "test_POST_rename_file_slash_fail",
+                             "400 Bad Request",
+                             "to_name= may not contain a slash",
+                             self.POST, self.public_url + "/foo", t="move",
+                             from_name="bar.txt",
+                             to_name="slash/fail.txt", to_dir="sub")
         d.addCallback(lambda res:
                       self.failUnlessNodeHasChild(self._foo_node, u"bar.txt"))
         d.addCallback(lambda res:
                       self.failIfNodeHasChild(self._sub_node, u"slash/fail.txt"))
+        d.addCallback(lambda ign:
+                      self.shouldFail2(error.Error,
+                                       "test_POST_rename_file_slash_fail2",
+                                       "400 Bad Request",
+                                       "from_name= may not contain a slash",
+                                       self.POST, self.public_url + "/foo",
+                                       t="move",
+                                       from_name="nope/bar.txt",
+                                       to_name="fail.txt", to_dir="sub"))
         return d
 
     def test_POST_move_file_no_target(self):
-        d = self.POST(self.public_url + "/foo", t="move",
-                      from_name="bar.txt", to_name="baz.txt")
-        d.addBoth(self.shouldFail, error.Error,
-                  "POST_move_file_no_target",
-                  "400 Bad Request",
-                  "move requires from_name and to_dir")
+        d = self.shouldFail2(error.Error, "POST_move_file_no_target",
+                             "400 Bad Request",
+                             "move requires from_name and to_dir",
+                             self.POST, self.public_url + "/foo", t="move",
+                             from_name="bar.txt", to_name="baz.txt")
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt"))
         d.addCallback(self.failUnlessIsBarDotTxt)
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json"))
@@ -3331,13 +3337,12 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def test_POST_move_file_bad_target_type(self):
-        d = self.POST(self.public_url + "/foo", t="move", target_type="*D",
-                      from_name="bar.txt", to_dir="sub")
-        d.addBoth(self.shouldFail, error.Error,
-                  "test_POST_rename_file_slash_fail",
-                  "400 Bad Request",
-                  "invalid target_type parameter",
-                  )
+        d = self.shouldFail2(error.Error, "test_POST_move_file_bad_target_type",
+                             "400 Bad Request", "invalid target_type parameter",
+                             self.POST,
+                             self.public_url + "/foo", t="move",
+                             target_type="*D", from_name="bar.txt",
+                             to_dir="sub")
         return d
 
     def test_POST_move_file_multi_level(self):
@@ -3364,21 +3369,17 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def test_POST_move_file_to_nonexist_dir(self):
-        d = self.POST(self.public_url + "/foo", t="move",
-                      from_name="bar.txt", to_dir="notchucktesta")
-        d.addBoth(self.shouldFail, error.Error,
-                  "POST_move_file_to_nonexist_dir",
-                  "404 Not Found",
-                  "No such child: notchucktesta")
+        d = self.shouldFail2(error.Error, "POST_move_file_to_nonexist_dir",
+                            "404 Not Found", "No such child: nopechucktesta",
+                            self.POST, self.public_url + "/foo", t="move",
+                            from_name="bar.txt", to_dir="nopechucktesta")
         return d
 
     def test_POST_move_file_into_file(self):
-        d = self.POST(self.public_url + "/foo", t="move",
-                      from_name="bar.txt", to_dir="baz.txt")
-        d.addBoth(self.shouldFail, error.Error,
-                  "POST_move_file_into_file",
-                  "410 Gone",
-                  "to_dir is not a usable directory")
+        d = self.shouldFail2(error.Error, "POST_move_file_into_file",
+                             "400 Bad Request", "to_dir is not a directory",
+                             self.POST, self.public_url + "/foo", t="move",
+                             from_name="bar.txt", to_dir="baz.txt")
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt"))
         d.addCallback(self.failUnlessIsBazDotTxt)
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt"))
@@ -3388,13 +3389,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
         return d
 
     def test_POST_move_file_to_bad_uri(self):
-        d = self.POST(self.public_url + "/foo", t="move",
-                      from_name="bar.txt", target_type="uri",
-                      to_dir="URI:DIR2:mn5jlyjnrjeuydyswlzyui72i:rmneifcj6k6sycjljjhj3f6majsq2zqffydnnul5hfa4j577arma")
-        d.addBoth(self.shouldFail, error.Error,
-                  "POST_move_file_to_bad_uri",
-                  "410 Gone",
-                  "to_dir is not a usable directory")
+        d =  self.shouldFail2(error.Error, "POST_move_file_to_bad_uri",
+                              "400 Bad Request", "to_dir is not a directory",
+                              self.POST, self.public_url + "/foo", t="move",
+                              from_name="bar.txt", target_type="uri",
+                              to_dir="URI:DIR2:mn5jlyjnrjeuydyswlzyui72i:rmneifcj6k6sycjljjhj3f6majsq2zqffydnnul5hfa4j577arma")
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt"))
         d.addCallback(self.failUnlessIsBarDotTxt)
         d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json"))

diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py
index c7f9af2d3d253fae1dd6be769e04409b31fb595b..112595cd6ec74e44e187107dd459cb69cbf85070 100644 (file)
--- a/src/allmydata/web/directory.py
+++ b/src/allmydata/web/directory.py
@@ -444,34 +444,33 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
         if not from_name or not to_dir:
             raise WebError("move requires from_name and to_dir")
         replace = boolean_of_arg(get_arg(req, "replace", "true"))
-        target_type = get_arg(req, "target_type", "name")
-        if not target_type in ["name", "uri"]:
-            raise WebError("invalid target_type parameter",
-                           http.BAD_REQUEST)
 
-        # allow from_name to contain slashes, so they can fix names that
-        # were accidentally created with them. But disallow them in to_name
-        # (if it's specified), to discourage the practice.
-        if to_name and "/" in to_name:
+        # Disallow slashes in both from_name and to_name, that would only
+        # cause confusion. t=move is only for moving things from the
+        # *current* directory into a second directory named by to_dir=
+        if "/" in from_name:
+            raise WebError("from_name= may not contain a slash",
+                           http.BAD_REQUEST)
+        if "/" in to_name:
             raise WebError("to_name= may not contain a slash",
                            http.BAD_REQUEST)
 
-        d = defer.Deferred()
-        def get_target_node(target_type):
-            if target_type == "name":
-                return self.node.get_child_at_path(to_dir)
-            elif target_type == "uri":
-                return self.client.create_node_from_uri(str(to_dir))
-        d.addCallback(get_target_node)
-        d.callback(target_type)
+        target_type = get_arg(req, "target_type", "name")
+        if target_type == "name":
+            d = self.node.get_child_at_path(to_dir)
+        elif target_type == "uri":
+            d = defer.succeed(self.client.create_node_from_uri(str(to_dir)))
+        else:
+            raise WebError("invalid target_type parameter", http.BAD_REQUEST)
+
         def is_target_node_usable(target_node):
             if not IDirectoryNode.providedBy(target_node):
-                raise WebError("to_dir is not a usable directory",
-                               http.GONE)
+                raise WebError("to_dir is not a directory", http.BAD_REQUEST)
             return target_node
         d.addCallback(is_target_node_usable)
-        d.addCallback(lambda new_parent: self.node.move_child_to(
-                      from_name, new_parent, to_name, replace))
+        d.addCallback(lambda new_parent:
+                      self.node.move_child_to(from_name, new_parent,
+                                              to_name, replace))
         d.addCallback(lambda res: "thing moved")
         return d