launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03461
[Merge] lp:~sinzui/launchpad/question-email-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/question-email-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/question-email-0/+merge/59247
Create a QuestionJob that can queue emails.
Launchpad bug: https://bugs.launchpad.net/bugs/771244
Pre-implementation: jcsackett, lifeless
Sending email is the primary reason question pages time out. The emails can be
queued, and the Lp job process could send them. The job wants the common
message and common headers. The job will get the question recipients, build
the message by adding the recipient's reason footer and header, then send it.
This branch just queue messages, the run() method is just a stub that I will
implement in my next branch. The work is the first implementation for the
QuestionJob table that was added to the schema 6 months ago.
--------------------------------------------------------------------
RULES
The schema already defines QuestionJob. This branch added code to use it.
* Add the interfaces to define the new job type.
* Add model and utility classes to manage the jobs
* Store the common subject, body and headers in the metadata and provide
properties to access them.
QA
None. This is an incremental branch for unreachable code.
LINT
lib/lp/answers/enums.py
lib/lp/answers/interfaces/questionjob.py
lib/lp/answers/model/questionjob.py
lib/lp/answers/tests/test_questionjob.py
lib/lp/testing/factory.py
TEST
./bin/test -vv -test_questionjob
IMPLEMENTATION
Allow tests to create questions without the need to login.
lib/lp/testing/factory.py
Added an enum to describe question email jobs.
lib/lp/answers/enums.py
Created QuestionJob and a derived QuestionEmailJob class. The latter defines
all the properties and methods needed to do the work, but the work,
the run() method, will be implemented in my next branch.
lib/lp/answers/interfaces/questionjob.py
lib/lp/answers/model/questionjob.py
lib/lp/answers/tests/test_questionjob.py
--
https://code.launchpad.net/~sinzui/launchpad/question-email-0/+merge/59247
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/question-email-0 into lp:launchpad.
=== modified file 'lib/lp/answers/enums.py'
--- lib/lp/answers/enums.py 2011-04-26 15:52:25 +0000
+++ lib/lp/answers/enums.py 2011-04-27 15:51:00 +0000
@@ -11,6 +11,7 @@
__all__ = [
'QuestionAction',
+ 'QuestionJobType',
'QuestionParticipation',
'QuestionPriority',
'QUESTION_STATUS_DEFAULT_SEARCH',
@@ -93,6 +94,16 @@
""")
+class QuestionJobType(DBEnumeratedType):
+ """Values that IQuestionJob.job_type can take."""
+
+ EMAIL = DBItem(0, """
+ Question email notification
+
+ Notify question subscribers about a question via email.
+ """)
+
+
class QuestionParticipation(EnumeratedType):
"""The different ways a person can be involved in a question.
=== added file 'lib/lp/answers/interfaces/questionjob.py'
--- lib/lp/answers/interfaces/questionjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/interfaces/questionjob.py 2011-04-27 15:51:00 +0000
@@ -0,0 +1,78 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface for the Jobs system for questions."""
+
+__metaclass__ = type
+__all__ = [
+ 'IQuestionJob',
+ 'IQuestionEmailJob',
+ 'IQuestionEmailJobSource',
+ ]
+
+from zope.interface import Attribute
+from zope.schema import (
+ Choice,
+ Field,
+ Int,
+ Object,
+ )
+
+from canonical.launchpad import _
+from lp.answers.enums import QuestionJobType
+from lp.services.job.interfaces.job import (
+ IJob,
+ IJobSource,
+ IRunnableJob,
+ )
+
+
+class IQuestionJob(IRunnableJob):
+ """A Job related to a question."""
+
+ id = Int(
+ title=_('DB ID'), required=True, readonly=True,
+ description=_("The tracking number for this job."))
+
+ job = Object(
+ title=_('The common Job attributes'),
+ schema=IJob, required=True)
+
+ job_type = Choice(
+ title=_('Job type'), vocabulary=QuestionJobType,
+ required=True, readonly=True)
+
+ question = Field(
+ title=_("The question related to this job."),
+ description=_("An IQuestion."), required=True, readonly=True)
+
+ metadata = Attribute('A dict of data about the job.')
+
+
+class IQuestionEmailJob(IQuestionJob):
+
+ user = Attribute('The `IPerson` who triggered the email.')
+
+ body = Attribute(
+ 'The subject of the email.')
+
+ body = Attribute(
+ 'The body of the email that is common to all recpients.')
+
+ headers = Attribute(
+ 'The headers of the email that are common to all recpients.')
+
+
+class IQuestionEmailJobSource(IJobSource):
+ """An interface for acquiring IQuestionJob."""
+
+ def create(question, user, subject, body, headers):
+ """Create a new IQuestionJob.
+
+ :param question: An `IQuestion`.
+ :param user: An `IPerson`.
+ :param subject: A'The subject of the email.
+ :param body: The text of the email that is common to all recpients.
+ :parma headers: A dict of headers for the email that are common to
+ all recpients.
+ """
=== added file 'lib/lp/answers/model/questionjob.py'
--- lib/lp/answers/model/questionjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/model/questionjob.py 2011-04-27 15:51:00 +0000
@@ -0,0 +1,172 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Job classes related to QuestionJob."""
+
+__metaclass__ = type
+__all__ = [
+ 'QuestionJob',
+ ]
+
+import simplejson
+from storm.expr import (
+ And,
+ )
+from storm.locals import (
+ Int,
+ Reference,
+ Unicode,
+ )
+from zope.component import getUtility
+from zope.interface import (
+ classProvides,
+ implements,
+ )
+
+from lazr.delegates import delegates
+
+from canonical.database.enumcol import EnumCol
+from canonical.launchpad.interfaces.lpstorm import (
+ IMasterStore,
+ )
+from canonical.launchpad.scripts import log
+from lp.answers.enums import QuestionJobType
+from lp.answers.interfaces.questionjob import (
+ IQuestionJob,
+ IQuestionEmailJob,
+ IQuestionEmailJobSource,
+ )
+from lp.answers.model.question import Question
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.database.stormbase import StormBase
+from lp.services.job.model.job import Job
+from lp.services.job.runner import BaseRunnableJob
+from lp.services.propertycache import cachedproperty
+
+
+class QuestionJob(StormBase):
+ """A Job for queued question emails."""
+
+ implements(IQuestionJob)
+
+ __storm_table__ = 'QuestionJob'
+
+ id = Int(primary=True)
+
+ job_id = Int(name='job')
+ job = Reference(job_id, Job.id)
+
+ job_type = EnumCol(enum=QuestionJobType, notNull=True)
+
+ question_id = Int(name='question')
+ question = Reference(question_id, Question.id)
+
+ _json_data = Unicode('json_data')
+
+ def __init__(self, question, job_type, metadata):
+ """Constructor.
+
+ :param question: The question related to this job.
+ :param job_type: The specific job being performed for the question.
+ :param metadata: The type-specific variables, as a JSON-compatible
+ dict.
+ """
+ super(QuestionJob, self).__init__()
+ self.job = Job()
+ self.job_type = job_type
+ self.question = question
+ json_data = simplejson.dumps(metadata)
+ self._json_data = json_data.decode('utf-8')
+
+ def __repr__(self):
+ return (
+ "<{self.__class__.__name__} for question {self.question.id}; "
+ "status={self.job.status}>").format(self=self)
+
+ @property
+ def metadata(self):
+ """See `IQuestionJob`."""
+ return simplejson.loads(self._json_data)
+
+
+class QuestionEmailJob(BaseRunnableJob):
+ """Intermediate class for deriving from QuestionJob."""
+ delegates(IQuestionJob)
+ implements(IQuestionEmailJob)
+ classProvides(IQuestionEmailJobSource)
+
+ def __init__(self, job):
+ self.context = job
+
+ class_job_type = QuestionJobType.EMAIL
+
+ @classmethod
+ def create(cls, question, user, subject, body, headers):
+ """See `IQuestionJob`."""
+ metadata = {
+ 'user': user.id,
+ 'subject': subject,
+ 'body': body,
+ 'headers': headers,
+ }
+ job = QuestionJob(
+ question=question, job_type=cls.class_job_type, metadata=metadata)
+ return cls(job)
+
+ @classmethod
+ def iterReady(cls):
+ """See `IJobSource`."""
+ store = IMasterStore(QuestionJob)
+ jobs = store.find(
+ QuestionJob,
+ And(QuestionJob.job_type == cls.class_job_type,
+ QuestionJob.job_id.is_in(Job.ready_jobs)))
+ return (cls(job) for job in jobs)
+
+ @cachedproperty
+ def user(self):
+ """See `IQuestionEmailJob`."""
+ return getUtility(IPersonSet).get(self.metadata['user'])
+
+ @property
+ def subject(self):
+ """See `IQuestionEmailJob`."""
+ return self.metadata['subject']
+
+ @property
+ def body(self):
+ """See `IQuestionEmailJob`."""
+ return self.metadata['body']
+
+ @property
+ def headers(self):
+ """See `IQuestionEmailJob`."""
+ return self.metadata['headers']
+
+ @property
+ def log_name(self):
+ """See `IRunnableJob`."""
+ return self.__class__.__name__
+
+ def getOopsVars(self):
+ """See `IRunnableJob`."""
+ vars = BaseRunnableJob.getOopsVars(self)
+ vars.extend([
+ ('question', self.question.id),
+ ('user', self.user.name),
+ ])
+ return vars
+
+ def getErrorRecipients(self):
+ """See `IRunnableJob`."""
+ return self.user
+
+ def run(self):
+ """Send emails."""
+ log.debug(
+ "%s will send email for question %s.",
+ self.log_name, self.question.id)
+ # Extract and adapt QuestionNotification.send().
+ log.debug(
+ "%s has sent email for question %s.",
+ self.log_name, self.question.id)
=== added file 'lib/lp/answers/tests/test_questionjob.py'
--- lib/lp/answers/tests/test_questionjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/tests/test_questionjob.py 2011-04-27 15:51:00 +0000
@@ -0,0 +1,141 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for QuestionJobs classes."""
+
+__metaclass__ = type
+
+from canonical.testing import DatabaseFunctionalLayer
+from lp.answers.enums import QuestionJobType
+from lp.answers.model.questionjob import (
+ QuestionJob,
+ QuestionEmailJob,
+ )
+from lp.testing import TestCaseWithFactory
+
+
+class QuestionJobTestCase(TestCaseWithFactory):
+ """Test case for base QuestionJob class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_instantiate(self):
+ question = self.factory.makeQuestion()
+ metadata = ('some', 'arbitrary', 'metadata')
+ job = QuestionJob(
+ question, QuestionJobType.EMAIL, metadata)
+ self.assertEqual(QuestionJobType.EMAIL, job.job_type)
+ self.assertEqual(question, job.question)
+ # Metadata is unserialized from JSON.
+ metadata_expected = list(metadata)
+ self.assertEqual(metadata_expected, job.metadata)
+
+ def test_repr(self):
+ question = self.factory.makeQuestion()
+ metadata = []
+ question_job = QuestionJob(
+ question, QuestionJobType.EMAIL, metadata)
+ self.assertEqual(
+ '<QuestionJob for question %s; status=Waiting>' % question.id,
+ repr(question_job))
+
+
+class QuestionEmailJobTestCase(TestCaseWithFactory):
+ """Test case for QuestionEmailJob class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def makeUserSubjectBodyHeaders(self):
+ user = self.factory.makePerson()
+ subject = 'email subject'
+ body = 'email body'
+ headers = {'X-Launchpad-Question': 'question metadata'}
+ return user, subject, body, headers
+
+ def test_create(self):
+ # The create class method converts the extra job arguments
+ # to metadata.
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(QuestionJobType.EMAIL, job.job_type)
+ self.assertEqual(question, job.question)
+ self.assertEqual(
+ ['body', 'headers', 'subject', 'user'],
+ sorted(job.metadata.keys()))
+ self.assertEqual(user.id, job.metadata['user'])
+ self.assertEqual(subject, job.metadata['subject'])
+ self.assertEqual(body, job.metadata['body'])
+ self.assertEqual(
+ headers['X-Launchpad-Question'],
+ job.metadata['headers']['X-Launchpad-Question'])
+
+ def test_iterReady(self):
+ # Jobs in the ready state are returned by the iterator.
+ question = self.factory.makeQuestion()
+ user, subject, ignore, headers = self.makeUserSubjectBodyHeaders()
+ job_1 = QuestionEmailJob.create(
+ question, user, subject, 'one', headers)
+ job_2 = QuestionEmailJob.create(
+ question, user, subject, 'two', headers)
+ job_3 = QuestionEmailJob.create(
+ question, user, subject, 'three', headers)
+ job_1.start()
+ job_1.complete()
+ job_ids = sorted(job.id for job in QuestionEmailJob.iterReady())
+ self.assertEqual(sorted([job_2.id, job_3.id]), job_ids)
+
+ def test_user(self):
+ # The user property matches the user passed to create().
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(user, job.user)
+
+ def test_subject(self):
+ # The subject property matches the subject passed to create().
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(body, job.body)
+
+ def test_body(self):
+ # The body property matches the body passed to create().
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(body, job.body)
+
+ def test_headers(self):
+ # The headers property matches the headers passed to create().
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(headers, job.headers)
+
+ def test_log_name(self):
+ # The log_name property matches the class name.
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(job.__class__.__name__, job.log_name)
+
+ def test_getOopsVars(self):
+ # The getOopsVars() method adds the question and user to the vars.
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ oops_vars = job.getOopsVars()
+ self.assertTrue(('question', question.id) in oops_vars)
+ self.assertTrue(('user', user.name) in oops_vars)
+
+ def test_getErrorRecipients(self):
+ # The getErrorRecipients method matches the user.
+ question = self.factory.makeQuestion()
+ user, subject, body, headers = self.makeUserSubjectBodyHeaders()
+ job = QuestionEmailJob.create(question, user, subject, body, headers)
+ self.assertEqual(user, job.getErrorRecipients())
+
+ def test_run(self):
+ # The email is sent to all the recipents.
+ pass
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-04-18 05:25:38 +0000
+++ lib/lp/testing/factory.py 2011-04-27 15:51:00 +0000
@@ -1992,8 +1992,10 @@
target = self.makeProduct()
if title is None:
title = self.getUniqueString('title')
- return target.newQuestion(
- owner=target.owner, title=title, description='description')
+ with person_logged_in(target.owner):
+ question = target.newQuestion(
+ owner=target.owner, title=title, description='description')
+ return question
def makeFAQ(self, target=None, title=None):
"""Create and return a new, arbitrary FAQ.
