Merge lp:~mwhudson/launchpad/vostok-main-template into lp:launchpad

[Michael Hudson-Doyle]

Add the publication plumbing and very basic root page for the 'vostok' vhost

Hi,

This branch adds a very simple main template METAL macro for the vostok layer
and makes the view for the root object use it.

The macro is done in an old-school way compared to how it's done in Launchpad
today, mostly because we couldn't be bothered to figure out how to make the
macro: tales stuff be layer dependent and also because we should be able to get
away with all our pages using the same main template macro.

Cheers,
mwh

[Tim Penhey (thumper)]

> class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
> pass

Instead of pass, how about a docstring?

The browser tests are missing docstrings too. I know they are (most likely)
boring, but should be there anyway.

> def test_distributions(self):
> # VostokRootView.distributions is an iterable of all registered
> # distributions.
> root_view = self.view()

This likes you are actually doing

     VostokRootView.__call__

Is that what you want? I wouldn't have thought so.

Are you familiar at all with the new lp.testing.BrowserTestCase?
It might be better than manually creating the view, initializing it, and
rendering it.

VostokBrowserRequest could benefit from a docstring rather than pass too.

I think that your main-template.pt macro should be a valid HTML file rather
than just <h1> and <div>. I'm pretty sure that most of the HTML in root.pt is
discarded given that it is using the master macro.

review: Needs Information

[Michael Hudson-Doyle (mwhudson)]

On Thu, 29 Jul 2010 23:38:41 -0000, Tim Penhey <email address hidden> wrote:
> Review: Needs Information
> > class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
> > pass
>
> Instead of pass, how about a docstring?

OK.

> The browser tests are missing docstrings too. I know they are (most likely)
> boring, but should be there anyway.

Added.

> > def test_distributions(self):
> > # VostokRootView.distributions is an iterable of all registered
> > # distributions.
> > root_view = self.view()
>
> This likes you are actually doing
>
> VostokRootView.__call__
>
> Is that what you want? I wouldn't have thought so.

view is a method, not a property.

> Are you familiar at all with the new lp.testing.BrowserTestCase?
> It might be better than manually creating the view, initializing it, and
> rendering it.

The basic reason I didn't use testbrowser was that I wanted to have the
view object around so I can check the list matches up with what's in the
template -- the idea being to test the template. I agree it's a bit
ugly.

> VostokBrowserRequest could benefit from a docstring rather than pass too.

I sprinkled a few docstrings around.

> I think that your main-template.pt macro should be a valid HTML file rather
> than just <h1> and <div>. I'm pretty sure that most of the HTML in root.pt is
> discarded given that it is using the master macro.

You were right. Fixed.

Cheers,
mwh

[Tim Penhey (thumper)]

Changes look good.

review: Approve

=== modified file 'Makefile'
--- Makefile	2010-08-05 01:32:59 +0000
+++ Makefile	2010-08-05 01:33:00 +0000
@@ -200,6 +200,7 @@
                 configuration:instance_name=${LPCONFIG} -c $(BUILDOUT_CFG)
 
 compile: $(PY) $(BZR_VERSION_INFO)
+	mkdir -p /var/tmp/vostok-archive
 	${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
 	    LPCONFIG=${LPCONFIG}
 	${SHHH} LPCONFIG=${LPCONFIG} ${PY} -t buildmailman.py
 
=== modified file 'lib/canonical/launchpad/doc/webapp-publication.txt'
--- lib/canonical/launchpad/doc/webapp-publication.txt	2010-07-23 08:50:49 +0000
+++ lib/canonical/launchpad/doc/webapp-publication.txt	2010-08-05 01:33:00 +0000
@@ -93,6 +93,10 @@
     rooturl: http://ubuntu-openid.launchpad.dev/
     althosts:
     ----
+    vostok @ vostok.dev
+    rooturl: http://vostok.dev/
+    althosts:
+    ----
     xmlrpc @ xmlrpc.launchpad.dev
     rooturl: http://launchpad.dev/
     althosts:
@@ -124,6 +128,7 @@
     testopenid.dev
     translations.launchpad.dev
     ubuntu-openid.launchpad.dev
+    vostok.dev
     xmlrpc-private.launchpad.dev
     xmlrpc.launchpad.dev
 
 
=== modified file 'lib/lp/vostok/browser/configure.zcml'
--- lib/lp/vostok/browser/configure.zcml	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/browser/configure.zcml	2010-08-05 01:33:00 +0000
@@ -5,6 +5,14 @@
    xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
    i18n_domain="launchpad">
 
+  <browser:page
+     for="*"
+     name="main_template"
+     template="../templates/main-template.pt"
+     permission="zope.Public"
+     layer="lp.vostok.publisher.VostokLayer"
+     />
+
   <browser:defaultView
      for="lp.vostok.publisher.IVostokRoot"
      name="+index"
 
=== modified file 'lib/lp/vostok/browser/root.py'
--- lib/lp/vostok/browser/root.py	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/browser/root.py	2010-08-05 01:33:00 +0000
@@ -8,8 +8,17 @@
     'VostokRootView',
     ]
 
+from zope.component import getUtility
+
 from canonical.launchpad.webapp import LaunchpadView
 
+from lp.registry.interfaces.distribution import IDistributionSet
+
 
 class VostokRootView(LaunchpadView):
     """The view for the Vostok root object."""
+
+    @property
+    def distributions(self):
+        """An iterable of all registered distributions."""
+        return getUtility(IDistributionSet)
 
=== modified file 'lib/lp/vostok/browser/tests/request.py'
--- lib/lp/vostok/browser/tests/request.py	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/browser/tests/request.py	2010-08-05 01:33:00 +0000
@@ -8,12 +8,10 @@
     'VostokTestRequest',
     ]
 
-from zope.interface import implements
-
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
-from lp.vostok.publisher import VostokLayer
-
-
-class VostokTestRequest(LaunchpadTestRequest):
-    implements(VostokLayer)
+from lp.vostok.publisher import VostokRequestMixin
+
+
+class VostokTestRequest(VostokRequestMixin, LaunchpadTestRequest):
+    """A test request for `VostokLayer`."""
 
=== added file 'lib/lp/vostok/browser/tests/test_main_template.py'
--- lib/lp/vostok/browser/tests/test_main_template.py	1970-01-01 00:00:00 +0000
+++ lib/lp/vostok/browser/tests/test_main_template.py	2010-08-05 01:33:00 +0000
@@ -0,0 +1,33 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the vostok 'main_template'."""
+
+__metaclass__ = type
+
+import unittest
+
+from zope.component import getMultiAdapter
+
+from canonical.testing.layers import FunctionalLayer
+
+from lp.testing import TestCase
+from lp.vostok.browser.tests.request import VostokTestRequest
+
+
+class TestMainTemplate(TestCase):
+    """Tests for our main template."""
+
+    layer = FunctionalLayer
+
+    def test_main_template_defines_master_macro(self):
+        # The main template, which is registered as a view for any object at
+        # all when in the VostokLayer, defines a 'master' macro.
+        adapter = getMultiAdapter(
+            (None, VostokTestRequest()), name='main_template')
+        self.assertEqual(['master'], adapter.index.macros.keys())
+        self.assertIn('lp/vostok', adapter.index.filename)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/vostok/browser/tests/test_root.py'
--- lib/lp/vostok/browser/tests/test_root.py	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/browser/tests/test_root.py	2010-08-05 01:33:00 +0000
@@ -9,16 +9,19 @@
 
 from zope.app.publisher.browser import getDefaultViewName
 
-from canonical.testing.layers import FunctionalLayer
+from canonical.testing.layers import DatabaseFunctionalLayer, FunctionalLayer
+from canonical.launchpad.testing.pages import extract_text, find_tag_by_id
 
-from lp.testing import TestCase
+from lp.testing import TestCase, TestCaseWithFactory
 from lp.testing.views import create_initialized_view
 from lp.vostok.browser.root import VostokRootView
 from lp.vostok.browser.tests.request import VostokTestRequest
 from lp.vostok.publisher import VostokLayer, VostokRoot
 
 
-class TestBrowseRoot(TestCase):
+
+class TestRootRegistrations(TestCase):
+    """Test the registration of views for `VostokRoot`."""
 
     layer = FunctionalLayer
 
@@ -34,5 +37,40 @@
         self.assertIsInstance(view, VostokRootView)
 
 
+class TestRootView(TestCaseWithFactory):
+    """Tests for `VostokRootView`."""
+
+    layer = DatabaseFunctionalLayer
+
+    def view(self):
+        return create_initialized_view(
+            VostokRoot(), name='+index', layer=VostokLayer)
+
+    def test_distributions(self):
+        # VostokRootView.distributions is an iterable of all registered
+        # distributions.
+        root_view = self.view()
+        new_distro = self.factory.makeDistribution()
+        self.assertIn(new_distro, list(root_view.distributions))
+
+
+class TestRootTemplate(TestCaseWithFactory):
+    """Tests for the templates used by views of `VostokRoot`."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_distribution_list(self):
+        # The element with id 'distro-list' on the root page contains a list
+        # of links to all registered distributions.
+        v = create_initialized_view(
+            VostokRoot(), name='+index', layer=VostokLayer)
+        contents = v.render()
+        link_list = find_tag_by_id(contents, 'distro-list')('a')
+        distro_list = list(v.distributions)
+        self.assertEqual(len(link_list), len(distro_list))
+        for distro, link in zip(distro_list, link_list):
+            self.assertEqual(distro.displayname, extract_text(link))
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/vostok/publisher.py'
--- lib/lp/vostok/publisher.py	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/publisher.py	2010-08-05 01:33:00 +0000
@@ -18,21 +18,38 @@
 from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
 from canonical.launchpad.webapp.servers import (
     LaunchpadBrowserRequest, VirtualHostRequestPublicationFactory)
+from canonical.launchpad.webapp.vhosts import allvhosts
 
 
 class VostokLayer(IBrowserRequest, IDefaultBrowserLayer):
     """The Vostok layer."""
 
 
-class VostokBrowserRequest(LaunchpadBrowserRequest):
+class VostokRequestMixin:
+    """This mixin defines behaviour for the real and test Vostok requests."""
+
     implements(VostokLayer)
 
+    def getRootURL(self, rootsite):
+        """See `IBasicLaunchpadRequest`."""
+        return allvhosts.configs['vostok'].rooturl
+
+
+class VostokBrowserRequest(VostokRequestMixin, LaunchpadBrowserRequest):
+    """Request class for Vostok layer."""
+
 
 class IVostokRoot(Interface):
     """Marker interface for the root vostok object."""
 
 
 class VostokRoot:
+    """The root object for the Vostok site.
+
+    No behaviour here, it just exists so it can have view and navigation
+    registrations attached to it.
+    """
+
     implements(IVostokRoot)
 
 
 
=== added file 'lib/lp/vostok/templates/main-template.pt'
--- lib/lp/vostok/templates/main-template.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/vostok/templates/main-template.pt	2010-08-05 01:33:00 +0000
@@ -0,0 +1,15 @@
+<metal:page
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  define-macro="master"><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <head>
+    <!-- Obviously, we'll need to do something better here. -->
+    <title>Vostok page</title>
+  </head>
+  <body>
+    <h1 metal:define-slot="heading" />
+    <div metal:define-slot="content" />
+  </body>
+</html>
+</metal:page>
 
=== modified file 'lib/lp/vostok/templates/root.pt'
--- lib/lp/vostok/templates/root.pt	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/templates/root.pt	2010-08-05 01:33:00 +0000
@@ -1,3 +1,20 @@
-<big>
-THIS IS VOSTOK
-</big>
+<html
+   xmlns="http://www.w3.org/1999/xhtml"
+   xmlns:tal="http://xml.zope.org/namespaces/tal"
+   xmlns:metal="http://xml.zope.org/namespaces/metal"
+   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+   metal:use-macro="context/@@main_template/master"
+   i18n:domain="vostok">
+  <body>
+    <tal:heading metal:fill-slot="heading">
+      <h1>Vostok</h1>
+    </tal:heading>
+    <tal:content metal:fill-slot="content">
+      <ul id="distro-list">
+        <tal:loop tal:repeat="distro view/distributions">
+          <li tal:content="structure distro/fmt:link" />
+        </tal:loop>
+      </ul>
+    </tal:content>
+  </body>
+</html>
 
=== modified file 'lib/lp/vostok/tests/test_publisher.py'
--- lib/lp/vostok/tests/test_publisher.py	2010-08-05 01:32:59 +0000
+++ lib/lp/vostok/tests/test_publisher.py	2010-08-05 01:33:00 +0000
@@ -9,15 +9,12 @@
 
 from canonical.config import config
 from canonical.testing.layers import FunctionalLayer
-from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
 
 from lp.testing import TestCase
 from lp.testing.publication import get_request_and_publication
 
 from lp.vostok.publisher import VostokLayer, VostokRoot
 
-from zope.component import getUtility
-
 
 class TestRegistration(TestCase):
     """Vostok's publication customizations are installed correctly."""
@@ -37,9 +34,6 @@
         request, publication = get_request_and_publication(
             host=config.vhost.vostok.hostname)
         self.assertProvides(request, VostokLayer)
-        # XXX getApplication caches the root object in the LaunchBag, so we
-        # need to set it up, or it crashes.
-        getUtility(IOpenLaunchBag).clear()
         root = publication.getApplication(request)
         self.assertIsInstance(root, VostokRoot)