--- a/pylib/mozreview/mozreview/resources/batch_review_request.py
+++ b/pylib/mozreview/mozreview/resources/batch_review_request.py
@@ -339,16 +339,17 @@ class BatchReviewRequestResource(WebAPIR
# A mapping from review request id to the corresponding ReviewRequest.
review_requests = {}
# A mapping of review request id to dicts of additional metadata.
review_data = {}
squashed_reviewers = set()
+ reviewer_cache = ReviewerCache(request)
warnings = []
# Do a pass and find all commits that map cleanly to old review requests.
for commit in commits['individual']:
node = commit['id']
if node not in remaining_nodes:
@@ -385,17 +386,18 @@ class BatchReviewRequestResource(WebAPIR
rid = remaining_nodes.get(precursor)
if not rid:
continue
del remaining_nodes[precursor]
unclaimed_rids.remove(rid)
rr = ReviewRequest.objects.get(pk=rid)
- draft, warns = update_review_request(local_site, request, rr, commit)
+ draft, warns = update_review_request(local_site, request,
+ reviewer_cache, rr, commit)
squashed_reviewers.update(u for u in draft.target_people.all())
warnings.extend(warns)
processed_nodes.add(node)
node_to_rid[node] = rid
review_requests[rid] = rr
review_data[rid] = get_review_request_data(rr)
try:
@@ -421,17 +423,18 @@ class BatchReviewRequestResource(WebAPIR
# For now, match the commit up against the next one in the index.
# The unclaimed rids list contains review requests which were created
# when previously updating this review identifier, but not published.
# If we have more commits than were previously published we'll start
# reusing these private review requests before creating new ones.
if unclaimed_rids:
assumed_old_rid = unclaimed_rids.pop(0)
rr = ReviewRequest.objects.get(pk=assumed_old_rid)
- draft, warns = update_review_request(local_site, request, rr, commit)
+ draft, warns = update_review_request(local_site, request,
+ reviewer_cache, rr, commit)
squashed_reviewers.update(u for u in draft.target_people.all())
warnings.extend(warns)
processed_nodes.add(commit['id'])
node_to_rid[node] = assumed_old_rid
review_requests[assumed_old_rid] = rr
review_data[assumed_old_rid] = get_review_request_data(rr)
try:
@@ -448,17 +451,18 @@ class BatchReviewRequestResource(WebAPIR
commit_id=None,
local_site=local_site)
rr.extra_data['p2rb'] = True
rr.extra_data['p2rb.is_squashed'] = False
rr.extra_data['p2rb.identifier'] = identifier
rr.save(update_fields=['extra_data'])
logger.info('created commit review request #%d' % rr.id)
- draft, warns = update_review_request(local_site, request, rr, commit)
+ draft, warns = update_review_request(local_site, request,
+ reviewer_cache, rr, commit)
squashed_reviewers.update(u for u in draft.target_people.all())
warnings.extend(warns)
processed_nodes.add(commit['id'])
node_to_rid[node] = rr.id
review_requests[rr.id] = rr
review_data[rr.id] = get_review_request_data(rr)
unpublished_rids.append(rr.id)
@@ -581,16 +585,39 @@ def previous_reviewers(rr):
# consistent. For now, we duplicate that logic here.
pr = {}
for review in gen_latest_reviews(rr):
pr[review.user.username] = review.ship_it
return pr
+class ReviewerCache(object):
+ """Caches lookups from reviewer/username to User instances.
+
+ Resolving a username string to a User potentially requires querying
+ Bugzilla via an HTTP request. To minimize the number of HTTP requests,
+ we cache lookups.
+ """
+
+ def __init__(self, request):
+ self._request = request
+ self._d = {}
+
+ def resolve_reviewer(self, reviewer):
+ """Try to obtain a User for a reviewer.
+
+ Returns None if the reviewer/user is unknown.
+ """
+ if reviewer not in self._d:
+ self._d[reviewer] = get_user_from_reviewer(self._request, reviewer)
+
+ return self._d[reviewer]
+
+
def get_user_from_reviewer(request, reviewer):
for backend in get_enabled_auth_backends():
backend.query_users(reviewer, request)
q = Q(username__icontains=reviewer)
q = q | Q(is_active=True)
users = User.objects.filter(q).all()
@@ -602,51 +629,51 @@ def get_user_from_reviewer(request, revi
# Try case insensitive username match.
for user in users:
if user.username.lower() == reviewer.lower():
return user
return None
-def resolve_reviewers(request, requested_reviewers):
+def resolve_reviewers(cache, requested_reviewers):
reviewers = set()
unrecognized = set()
# TODO track mapping for multiple review requests and cache results.
for reviewer in requested_reviewers:
- user = get_user_from_reviewer(request, reviewer)
+ user = cache.resolve_reviewer(reviewer)
if user:
if not any(u.username == user.username for u in reviewers):
reviewers.add(user)
else:
unrecognized.add(reviewer)
return reviewers, unrecognized
-def update_review_request(local_site, request, rr, commit):
+def update_review_request(local_site, request, reviewer_cache, rr, commit):
"""Synchronize the state of a review request with a commit.
Updates the commit message, refreshes the diff, etc.
"""
try:
draft = rr.draft.get()
except ReviewRequestDraft.DoesNotExist:
draft = ReviewRequestDraft.create(rr)
draft.summary = commit['message'].splitlines()[0]
draft.description = commit['message']
draft.bugs_closed = commit['bug']
draft.extra_data['p2rb.commit_id'] = commit['id']
draft.extra_data['p2rb.first_public_ancestor'] = commit['first_public_ancestor']
reviewer_users, unrecognized_reviewers = \
- resolve_reviewers(request, commit.get('reviewers', []))
+ resolve_reviewers(reviewer_cache, commit.get('reviewers', []))
requal_reviewer_users, unrecognized_requal_reviewers = \
- resolve_reviewers(request, commit.get('requal_reviewers', []))
+ resolve_reviewers(reviewer_cache, commit.get('requal_reviewers', []))
warnings = []
for reviewer in unrecognized_reviewers | unrecognized_requal_reviewers:
warnings.append('unrecognized reviewer: %s' % reviewer)
logger.info('unrecognized reviewer: %s' % reviewer)
if requal_reviewer_users:
@@ -690,17 +717,16 @@ def update_review_request(local_site, re
draft.save()
if discarded_diffset:
discarded_diffset.delete()
return draft, warnings
-
def get_review_request_data(rr):
"""Obtain a dictionary containing review request metadata.
The dict consists of plain types (as opposed to ReviewBoard types).
Some values may be unicode, not str.
"""
rd = {