launchpad-dev team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Working yesterday on lp:~adeuring/launchpad/bug-746460 , I found some
quire weird code in ProductSeries.setPackaging():

- We can link a product series with two sourcepackages of the same
  distroseries, if we create the links via sourcepackage.setPackaging()
- If we try to link the two sourcepackages by calling
  productseries.setPackaging(), the second call simply changes the
  packaging record created during the first call.
- If we create the two packagings via sourcepackage.setPackaging()
  and then try to call productseries.setPackaging()

Is there any reason why we ProductSeries.setPackaging() tries to change
the sourcpackagename of an existing packaging link? In other words: Is
there any need for a constraint "A product series should have at most
one packaging link per distroseries"?

See the attached file for unit tests describing the issue formally. All
tests pass, but only the first one should...

from storm.exceptions import IntegrityError
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.testing import (
    person_logged_in,
    TestCaseWithFactory,
    )


class TestSetPackaging(TestCaseWithFactory):

    layer = DatabaseFunctionalLayer

    def setUp(self):
        super(TestSetPackaging, self).setUp()
        self.distroseries = self.factory.makeDistroSeries()
        self.sourcepackage_a = self.factory.makeSourcePackage(
            distroseries = self.distroseries)
        self.sourcepackage_b = self.factory.makeSourcePackage(
            distroseries = self.distroseries)
        self.productseries = self.factory.makeProductSeries()

    def test_sourcepackage_makes_packagings(self):
        self.sourcepackage_a.setPackaging(
            self.productseries, self.factory.makePerson())
        self.sourcepackage_b.setPackaging(
            self.productseries, self.factory.makePerson())
        # The productseries now has TWO packagings.
        self.assertEqual(
            [self.sourcepackage_b, self.sourcepackage_a],
            [packaging.sourcepackage
             for packaging in self.productseries.packagings])

    def test_productseries_makes_packagings(self):
        self.productseries.setPackaging(
            self.distroseries, self.sourcepackage_a.sourcepackagename,
            self.factory.makePerson())
        self.productseries.setPackaging(
            self.distroseries, self.sourcepackage_b.sourcepackagename,
            self.factory.makePerson())
        # The second call of ProductSeries.setPackaging() changes
        # the packaging record created in the first call;
        # productseries.packagings has only ONE packaging.
        self.assertEqual(
            [self.sourcepackage_b],
            [packaging.sourcepackage
             for packaging in self.productseries.packagings])

    def test_sourcepackage_makes_packagings_productseries_overrides(self):
        self.sourcepackage_a.setPackaging(
            self.productseries, self.factory.makePerson())
        self.sourcepackage_b.setPackaging(
            self.productseries, self.factory.makePerson())
        self.assertEqual(
            [self.sourcepackage_b, self.sourcepackage_a],
            [packaging.sourcepackage
             for packaging in self.productseries.packagings])
        # Calling setPackaging for the first record of series.packagings
        # does not have any real effect.
        self.productseries.setPackaging(
            self.distroseries, self.sourcepackage_b.sourcepackagename,
            self.factory.makePerson())
        self.assertEqual(
            [self.sourcepackage_b, self.sourcepackage_a],
            [packaging.sourcepackage
             for packaging in self.productseries.packagings])

        # Calling setPackaging for the second record of series.packagings
        # causes an IntegrityError, because the method wants to
        # change the first entry of productseries.packagings, which
        # currently links the series with sourcepackage_b.
        # This fails because the second record already links the
        # series and sourcepackage_a.
        self.assertRaises(
            IntegrityError, self.productseries.setPackaging,
            self.distroseries, self.sourcepackage_a.sourcepackagename,
            self.factory.makePerson())