Merge lp:~julian-edwards/launchpad/publisher-config-ppa-schema-bug-734807 into lp:launchpad/db-devel

[Julian Edwards]

= Summary =
Add PPA publisher config fields to PublisherConfig.

== Proposed fix ==
A fairly mechanical branch to add PPA-related config fields to the
PublisherConfig table and the edit form.

== Tests ==
bin/test -cvvt test_distribution_views -t test_publisherconfig

== Demo and Q/A ==
See http://launchpad.dev/ubuntu/+pubconf

[j.c.sackett (jcsackett)]

As we discussed on IRC, there should probably be a way that you can get an instance out of the factory without the new fields set. Besides that, this looks good.

review: Approve

[lp:~julian-edwards/launchpad/publisher-config-ppa-schema-bug-734807]

10312. By Julian Edwards on 2011-03-17

Remove factory code that makes it impossible to set optional fields to None.

[Julian Edwards (julian-edwards)]

I am putting this on hold until I work out some other issues to do with non-Ubuntu PPAs.

Unmerged revisions

10312. By Julian Edwards on 2011-03-17

Remove factory code that makes it impossible to set optional fields to None.

10311. By Julian Edwards on 2011-03-17

Put the extra fields on the form and make the tests pass

10310. By Julian Edwards on 2011-03-17

failing tests for the form

10309. By Julian Edwards on 2011-03-17

Update tests

10308. By Julian Edwards on 2011-03-17

model, interface and factory changes

10307. By Julian Edwards on 2011-03-17

add schema patch

=== added file 'database/schema/patch-2208-99-0.sql'
--- database/schema/patch-2208-99-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-0.sql	2011-03-17 14:37:44 +0000
@@ -0,0 +1,13 @@
+-- Copyright 2011 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE PublisherConfig
+    ADD COLUMN ppa_root_dir text,
+    ADD COLUMN ppa_private_root_dir text,
+    ADD COLUMN ppa_base_url text,
+    ADD COLUMN ppa_private_base_url text,
+    ADD COLUMN ppa_signing_keys_root text;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);

=== modified file 'lib/lp/archivepublisher/interfaces/publisherconfig.py'
--- lib/lp/archivepublisher/interfaces/publisherconfig.py	2011-03-04 17:05:45 +0000
+++ lib/lp/archivepublisher/interfaces/publisherconfig.py	2011-03-17 14:37:44 +0000
@@ -44,6 +44,28 @@
         title=_("Copy Base URL"), required=True,
         description=_("The base URL for published copy archives"))
 
+    ppa_root_dir = TextLine(
+        title=_("PPA Root Directory"), required=False,
+        description=_("The root directory for published PPAs."))
+
+    ppa_private_root_dir = TextLine(
+        title=_("Private PPA Root Directory"), required=False,
+        description=_("The root directory for published private PPAs."))
+
+    ppa_base_url = TextLine(
+        title=_("PPA Base URL"), required=False,
+        description=_("The base URL for published PPAs."))
+
+    ppa_private_base_url = TextLine(
+        title=_("Private PPA Base URL"), required=False,
+        description=_("The base URL for published private PPAs."))
+
+    ppa_signing_keys_root = TextLine(
+        title=_("Root Directory For PPA Signing Keys"), required=False,
+        description=_(
+            "The directory containing the GPG signing keys for PPAs."))
+
+
 
 class IPublisherConfigSet(Interface):
     """`PublisherConfigSet` interface."""

=== modified file 'lib/lp/archivepublisher/model/publisherconfig.py'
--- lib/lp/archivepublisher/model/publisherconfig.py	2011-03-08 12:51:02 +0000
+++ lib/lp/archivepublisher/model/publisherconfig.py	2011-03-17 14:37:44 +0000
@@ -43,13 +43,28 @@
 
     copy_base_url = Unicode(name='copy_base_url', allow_none=False)
 
+    ppa_root_dir = Unicode(name='ppa_root_dir', allow_none=True)
+
+    ppa_private_root_dir = Unicode(
+        name='ppa_private_root_dir', allow_none=True)
+
+    ppa_base_url = Unicode(name='ppa_base_url', allow_none=True)
+
+    ppa_private_base_url = Unicode(
+        name='ppa_private_base_url', allow_none=True)
+
+    ppa_signing_keys_root = Unicode(
+        name='ppa_signing_keys_root', allow_none=True)
+
 
 class PublisherConfigSet:
     """See `IPublisherConfigSet`."""
     implements(IPublisherConfigSet)
     title = "Soyuz Publisher Configurations"
 
-    def new(self, distribution, root_dir, base_url, copy_base_url):
+    def new(self, distribution, root_dir, base_url, copy_base_url,
+            ppa_root_dir=None, ppa_private_root_dir=None, ppa_base_url=None,
+            ppa_private_base_url=None, ppa_signing_keys_root=None):
         """Make and return a new `PublisherConfig`."""
         store = IMasterStore(PublisherConfig)
         pubconf = PublisherConfig()
@@ -57,6 +72,11 @@
         pubconf.root_dir = root_dir
         pubconf.base_url = base_url
         pubconf.copy_base_url = copy_base_url
+        pubconf.ppa_root_dir = ppa_root_dir
+        pubconf.ppa_private_root_dir = ppa_private_root_dir
+        pubconf.ppa_base_url = ppa_base_url
+        pubconf.ppa_private_base_url = ppa_private_base_url
+        pubconf.ppa_signing_keys_root = ppa_signing_keys_root
         store.add(pubconf)
         return pubconf
 

=== modified file 'lib/lp/archivepublisher/tests/test_publisherconfig.py'
--- lib/lp/archivepublisher/tests/test_publisherconfig.py	2011-03-16 17:04:49 +0000
+++ lib/lp/archivepublisher/tests/test_publisherconfig.py	2011-03-17 14:37:44 +0000
@@ -47,17 +47,32 @@
         ROOT_DIR = u"rootdir/test"
         BASE_URL = u"http://base.url"
         COPY_BASE_URL = u"http://base.url"
+        PPA_ROOT = u"root/foo"
+        PPA_PRIVATE_ROOT = u"private/foo"
+        PPA_BASE_URL = u"http://ppa.foo"
+        PPA_PRIVATE_BASE_URL = u"http://private.foo"
+        PPA_KEYS_ROOT = u"keyroot/foo"
         pubconf = self.factory.makePublisherConfig(
             distribution=self.distribution,
             root_dir=ROOT_DIR,
             base_url=BASE_URL,
             copy_base_url=COPY_BASE_URL,
+            ppa_root_dir=PPA_ROOT,
+            ppa_private_root_dir=PPA_PRIVATE_ROOT,
+            ppa_base_url=PPA_BASE_URL,
+            ppa_private_base_url=PPA_PRIVATE_BASE_URL,
+            ppa_signing_keys_root=PPA_KEYS_ROOT
             )
 
         self.assertEqual(self.distribution.name, pubconf.distribution.name)
         self.assertEqual(ROOT_DIR, pubconf.root_dir)
         self.assertEqual(BASE_URL, pubconf.base_url)
         self.assertEqual(COPY_BASE_URL, pubconf.copy_base_url)
+        self.assertEqual(PPA_ROOT, pubconf.ppa_root_dir)
+        self.assertEqual(PPA_PRIVATE_ROOT, pubconf.ppa_private_root_dir)
+        self.assertEqual(PPA_BASE_URL, pubconf.ppa_base_url)
+        self.assertEqual(PPA_PRIVATE_BASE_URL, pubconf.ppa_private_base_url)
+        self.assertEqual(PPA_KEYS_ROOT, pubconf.ppa_signing_keys_root)
 
     def test_one_config_per_distro(self):
         # Only one config for each distro is allowed.

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2011-03-10 17:02:15 +0000
+++ lib/lp/registry/browser/distribution.py	2011-03-17 14:37:44 +0000
@@ -1108,7 +1108,11 @@
     It redirects to the main distroseries page after a successful edit.
     """
     schema = IPublisherConfig
-    field_names = ['root_dir', 'base_url', 'copy_base_url']
+    field_names = [
+        'root_dir', 'base_url', 'copy_base_url', 'ppa_root_dir',
+        'ppa_private_root_dir', 'ppa_base_url', 'ppa_private_base_url',
+        'ppa_signing_keys_root',
+        ]
 
     @property
     def label(self):
@@ -1147,7 +1151,14 @@
                 distribution=self.context,
                 root_dir=data['root_dir'],
                 base_url=data['base_url'],
-                copy_base_url=data['copy_base_url'])
+                copy_base_url=data['copy_base_url'],
+                # The following fields are optional.
+                ppa_root_dir=data.get('ppa_root_dir'),
+                ppa_private_root_dir=data.get('ppa_private_root_dir'),
+                ppa_base_url=data.get('ppa_base_url'),
+                ppa_private_base_url=data.get('ppa_private_base_url'),
+                ppa_signing_keys_root=data.get('ppa_signing_keys_root')
+                )
         else:
             form.applyChanges(config, self.form_fields, data, self.adapters)
 

=== modified file 'lib/lp/registry/browser/tests/test_distribution_views.py'
--- lib/lp/registry/browser/tests/test_distribution_views.py	2011-03-16 17:14:26 +0000
+++ lib/lp/registry/browser/tests/test_distribution_views.py	2011-03-17 14:37:44 +0000
@@ -34,6 +34,11 @@
         self.ROOT_DIR = u"rootdir/test"
         self.BASE_URL = u"http://base.url"
         self.COPY_BASE_URL = u"http://copybase.url"
+        self.PPA_ROOT_DIR = u"ppa/root"
+        self.PPA_PRIVATE_ROOT_DIR = u"ppa/private"
+        self.PPA_BASE_URL = u"http://ppa.base"
+        self.PPA_PRIVATE_BASE_URL = u"http://private.ppa"
+        self.PPA_SIGNING_KEYS_ROOT = u"keys/root"
 
     def test_empty_initial_values(self):
         # Test that the page will display empty field values with no
@@ -58,13 +63,15 @@
         self.assertEqual(
             pubconf.copy_base_url, view.initial_values["copy_base_url"])
 
-    def _change_and_test_config(self):
+    def _change_and_test_config(self, extra_fields=None):
         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,
         }
+        if extra_fields is not None:
+            form.update(extra_fields)
 
         view = DistributionPublisherConfigView(
             self.distro, LaunchpadTestRequest(method='POST', form=form))
@@ -76,6 +83,7 @@
         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)
+        return config
 
     def test_add_new_config(self):
         # Test POSTing a new config.
@@ -91,6 +99,30 @@
             )
         self._change_and_test_config()
 
+    def test_ppa_config_fields(self):
+        # The other tests don't set PPA fields and continue to work
+        # because they are optional.  Here we check that they can be
+        # set.
+        form = {
+            'field.ppa_root_dir': self.PPA_ROOT_DIR,
+            'field.ppa_private_root_dir': self.PPA_PRIVATE_ROOT_DIR,
+            'field.ppa_base_url': self.PPA_BASE_URL,
+            'field.ppa_private_base_url': self.PPA_PRIVATE_BASE_URL,
+            'field.ppa_signing_keys_root': self.PPA_SIGNING_KEYS_ROOT,
+        }
+
+        config = self._change_and_test_config(extra_fields=form)
+
+        self.assertEqual(self.PPA_ROOT_DIR, config.ppa_root_dir)
+        self.assertEqual(
+            self.PPA_PRIVATE_ROOT_DIR, config.ppa_private_root_dir)
+        self.assertEqual(self.PPA_BASE_URL, config.ppa_base_url)
+        self.assertEqual(
+            self.PPA_PRIVATE_BASE_URL, config.ppa_private_base_url)
+        self.assertEqual(
+            self.PPA_SIGNING_KEYS_ROOT, config.ppa_signing_keys_root)
+
+
 
 class TestDistroAddView(TestCaseWithFactory):
     """Test the +add page for a new distribution."""

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-03-17 14:28:30 +0000
+++ lib/lp/testing/factory.py	2011-03-17 14:37:44 +0000
@@ -3908,7 +3908,10 @@
         return getUtility(ICveSet).new(sequence, description, cvestate)
 
     def makePublisherConfig(self, distribution=None, root_dir=None,
-                            base_url=None, copy_base_url=None):
+                            base_url=None, copy_base_url=None,
+                            ppa_root_dir=None, ppa_private_root_dir=None,
+                            ppa_base_url=None, ppa_private_base_url=None,
+                            ppa_signing_keys_root=None):
         """Create a new `PublisherConfig` record."""
         if distribution is None:
             distribution = self.makeDistribution()
@@ -3918,8 +3921,11 @@
             base_url = self.getUniqueUnicode()
         if copy_base_url is None:
             copy_base_url = self.getUniqueUnicode()
+
         return getUtility(IPublisherConfigSet).new(
-            distribution, root_dir, base_url, copy_base_url)
+            distribution, root_dir, base_url, copy_base_url, ppa_root_dir,
+            ppa_private_root_dir, ppa_base_url, ppa_private_base_url,
+            ppa_signing_keys_root)
 
 
 # Some factory methods return simple Python types. We don't add