Added unit tests covering #466:comment-15. Refactored the 'etag' behavior for immutable files to respond to all GET '?t=' flags, not just t=None

Signed-off-by: Andrew Miller <amiller@dappervision.com>

src/allmydata/test/test_web.py

@@ -900,6 +900,38 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi
                              self.PUT, base, "new_data"))
         return d
 
+    def test_GET_etags(self):
+
+        def _check_etags(uri):
+            d1 = _get_etag(uri)
+            d2 = _get_etag(uri, 'json')
+            d = defer.DeferredList([d1, d2], consumeErrors=True)
+            def _check(results):
+                assert all([r[0] for r in results])  # All deferred must succeed
+                assert results[0][1] + 'json' == results[1][1]
+            d.addCallback(_check)
+            return d
+
+        def _get_etag(uri, t=''):
+            targetbase = "/uri/%s?t=%s" % (urllib.quote(uri.strip()), t)
+            d = self.GET(targetbase, return_response=True, followRedirect=True)
+            def _just_the_etag(result):
+                data, response, headers = result
+                etag = headers['etag'][0]
+                if uri.startswith('URI:DIR'): assert etag.startswith('DIR:')
+                return etag
+            return d.addCallback(_just_the_etag)
+
+        # Check that etags work with immutable directories
+        (newkids, caps) = self._create_immutable_children()
+        d = self.POST2(self.public_url + "/foo/newdir?t=mkdir-immutable",
+                      simplejson.dumps(newkids))
+        d.addCallback(_check_etags)
+
+        # Check that etags work with immutable files
+        d.addCallback(lambda _: _check_etags(self._bar_txt_uri))
+        return d
+
     # TODO: version of this with a Unicode filename
     def test_GET_FILEURL_save(self):
         d = self.GET(self.public_url + "/foo/bar.txt?filename=bar.txt&save=true",

src/allmydata/web/filenode.py

@@ -157,6 +157,14 @@ class FileNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
     def render_GET(self, ctx):
         req = IRequest(ctx)
         t = get_arg(req, "t", "").strip()
+
+        if not self.node.is_mutable():
+            # if the client already has the ETag then we can
+            # short-circuit the whole process.
+            si = self.node.get_storage_index()
+            if si and req.setETag('%s-%s' % (base32.b2a(si), t or "")):
+                return ""
+
         if not t:
             # just get the contents
             # the filename arrives as part of the URL or in a form input
@@ -206,7 +214,7 @@ class FileNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
         req = IRequest(ctx)
         t = get_arg(req, "t", "").strip()
         if t:
-            raise WebError("GET file: bad t=%s" % t)
+            raise WebError("HEAD file: bad t=%s" % t)
         filename = get_arg(req, "filename", self.name) or "unknown"
         d = self.node.get_best_readable_version()
         d.addCallback(lambda dn: FileDownloader(dn, filename))
@@ -414,12 +422,6 @@ class FileDownloader(rend.Page):
         first, size = 0, None
         contentsize = filesize
         req.setHeader("accept-ranges", "bytes")
-        if not self.filenode.is_mutable():
-            # if the client already has the ETag then we can short-circuit
-            # the whole process.
-            si = self.filenode.get_storage_index()
-            if si and req.setETag(base32.b2a(si)):
-                return ""
 
         # TODO: for mutable files, use the roothash. For LIT, hash the data.
         # or maybe just use the URI for CHK and LIT.