launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04762
[Merge] lp:~sinzui/launchpad/private-bug-2 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/private-bug-2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/private-bug-2/+merge/72788
Refactor the bug command process loop so that I can see what is happening.
Pre-implementation: lifeless, jcsackett
This branch is a refactoring that extracts the event, error and comment
addition rules to methods so that I can see is happening in the loop.
My goal is to fix a cluster of bugs were email commands are processed in
the wrong order or without context which causes security and privacy
leaks.
--------------------------------------------------------------------
RULES
* Extract duplicate code for events and errors to helper methods.
* Extract the comment addition rules to a helper method
QA
* None, this is a refactoring
LINT
lib/lp/bugs/mail/handler.py
TEST
./bin/test -vv -t bugs-emailinterface.txt -t bugs/.*/test_handler
IMPLEMENTATION
Extracted code while running the tests to ensure nothing broke.
lib/lp/bugs/mail/handler.py
--
https://code.launchpad.net/~sinzui/launchpad/private-bug-2/+merge/72788
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/private-bug-2 into lp:launchpad.
=== modified file 'lib/lp/bugs/mail/handler.py'
--- lib/lp/bugs/mail/handler.py 2011-08-12 18:43:14 +0000
+++ lib/lp/bugs/mail/handler.py 2011-08-24 20:52:24 +0000
@@ -143,78 +143,35 @@
command = commands.pop(0)
try:
if IBugEmailCommand.providedBy(command):
- if bug_event is not None:
- try:
- notify(bug_event)
- except CreatedBugWithNoBugTasksError:
- rollback()
- raise IncomingEmailError(
- get_error_message(
- 'no-affects-target-on-submit.txt',
- error_templates=error_templates))
- if (bugtask_event is not None and
- not IObjectCreatedEvent.providedBy(bug_event)):
- notify(bugtask_event)
+ # Finish outstanding work from the previous bug.
+ self.notify_bug_event(bug_event)
+ self.notify_bugtask_event(bugtask_event, bug_event)
bugtask = None
bugtask_event = None
-
+ # Get or start building a new bug.
bug, bug_event = command.execute(
signed_msg, filealias)
if add_comment_to_bug:
- messageset = getUtility(IMessageSet)
- message = messageset.fromEmail(
- signed_msg.as_string(),
- owner=getUtility(ILaunchBag).user,
- filealias=filealias,
- parsed_message=signed_msg,
- fallback_parent=bug.initial_message)
-
- # If the new message's parent is linked to
- # a bug watch we also link this message to
- # that bug watch.
- bug_message_set = getUtility(IBugMessageSet)
- parent_bug_message = (
- bug_message_set.getByBugAndMessage(
- bug, message.parent))
-
- if (parent_bug_message is not None and
- parent_bug_message.bugwatch):
- bug_watch = parent_bug_message.bugwatch
- else:
- bug_watch = None
-
- bugmessage = bug.linkMessage(
- message, bug_watch)
-
- notify(ObjectCreatedEvent(bugmessage))
+ message = self.appendBugComment(
+ bug, signed_msg, filealias)
add_comment_to_bug = False
else:
message = bug.initial_message
self.processAttachments(bug, message, signed_msg)
elif IBugTaskEmailCommand.providedBy(command):
- if bugtask_event is not None:
- if not IObjectCreatedEvent.providedBy(bug_event):
- notify(bugtask_event)
- bugtask_event = None
- bugtask, bugtask_event = command.execute(bug)
+ self.notify_bugtask_event(bugtask_event, bug_event)
+ bugtask, bugtask_event = command.execute(
+ bug)
elif IBugEditEmailCommand.providedBy(command):
bug, bug_event = command.execute(bug, bug_event)
elif IBugTaskEditEmailCommand.providedBy(command):
if bugtask is None:
if len(bug.bugtasks) == 0:
- rollback()
- raise IncomingEmailError(
- get_error_message(
- 'no-affects-target-on-submit.txt',
- error_templates=error_templates))
+ self.handleNoAffectsTarget()
bugtask = guess_bugtask(
bug, getUtility(ILaunchBag).user)
if bugtask is None:
- raise IncomingEmailError(get_error_message(
- 'no-default-affects.txt',
- error_templates=error_templates,
- bug_id=bug.id,
- nr_of_bugtasks=len(bug.bugtasks)))
+ self.handleNoDefaultAffectsTarget(bug)
bugtask, bugtask_event = command.execute(
bugtask, bugtask_event)
@@ -231,19 +188,8 @@
'\n'.join(str(error) for error, command
in processing_errors),
[command for error, command in processing_errors])
-
- if bug_event is not None:
- try:
- notify(bug_event)
- except CreatedBugWithNoBugTasksError:
- rollback()
- raise IncomingEmailError(
- get_error_message(
- 'no-affects-target-on-submit.txt',
- error_templates=error_templates))
- if bugtask_event is not None:
- if not IObjectCreatedEvent.providedBy(bug_event):
- notify(bugtask_event)
+ self.notify_bug_event(bug_event)
+ self.notify_bugtask_event(bugtask_event, bug_event)
except IncomingEmailError, error:
send_process_error_notification(
@@ -320,3 +266,56 @@
getUtility(IBugAttachmentSet).create(
bug=bug, filealias=blob, attach_type=attach_type,
title=blob.filename, message=message, send_notifications=True)
+
+ def appendBugComment(self, bug, signed_msg, filealias=None):
+ """Append the message text to the bug comments."""
+ messageset = getUtility(IMessageSet)
+ message = messageset.fromEmail(
+ signed_msg.as_string(),
+ owner=getUtility(ILaunchBag).user,
+ filealias=filealias,
+ parsed_message=signed_msg,
+ fallback_parent=bug.initial_message)
+ # If the new message's parent is linked to
+ # a bug watch we also link this message to
+ # that bug watch.
+ bug_message_set = getUtility(IBugMessageSet)
+ parent_bug_message = (
+ bug_message_set.getByBugAndMessage(bug, message.parent))
+ if (parent_bug_message is not None and
+ parent_bug_message.bugwatch):
+ bug_watch = parent_bug_message.bugwatch
+ else:
+ bug_watch = None
+ bugmessage = bug.linkMessage(
+ message, bug_watch)
+ notify(ObjectCreatedEvent(bugmessage))
+ return message
+
+ def notify_bug_event(self, bug_event):
+ if bug_event is None:
+ return
+ try:
+ notify(bug_event)
+ except CreatedBugWithNoBugTasksError:
+ self.handleNoAffectsTarget()
+
+ def notify_bugtask_event(self, bugtask_event, bug_event):
+ if bugtask_event is None:
+ return
+ if not IObjectCreatedEvent.providedBy(bug_event):
+ notify(bugtask_event)
+
+ def handleNoAffectsTarget(self):
+ rollback()
+ raise IncomingEmailError(
+ get_error_message(
+ 'no-affects-target-on-submit.txt',
+ error_templates=error_templates))
+
+ def handleNoDefaultAffectsTarget(self, bug):
+ raise IncomingEmailError(get_error_message(
+ 'no-default-affects.txt',
+ error_templates=error_templates,
+ bug_id=bug.id,
+ nr_of_bugtasks=len(bug.bugtasks)))
