launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05031
[Merge] lp:~sinzui/launchpad/dsp-official-branches-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-official-branches-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #851276 in Launchpad itself: "Update DSP when an official package branch is linked"
https://bugs.launchpad.net/launchpad/+bug/851276
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/dsp-official-branches-0/+merge/76128
Ensure official package branches are represented in the DSP table.
Launchpad bug: https://bugs.launchpad.net/bugs/851276
Pre-implementation: jcsackett
Ensure that official package branches have an entry in the DSP table.
Any branch linked to a package name or uploaded package to a distribution
represents an official package that user can report bugs or ask questions
about.
--------------------------------------------------------------------
RULES
* Update DSP.ensure to accept an alternate argument for SPPH that
provides the distro and spn to create the DSP table row.
* Both DistributionPackageLinkedBranch and PackageLinkedBranch
require a SuiteSourcePackage to accomplish the setBranch(). This
object provides both a SPN and a distro.
* There are no unittests or doctests that specifically verify that
DSP.ensure works.
* ADDENDUM: add a delete method that only deletes if it is safe to
delete a persistent DSP.
* Update SourcePackage.setBranch() to call DSP.ensure()
* Update test_sourcepackage testcases to verify that linking to
a branch creates an official dsp.
* Verify that the the persistent DSP is removed if there is not SPPH.
QA
* For a distro you can set the official branches find a package that
has never been uploaded/published.
* Verify the package is not listed in the package picker if the dsp flag
is enabled, or check that the DSP table *does not* have an entry.
* Link the distro or current series package to a branch.
* Verify that the package is listed in the package picker if the dsp flag
is enabled, or check that the DSP table *does* have an entry.
* Unlink the the package.
* Verify the package is not listed in the package picker if the dsp flag
is enabled, or check that the DSP table *does not* have an entry.
LINT
lib/lp/registry/configure.zcml
lib/lp/registry/interfaces/distributionsourcepackage.py
lib/lp/registry/model/distributionsourcepackage.py
lib/lp/registry/model/sourcepackage.py
lib/lp/registry/tests/test_distributionsourcepackage.py
lib/lp/registry/tests/test_sourcepackage.py
TEST
./bin/test -vvc lp.registry.tests.test_distributionsourcepackage
./bin/test -vvc lp.registry.tests.test_sourcepackage
IMPLEMENTATION
Updated ensure() to accept a SourcePackage or SPPH. Added a delete method
that only deletes a persistent DSP if there is not SPPH.
lib/lp/registry/configure.zcml
lib/lp/registry/interfaces/distributionsourcepackage.py
lib/lp/registry/model/distributionsourcepackage.py
lib/lp/registry/tests/test_distributionsourcepackage.py
Updated setBranch() to call DSP.ensure() after setting the branch, and
call DSP.delete() when the branch was set to None.
lib/lp/registry/model/sourcepackage.py
lib/lp/registry/tests/test_sourcepackage.py
--
https://code.launchpad.net/~sinzui/launchpad/dsp-official-branches-0/+merge/76128
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-official-branches-0 into lp:launchpad.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2011-09-15 18:45:21 +0000
+++ lib/lp/registry/configure.zcml 2011-09-19 23:00:30 +0000
@@ -481,6 +481,7 @@
critical_bugtasks
current_publishing_records
currentrelease
+ delete
development_version
displayname
distribution
=== modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py'
--- lib/lp/registry/interfaces/distributionsourcepackage.py 2011-05-12 21:51:01 +0000
+++ lib/lp/registry/interfaces/distributionsourcepackage.py 2011-09-19 23:00:30 +0000
@@ -210,3 +210,9 @@
Distro sourcepackages compare not equal if either of their
distribution or sourcepackagename compare not equal.
"""
+
+ def delete():
+ """Delete the persistent DSP if it exists.
+
+ :return: True if a persistent object was removed, otherwise False.
+ """
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2011-09-16 19:02:49 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2011-09-19 23:00:30 +0000
@@ -207,6 +207,16 @@
# in the database.
return self._get(self.distribution, self.sourcepackagename)
+ def delete(self):
+ """See `DistributionSourcePackage`."""
+ dsp_in_db = self._self_in_database
+ no_spph = self.publishing_history.count() == 0
+ if dsp_in_db is not None and no_spph:
+ store = IStore(dsp_in_db)
+ store.remove(dsp_in_db)
+ return True
+ return False
+
def recalculateBugHeatCache(self):
"""See `IHasBugHeat`."""
row = IStore(Bug).find(
@@ -527,16 +537,30 @@
return dsp
@classmethod
- def ensure(cls, spph):
+ def ensure(cls, spph=None, sourcepackage=None):
"""Create DistributionSourcePackage record, if necessary.
- Only create a record for primary archives (i.e. not for PPAs).
+ Only create a record for primary archives (i.e. not for PPAs) or
+ for official package branches. Requires either a SourcePackage
+ or a SourcePackagePublishingHistory.
+
+ :param spph: A SourcePackagePublishingHistory to create a DSP
+ to represent an official uploaded/published package.
+ :param sourcepackage: A SourcePackage to create a DSP to represent an
+ official package branch.
"""
- if spph.archive.purpose != ArchivePurpose.PRIMARY:
- return
-
- distribution = spph.distroseries.distribution
- sourcepackagename = spph.sourcepackagerelease.sourcepackagename
+ if spph is None and sourcepackage is None:
+ raise ValueError(
+ 'ensure() must be called with either a SPPH '
+ 'or a SourcePackage.')
+ if spph is not None:
+ if spph.archive.purpose != ArchivePurpose.PRIMARY:
+ return
+ distribution = spph.distroseries.distribution
+ sourcepackagename = spph.sourcepackagerelease.sourcepackagename
+ else:
+ distribution = sourcepackage.distribution
+ sourcepackagename = sourcepackage.sourcepackagename
dsp = cls._get(distribution, sourcepackagename)
if dsp is None:
upstream_link_allowed = is_upstream_link_allowed(spph)
@@ -572,8 +596,7 @@
po_message_count = Int()
is_upstream_link_allowed = Bool()
enable_bugfiling_duplicate_search = Bool()
-
- # XXX kiko 2006-08-16: Bad method name, no need to be a property.
+
@property
def currentrelease(self):
"""See `IDistributionSourcePackage`."""
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py 2011-09-13 05:23:16 +0000
+++ lib/lp/registry/model/sourcepackage.py 2011-09-19 23:00:30 +0000
@@ -754,6 +754,14 @@
SeriesSourcePackageBranchSet.new(
self.distroseries, pocket, self.sourcepackagename, branch,
registrant)
+ # Avoid circular imports.
+ from lp.registry.model.distributionsourcepackage import (
+ DistributionSourcePackage,
+ )
+ DistributionSourcePackage.ensure(sourcepackage=self)
+ else:
+ # Delete the official DSP if there is no publishing history.
+ self.distribution_sourcepackage.delete()
@property
def linked_branches(self):
=== modified file 'lib/lp/registry/tests/test_distributionsourcepackage.py'
--- lib/lp/registry/tests/test_distributionsourcepackage.py 2011-08-12 11:37:08 +0000
+++ lib/lp/registry/tests/test_distributionsourcepackage.py 2011-09-19 23:00:30 +0000
@@ -9,11 +9,16 @@
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.model.distributionsourcepackage import (
+ DistributionSourcePackage,
+ DistributionSourcePackageInDatabase,
+ )
from lp.registry.model.karma import KarmaTotalCache
from lp.soyuz.enums import PackagePublishingStatus
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
@@ -37,6 +42,79 @@
dsp = naked_distribution.getSourcePackage(name='pmount')
self.assertEqual(None, dsp.summary)
+ def test_ensure_spph_creates_a_dsp_in_db(self):
+ # The DSP.ensure() class methods creates a persistent instance
+ # if one does not exist.
+ spph = self.factory.makeSourcePackagePublishingHistory()
+ spph_dsp = spph.sourcepackagerelease.distrosourcepackage
+ DistributionSourcePackage.ensure(spph)
+ new_dsp = DistributionSourcePackage._get(
+ spph_dsp.distribution, spph_dsp.sourcepackagename)
+ self.assertIsNot(None, new_dsp)
+ self.assertIsNot(spph_dsp, new_dsp)
+ self.assertEqual(spph_dsp.distribution, new_dsp.distribution)
+ self.assertEqual(
+ spph_dsp.sourcepackagename, new_dsp.sourcepackagename)
+
+ def test_ensure_spph_dsp_in_db_exists(self):
+ # The DSP.ensure() class methods does not create duplicate
+ # persistent instances; it skips the query to create the DSP.
+ store = IStore(DistributionSourcePackageInDatabase)
+ start_count = store.find(DistributionSourcePackageInDatabase).count()
+ spph = self.factory.makeSourcePackagePublishingHistory()
+ DistributionSourcePackage.ensure(spph)
+ new_count = store.find(DistributionSourcePackageInDatabase).count()
+ self.assertEqual(start_count + 1, new_count)
+ final_count = store.find(DistributionSourcePackageInDatabase).count()
+ self.assertEqual(new_count, final_count)
+
+ def test_ensure_spph_does_not_create_dsp_in_db_non_primary_archive(self):
+ # The DSP.ensure() class methods creates a persistent instance
+ # if one does not exist.
+ archive = self.factory.makeArchive()
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ archive=archive)
+ spph_dsp = spph.sourcepackagerelease.distrosourcepackage
+ DistributionSourcePackage.ensure(spph)
+ new_dsp = DistributionSourcePackage._get(
+ spph_dsp.distribution, spph_dsp.sourcepackagename)
+ self.assertIs(None, new_dsp)
+
+ def test_ensure_suitesourcepackage_creates_a_dsp_in_db(self):
+ # The DSP.ensure() class methods creates a persistent instance
+ # if one does not exist.
+ sourcepackage = self.factory.makeSourcePackage()
+ DistributionSourcePackage.ensure(sourcepackage=sourcepackage)
+ new_dsp = DistributionSourcePackage._get(
+ sourcepackage.distribution, sourcepackage.sourcepackagename)
+ self.assertIsNot(None, new_dsp)
+ self.assertEqual(sourcepackage.distribution, new_dsp.distribution)
+ self.assertEqual(
+ sourcepackage.sourcepackagename, new_dsp.sourcepackagename)
+
+ def test_delete_without_dsp_in_db(self):
+ # Calling delete() on a DSP without persistence returns False.
+ dsp = self.factory.makeDistributionSourcePackage()
+ self.assertFalse(dsp.delete())
+
+ def test_delete_with_dsp_in_db_with_history(self):
+ # Calling delete() on a persistent DSP with SPPH returns False.
+ # Once a package is uploaded, it cannot be deleted.
+ spph = self.factory.makeSourcePackagePublishingHistory()
+ dsp = spph.sourcepackagerelease.distrosourcepackage
+ DistributionSourcePackage.ensure(spph=spph)
+ transaction.commit()
+ self.assertFalse(dsp.delete())
+
+ def test_delete_with_dsp_in_db_without_history(self):
+ # Calling delete() on a persistent DSP without SPPH returns True.
+ # A package without history was a mistake.
+ sp = self.factory.makeSourcePackage()
+ DistributionSourcePackage.ensure(sourcepackage=sp)
+ transaction.commit()
+ dsp = sp.distribution_sourcepackage
+ self.assertTrue(dsp.delete())
+
class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py 2011-08-12 11:37:08 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py 2011-09-19 23:00:30 +0000
@@ -24,6 +24,9 @@
from lp.registry.interfaces.distribution import NoPartnerArchive
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.model.distributionsourcepackage import (
+ DistributionSourcePackage,
+ )
from lp.registry.model.packaging import Packaging
from lp.soyuz.enums import (
ArchivePurpose,
@@ -81,6 +84,10 @@
with person_logged_in(sourcepackage.distribution.owner):
sourcepackage.setBranch(pocket, branch, registrant)
self.assertEqual(branch, sourcepackage.getBranch(pocket))
+ # A DSP was created for the official branch.
+ new_dsp = DistributionSourcePackage._get(
+ sourcepackage.distribution, sourcepackage.sourcepackagename)
+ self.assertIsNot(None, new_dsp)
def test_change_branch_once_set(self):
# We can change the official branch for a a pocket of a source package
@@ -108,6 +115,20 @@
sourcepackage.setBranch(pocket, None, registrant)
self.assertIs(None, sourcepackage.getBranch(pocket))
+ def test_unsetBranch_delete_unpublished_dsp(self):
+ # Setting the official branch for a pocket to 'None' deletes the
+ # official DSP record if there is no SPPH.
+ sourcepackage = self.factory.makeSourcePackage()
+ pocket = PackagePublishingPocket.RELEASE
+ registrant = self.factory.makePerson()
+ branch = self.factory.makePackageBranch(sourcepackage=sourcepackage)
+ with person_logged_in(sourcepackage.distribution.owner):
+ sourcepackage.setBranch(pocket, branch, registrant)
+ sourcepackage.setBranch(pocket, None, registrant)
+ new_dsp = DistributionSourcePackage._get(
+ sourcepackage.distribution, sourcepackage.sourcepackagename)
+ self.assertIs(None, new_dsp)
+
def test_linked_branches(self):
# ISourcePackage.linked_branches is a mapping of pockets to branches.
sourcepackage = self.factory.makeSourcePackage()
