Merge PR429: refactor response format selection logic

Factor out the logic for detecting a query argument in an HTTP request
selecting an alternate response format.  Use the resulting helper
throughout to reduce duplication for this feature.

Closes ticket:2893

src/allmydata/test/web/test_web.py

@@ -5,12 +5,24 @@ import treq
 from twisted.application import service
 from twisted.trial import unittest
 from twisted.internet import defer, reactor
-from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.defer import inlineCallbacks, returnValue, maybeDeferred
 from twisted.internet.task import Clock
 from twisted.web import client, error, http
 from twisted.python import failure, log
 
+from nevow.context import WebContext
+from nevow.inevow import (
+    ICanHandleException,
+    IRequest,
+    IData,
+)
 from nevow.util import escapeToXML
+from nevow.loaders import stan
+from nevow.testutil import FakeRequest
+from nevow.appserver import (
+    processingFailed,
+    DefaultExceptionHandler,
+)
 
 from allmydata import interfaces, uri, webish
 from allmydata.storage_client import StorageFarmBroker, StubServer
@@ -19,6 +31,7 @@ from allmydata.immutable.downloader.status import DownloadStatus
 from allmydata.dirnode import DirectoryNode
 from allmydata.nodemaker import NodeMaker
 from allmydata.web import status
+from allmydata.web.common import WebError, MultiFormatPage
 from allmydata.util import fileutil, base32, hashutil
 from allmydata.util.consumer import download_to_data
 from allmydata.util.encodingutil import to_str
@@ -29,7 +42,11 @@ from ..common import FakeCHKFileNode, FakeMutableFileNode, \
 from allmydata.interfaces import IMutableFileNode, SDMF_VERSION, MDMF_VERSION
 from allmydata.mutable import servermap, publish, retrieve
 from .. import common_util as testutil
-from ..common_web import HTTPClientGETFactory, do_http, Error
+from ..common_web import (
+    HTTPClientGETFactory,
+    do_http,
+    Error,
+)
 from allmydata.client import Client, SecretHolder
 from .common import unknown_rwcap, unknown_rocap, unknown_immcap, FAVICON_MARKUP
 # create a fake uploader/downloader, and a couple of fake dirnodes, then
@@ -602,6 +619,117 @@ class WebMixin(testutil.TimezoneMixin):
             self.fail("%s was supposed to Error(302), not get '%s'" %
                       (which, res))
 
+
+
+class MultiFormatPageTests(unittest.TestCase):
+    """
+    Tests for ``MultiFormatPage``.
+    """
+    def resource(self):
+        """
+        Create and return an instance of a ``MultiFormatPage`` subclass with two
+        formats: ``a`` and ``b``.
+        """
+        class Content(MultiFormatPage):
+            docFactory = stan("doc factory")
+
+            def render_A(self, req):
+                return "a"
+
+            def render_B(self, req):
+                return "b"
+        return Content()
+
+
+    def render(self, resource, **query_args):
+        """
+        Render a Nevow ``Page`` against a request with the given query arguments.
+
+        :param resource: The Nevow resource to render.
+
+        :param query_args: The query arguments to put into the request being
+            rendered.  A mapping from ``bytes`` to ``list`` of ``bytes``.
+
+        :return: The rendered response body as ``bytes``.
+        """
+        ctx = WebContext(tag=resource)
+        req = FakeRequest(args=query_args)
+        ctx.remember(DefaultExceptionHandler(), ICanHandleException)
+        ctx.remember(req, IRequest)
+        ctx.remember(None, IData)
+
+        d = maybeDeferred(resource.renderHTTP, ctx)
+        d.addErrback(processingFailed, req, ctx)
+        res = self.successResultOf(d)
+        if isinstance(res, bytes):
+            return req.v + res
+        return req.v
+
+
+    def test_select_format(self):
+        """
+        The ``formatArgument`` attribute of a ``MultiFormatPage`` subclass
+        identifies the query argument which selects the result format.
+        """
+        resource = self.resource()
+        resource.formatArgument = "foo"
+        self.assertEqual("a", self.render(resource, foo=["a"]))
+
+
+    def test_default_format_argument(self):
+        """
+        If a ``MultiFormatPage`` subclass does not set ``formatArgument`` then the
+        ``t`` argument is used.
+        """
+        resource = self.resource()
+        self.assertEqual("a", self.render(resource, t=["a"]))
+
+
+    def test_no_format(self):
+        """
+        If no value is given for the format argument and no default format has
+        been defined, the base Nevow rendering behavior is used
+        (``renderHTTP``).
+        """
+        resource = self.resource()
+        self.assertEqual("doc factory", self.render(resource))
+
+
+    def test_default_format(self):
+        """
+        If no value is given for the format argument and the ``MultiFormatPage``
+        subclass defines a ``formatDefault``, that value is used as the format
+        to render.
+        """
+        resource = self.resource()
+        resource.formatDefault = "b"
+        self.assertEqual("b", self.render(resource))
+
+
+    def test_explicit_none_format_renderer(self):
+        """
+        If a format is selected which has a renderer set to ``None``, the base
+        Nevow rendering behavior is used (``renderHTTP``).
+        """
+        resource = self.resource()
+        resource.render_FOO = None
+        self.assertEqual("doc factory", self.render(resource, t=["foo"]))
+
+
+    def test_unknown_format(self):
+        """
+        If a format is selected for which there is no renderer, an error is
+        returned.
+        """
+        resource = self.resource()
+        self.assertIn(
+            "<title>Exception</title>",
+            self.render(resource, t=["foo"]),
+        )
+        self.flushLoggedErrors(WebError)
+
+
+
 class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixin, unittest.TestCase):
     def test_create(self):
         pass

src/allmydata/web/common.py

@@ -7,6 +7,7 @@ from twisted.python import log
 from twisted.python.failure import Failure
 from zope.interface import Interface
 from nevow import loaders, appserver
+from nevow.rend import Page
 from nevow.inevow import IRequest
 from nevow.util import resource_filename
 from allmydata import blacklist
@@ -387,6 +388,64 @@ class RenderMixin:
         return m(ctx)
 
 
+
+class MultiFormatPage(Page):
+    """
+    ```MultiFormatPage`` is a ``rend.Page`` that can be rendered in a number
+    of different formats.
+
+    Rendered format is controlled by a query argument (given by
+    ``self.formatArgument``).  Different resources may support different
+    formats but ``json`` is a pretty common one.
+    """
+    formatArgument = "t"
+    formatDefault = None
+
+    def renderHTTP(self, ctx):
+        """
+        Dispatch to a renderer for a particular format, as selected by a query
+        argument.
+
+        A renderer for the format given by the query argument matching
+        ``formatArgument`` will be selected and invoked.  The default ``Page``
+        rendering behavior will be used if no format is selected (either by
+        query arguments or by ``formatDefault``).
+
+        :return: The result of the selected renderer.
+        """
+        req = IRequest(ctx)
+        t = get_arg(req, self.formatArgument, self.formatDefault)
+        renderer = self._get_renderer(t)
+        result = renderer(ctx)
+        return result
+
+
+    def _get_renderer(self, fmt):
+        """
+        Get the renderer for the indicated format.
+
+        :param bytes fmt: The format.  If a method with a prefix of
+            ``render_`` and a suffix of this format (upper-cased) is found, it
+            will be used.
+
+        :return: A callable which takes a Nevow context and renders a
+            response.
+        """
+        if fmt is None:
+            return super(MultiFormatPage, self).renderHTTP
+        try:
+            renderer = getattr(self, "render_{}".format(fmt.upper()))
+        except AttributeError:
+            raise WebError(
+                "Unknown {} value: {!r}".format(self.formatArgument, fmt),
+            )
+        else:
+            if renderer is None:
+                return super(MultiFormatPage, self).renderHTTP
+            return lambda ctx: renderer(IRequest(ctx))
+
+
+
 class TokenOnlyWebApi(resource.Resource):
     """
     I provide a rend.Page implementation that only accepts POST calls,

src/allmydata/web/directory.py

@@ -26,7 +26,8 @@ from allmydata.web.common import text_plain, WebError, \
      boolean_of_arg, get_arg, get_root, parse_replace_arg, \
      should_create_intermediate_directories, \
      getxmlfile, RenderMixin, humanize_failure, convert_children_json, \
-     get_format, get_mutable_type, get_filenode_metadata, render_time
+     get_format, get_mutable_type, get_filenode_metadata, render_time, \
+     MultiFormatPage
 from allmydata.web.filenode import ReplaceMeMixin, \
      FileNodeHandler, PlaceHolderNodeHandler
 from allmydata.web.check_results import CheckResultsRenderer, \
@@ -954,28 +955,26 @@ class RenameForm(rend.Page):
         ctx.tag.attributes['value'] = name
         return ctx.tag
 
-class ManifestResults(rend.Page, ReloadMixin):
+class ManifestResults(MultiFormatPage, ReloadMixin):
     docFactory = getxmlfile("manifest.xhtml")
 
+    # Control MultiFormatPage
+    formatArgument = "output"
+    formatDefault = "html"
+
     def __init__(self, client, monitor):
         self.client = client
         self.monitor = monitor
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        output = get_arg(req, "output", "html").lower()
-        if output == "text":
-            return self.text(req)
-        if output == "json":
-            return self.json(req)
-        return rend.Page.renderHTTP(self, ctx)
+    # The default format is HTML but the HTML renderer is just renderHTTP.
+    render_HTML = None
 
     def slashify_path(self, path):
         if not path:
             return ""
         return "/".join([p.encode("utf-8") for p in path])
 
-    def text(self, req):
+    def render_TEXT(self, req):
         req.setHeader("content-type", "text/plain")
         lines = []
         is_finished = self.monitor.is_finished()
@@ -984,7 +983,7 @@ class ManifestResults(rend.Page, ReloadMixin):
             lines.append(self.slashify_path(path) + " " + cap)
         return "\n".join(lines) + "\n"
 
-    def json(self, req):
+    def render_JSON(self, req):
         req.setHeader("content-type", "text/plain")
         m = self.monitor
         s = m.get_status()
@@ -1046,18 +1045,16 @@ class ManifestResults(rend.Page, ReloadMixin):
             ctx.fillSlots("cap", "")
         return ctx.tag
 
-class DeepSizeResults(rend.Page):
+class DeepSizeResults(MultiFormatPage):
+    # Control MultiFormatPage
+    formatArgument = "output"
+    formatDefault = "html"
+
     def __init__(self, client, monitor):
         self.client = client
         self.monitor = monitor
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        output = get_arg(req, "output", "html").lower()
-        req.setHeader("content-type", "text/plain")
-        if output == "json":
-            return self.json(req)
-        # plain text
+    def render_HTML(self, req):
         is_finished = self.monitor.is_finished()
         output = "finished: " + {True: "yes", False: "no"}[is_finished] + "\n"
         if is_finished:
@@ -1067,8 +1064,10 @@ class DeepSizeResults(rend.Page):
                      + stats.get("size-directories", 0))
             output += "size: %d\n" % total
         return output
+    render_TEXT = render_HTML
 
-    def json(self, req):
+    def render_JSON(self, req):
+        req.setHeader("content-type", "text/plain")
         status = {"finished": self.monitor.is_finished(),
                   "size": self.monitor.get_status(),
                   }

src/allmydata/web/introweb.py

@@ -1,16 +1,20 @@
 
 import time, os
-from nevow import rend, inevow
+from nevow import rend
 from nevow.static import File as nevow_File
 from nevow.util import resource_filename
 import allmydata
 import json
 from allmydata import get_package_versions_string
 from allmydata.util import idlib
-from allmydata.web.common import getxmlfile, get_arg, render_time
+from allmydata.web.common import (
+    getxmlfile,
+    render_time,
+    MultiFormatPage,
+)
 
 
-class IntroducerRoot(rend.Page):
+class IntroducerRoot(MultiFormatPage):
 
     addSlash = True
     docFactory = getxmlfile("introducer.xhtml")
@@ -25,13 +29,7 @@ class IntroducerRoot(rend.Page):
         for filen in os.listdir(static_dir):
             self.putChild(filen, nevow_File(os.path.join(static_dir, filen)))
 
-    def renderHTTP(self, ctx):
-        t = get_arg(inevow.IRequest(ctx), "t")
-        if t == "json":
-            return self.render_JSON(ctx)
-        return rend.Page.renderHTTP(self, ctx)
-
-    def render_JSON(self, ctx):
+    def render_JSON(self, req):
         res = {}
 
         counts = {}
@@ -110,4 +108,3 @@ class IntroducerRoot(rend.Page):
         ctx.fillSlots("version", s.version)
         ctx.fillSlots("service_name", s.service_name)
         return ctx.tag
-

src/allmydata/web/status.py

@@ -4,8 +4,16 @@ import json
 from twisted.internet import defer
 from nevow import rend, inevow, tags as T
 from allmydata.util import base32, idlib
-from allmydata.web.common import getxmlfile, get_arg, \
-     abbreviate_time, abbreviate_rate, abbreviate_size, plural, compute_rate, render_time
+from allmydata.web.common import (
+    getxmlfile,
+    abbreviate_time,
+    abbreviate_rate,
+    abbreviate_size,
+    plural,
+    compute_rate,
+    render_time,
+    MultiFormatPage,
+)
 from allmydata.interfaces import IUploadStatus, IDownloadStatus, \
      IPublishStatus, IRetrieveStatus, IServermapUpdaterStatus
 
@@ -950,7 +958,8 @@ class MapupdateStatusPage(rend.Page, RateAndTimeMixin):
         return T.li["Per-Server Response Times: ", l]
 
 
-class Status(rend.Page):
+
+class Status(MultiFormatPage):
     docFactory = getxmlfile("status.xhtml")
     addSlash = True
 
@@ -958,14 +967,7 @@ class Status(rend.Page):
         rend.Page.__init__(self, history)
         self.history = history
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        t = get_arg(req, "t")
-        if t == "json":
-            return self.json(req)
-        return rend.Page.renderHTTP(self, ctx)
-
-    def json(self, req):
+    def render_JSON(self, req):
         # modern browsers now render this instead of forcing downloads
         req.setHeader("content-type", "application/json")
         data = {}
@@ -1128,20 +1130,13 @@ class Status(rend.Page):
                     return RetrieveStatusPage(s)
 
 
-class HelperStatus(rend.Page):
+class HelperStatus(MultiFormatPage):
     docFactory = getxmlfile("helper.xhtml")
 
     def __init__(self, helper):
         rend.Page.__init__(self, helper)
         self.helper = helper
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        t = get_arg(req, "t")
-        if t == "json":
-            return self.render_JSON(req)
-        return rend.Page.renderHTTP(self, ctx)
-
     def data_helper_stats(self, ctx, data):
         return self.helper.get_stats()
 
@@ -1179,21 +1174,17 @@ class HelperStatus(rend.Page):
         return str(data["chk_upload_helper.encoded_bytes"])
 
 
-class Statistics(rend.Page):
+class Statistics(MultiFormatPage):
     docFactory = getxmlfile("statistics.xhtml")
 
     def __init__(self, provider):
         rend.Page.__init__(self, provider)
         self.provider = provider
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        t = get_arg(req, "t")
-        if t == "json":
-            stats = self.provider.get_stats()
-            req.setHeader("content-type", "text/plain")
-            return json.dumps(stats, indent=1) + "\n"
-        return rend.Page.renderHTTP(self, ctx)
+    def render_JSON(self, req):
+        stats = self.provider.get_stats()
+        req.setHeader("content-type", "text/plain")
+        return json.dumps(stats, indent=1) + "\n"
 
     def data_get_stats(self, ctx, data):
         return self.provider.get_stats()

src/allmydata/web/storage.py

@@ -1,7 +1,11 @@
 
 import time, json
-from nevow import rend, tags as T, inevow
-from allmydata.web.common import getxmlfile, abbreviate_time, get_arg
+from nevow import rend, tags as T
+from allmydata.web.common import (
+    getxmlfile,
+    abbreviate_time,
+    MultiFormatPage,
+)
 from allmydata.util.abbreviate import abbreviate_space
 from allmydata.util import time_format, idlib
 
@@ -10,7 +14,7 @@ def remove_prefix(s, prefix):
         return None
     return s[len(prefix):]
 
-class StorageStatus(rend.Page):
+class StorageStatus(MultiFormatPage):
     docFactory = getxmlfile("storage_status.xhtml")
     # the default 'data' argument is the StorageServer instance
 
@@ -19,13 +23,6 @@ class StorageStatus(rend.Page):
         self.storage = storage
         self.nickname = nickname
 
-    def renderHTTP(self, ctx):
-        req = inevow.IRequest(ctx)
-        t = get_arg(req, "t")
-        if t == "json":
-            return self.render_JSON(req)
-        return rend.Page.renderHTTP(self, ctx)
-
     def render_JSON(self, req):
         req.setHeader("content-type", "text/plain")
         d = {"stats": self.storage.get_stats(),
@@ -266,4 +263,3 @@ class StorageStatus(rend.Page):
                              ]]])
 
         return ctx.tag[p]
-