launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02898
[Merge] lp:~julian-edwards/launchpad/publisher-config-db-ui into lp:launchpad/db-devel
Julian Edwards has proposed merging lp:~julian-edwards/launchpad/publisher-config-db-ui into lp:launchpad/db-devel with lp:~julian-edwards/launchpad/publisher-config-db-schema as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/publisher-config-db-ui/+merge/52584
This branch builds on a previous one that adds a new database table, PublisherConfig, by letting you edit its values in a simple form. See the pre-requisite branch merge proposal for more details.
The long-term goal of this page is to also support the admin setting publishing schedules instead of having to do it via a crontab entry, but that will come later of course.
For now, only admins are allowed to edit this page and its settings. I also added a link to the new page on the Distribution:+index page which is only presented to admins. I've not bothered hiding anything behind a feature flag because of this.
Note that I could have used a LaunchpadEditFormView, but this depends on the object being edited already present in the database. Instead you can just do a custom initial_values property on the view that DTRT. The save_action also DTRT depending on whether it's a new entry or an edit.
--
https://code.launchpad.net/~julian-edwards/launchpad/publisher-config-db-ui/+merge/52584
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/publisher-config-db-ui into lp:launchpad/db-devel.
=== modified file 'lib/lp/registry/adapters.py'
--- lib/lp/registry/adapters.py 2011-01-04 16:08:57 +0000
+++ lib/lp/registry/adapters.py 2011-03-08 17:19:32 +0000
@@ -18,6 +18,9 @@
from zope.interface import implements
from canonical.launchpad.webapp.interfaces import ILaunchpadPrincipal
+from lp.archivepublisher.interfaces.publisherconfig import (
+ IPublisherConfigSet,
+ )
from lp.registry.interfaces.poll import (
IPollSet,
IPollSubset,
@@ -112,3 +115,10 @@
or `ILaunchpadUsage`.
"""
return productseries.product
+
+
+def distribution_to_publisherconfig(distro):
+ """Adapts `IDistribution` to `IPublisherConfig`."""
+ # Used for traversal from distro to +pubconf.
+ config = getUtility(IPublisherConfigSet).getByDistribution(distro)
+ return config
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2011-01-24 20:53:10 +0000
+++ lib/lp/registry/browser/configure.zcml 2011-03-08 17:19:32 +0000
@@ -1992,6 +1992,13 @@
facet="overview"
permission="launchpad.Edit"
template="../../app/templates/generic-edit.pt"/>
+ <browser:page
+ name="+pubconf"
+ for="lp.registry.interfaces.distribution.IDistribution"
+ class="lp.registry.browser.distribution.DistributionPublisherConfigView"
+ template="../../app/templates/generic-edit.pt"
+ facet="overview"
+ permission="launchpad.Admin"/>
<browser:defaultView
for="lp.registry.interfaces.distribution.IDistributionSet"
name="+index"/>
=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py 2011-02-03 10:35:36 +0000
+++ lib/lp/registry/browser/distribution.py 2011-03-08 17:19:32 +0000
@@ -21,6 +21,7 @@
'DistributionPPASearchView',
'DistributionPackageSearchView',
'DistributionPendingReviewMirrorsView',
+ 'DistributionPublisherConfigView',
'DistributionSeriesView',
'DistributionSeriesMirrorsRSSView',
'DistributionSeriesMirrorsView',
@@ -40,6 +41,7 @@
from zope.component import getUtility
from zope.event import notify
+from zope.formlib import form
from zope.interface import implements
from zope.lifecycleevent import ObjectCreatedEvent
from zope.security.interfaces import Unauthorized
@@ -78,6 +80,10 @@
)
from lp.app.errors import NotFoundError
from lp.app.widgets.image import ImageChangeWidget
+from lp.archivepublisher.interfaces.publisherconfig import (
+ IPublisherConfig,
+ IPublisherConfigSet,
+ )
from lp.blueprints.browser.specificationtarget import (
HasSpecificationsMenuMixin,
)
@@ -278,7 +284,12 @@
"""A menu of context actions."""
usedfor = IDistribution
facet = 'overview'
- links = ['edit']
+ links = ['edit', 'pubconf']
+
+ @enabled_with_permission("launchpad.Admin")
+ def pubconf(self):
+ text = "Configure publisher"
+ return Link("+pubconf", text, icon="edit")
class DistributionOverviewMenu(ApplicationMenu, DistributionLinksMixin):
@@ -1079,3 +1090,57 @@
@cachedproperty
def mirrors(self):
return self.context.disabled_mirrors
+
+
+class DistributionPublisherConfigView(LaunchpadFormView):
+ """View class for configuring publisher options for a DistroSeries.
+
+ It redirects to the main distroseries page after a successful edit.
+ """
+ schema = IPublisherConfig
+ field_names = ['root_dir', 'base_url', 'copy_base_url']
+
+ @property
+ def label(self):
+ """See `LaunchpadFormView`."""
+ return 'Publisher configuration for %s' % self.context.title
+
+ @property
+ def page_title(self):
+ """The page title."""
+ return self.label
+
+ @property
+ def cancel_url(self):
+ """See `LaunchpadFormView`."""
+ return canonical_url(self.context)
+
+ @property
+ def initial_values(self):
+ """If the config already exists, set up the fields with data."""
+ config = getUtility(
+ IPublisherConfigSet).getByDistribution(self.context)
+ values = {}
+ if config is not None:
+ for name in self.field_names:
+ values[name] = getattr(config, name)
+
+ return values
+
+ @action("Save")
+ def save_action(self, action, data):
+ """Update the context and redirect to its overview page."""
+ config = getUtility(IPublisherConfigSet).getByDistribution(
+ self.context)
+ if config is None:
+ config = getUtility(IPublisherConfigSet).new(
+ distribution=self.context,
+ root_dir=data['root_dir'],
+ base_url=data['base_url'],
+ copy_base_url=data['copy_base_url'])
+ else:
+ form.applyChanges(config, self.form_fields, data, self.adapters)
+
+ self.request.response.addInfoNotification(
+ 'Your changes have been applied.')
+ self.next_url = canonical_url(self.context)
=== added file 'lib/lp/registry/browser/tests/test_distribution.py'
--- lib/lp/registry/browser/tests/test_distribution.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_distribution.py 2011-03-08 17:19:32 +0000
@@ -0,0 +1,90 @@
+# Copyright 2009 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.launchpad.ftests import login
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
+from lp.registry.browser.distribution import DistributionPublisherConfigView
+from lp.testing import TestCaseWithFactory
+from lp.testing.sampledata import LAUNCHPAD_ADMIN
+
+
+class TestDistributionPublisherConfigView(TestCaseWithFactory):
+ """Test `DistributionPublisherConfigView`."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ # Create a test distribution.
+ super(TestDistributionPublisherConfigView, self).setUp()
+ self.distro = self.factory.makeDistribution()
+ login(LAUNCHPAD_ADMIN)
+
+ self.ROOT_DIR = u"rootdir/test"
+ self.BASE_URL = u"http://base.url";
+ self.COPY_BASE_URL = u"http://copybase.url";
+
+ def test_empty_initial_values(self):
+ # Test that the page will display empty field values with no
+ # existing config set up.
+ view = DistributionPublisherConfigView(
+ self.distro, LaunchpadTestRequest())
+
+ for value in view.initial_values:
+ self.assertEqual(u"", value)
+
+ def test_previous_initial_values(self):
+ # Test that the initial values are the same as the ones in the
+ # existing database record.
+ pubconf = self.factory.makePublisherConfig(
+ distribution=self.distro,
+ root_dir=self.ROOT_DIR,
+ base_url=self.BASE_URL,
+ copy_base_url=self.COPY_BASE_URL,
+ )
+
+ view = DistributionPublisherConfigView(
+ self.distro, LaunchpadTestRequest())
+
+ self.assertEqual(self.ROOT_DIR, view.initial_values["root_dir"])
+ self.assertEqual(self.BASE_URL, view.initial_values["base_url"])
+ self.assertEqual(
+ self.COPY_BASE_URL, view.initial_values["copy_base_url"])
+
+ def _change_and_test_config(self):
+ form = {
+ 'field.actions.save': 'save',
+ 'field.root_dir': self.ROOT_DIR,
+ 'field.base_url': self.BASE_URL,
+ 'field.copy_base_url': self.COPY_BASE_URL,
+ }
+
+ view = DistributionPublisherConfigView(
+ self.distro, LaunchpadTestRequest(method='POST', form=form))
+ view.initialize()
+
+ config = getUtility(
+ IPublisherConfigSet).getByDistribution(self.distro)
+
+ self.assertEqual(self.ROOT_DIR, config.root_dir)
+ self.assertEqual(self.BASE_URL, config.base_url)
+ self.assertEqual(self.COPY_BASE_URL, config.copy_base_url)
+
+ def test_add_new_config(self):
+ # Test POSTing a new config.
+ self._change_and_test_config()
+
+ def test_change_existing_config(self):
+ # Test POSTing to change existing config.
+ pubconf = self.factory.makePublisherConfig(
+ distribution=self.distro,
+ root_dir=u"random",
+ base_url=u"blah",
+ copy_base_url=u"foo",
+ )
+ self._change_and_test_config()
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2011-02-16 21:34:13 +0000
+++ lib/lp/registry/configure.zcml 2011-03-08 17:19:32 +0000
@@ -1535,6 +1535,7 @@
addAnswerContact
removeAnswerContact"/>
+
<!-- IFAQTarget -->
<allow
@@ -1578,6 +1579,12 @@
for="lp.registry.interfaces.distribution.IDistributionSet"
factory="lp.registry.browser.distribution.DistributionSetBreadcrumb"
permission="zope.Public"/>
+ <adapter
+ provides="lp.archivepublisher.interfaces.publisherconfig.IPublisherConfig"
+ for="lp.registry.interfaces.distribution.IDistribution"
+ factory="lp.registry.adapters.distribution_to_publisherconfig"
+ permission="zope.Public"/>
+
<!-- DistributionSet -->
=== modified file 'lib/lp/registry/stories/distribution/xx-distribution-launchpad-usage.txt'
--- lib/lp/registry/stories/distribution/xx-distribution-launchpad-usage.txt 2010-11-30 15:11:48 +0000
+++ lib/lp/registry/stories/distribution/xx-distribution-launchpad-usage.txt 2011-03-08 17:19:32 +0000
@@ -12,6 +12,10 @@
Traceback (most recent call last):
...
LinkNotFound...
+ >>> user_browser.getLink('Configure publisher')
+ Traceback (most recent call last):
+ ...
+ LinkNotFound...
>>> user_browser.open('http://launchpad.dev/ubuntu/+edit')
Traceback (most recent call last):
...
@@ -62,12 +66,31 @@
>>> admin_browser.getControl(name='field.official_malone').value = False
>>> admin_browser.getControl(name='field.blueprints_usage').value = (
... ['UNKNOWN'])
- >>> admin_browser.getControl(name='field.answers_usage').value = ['UNKNOWN']
+ >>> admin_browser.getControl(
+ ... name='field.answers_usage').value = ['UNKNOWN']
>>> admin_browser.getControl('Change', index=3).click()
>>> print admin_browser.url
http://launchpad.dev/ubuntu
+Only administators can configure the publisher for the distribution:
+
+ >>> admin_browser.getLink('Configure publisher').click()
+ >>> print admin_browser.title
+ Publisher configuration for...
+
+ >>> admin_browser.getControl(
+ ... name='field.root_dir').value = "/tmp/root_dir"
+ >>> admin_browser.getControl(
+ ... name='field.base_url').value = "http://base.url/";
+ >>> admin_browser.getControl(
+ ... name='field.copy_base_url').value = "http://copy.base.url/";
+ >>> admin_browser.getControl('Save').click()
+
+ >>> print admin_browser.url
+ http://launchpad.dev/ubuntu
+
+
enable_bug_expiration and JavaScript
====================================
=== removed file 'lib/lp/services/googlesearch/__init__.py'
--- lib/lp/services/googlesearch/__init__.py 2011-03-07 16:32:12 +0000
+++ lib/lp/services/googlesearch/__init__.py 1970-01-01 00:00:00 +0000
@@ -1,326 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# pylint: disable-msg=E0211,E0213
-
-"""Interfaces for searching and working with results."""
-
-__metaclass__ = type
-
-__all__ = [
- 'GoogleSearchService',
- 'PageMatch',
- 'PageMatches',
- ]
-
-import xml.etree.cElementTree as ET
-import urllib
-import urllib2
-from urlparse import urlunparse
-
-from lazr.restful.utils import get_current_browser_request
-from lazr.uri import URI
-from zope.interface import implements
-
-from canonical.config import config
-from canonical.lazr.timeout import TimeoutError
-from lp.services.googlesearch.interfaces import (
- GoogleResponseError,
- GoogleWrongGSPVersion,
- ISearchResult,
- ISearchResults,
- ISearchService,
- )
-from canonical.launchpad.webapp import urlparse
-from lp.services.timeline.requesttimeline import get_request_timeline
-
-
-class PageMatch:
- """See `ISearchResult`.
-
- A search result that represents a web page.
- """
- implements(ISearchResult)
-
- @property
- def url_rewrite_exceptions(self):
- """A list of launchpad.net URLs that must not be rewritten.
-
- Configured in config.google.url_rewrite_exceptions.
- """
- return config.google.url_rewrite_exceptions.split()
-
- @property
- def url_rewrite_scheme(self):
- """The URL scheme used in rewritten URLs.
-
- Configured in config.vhosts.use_https.
- """
- if config.vhosts.use_https:
- return 'https'
- else:
- return 'http'
-
- @property
- def url_rewrite_hostname(self):
- """The network location used in rewritten URLs.
-
- Configured in config.vhost.mainsite.hostname.
- """
- return config.vhost.mainsite.hostname
-
- def __init__(self, title, url, summary):
- """initialize a PageMatch.
-
- :param title: A string. The title of the item.
- :param url: A string. The full URL of the item.
- :param summary: A string. A summary of the item.
- """
- self.title = title
- self.summary = summary
- self.url = self._rewrite_url(url)
-
- def _strip_trailing_slash(self, url):
- """Return the url without a trailing slash."""
- uri = URI(url).ensureNoSlash()
- return str(uri)
-
- def _rewrite_url(self, url):
- """Rewrite the url to the local environment.
-
- Links with launchpad.net are rewritten to the local hostname,
- except if the domain matches a domain in the url_rewrite_exceptions.
- property.
-
- :param url: A URL str that may be rewritten to the local
- launchpad environment.
- :return: A URL str.
- """
- if self.url_rewrite_hostname == 'launchpad.net':
- # Do not rewrite the url is the hostname is the public hostname.
- return self._strip_trailing_slash(url)
- parts = urlparse(url)
- for netloc in self.url_rewrite_exceptions:
- # The network location is parts[1] in the tuple.
- if netloc in parts[1]:
- return url
- local_scheme = self.url_rewrite_scheme
- local_hostname = parts[1].replace(
- 'launchpad.net', self.url_rewrite_hostname)
- local_parts = tuple(
- [local_scheme] + [local_hostname] + list(parts[2:]))
- url = urlunparse(local_parts)
- return self._strip_trailing_slash(url)
-
-
-class PageMatches:
- """See `ISearchResults`.
-
- A collection of PageMatches.
- """
- implements(ISearchResults)
-
- def __init__(self, matches, start, total):
- """initialize a PageMatches.
-
- :param matches: A list of `PageMatch` objects.
- :param start: The index of the first item in the collection relative
- to the total number of items.
- :param total: The total number of items that matched a search.
- """
- self._matches = matches
- self.start = start
- self.total = total
-
- def __len__(self):
- """See `ISearchResults`."""
- return len(self._matches)
-
- def __getitem__(self, index):
- """See `ISearchResults`."""
- return self._matches[index]
-
- def __iter__(self):
- """See `ISearchResults`."""
- return iter(self._matches)
-
-
-class GoogleSearchService:
- """See `ISearchService`.
-
- A search service that search Google for launchpad.net pages.
- """
- implements(ISearchService)
-
- _default_values = {
- 'client': 'google-csbe',
- 'cx': None,
- 'ie': 'utf8',
- 'num': 20,
- 'oe': 'utf8',
- 'output': 'xml_no_dtd',
- 'start': 0,
- 'q': None,
- }
-
- @property
- def client_id(self):
- """The client-id issued by Google.
-
- Google requires that each client of the Google Search Engine
- service to pass its id as a parameter in the request URL.
- """
- return config.google.client_id
-
- @property
- def site(self):
- """The URL to the Google Search Engine service.
-
- The URL is probably http://www.google.com/search.
- """
- return config.google.site
-
- def search(self, terms, start=0):
- """See `ISearchService`.
-
- The config.google.client_id is used as Google client-id in the
- search request. Search returns 20 or fewer results for each query.
- For terms that match more than 20 results, the start param can be
- used over multiple queries to get successive sets of results.
-
- :return: `ISearchResults` (PageMatches).
- :raise: `GoogleWrongGSPVersion` if the xml cannot be parsed.
- """
- search_url = self.create_search_url(terms, start=start)
- from canonical.lazr.timeout import urlfetch
- request = get_current_browser_request()
- timeline = get_request_timeline(request)
- action = timeline.start("google-search-api", search_url)
- try:
- gsp_xml = urlfetch(search_url)
- except (TimeoutError, urllib2.HTTPError, urllib2.URLError), error:
- # Google search service errors are not code errors. Let the
- # call site choose to handle the unavailable service.
- raise GoogleResponseError(
- "The response errored: %s" % str(error))
- finally:
- action.finish()
- page_matches = self._parse_google_search_protocol(gsp_xml)
- return page_matches
-
- def _checkParameter(self, name, value, is_int=False):
- """Check that a parameter value is not None or an empty string."""
- if value in (None, ''):
- raise AssertionError("Missing value for parameter '%s'." % name)
- if is_int:
- try:
- int(value)
- except ValueError:
- raise AssertionError(
- "Value for parameter '%s' is not an int." % name)
-
- def create_search_url(self, terms, start=0):
- """Return a Google search url."""
- self._checkParameter('q', terms)
- self._checkParameter('start', start, is_int=True)
- self._checkParameter('cx', self.client_id)
- safe_terms = urllib.quote_plus(terms.encode('utf8'))
- search_params = dict(self._default_values)
- search_params['q'] = safe_terms
- search_params['start'] = start
- search_params['cx'] = self.client_id
- search_param_list = []
- for name in sorted(search_params):
- value = search_params[name]
- search_param_list.append('%s=%s' % (name, value))
- query_string = '&'.join(search_param_list)
- return self.site + '?' + query_string
-
- def _getElementsByAttributeValue(self, doc, path, name, value):
- """Return a list of elements whose named attribute matches the value.
-
- The cElementTree implementation does not support attribute selection
- (@) or conditional expressions (./PARAM[@name = 'start']).
-
- :param doc: An ElementTree of an XML document.
- :param path: A string path to match the first element.
- :param name: The attribute name to check.
- :param value: The string value of the named attribute.
- """
- elements = doc.findall(path)
- return [element for element in elements
- if element.get(name) == value]
-
- def _getElementByAttributeValue(self, doc, path, name, value):
- """Return the first element whose named attribute matches the value.
-
- :param doc: An ElementTree of an XML document.
- :param path: A string path to match an element.
- :param name: The attribute name to check.
- :param value: The string value of the named attribute.
- """
- return self._getElementsByAttributeValue(doc, path, name, value)[0]
-
- def _parse_google_search_protocol(self, gsp_xml):
- """Return a `PageMatches` object.
-
- :param gsp_xml: A string that should be Google Search Protocol
- version 3.2 XML. There is no guarantee that other GSP versions
- can be parsed.
- :return: `ISearchResults` (PageMatches).
- :raise: `GoogleResponseError` if the xml is incomplete.
- :raise: `GoogleWrongGSPVersion` if the xml cannot be parsed.
- """
- try:
- gsp_doc = ET.fromstring(gsp_xml)
- start_param = self._getElementByAttributeValue(
- gsp_doc, './PARAM', 'name', 'start')
- except (SyntaxError, IndexError):
- raise GoogleResponseError("The response was incomplete, no xml.")
- try:
- start = int(start_param.get('value'))
- except (AttributeError, ValueError):
- # The datatype is not what PageMatches requires.
- raise GoogleWrongGSPVersion(
- "Could not get the 'start' from the GSP XML response.")
- page_matches = []
- total = 0
- results = gsp_doc.find('RES')
- if results is None:
- # Google did not match any pages. Return an empty PageMatches.
- return PageMatches(page_matches, start, total)
-
- try:
- total = int(results.find('M').text)
- except (AttributeError, ValueError):
- # The datatype is not what PageMatches requires.
- raise GoogleWrongGSPVersion(
- "Could not get the 'total' from the GSP XML response.")
- if total < 0:
- # See bug 683115.
- total = 0
- for result in results.findall('R'):
- url_tag = result.find('U')
- title_tag = result.find('T')
- summary_tag = result.find('S')
- if None in (url_tag, title_tag, summary_tag):
- # Google indexed a bad page, or the page may be marked for
- # removal from the index. We should not include this.
- continue
- title = title_tag.text
- url = url_tag.text
- summary = summary_tag.text
- if None in (url, title, summary):
- # There is not enough data to create a PageMatch object.
- # This can be caused by an empty title or summary which
- # has been observed for pages that are from vhosts that
- # should not be indexed.
- continue
- summary = summary.replace('<br>', '')
- page_matches.append(PageMatch(title, url, summary))
- if len(page_matches) == 0 and total > 20:
- # No viable page matches could be found in the set and there
- # are more possible matches; the XML may be the wrong version.
- raise GoogleWrongGSPVersion(
- "Could not get any PageMatches from the GSP XML response.")
- return PageMatches(page_matches, start, total)
Follow ups
