launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/feeds-favicon into lp:launchpad.

Commit message:
Allow serving favicon.ico from the feeds root.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/feeds-favicon/+merge/274087

We get fairly frequent NotFound OOPSes for http://feeds.launchpad.net/favicon.ico.  It's a reasonably simple matter to make this serve the usual favicon image rather than 404ing.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/feeds-favicon into lp:launchpad.

=== modified file 'lib/lp/services/feeds/stories/xx-navigation.txt'
--- lib/lp/services/feeds/stories/xx-navigation.txt	2011-12-29 05:29:36 +0000
+++ lib/lp/services/feeds/stories/xx-navigation.txt	2015-10-11 20:52:10 +0000
@@ -132,3 +132,12 @@
 Revert configuration change after tests are finished.
 
     >>> config_data = config.pop('bug_search_feed_data')
+
+
+== Favicon ==
+
+feeds.launchpad.dev has a favicon.
+
+    >>> browser.open('http://feeds.launchpad.dev/favicon.ico')
+    >>> print browser.headers['Content-Type']
+    image/png

=== modified file 'lib/lp/services/webapp/interfaces.py'
--- lib/lp/services/webapp/interfaces.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/interfaces.py	2015-10-11 20:52:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -25,6 +25,7 @@
 from zope.publisher.interfaces.browser import IBrowserApplicationRequest
 from zope.schema import (
     Bool,
+    Bytes,
     Choice,
     Datetime,
     Int,
@@ -275,6 +276,15 @@
             )
 
 
+class IFavicon(Interface):
+    """A favicon."""
+
+    path = TextLine(
+        title=u"The name of the file containing the favicon data.",
+        required=True)
+    data = Bytes(title=u"The favicon data.", required=True)
+
+
 # XXX kiko 2007-02-08: this needs reconsideration if we are to make it a truly
 # generic thing. The problem lies in the fact that half of this (user, login,
 # time zone, developer) is actually useful inside webapp/, and the other half

=== modified file 'lib/lp/services/webapp/metazcml.py'
--- lib/lp/services/webapp/metazcml.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/metazcml.py	2015-10-11 20:52:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -57,6 +57,7 @@
     ICanonicalUrlData,
     IContextMenu,
     IFacetMenu,
+    IFavicon,
     INavigationMenu,
     )
 from lp.services.webapp.publisher import RenamedView
@@ -407,8 +408,6 @@
 
 
 class FaviconRendererBase:
-    # subclasses must provide a 'fileobj' member that has 'contentType'
-    # and 'data' attributes.
 
     def __call__(self):
         self.request.response.setHeader(
@@ -417,6 +416,7 @@
 
 
 def favicon(_context, for_, file):
+    @implementer(IFavicon)
     class Favicon(FaviconRendererBase):
         path = file
         data = open(file, 'rb').read()

=== modified file 'lib/lp/services/webapp/servers.py'
--- lib/lp/services/webapp/servers.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/servers.py	2015-10-11 20:52:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Definition of the internet servers that Launchpad uses."""
@@ -85,6 +85,7 @@
     FinishReadOnlyRequestEvent,
     IBasicLaunchpadRequest,
     IBrowserFormNG,
+    IFavicon,
     ILaunchpadBrowserApplicationRequest,
     ILaunchpadProtocolError,
     INotificationRequest,
@@ -1125,9 +1126,10 @@
         result = super(FeedsPublication, self).traverseName(request, ob, name)
         if len(request.stepstogo) == 0:
             # The url has been fully traversed. Now we can check that
-            # the result is a feed, an image, or a redirection.
+            # the result is a feed, a favicon, an image, or a redirection.
             naked_result = removeSecurityProxy(result)
             if (IFeed.providedBy(result) or
+                IFavicon.providedBy(result) or
                 isinstance(naked_result, LaunchpadImageFolder) or
                 getattr(naked_result, 'status', None) == 301):
                 return result