Get rid of ?replace= handling entirely and more-correctly support /uri variants

src/allmydata/test/test_testing.py

@@ -21,6 +21,11 @@ from allmydata.uri import (
 )
 from allmydata.testing.web import (
     create_tahoe_treq_client,
+    capability_generator,
+)
+
+from hyperlink import (
+    DecodedURL,
 )
 
 from hypothesis import (
@@ -35,9 +40,12 @@ from testtools import (
 )
 from testtools.matchers import (
     Always,
+    Equals,
+    MatchesStructure,
 )
 from testtools.twistedsupport import (
     succeeded,
+    failed,
 )
 
 
@@ -46,9 +54,9 @@ class FakeWebTest(TestCase):
     Test the WebUI verified-fakes infrastucture
     """
 
-    def setUp(self):
+    # Note: do NOT use setUp() because Hypothesis doesn't work
-        super(FakeWebTest, self).setUp()
+    # properly with it. You must instead do all fixture-type work
-        self.http_client = create_tahoe_treq_client()
+    # yourself in each test.
 
     @given(
         content=binary(),
@@ -58,21 +66,31 @@ class FakeWebTest(TestCase):
         Upload some content (via 'PUT /uri') and then download it (via
         'GET /uri?uri=...')
         """
+        http_client = create_tahoe_treq_client()
 
         @inlineCallbacks
         def do_test():
-            resp = yield self.http_client.put("http://example.com/uri?replace=true", content)
+            resp = yield http_client.put("http://example.com/uri", content)
             self.assertEqual(resp.code, 201)
 
             cap_raw = yield resp.content()
             cap = from_string(cap_raw)
             self.assertIsInstance(cap, CHKFileURI)
 
-            resp = yield self.http_client.get(
+            resp = yield http_client.get(
                 "http://example.com/uri?uri={}".format(cap.to_string())
             )
             self.assertEqual(resp.code, 200)
 
+            round_trip_content = yield resp.content()
+
+            # using the form "/uri/<cap>" is also valid
+
+            resp = yield http_client.get(
+                "http://example.com/uri/{}".format(cap.to_string())
+            )
+            self.assertEqual(resp.code, 200)
+
             round_trip_content = yield resp.content()
             self.assertEqual(content, round_trip_content)
         self.assertThat(
@@ -80,21 +98,67 @@ class FakeWebTest(TestCase):
             succeeded(Always()),
         )
 
+    @given(
+        content=binary(),
+    )
+    def test_duplicate_upload(self, content):
+        """
+        Upload the same content (via 'PUT /uri') twice
+        """
+
+        http_client = create_tahoe_treq_client()
+
         @inlineCallbacks
-    def test_duplicate_upload(self):
+        def do_test():
-        """
+            resp = yield http_client.put("http://example.com/uri", content)
-        Upload the same content (via 'PUT /uri') twice with no overwrite
-        """
-
-        content = "fake content\n" * 200
-
-        resp = yield self.http_client.put("http://example.com/uri", content)
             self.assertEqual(resp.code, 201)
 
             cap_raw = yield resp.content()
             cap = from_string(cap_raw)
             self.assertIsInstance(cap, CHKFileURI)
 
-        # this one fails: same content but no ?replace=true
+            resp = yield http_client.put("http://example.com/uri", content)
-        resp = yield self.http_client.put("http://example.com/uri", content)
+            self.assertThat(resp.code, Equals(200))
-        self.assertEqual(resp.code, 409)
+        self.assertThat(
+            do_test(),
+            succeeded(Always()),
+        )
+
+    def test_download_missing(self):
+        """
+        Error if we download a capability that doesn't exist
+        """
+
+        http_client = create_tahoe_treq_client()
+        cap_gen = capability_generator("URI:CHK:")
+
+        uri = DecodedURL.from_text(u"http://example.com/uri?uri={}".format(next(cap_gen)))
+        resp = http_client.get(uri.to_uri().to_text())
+
+        self.assertThat(
+            resp,
+            succeeded(
+                MatchesStructure(
+                    code=Equals(500)
+                )
+            )
+        )
+
+    def test_download_no_arg(self):
+        """
+        Error if we GET from "/uri" with no ?uri= query-arg
+        """
+
+        http_client = create_tahoe_treq_client()
+
+        uri = DecodedURL.from_text(u"http://example.com/uri/")
+        resp = http_client.get(uri.to_uri().to_text())
+
+        self.assertThat(
+            resp,
+            succeeded(
+                MatchesStructure(
+                    code=Equals(400)
+                )
+            )
+        )

src/allmydata/testing/web.py

@@ -76,7 +76,8 @@ class _FakeTahoeRoot(Resource, object):
         self.putChild(b"uri", self._uri)
 
     def add_data(self, kind, data):
-        return self._uri.add_data(kind, data)
+        fresh, cap = self._uri.add_data(kind, data)
+        return cap
 
 
 KNOWN_CAPABILITIES = [
@@ -156,39 +157,32 @@ class _FakeTahoeUriHandler(Resource, object):
         capability = next(self.capability_generators[kind])
         return capability
 
-    def add_data(self, kind, data, allow_duplicate=False):
+    def add_data(self, kind, data):
         """
         adds some data to our grid
 
-        :returns: a capability-string
+        :returns: a two-tuple: a bool (True if the data is freshly added) and a capability-string
         """
         if not isinstance(data, bytes):
             raise TypeError("'data' must be bytes")
 
         for k in self.data:
             if self.data[k] == data:
-                if allow_duplicate:
+                return (False, k)
-                    return k
-                raise ValueError(
-                    "Duplicate data"
-                )
 
         cap = self._generate_capability(kind)
-        if cap in self.data:
+        assert cap not in self.data, "logic error"  # shouldn't happen
-            raise ValueError("already have '{}'".format(cap))
         self.data[cap] = data
-        return cap
+        return (True, cap)
 
     def render_PUT(self, request):
         data = request.content.read()
+        fresh, cap = self.add_data("URI:CHK:", data)
+        if fresh:
             request.setResponseCode(http.CREATED)  # real code does this for brand-new files
-        replace = request.args.get("replace", None)
+        else:
-        try:
+            request.setResponseCode(http.OK)  # replaced/modified files
-            return self.add_data("URI:CHK:", data, allow_duplicate=replace)
+        return cap
-        except ValueError:
-            msg, code = humanize_failure(Failure(ExistingChildError()))
-            request.setResponseCode(code)
-            return msg
 
     def render_POST(self, request):
         t = request.args[u"t"][0]
@@ -198,7 +192,8 @@ class _FakeTahoeUriHandler(Resource, object):
             "mkdir-immutable": "URI:DIR2-CHK:"
         }
         kind = type_to_kind[t]
-        return self.add_data(kind, data)
+        fresh, cap = self.add_data(kind, data)
+        return cap
 
     def render_GET(self, request):
         uri = DecodedURL.from_text(request.uri.decode('utf8'))
@@ -206,14 +201,19 @@ class _FakeTahoeUriHandler(Resource, object):
         for arg, value in uri.query:
             if arg == u"uri":
                 capability = value
-        if capability is None:
+        # it's legal to use the form "/uri/<capability>"
-            raise Exception(
+        if capability is None and request.postpath and request.postpath[0]:
-                "No ?uri= arguent in GET '{}'".format(
+            capability = request.postpath[0]
-                    uri.to_string()
-                )
-            )
 
+        # if we don't yet have a capability, that's an error
+        if capability is None:
+            request.setResponseCode(http.BAD_REQUEST)
+            return b"GET /uri requires uri="
+
+        # the user gave us a capability; if our Grid doesn't have any
+        # data for it, that's an error.
         if capability not in self.data:
+            request.setResponseCode(http.BAD_REQUEST)
             return u"No data for '{}'".format(capability).decode("ascii")
 
         return self.data[capability]