mozreview: fail silently when BugzillaUserMap refers to non-existing Bugzilla user (
bug 1290889), r=mars
If BugzillaUserMap is out of sync with Bugzilla database push is failing
without explanation. This situation might happen for merged Bugzilla accounts.
Instead of failing without explanation MozReview is now giving a message:
Error publishing: Chosen reviewer (username) is assigned to a non-existing
Bugzilla user. If you believe you received this message in error, please ask
for help on IRC#mozreview. (HTTP 500, API Error 225)
MozReview-Commit-ID: APwKXOyYaYD
--- a/pylib/mozreview/mozreview/bugzilla/attachments.py
+++ b/pylib/mozreview/mozreview/bugzilla/attachments.py
@@ -1,13 +1,16 @@
from __future__ import unicode_literals
from mozreview.bugzilla.client import (
BugzillaAttachmentUpdates,
)
+from mozreview.errors import (
+ BugzillaUserMapError,
+)
from mozreview.extra_data import (
REVIEW_FLAG_KEY,
)
from mozreview.models import (
BugzillaUserMap,
get_or_create_bugzilla_users,
)
from mozreview.rb_utils import (
@@ -57,16 +60,22 @@ def update_bugzilla_attachments(bugzilla
email = user_email_cache.get(bum.bugzilla_user_id)
if email is None:
user_data = bugzilla.get_user_from_userid(bum.bugzilla_user_id)
# Since we're making the API call, we might as well ensure the
# local database is up to date.
users = get_or_create_bugzilla_users(user_data)
+ # There is a chance the user requested a merge of accounts.
+ # In such case bum will exist, but no user will be found
+ # with userid = bum.bugzilla_user_id. MozReview should fail
+ # gracefully
+ if len(users) == 0:
+ raise BugzillaUserMapError(u.username)
email = users[0].email
user_email_cache[bum.bugzilla_user_id] = email
carry_forward[email] = False
for review in gen_latest_reviews(review_request):
if has_code_changes:
# The code has changed, we need to determine what needs to
--- a/pylib/mozreview/mozreview/errors.py
+++ b/pylib/mozreview/mozreview/errors.py
@@ -25,16 +25,28 @@ class InvalidBugIdError(PublishError):
class ConfidentialBugError(PublishError):
def __init__(self):
PublishError.__init__(self, 'This bug is confidential; please attach '
'the patch directly to the bug.')
+class BugzillaUserMapError(Exception):
+ "Non existing Bugzilla user is mapped to MozReview user."
+
+
+class ReviewerUserMapError(PublishError):
+ def __init__(self, username):
+ PublishError.__init__(self, 'Chosen reviewer (%s) is assigned to a '
+ 'non-existing Bugzilla user. If you believe you '
+ 'received this message in error, please ask for '
+ 'help on IRC#mozreview.' % username)
+
+
NOT_PARENT = WebAPIError(
1001,
"Review request is not a parent",
http_status=400) # 400 Bad Request
CHILD_DOES_NOT_EXIST = WebAPIError(
1002,
"Child review request does not exist",
--- a/pylib/mozreview/mozreview/signal_handlers.py
+++ b/pylib/mozreview/mozreview/signal_handlers.py
@@ -49,20 +49,22 @@ from mozreview.bugzilla.client import (
from mozreview.bugzilla.errors import (
BugzillaError,
bugzilla_to_publish_errors,
)
from mozreview.diffs import (
build_plaintext_review,
)
from mozreview.errors import (
+ BugzillaUserMapError,
CommitPublishProhibited,
ConfidentialBugError,
InvalidBugIdError,
ParentShipItError,
+ ReviewerUserMapError,
)
from mozreview.extra_data import (
DISCARD_ON_PUBLISH_KEY,
DRAFTED_COMMIT_DATA_KEYS,
fetch_commit_data,
gen_child_rrs,
gen_rrs_by_extra_data_key,
gen_rrs_by_rids,
@@ -338,18 +340,25 @@ def on_review_request_publishing(user, r
if child_draft:
if child.id in discard_on_publish_rids:
children_to_obsolete.append(child)
children_to_post.append((child_draft, child))
if children_to_post or children_to_obsolete:
- update_bugzilla_attachments(b, bug_id, children_to_post,
- children_to_obsolete)
+ try:
+ update_bugzilla_attachments(b, bug_id, children_to_post,
+ children_to_obsolete)
+ except BugzillaUserMapError, e:
+ logger.error(
+ 'User (%s) tried to request a review from reviewer '
+ '(%s) who\'s account is assigned to a non-existing'
+ 'Bugzilla user.', user.username, unicode(e))
+ raise ReviewerUserMapError(unicode(e))
# Publish draft commits. This will already include items that are in
# unpublished_rids, so we'll remove anything we publish out of
# unpublished_rids.
for child in child_rrs:
if child.get_draft(user=user) or not child.public:
def approve_publish(sender, user, review_request_draft,
**kwargs):
new file mode 100644
--- /dev/null
+++ b/pylib/mozreview/mozreview/tests/test-merged-account.py
@@ -0,0 +1,66 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+# It is very rare to remove account by Bugzilla.
+# One of such is merging user accounts. As a result old account is deleted.
+# MozReview holds a table in database for BugzillaUserMap which links Mozreview
+# and Bugzilla accounts. If user account was merged the link remains. In such
+# case Mozreview has to provide a meaningful info.
+
+import pytest
+
+from mock import patch
+
+from mozreview.bugzilla import attachments
+from mozreview.errors import BugzillaUserMapError
+from mozreview.tests.helpers import UserFactory
+
+
+@patch.object(attachments.BugzillaUserMap.objects, 'get')
+@patch('mozreview.bugzilla.attachments.get_or_create_bugzilla_users')
+def test_bugzilla_usermap_excetion(mock_get_or_create_bugzilla_users,
+ mock_bum_get):
+ class ReviewRequestDraft:
+ class TargetPeople:
+ user = UserFactory()
+
+ def all(self):
+ return [self.user]
+
+ diffset = None
+ target_people = TargetPeople()
+
+ class Bugzilla:
+ """ Fake Bugzilla client. """
+ def get_user_from_userid(self, *args):
+ # No need to return any value as it is just used to pass to
+ # a mocked function
+ return None
+
+ # BugzillaUserMap.objects.get should return an object with just
+ # bugzilla_user_id
+ class BugzillaUserMap:
+ bugzilla_user_id = 1
+
+ mock_bum_get.return_value = BugzillaUserMap()
+
+ # mrmodels.get_or_create_bugzilla_users is returning an empty array
+ # if no Bugzilla user is found
+ mock_get_or_create_bugzilla_users.return_value = []
+
+ bugzilla = Bugzilla()
+ draft = ReviewRequestDraft()
+ children_to_post = ((draft, None),)
+ children_to_obsolete = []
+ bug_id = 1
+
+ # In case there is an entry in BugzillaUserMap and no user is found
+ # in Bugzilla with given id a BugzillaUserMapError should be raised
+ with pytest.raises(BugzillaUserMapError) as excinfo:
+ attachments.update_bugzilla_attachments(bugzilla, bug_id,
+ children_to_post,
+ children_to_obsolete)
+ # BugzillaUserMapError should pass the username of the non-existing
+ # reviewer
+ excinfo.match(draft.target_people.user.username)