launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/schema-distro-parents into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/schema-distro-parents/+merge/57898

Add a new database table, DistroSeriesParent, that will track all of a distroseries' derived distroseries and whether they've been initialized or not (initialized is when the packages get copied from the parent, this needs to be tracked so it can't happen twice).

Most of the changes are boilerplate.  The security declarations are set up so the person changing the data must have edit permission on the derived series.

Other than that, a routine and dull branch.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/schema-distro-parents/+merge/57898
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/schema-distro-parents into lp:launchpad/db-devel.

=== 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-04-15 16:17:34 +0000
@@ -0,0 +1,20 @@
+-- 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;
+
+CREATE TABLE DistroSeriesParent (
+    id serial PRIMARY KEY,
+    derived_series integer NOT NULL
+        CONSTRAINT distroseriesparent__derivedseries__fk REFERENCES distroseries,
+    parent_series integer NOT NULL
+        CONSTRAINT distroseriesparent__parentseries__fk REFERENCES distroseries,
+    initialized boolean NOT NULL
+);
+
+CREATE INDEX distroseriesparent__derivedseries__idx
+    ON DistroSeriesParent (derived_series);
+CREATE INDEX distroseriesparent__parentseries__idx
+    ON DistroSeriesParent (parent_series);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-04-11 09:01:12 +0000
+++ database/schema/security.cfg	2011-04-15 16:17:34 +0000
@@ -172,6 +172,7 @@
 public.distributionsourcepackagecache   = SELECT
 public.distroseriesdifference           = SELECT, INSERT, UPDATE
 public.distroseriesdifferencemessage    = SELECT, INSERT, UPDATE
+public.distroseriesparent               = SELECT, INSERT, UPDATE, DELETE
 public.distroserieslanguage             = SELECT, INSERT, UPDATE
 public.distroseriespackagecache         = SELECT
 public.emailaddress                     = SELECT, INSERT, UPDATE, DELETE

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-04-14 15:26:55 +0000
+++ lib/canonical/launchpad/security.py	2011-04-15 16:17:34 +0000
@@ -113,9 +113,13 @@
     IDistributionSourcePackage,
     )
 from lp.registry.interfaces.distroseries import IDistroSeries
+<<<<<<< TREE
 from lp.registry.interfaces.distroseriesdifference import (
     IDistroSeriesDifferenceEdit,
     )
+=======
+from lp.registry.interfaces.distroseriesparent import IDistroSeriesParent
+>>>>>>> MERGE-SOURCE
 from lp.registry.interfaces.entitlement import IEntitlement
 from lp.registry.interfaces.gpg import IGPGKey
 from lp.registry.interfaces.irc import IIrcID
@@ -1014,6 +1018,16 @@
     usedfor = IDistroSeries
 
 
+class EditDistroSeriesParent(AuthorizationBase):
+    """DistroSeriesParent can be edited by the same people who can edit
+    the derived_distroseries."""
+    permission = "launchpad.Edit"
+    usedfor = IDistroSeriesParent
+
+    def checkAuthenticated(self, user):
+        return check_permission(self.permission, self.obj.derived_series)
+
+
 class ViewCountry(AnonymousAuthorization):
     """Anyone can view a Country."""
     usedfor = ICountry

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-04-12 23:25:45 +0000
+++ lib/lp/registry/configure.zcml	2011-04-15 16:17:34 +0000
@@ -178,6 +178,25 @@
         <allow interface="lp.registry.interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"/>
     </class>
 
+    <!-- DistroSeriesParent -->
+    <securedutility
+        class="lp.registry.model.distroseriesparent.DistroSeriesParentSet"
+        provides="lp.registry.interfaces.distroseriesparent.IDistroSeriesParentSet">
+        <require
+            permission="launchpad.Edit"
+            set_attributes="new"/>
+        <allow
+            interface="lp.registry.interfaces.distroseriesparent.IDistroSeriesParentSet"/>
+    </securedutility>
+    <class
+        class="lp.registry.model.distroseriesparent.DistroSeriesParent">
+        <require
+            permission="launchpad.Edit"
+            set_schema="lp.registry.interfaces.distroseriesparent.IDistroSeriesParent"/>
+        <allow
+            interface="lp.registry.interfaces.distroseriesparent.IDistroSeriesParent"/>
+    </class>
+
     <class
         class="lp.registry.model.distroseries.DistroSeries">
         <allow

=== added file 'lib/lp/registry/interfaces/distroseriesparent.py'
--- lib/lp/registry/interfaces/distroseriesparent.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/interfaces/distroseriesparent.py	2011-04-15 16:17:34 +0000
@@ -0,0 +1,62 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# pylint: disable-msg=E0211,E0213
+
+"""DistroSeriesParent interface."""
+
+__metaclass__ = type
+
+__all__ = [
+    'IDistroSeriesParent',
+    'IDistroSeriesParentSet',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+    Int,
+    Bool,
+    )
+
+from canonical.launchpad import _
+from lp.registry.interfaces.distroseries import IDistroSeries
+
+
+class IDistroSeriesParent(Interface):
+    """`DistroSeriesParent` interface."""
+
+    id = Int(title=_('ID'), required=True, readonly=True)
+
+    derived_series = Reference(
+        IDistroSeries, title=_("Derived Series"), required=True,
+        description=_("The derived distribution series."))
+
+    parent_series = Reference(
+        IDistroSeries, title=_("Parent Series"), required=True,
+        description=_("The parent distribution series."))
+
+    initialized = Bool(
+        title=_("Initialized"), required=True,
+        description=_(
+            "Whether or not the derived_series has been populated with "
+            "packages from its parent_series."))
+
+
+class IDistroSeriesParentSet(Interface):
+    """`DistroSeriesParentSet` interface."""
+
+    def new(derived_series, parent_series, initialized):
+        """Create a new `DistroSeriesParent`."""
+
+    def getByDerivedSeries(derived_series):
+        """Get the `DistroSeriesParent` by derived series.
+
+        :param derived_series: An `IDistroseries`
+        """
+
+    def getByParentSeries(parent_series):
+        """Get the `DistroSeriesParent` by parent series.
+
+        :param parent_series: An `IDistroseries`
+        """

=== added file 'lib/lp/registry/model/distroseriesparent.py'
--- lib/lp/registry/model/distroseriesparent.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/model/distroseriesparent.py	2011-04-15 16:17:34 +0000
@@ -0,0 +1,74 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Database class for table DistroSeriesParent."""
+
+__metaclass__ = type
+
+__all__ = [
+    'DistroSeriesParent',
+    'DistroSeriesParentSet',
+    ]
+
+from storm.locals import (
+    Int,
+    Reference,
+    Storm,
+    Bool,
+    )
+from zope.interface import implements
+
+from canonical.launchpad.interfaces.lpstorm import (
+    IMasterStore,
+    IStore,
+    )
+from lp.registry.interfaces.distroseriesparent import (
+    IDistroSeriesParent,
+    IDistroSeriesParentSet,
+    )
+
+
+class DistroSeriesParent(Storm):
+    """See `IDistroSeriesParent`."""
+    implements(IDistroSeriesParent)
+    __storm_table__ = 'DistroSeriesParent'
+
+    id = Int(primary=True)
+
+    parent_series_id = Int(name='parent_series', allow_none=False)
+    parent_series = Reference(parent_series_id, 'DistroSeries.id')
+
+    derived_series_id = Int(name='derived_series', allow_none=False)
+    derived_series = Reference(derived_series_id, 'DistroSeries.id')
+
+    initialized = Bool(allow_none=False)
+
+
+class DistroSeriesParentSet:
+    """See `IDistroSeriesParentSet`."""
+    implements(IDistroSeriesParentSet)
+    title = "Cross reference of parent and derived distroseries."
+
+    def new(self, derived_series, parent_series, initialized):
+        """Make and return a new `DistroSeriesParent`."""
+        store = IMasterStore(DistroSeriesParent)
+        dsp = DistroSeriesParent()
+        dsp.derived_series = derived_series
+        dsp.parent_series = parent_series
+        dsp.initialized = initialized
+        store.add(dsp)
+        return dsp
+
+    def getByDerivedSeries(self, derived_series):
+        """See `IDistroSeriesParentSet`."""
+        store = IStore(DistroSeriesParent)
+        return store.find(
+            DistroSeriesParent,
+            DistroSeriesParent.derived_series_id == derived_series.id)
+
+    def getByParentSeries(self, parent_series):
+        """See `IDistroSeriesParentSet`."""
+        store = IStore(DistroSeriesParent)
+        return store.find(
+            DistroSeriesParent,
+            DistroSeriesParent.parent_series_id == parent_series.id)

=== added file 'lib/lp/registry/tests/test_distroseriesparent.py'
--- lib/lp/registry/tests/test_distroseriesparent.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_distroseriesparent.py	2011-04-15 16:17:34 +0000
@@ -0,0 +1,145 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for DistroSeriesParent model class."""
+
+__metaclass__ = type
+
+from testtools.matchers import (
+    Equals,
+    MatchesStructure,
+    )
+
+from zope.component import getUtility
+from zope.interface.verify import verifyObject
+from zope.security.interfaces import Unauthorized
+
+from canonical.launchpad.ftests import login
+from canonical.testing.layers import (
+    DatabaseFunctionalLayer,
+    ZopelessDatabaseLayer,
+    )
+from lp.registry.interfaces.distroseriesparent import (
+    IDistroSeriesParent,
+    IDistroSeriesParentSet,
+    )
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.sampledata import LAUNCHPAD_ADMIN
+
+
+class TestDistroSeriesParent(TestCaseWithFactory):
+    """Test the `DistroSeriesParent` model."""
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        #self.distribution = self.factory.makeDistribution(name='conftest')
+
+    def test_verify_interface(self):
+        # Test the interface for the model.
+        dsp = self.factory.makeDistroSeriesParent()
+        verified = verifyObject(IDistroSeriesParent, dsp)
+        self.assertTrue(verified)
+
+    def test_properties(self):
+        # Test the model properties.
+        parent_series = self.factory.makeDistroSeries()
+        derived_series = self.factory.makeDistroSeries()
+        dsp = self.factory.makeDistroSeriesParent(
+            derived_series=derived_series,
+            parent_series=parent_series,
+            initialized=True
+            )
+
+        self.assertThat(
+            dsp,
+            MatchesStructure(
+                derived_series=Equals(derived_series),
+                parent_series=Equals(parent_series),
+                initialized=Equals(True)
+                ))
+
+    def test_getByDerivedSeries(self):
+        parent_series = self.factory.makeDistroSeries()
+        derived_series = self.factory.makeDistroSeries()
+        self.factory.makeDistroSeriesParent(
+            derived_series, parent_series)
+        results = getUtility(IDistroSeriesParentSet).getByDerivedSeries(
+            derived_series)
+        self.assertEqual(1, results.count())
+        self.assertEqual(parent_series, results[0].parent_series)
+
+        # Making a second parent should add it to the results.
+        self.factory.makeDistroSeriesParent(
+            derived_series, self.factory.makeDistroSeries())
+        results = getUtility(IDistroSeriesParentSet).getByDerivedSeries(
+            derived_series)
+        self.assertEqual(2, results.count())
+
+    def test_getByParentSeries(self):
+        parent_series = self.factory.makeDistroSeries()
+        derived_series = self.factory.makeDistroSeries()
+        dsp = self.factory.makeDistroSeriesParent(
+            derived_series, parent_series)
+        results = getUtility(IDistroSeriesParentSet).getByParentSeries(
+            parent_series)
+        self.assertEqual(1, results.count())
+        self.assertEqual(derived_series, results[0].derived_series)
+
+        # Making a second child should add it to the results.
+        self.factory.makeDistroSeriesParent(
+            self.factory.makeDistroSeries(), parent_series)
+        results = getUtility(IDistroSeriesParentSet).getByParentSeries(
+            parent_series)
+        self.assertEqual(2, results.count())
+
+
+class TestDistroSeriesParentSecurity(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestDistroSeriesParentSecurity, self).setUp()
+        self.parent_series = self.factory.makeDistroSeries()
+        self.derived_series = self.factory.makeDistroSeries()
+        self.person = self.factory.makePerson()
+
+    def test_random_person_is_unauthorized(self):
+        dsp_set = getUtility(IDistroSeriesParentSet)
+        dsp = dsp_set.new(self.derived_series, self.parent_series, False)
+        with person_logged_in(self.person):
+            self.assertRaises(
+                Unauthorized,
+                setattr, dsp, "derived_series", self.parent_series)
+
+    def assertCanEdit(self):
+        dsp_set = getUtility(IDistroSeriesParentSet)
+        dsp = dsp_set.new(
+            self.derived_series, self.parent_series, False)
+        self.assertThat(
+            dsp,
+            MatchesStructure(
+                derived_series=Equals(self.derived_series),
+                parent_series=Equals(self.parent_series),
+                initialized=Equals(False)
+                ))
+
+    def test_distro_drivers_can_edit(self):
+        # Test that distro drivers can edit the data.
+        login(LAUNCHPAD_ADMIN)
+        self.derived_series.distribution.driver = self.person
+        with person_logged_in(self.person):
+            self.assertCanEdit()
+
+    def test_admins_can_edit(self):
+        login(LAUNCHPAD_ADMIN)
+        self.assertCanEdit()
+
+    def test_distro_owners_can_edit(self):
+        login(LAUNCHPAD_ADMIN)
+        self.derived_series.distribution.owner = self.person
+        with person_logged_in(self.person):
+            self.assertCanEdit()

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-04-14 16:10:39 +0000
+++ lib/lp/testing/factory.py	2011-04-15 16:17:34 +0000
@@ -190,6 +190,9 @@
 from lp.registry.interfaces.distroseriesdifferencecomment import (
     IDistroSeriesDifferenceCommentSource,
     )
+from lp.registry.interfaces.distroseriesparent import (
+    IDistroSeriesParentSet,
+    )
 from lp.registry.interfaces.gpg import (
     GPGKeyAlgorithm,
     IGPGKeySet,
@@ -2386,6 +2389,15 @@
         return getUtility(IDistroSeriesDifferenceCommentSource).new(
             distro_series_difference, owner, comment)
 
+    def makeDistroSeriesParent(self, derived_series=None, parent_series=None,
+                               initialized=False):
+        if parent_series is None:
+            parent_series = self.makeDistroSeries()
+        if derived_series is None:
+            derived_series = self.makeDistroSeries()
+        return getUtility(IDistroSeriesParentSet).new(
+            derived_series, parent_series, initialized)
+
     def makeDistroArchSeries(self, distroseries=None,
                              architecturetag=None, processorfamily=None,
                              official=True, owner=None,