Merge lp:~sinzui/launchpad/commercial-jobsource-zcml into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15422
Proposed branch: lp:~sinzui/launchpad/commercial-jobsource-zcml
Merge into: lp:launchpad
Diff against target: 219 lines (+95/-8)
7 files modified
cronscripts/daily_product_jobs.py (+1/-1)
database/schema/security.cfg (+6/-0)
lib/lp/registry/configure.zcml (+31/-0)
lib/lp/registry/model/productjob.py (+3/-0)
lib/lp/registry/tests/test_productjob.py (+52/-3)
lib/lp/services/config/schema-lazr.conf (+2/-2)
logs/README.txt (+0/-2)
To merge this branch: bzr merge lp:~sinzui/launchpad/commercial-jobsource-zcml
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+110365@code.launchpad.net

Commit message

Register the commercial email job sources and add db permissions.

Description of the change

process-job-source.py fails because the commercial notification classes
are not registered in ZCML. Clearly, a test is missing that demonstrates
the ZCML, schema-lazr.conf, and security.cfg are configured properly.

I really botched this. I new what test was missing when I discovered the
bug. There are two cronscripts used in this process, and I only wrote
tests for the script I wrote. I need to also write a test for the script
I reused.

--------------------------------------------------------------------

RULES

    Pre-implementation: no one
    * Add the missing ZCML needed to register the secured utilities.
    * Add tests to verify the three kinds of job are run.
      * Fix any db permissions that are proven to fail.
    * Fix the ICommercialExpiredJob to be ICommercialExpiredJobSource
      in the schema.

QA

    On qastaging, as a webops to run
    * cronscripts/process-job-source.py ICommercialExpiredJobSource
    * cronscripts/process-job-source.py ISevenDayCommercialExpirationJobSource
    * cronscripts/process-job-source.py IThirtyDayCommercialExpirationJobSource

LINT

    cronscripts/daily_product_jobs.py
    database/schema/security.cfg
    lib/lp/registry/configure.zcml
    lib/lp/registry/model/productjob.py
    lib/lp/registry/tests/test_productjob.py
    lib/lp/services/config/schema-lazr.conf

TEST

    ./bin/test -vvc lp.registry.tests.test_productjob

IMPLEMENTATION

Fixed the class name used to lookup jobs
ICommercialExpiredJob => ICommercialExpiredJobSource
    cronscripts/daily_product_jobs.py
    lib/lp/services/config/schema-lazr.conf

Added a test to the test mixin to verify that the three kinds of
commercial notification job can be run by process-job-source.py. I
discovered that there is two run_script() test helpers with different
IO. I chose to use update an existing test to use the one from
lp.testing because it allows me to pass the environ to get better test
output.
    lib/lp/registry/model/productjob.py
    lib/lp/registry/tests/test_productjob.py

Added the missing ZCML registration that permits process-job-source.py
to lookup the job source class and then have permission to use the job
instance.
    lib/lp/registry/configure.zcml

Fixed the db permissions demonstrated to be need by the new test. The
packaging/distro tables are needed because there is a sanity check in
the product deactivation code that ensure Ubuntu does not loose an
upstream packaging link
    database/schema/security.cfg

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Curtis--

This looks fine; I'm a little confused about the block of lp.services imports being moved--is there some sort of interference if they're sorted normally?

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cronscripts/daily_product_jobs.py'
--- cronscripts/daily_product_jobs.py 2012-06-07 19:14:04 +0000
+++ cronscripts/daily_product_jobs.py 2012-06-14 16:53:21 +0000
@@ -21,7 +21,7 @@
2121
22 def __init__(self):22 def __init__(self):
23 name = 'daily_product_jobs'23 name = 'daily_product_jobs'
24 dbuser = config.ICommercialExpiredJob.dbuser24 dbuser = config.ICommercialExpiredJobSource.dbuser
25 LaunchpadCronScript.__init__(self, name, dbuser)25 LaunchpadCronScript.__init__(self, name, dbuser)
2626
27 def main(self):27 def main(self):
2828
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-06-08 15:46:51 +0000
+++ database/schema/security.cfg 2012-06-14 16:53:21 +0000
@@ -2367,11 +2367,17 @@
2367[product-job]2367[product-job]
2368groups=script2368groups=script
2369public.account = SELECT2369public.account = SELECT
2370public.branch = SELECT
2370public.commercialsubscription = SELECT, UPDATE, DELETE2371public.commercialsubscription = SELECT, UPDATE, DELETE
2372public.distribution = SELECT
2373public.distroseries = SELECT
2371public.emailaddress = SELECT2374public.emailaddress = SELECT
2372public.job = SELECT, INSERT, UPDATE2375public.job = SELECT, INSERT, UPDATE
2373public.person = SELECT2376public.person = SELECT
2377public.packaging = SELECT
2374public.product = SELECT, UPDATE2378public.product = SELECT, UPDATE
2379public.productlicense = SELECT
2375public.productseries = SELECT, UPDATE2380public.productseries = SELECT, UPDATE
2376public.productjob = SELECT, INSERT2381public.productjob = SELECT, INSERT
2382public.sourcepackagename = SELECT
2377type=user2383type=user
23782384
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-05-29 03:59:16 +0000
+++ lib/lp/registry/configure.zcml 2012-06-14 16:53:21 +0000
@@ -110,6 +110,37 @@
110 <allow interface=".interfaces.persontransferjob.IPersonMergeJob"/>110 <allow interface=".interfaces.persontransferjob.IPersonMergeJob"/>
111 </class>111 </class>
112112
113 <!-- IProductNotificationJob -->
114 <securedutility
115 component=".model.productjob.CommercialExpiredJob"
116 provides=".interfaces.productjob.ICommercialExpiredJobSource">
117 <allow interface=".interfaces.productjob.ICommercialExpiredJobSource"/>
118 </securedutility>
119
120 <class class=".model.productjob.CommercialExpiredJob">
121 <allow interface=".interfaces.productjob.ICommercialExpiredJob"/>
122 </class>
123
124 <securedutility
125 component=".model.productjob.SevenDayCommercialExpirationJob"
126 provides=".interfaces.productjob.ISevenDayCommercialExpirationJobSource">
127 <allow interface=".interfaces.productjob.ISevenDayCommercialExpirationJobSource"/>
128 </securedutility>
129
130 <class class=".model.productjob.SevenDayCommercialExpirationJob">
131 <allow interface=".interfaces.productjob.ISevenDayCommercialExpirationJob"/>
132 </class>
133
134 <securedutility
135 component=".model.productjob.ThirtyDayCommercialExpirationJob"
136 provides=".interfaces.productjob.IThirtyDayCommercialExpirationJobSource">
137 <allow interface=".interfaces.productjob.IThirtyDayCommercialExpirationJobSource"/>
138 </securedutility>
139
140 <class class=".model.productjob.ThirtyDayCommercialExpirationJob">
141 <allow interface=".interfaces.productjob.IThirtyDayCommercialExpirationJob"/>
142 </class>
143
113 <!-- INameBlacklist -->144 <!-- INameBlacklist -->
114 <securedutility145 <securedutility
115 class="lp.registry.model.nameblacklist.NameBlacklistSet"146 class="lp.registry.model.nameblacklist.NameBlacklistSet"
116147
=== modified file 'lib/lp/registry/model/productjob.py'
--- lib/lp/registry/model/productjob.py 2012-06-14 05:18:22 +0000
+++ lib/lp/registry/model/productjob.py 2012-06-14 16:53:21 +0000
@@ -77,6 +77,7 @@
77 simple_sendmail,77 simple_sendmail,
78 )78 )
79from lp.services.propertycache import cachedproperty79from lp.services.propertycache import cachedproperty
80from lp.services.scripts import log
80from lp.services.webapp.publisher import canonical_url81from lp.services.webapp.publisher import canonical_url
8182
8283
@@ -332,6 +333,8 @@
332 body, headers = self.getBodyAndHeaders(333 body, headers = self.getBodyAndHeaders(
333 email_template, address, self.reply_to)334 email_template, address, self.reply_to)
334 simple_sendmail(from_address, address, subject, body, headers)335 simple_sendmail(from_address, address, subject, body, headers)
336 log.debug("%s has sent email to the maintainer of %s.",
337 self.log_name, self.product.name)
335338
336 def run(self):339 def run(self):
337 """See `BaseRunnableJob`.340 """See `BaseRunnableJob`.
338341
=== modified file 'lib/lp/registry/tests/test_productjob.py'
--- lib/lp/registry/tests/test_productjob.py 2012-06-14 05:18:22 +0000
+++ lib/lp/registry/tests/test_productjob.py 2012-06-14 16:53:21 +0000
@@ -12,6 +12,8 @@
1212
13import pytz13import pytz
14import transaction14import transaction
15from testtools.content import Content
16from testtools.content_type import UTF8_TEXT
15from zope.component import getUtility17from zope.component import getUtility
16from zope.interface import (18from zope.interface import (
17 classProvides,19 classProvides,
@@ -47,12 +49,14 @@
47 SevenDayCommercialExpirationJob,49 SevenDayCommercialExpirationJob,
48 ThirtyDayCommercialExpirationJob,50 ThirtyDayCommercialExpirationJob,
49 )51 )
52from lp.services.database.lpstorm import IStore
53from lp.services.job.interfaces.job import JobStatus
50from lp.services.log.logger import BufferLogger54from lp.services.log.logger import BufferLogger
51from lp.services.propertycache import clear_property_cache55from lp.services.propertycache import clear_property_cache
52from lp.services.scripts.tests import run_script
53from lp.services.webapp.publisher import canonical_url56from lp.services.webapp.publisher import canonical_url
54from lp.testing import (57from lp.testing import (
55 person_logged_in,58 person_logged_in,
59 run_script,
56 TestCaseWithFactory,60 TestCaseWithFactory,
57 )61 )
58from lp.testing.layers import (62from lp.testing.layers import (
@@ -147,8 +151,11 @@
147 # ProductJobManagerTestCase.test_createAllDailyJobs151 # ProductJobManagerTestCase.test_createAllDailyJobs
148 self.make_test_products()152 self.make_test_products()
149 transaction.commit()153 transaction.commit()
150 retcode, stdout, stderr = run_script(154 stdout, stderr, retcode = run_script(
151 'cronscripts/daily_product_jobs.py', [])155 'cronscripts/daily_product_jobs.py')
156 self.addDetail("stdout", Content(UTF8_TEXT, lambda: stdout))
157 self.addDetail("stderr", Content(UTF8_TEXT, lambda: stderr))
158 self.assertEqual(0, retcode)
152 self.assertIn('Requested 3 total product jobs.', stderr)159 self.assertIn('Requested 3 total product jobs.', stderr)
153160
154161
@@ -556,6 +563,48 @@
556 self.assertEqual(1, len(notifications))563 self.assertEqual(1, len(notifications))
557 self.assertIn(iso_date, notifications[0].get_payload())564 self.assertIn(iso_date, notifications[0].get_payload())
558565
566 def test_run_cronscript(self):
567 # Everything is configured: ZCML, schema-lazr.conf, and security.cfg.
568 product, reviewer = self.make_notification_data()
569 private_branch = self.factory.makeBranch(
570 owner=product.owner, product=product,
571 information_type=InformationType.USERDATA)
572 with person_logged_in(product.owner):
573 product.setPrivateBugs(True, product.owner)
574 product.development_focus.branch = private_branch
575 self.expire_commercial_subscription(product)
576 job = self.JOB_CLASS.create(product, reviewer)
577 # Create a proprietary project which will have different DB relations.
578 proprietary_product = self.factory.makeProduct(
579 licenses=[License.OTHER_PROPRIETARY])
580 self.expire_commercial_subscription(proprietary_product)
581 proprietary_job = self.JOB_CLASS.create(proprietary_product, reviewer)
582 transaction.commit()
583
584 out, err, exit_code = run_script(
585 "LP_DEBUG_SQL=1 cronscripts/process-job-source.py -vv %s" %
586 self.JOB_SOURCE_INTERFACE.getName())
587 self.addDetail("stdout", Content(UTF8_TEXT, lambda: out))
588 self.addDetail("stderr", Content(UTF8_TEXT, lambda: err))
589 self.assertEqual(0, exit_code)
590 self.assertTrue(
591 'Traceback (most recent call last)' not in err)
592 message = (
593 '%s has sent email to the maintainer of %s.' % (
594 self.JOB_CLASS.__name__, product.name))
595 self.assertTrue(
596 message in err,
597 'Cound not find "%s" in err log:\n%s.' % (message, err))
598 message = (
599 '%s has sent email to the maintainer of %s.' % (
600 self.JOB_CLASS.__name__, proprietary_product.name))
601 self.assertTrue(
602 message in err,
603 'Cound not find "%s" in err log:\n%s.' % (message, err))
604 IStore(job.job).invalidate()
605 self.assertEqual(JobStatus.COMPLETED, job.job.status)
606 self.assertEqual(JobStatus.COMPLETED, proprietary_job.job.status)
607
559608
560class SevenDayCommercialExpirationJobTestCase(CommericialExpirationMixin,609class SevenDayCommercialExpirationJobTestCase(CommericialExpirationMixin,
561 TestCaseWithFactory):610 TestCaseWithFactory):
562611
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2012-06-13 01:05:42 +0000
+++ lib/lp/services/config/schema-lazr.conf 2012-06-14 16:53:21 +0000
@@ -1749,7 +1749,7 @@
1749# dbuser, the crontab_group, and the module that the job source class1749# dbuser, the crontab_group, and the module that the job source class
1750# can be loaded from.1750# can be loaded from.
1751job_sources:1751job_sources:
1752 ICommercialExpiredJob,1752 ICommercialExpiredJobSource,
1753 IMembershipNotificationJobSource,1753 IMembershipNotificationJobSource,
1754 IPersonMergeJobSource,1754 IPersonMergeJobSource,
1755 IPlainPackageCopyJobSource,1755 IPlainPackageCopyJobSource,
@@ -1759,7 +1759,7 @@
1759 ISevenDayCommercialExpirationJobSource,1759 ISevenDayCommercialExpirationJobSource,
1760 IThirtyDayCommercialExpirationJobSource1760 IThirtyDayCommercialExpirationJobSource
17611761
1762[ICommercialExpiredJob]1762[ICommercialExpiredJobSource]
1763module: lp.registry.interfaces.productjob1763module: lp.registry.interfaces.productjob
1764dbuser: product-job1764dbuser: product-job
1765crontab_group: MAIN1765crontab_group: MAIN
17661766
=== removed directory 'logs'
=== removed file 'logs/README.txt'
--- logs/README.txt 2012-06-14 13:41:58 +0000
+++ logs/README.txt 1970-01-01 00:00:00 +0000
@@ -1,2 +0,0 @@
1This directory is included in revision control so that it exists for
2tests that need to write logs.