launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02790
[Merge] lp:~abentley/launchpad/translation-splitting-job into lp:launchpad/db-devel
Aaron Bentley has proposed merging lp:~abentley/launchpad/translation-splitting-job into lp:launchpad/db-devel with lp:~abentley/launchpad/refactor-merge-job as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/translation-splitting-job/+merge/51371
= Summary =
Fix bug #719521: translations should split when packaging links are removed
== Proposed fix ==
Implement TranslationSplitJob.
== Pre-implementation notes ==
Discussed with Henninge, Deryck
== Implementation details ==
Added an ObjectDeletedEvent when Packaging.destroySelf is invoked, ensured PackagingUtil.deletePackaging also emits it. (This only works if Store.remove is not invoked directly, but Storm does not provide a public event API that could be used to hook into remove calls.)
Added enumeration for TRANSLATION_SPLIT. Added TranslationSplitJob, using TRANSLATION_SPLIT as its class_job_type.
Tested that the existing packaging_translations job supports
TranslationSplitJob.
Converted TranslationMergeJob.findJobs into findJobs, and made it a client of PackagingJobDerived.iterReady. This means that it returns the correct subclass, and also makes the tests more obvious.
Extracted use_in_template, make_translation_splitter and make_shared_potmsgset
from test methods.
== Tests ==
bint/test -t test_packaging -t test_packaging_translations -t test_translationpackagingjob -t test_translationsplitter
== Demo and Q/A ==
None
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/scripts/tests/test_packaging_translations.py
lib/lp/registry/interfaces/packagingjob.py
lib/canonical/config/schema-lazr.conf
lib/lp/registry/model/packaging.py
lib/lp/registry/model/packagingjob.py
lib/lp/registry/tests/test_packaging.py
lib/lp/testing/factory.py
lib/lp/translations/interfaces/translationpackagingjob.py
lib/lp/translations/model/translationpackagingjob.py
lib/lp/translations/tests/test_translationpackagingjob.py
lib/lp/translations/configure.zcml
lib/lp/translations/tests/test_translationsplitter.py
configs/testrunner/launchpad-lazr.conf
lib/lp/translations/scripts/tests/test_merge_existing_packagings.py
configs/development/launchpad-lazr.conf
./lib/canonical/config/schema-lazr.conf
519: Line exceeds 78 characters.
602: Line exceeds 78 characters.
1053: Line exceeds 78 characters.
./configs/testrunner/launchpad-lazr.conf
144: Line exceeds 78 characters.
./configs/development/launchpad-lazr.conf
97: Line exceeds 78 characters.
116: Line exceeds 78 characters.
127: Line exceeds 78 characters.
--
https://code.launchpad.net/~abentley/launchpad/translation-splitting-job/+merge/51371
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/translation-splitting-job into lp:launchpad/db-devel.
=== modified file 'lib/lp/registry/model/packaging.py'
--- lib/lp/registry/model/packaging.py 2011-02-16 16:40:43 +0000
+++ lib/lp/registry/model/packaging.py 2011-02-25 20:59:20 +0000
@@ -6,10 +6,13 @@
__metaclass__ = type
__all__ = ['Packaging', 'PackagingUtil']
+from lazr.lifecycle.event import (
+ ObjectCreatedEvent,
+ ObjectDeletedEvent,
+ )
from sqlobject import ForeignKey
from zope.event import notify
from zope.interface import implements
-from lazr.lifecycle.event import ObjectCreatedEvent
from canonical.database.constants import (
DEFAULT,
@@ -61,6 +64,10 @@
super(Packaging, self).__init__(**kwargs)
notify(ObjectCreatedEvent(self))
+ def destroySelf(self):
+ notify(ObjectDeletedEvent(self))
+ super(Packaging, self).destroySelf()
+
class PackagingUtil:
"""Utilities for Packaging."""
=== modified file 'lib/lp/registry/model/packagingjob.py'
--- lib/lp/registry/model/packagingjob.py 2011-02-25 20:59:19 +0000
+++ lib/lp/registry/model/packagingjob.py 2011-02-25 20:59:20 +0000
@@ -43,6 +43,12 @@
Merge translations betweeen productseries and sourcepackage.
""")
+ TRANSLATION_SPLIT = DBItem(1, """
+ Split translations between productseries and sourcepackage.
+
+ Split translations between productseries and sourcepackage.
+ """)
+
class PackagingJob(StormBase):
"""Base class for jobs related to a packaging."""
@@ -104,6 +110,10 @@
_subclass = {}
_event_types = {}
+ @property
+ def sourcepackage(self):
+ return self.distroseries.getSourcePackage(self.sourcepackagename)
+
@staticmethod
def _register_subclass(cls):
"""Register this class with its enumeration."""
=== modified file 'lib/lp/registry/tests/test_packaging.py'
--- lib/lp/registry/tests/test_packaging.py 2011-02-10 21:58:48 +0000
+++ lib/lp/registry/tests/test_packaging.py 2011-02-25 20:59:20 +0000
@@ -7,8 +7,12 @@
from unittest import TestLoader
+from lazr.lifecycle.event import (
+ ObjectCreatedEvent,
+ ObjectDeletedEvent,
+ )
from zope.component import getUtility
-from lazr.lifecycle.event import ObjectCreatedEvent
+from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.registry.interfaces.distribution import IDistributionSet
@@ -41,6 +45,16 @@
self.assertIsInstance(event, ObjectCreatedEvent)
self.assertIs(packaging, event.object)
+ def test_destroySelf_notifies(self):
+ """destroySelf creates a notification."""
+ packaging_util = getUtility(IPackagingUtil)
+ packaging = self.factory.makePackaging()
+ with EventRecorder() as recorder:
+ removeSecurityProxy(packaging).destroySelf()
+ (event,) = recorder.events
+ self.assertIsInstance(event, ObjectDeletedEvent)
+ self.assertIs(removeSecurityProxy(packaging), event.object)
+
class PackagingUtilMixin:
"""Common items for testing IPackagingUtil."""
@@ -182,6 +196,18 @@
else:
self.fail("AssertionError was not raised.")
+ def test_deletePackaging_notifies(self):
+ """Deleting a Packaging creates a notification."""
+ packaging_util = getUtility(IPackagingUtil)
+ packaging = self.factory.makePackaging()
+ with EventRecorder() as recorder:
+ packaging_util.deletePackaging(
+ packaging.productseries, packaging.sourcepackagename,
+ packaging.distroseries)
+ (event,) = recorder.events
+ self.assertIsInstance(event, ObjectDeletedEvent)
+ self.assertIs(removeSecurityProxy(packaging), event.object)
+
def test_suite():
return TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-02-25 19:27:28 +0000
+++ lib/lp/testing/factory.py 2011-02-25 20:59:20 +0000
@@ -814,6 +814,14 @@
productseries=productseries, distroseries=distroseries,
name=name))
+ def makePackaging(self):
+ """Create a new Packaging."""
+ productseries = self.makeProductSeries()
+ sourcepackage = self.makeSourcePackage()
+ return productseries.setPackaging(
+ sourcepackage.distroseries, sourcepackage.sourcepackagename,
+ productseries.owner)
+
def makeProcessor(self, family=None, name=None, title=None,
description=None):
"""Create a new processor.
=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml 2011-02-25 20:59:19 +0000
+++ lib/lp/translations/configure.zcml 2011-02-25 20:59:20 +0000
@@ -639,6 +639,10 @@
class="lp.translations.model.translationpackagingjob.TranslationMergeJob">
<allow interface='lp.services.job.interfaces.job.IRunnableJob'/>
</class>
+ <class
+ class="lp.translations.model.translationpackagingjob.TranslationSplitJob">
+ <allow interface='lp.services.job.interfaces.job.IRunnableJob'/>
+ </class>
<utility
component="lp.translations.model.translationtemplatesbuildjob.TranslationTemplatesBuildJob"
provides="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob"
=== modified file 'lib/lp/translations/model/translationpackagingjob.py'
--- lib/lp/translations/model/translationpackagingjob.py 2011-02-25 20:59:19 +0000
+++ lib/lp/translations/model/translationpackagingjob.py 2011-02-25 20:59:20 +0000
@@ -8,6 +8,7 @@
from lazr.lifecycle.interfaces import (
IObjectCreatedEvent,
+ IObjectDeletedEvent,
)
from zope.interface import (
classProvides,
@@ -30,8 +31,10 @@
TransactionManager,
TranslationMerger,
)
-
-__all__ = ['TranslationMergeJob']
+from lp.translations.utilities.translationsplitter import TranslationSplitter
+
+
+__all__ = ['TranslationMergeJob', 'TranslationSplitJob']
def schedule_merge(packaging, event):
@@ -92,3 +95,17 @@
tm = TransactionManager(None, False)
TranslationMerger.mergePackagingTemplates(
self.productseries, self.sourcepackagename, self.distroseries, tm)
+
+
+class TranslationSplitJob(TranslationPackagingJob):
+ """Job for merging translations between a product and sourcepackage."""
+
+ implements(IRunnableJob)
+
+ class_job_type = PackagingJobType.TRANSLATION_SPLIT
+
+ create_on_event = IObjectDeletedEvent
+
+ def run(self):
+ """See `IRunnableJob`."""
+ TranslationSplitter(self.productseries, self.sourcepackage).split()
=== modified file 'lib/lp/translations/scripts/tests/test_packaging_translations.py'
--- lib/lp/translations/scripts/tests/test_packaging_translations.py 2011-02-25 20:59:19 +0000
+++ lib/lp/translations/scripts/tests/test_packaging_translations.py 2011-02-25 20:59:20 +0000
@@ -10,9 +10,10 @@
from canonical.launchpad.scripts.tests import run_script
from canonical.testing.layers import ZopelessAppServerLayer
+from lp.translations.model.translationpackagingjob import (
+ TranslationSplitJob)
from lp.testing import TestCaseWithFactory
from lp.translations.tests.test_translationpackagingjob import (
- count_translations,
make_translation_merge_job,
)
@@ -23,6 +24,8 @@
def test_merge_translations(self):
job = make_translation_merge_job(self.factory)
+ TranslationSplitJob.create(
+ job.productseries, job.distroseries, job.sourcepackagename)
transaction.commit()
retcode, stdout, stderr = run_script(
'cronscripts/run_jobs.py', ['packaging_translations'],
@@ -32,6 +35,6 @@
INFO Running synchronously.
INFO Deleted POTMsgSets: 1. TranslationMessages: 1.
INFO Ran 1 TranslationMergeJob jobs.
+ INFO Ran 1 TranslationSplitJob jobs.
"""), stderr)
self.assertEqual('', stdout)
- self.assertEqual(1, count_translations(job))
=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py 2011-02-25 20:59:19 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py 2011-02-25 20:59:20 +0000
@@ -6,16 +6,19 @@
__metaclass__ = type
-from storm.locals import Store
import transaction
+from zope.component import getUtility
from canonical.launchpad.webapp.testing import verifyObject
from canonical.testing.layers import (
LaunchpadZopelessLayer,
)
-from lp.registry.model.packagingjob import PackagingJob
+from lp.registry.interfaces.packaging import IPackagingUtil
+from lp.registry.model.packagingjob import PackagingJob, PackagingJobDerived
from lp.services.job.interfaces.job import IRunnableJob
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+ TestCaseWithFactory,
+ )
from lp.translations.interfaces.side import TranslationSide
from lp.translations.interfaces.translationpackagingjob import (
ITranslationPackagingJobSource,
@@ -24,6 +27,10 @@
from lp.translations.model.translationpackagingjob import (
TranslationMergeJob,
TranslationPackagingJob,
+ TranslationSplitJob,
+ )
+from lp.translations.tests.test_translationsplitter import (
+ make_shared_potmsgset,
)
@@ -84,6 +91,23 @@
return len(tm)
+class JobFinder:
+
+ def __init__(self, productseries, sourcepackage, job_class):
+ self.productseries = productseries
+ self.sourcepackagename = sourcepackage.sourcepackagename
+ self.distroseries = sourcepackage.distroseries
+ self.job_type = job_class.class_job_type
+
+ def find(self):
+ return list(PackagingJobDerived.iterReady([
+ PackagingJob.productseries_id == self.productseries.id,
+ PackagingJob.sourcepackagename_id == self.sourcepackagename.id,
+ PackagingJob.distroseries_id == self.distroseries.id,
+ PackagingJob.job_type == self.job_type,
+ ]))
+
+
class TestTranslationPackagingJob(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
@@ -134,25 +158,42 @@
job.run()
self.assertEqual(2, count_translations(job))
- @staticmethod
- def findJobs(productseries, sourcepackage):
- store = Store.of(productseries)
- result = store.find(
- PackagingJob,
- PackagingJob.productseries_id == productseries.id,
- PackagingJob.sourcepackagename_id ==
- sourcepackage.sourcepackagename.id,
- PackagingJob.distroseries_id ==
- sourcepackage.distroseries.id,
- )
- return list(result)
-
def test_create_packaging_makes_job(self):
"""Creating a Packaging should make a TranslationMergeJob."""
productseries = self.factory.makeProductSeries()
sourcepackage = self.factory.makeSourcePackage()
- self.assertEqual([], self.findJobs(productseries, sourcepackage))
+ finder = JobFinder(productseries, sourcepackage, TranslationMergeJob)
+ self.assertEqual([], finder.find())
sourcepackage.setPackaging(productseries, productseries.owner)
- self.assertNotEqual([], self.findJobs(productseries, sourcepackage))
+ self.assertNotEqual([], finder.find())
# Ensure no constraints were violated.
transaction.commit()
+
+
+class TestTranslationSplitJob(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def test_run_splits_translations(self):
+ upstream_item, ubuntu_item = make_shared_potmsgset(self.factory)
+ job = TranslationSplitJob.create(
+ upstream_item.potemplate.productseries,
+ ubuntu_item.potemplate.distroseries,
+ ubuntu_item.potemplate.sourcepackagename,
+ )
+ self.assertEqual(upstream_item.potmsgset, ubuntu_item.potmsgset)
+ job.run()
+ self.assertNotEqual(upstream_item.potmsgset, ubuntu_item.potmsgset)
+
+ def test_deletePackaging_makes_job(self):
+ """Creating a Packaging should make a TranslationMergeJob."""
+ packaging = self.factory.makePackaging()
+ finder = JobFinder(
+ packaging.productseries, packaging.sourcepackage,
+ TranslationSplitJob)
+ self.assertEqual([], finder.find())
+ getUtility(IPackagingUtil).deletePackaging(
+ packaging.productseries, packaging.sourcepackagename,
+ packaging.distroseries)
+ (job,) = finder.find()
+ self.assertIsInstance(job, TranslationSplitJob)
=== modified file 'lib/lp/translations/tests/test_translationsplitter.py'
--- lib/lp/translations/tests/test_translationsplitter.py 2011-02-23 16:23:12 +0000
+++ lib/lp/translations/tests/test_translationsplitter.py 2011-02-25 20:59:20 +0000
@@ -16,14 +16,34 @@
)
+def use_in_template(factory, potmsgset, potemplate):
+ return potmsgset.setSequence(
+ potemplate, factory.getUniqueInteger())
+
+
+def make_translation_splitter(factory):
+ return TranslationSplitter(
+ factory.makeProductSeries(), factory.makeSourcePackage())
+
+
+def make_shared_potmsgset(factory, splitter=None):
+ if splitter is None:
+ splitter = make_translation_splitter(factory)
+ upstream_template = factory.makePOTemplate(
+ productseries=splitter.productseries)
+ potmsgset = factory.makePOTMsgSet(
+ upstream_template, sequence=factory.getUniqueInteger())
+ (upstream_item,) = potmsgset.getAllTranslationTemplateItems()
+ ubuntu_template = factory.makePOTemplate(
+ sourcepackage=splitter.sourcepackage)
+ ubuntu_item = use_in_template(factory, potmsgset, ubuntu_template)
+ return upstream_item, ubuntu_item
+
+
class TestTranslationSplitter(TestCaseWithFactory):
layer = ZopelessDatabaseLayer
- def useInTemplate(self, potmsgset, potemplate):
- return potmsgset.setSequence(
- potemplate, self.factory.getUniqueInteger())
-
def test_findShared_requires_both(self):
"""Results are only included when both sides have the POTMsgSet."""
upstream_template = self.factory.makePOTemplate(
@@ -36,41 +56,27 @@
splitter = TranslationSplitter(productseries, package)
self.assertContentEqual([], splitter.findShared())
(upstream_item,) = potmsgset.getAllTranslationTemplateItems()
- ubuntu_item = self.useInTemplate(potmsgset, ubuntu_template)
+ ubuntu_item = use_in_template(
+ self.factory, potmsgset, ubuntu_template)
self.assertContentEqual(
[(upstream_item, ubuntu_item)], splitter.findShared())
removeSecurityProxy(upstream_item).destroySelf()
self.assertContentEqual([], splitter.findShared())
- def makeTranslationSplitter(self):
- return TranslationSplitter(
- self.factory.makeProductSeries(),
- self.factory.makeSourcePackage())
-
- def makeSharedPOTMsgSet(self, splitter):
- upstream_template = self.factory.makePOTemplate(
- productseries=splitter.productseries)
- potmsgset = self.factory.makePOTMsgSet(
- upstream_template, sequence=self.factory.getUniqueInteger())
- (upstream_item,) = potmsgset.getAllTranslationTemplateItems()
- ubuntu_template = self.factory.makePOTemplate(
- sourcepackage=splitter.sourcepackage)
- ubuntu_item = self.useInTemplate(potmsgset, ubuntu_template)
- return upstream_item, ubuntu_item
-
def test_findSharedGroupsPOTMsgSet(self):
"""POTMsgSets are correctly grouped."""
- splitter = self.makeTranslationSplitter()
- self.makeSharedPOTMsgSet(splitter)
- self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ make_shared_potmsgset(self.factory, splitter)
+ make_shared_potmsgset(self.factory, splitter)
for num, (upstream, ubuntu) in enumerate(splitter.findShared()):
self.assertEqual(upstream.potmsgset, ubuntu.potmsgset)
self.assertEqual(1, num)
def test_splitPOTMsgSet(self):
"""Splitting a POTMsgSet clones it and updates TemplateItem."""
- splitter = self.makeTranslationSplitter()
- upstream_item, ubuntu_item = self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ upstream_item, ubuntu_item = make_shared_potmsgset(
+ self.factory, splitter)
ubuntu_template = ubuntu_item.potemplate
ubuntu_sequence = ubuntu_item.sequence
new_potmsgset = splitter.splitPOTMsgSet(ubuntu_item)
@@ -78,8 +84,9 @@
def test_migrateTranslations_diverged_upstream(self):
"""Diverged upstream translation stays put."""
- splitter = self.makeTranslationSplitter()
- upstream_item, ubuntu_item = self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ upstream_item, ubuntu_item = make_shared_potmsgset(
+ self.factory, splitter)
upstream_message = self.factory.makeCurrentTranslationMessage(
potmsgset=upstream_item.potmsgset,
potemplate=upstream_item.potemplate, diverged=True)
@@ -94,8 +101,9 @@
def test_migrateTranslations_diverged_ubuntu(self):
"""Diverged ubuntu translation moves."""
- splitter = self.makeTranslationSplitter()
- upstream_item, ubuntu_item = self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ upstream_item, ubuntu_item = make_shared_potmsgset(
+ self.factory, splitter)
ubuntu_message = self.factory.makeCurrentTranslationMessage(
potmsgset=ubuntu_item.potmsgset,
potemplate=ubuntu_item.potemplate, diverged=True)
@@ -111,8 +119,9 @@
def test_migrateTranslations_shared(self):
"""Shared translation is copied."""
- splitter = self.makeTranslationSplitter()
- upstream_item, ubuntu_item = self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ upstream_item, ubuntu_item = make_shared_potmsgset(
+ self.factory, splitter)
self.factory.makeCurrentTranslationMessage(
potmsgset=upstream_item.potmsgset)
splitter.splitPOTMsgSet(ubuntu_item)
@@ -127,8 +136,9 @@
def test_split_translations(self):
"""Split translations splits POTMsgSet and TranslationMessage."""
- splitter = self.makeTranslationSplitter()
- upstream_item, ubuntu_item = self.makeSharedPOTMsgSet(splitter)
+ splitter = make_translation_splitter(self.factory)
+ upstream_item, ubuntu_item = make_shared_potmsgset(
+ self.factory, splitter)
upstream_message = self.factory.makeCurrentTranslationMessage(
potmsgset=upstream_item.potmsgset,
potemplate=upstream_item.potemplate)
