Changeset - 20a094053606
[Not reviewed]
default
0 4 0
Mads Kiilerich - 10 years ago 2016-03-14 16:17:46
madski@unity3d.com
pullrequests: optimize iteration over reviewers - avoid fetching users one by one

.reviewers was mainly used for iteration and then dereferencing .user one by
one. That gave lots of queries and round trips to the database and was slow.

Instead, do something else. Either query directly or use a new method for
getting the list of reviewer users.

Reviewers will explicitly be shown in the order they have been added (assuming
database id's are monotonic).
4 files changed with 18 insertions and 7 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/pullrequests.py
Show inline comments
 
@@ -161,50 +161,53 @@ class PullrequestsController(BaseRepoCon
 
                    selected = 'branch:%s:%s' % (tipbranch, tiprev)
 
                else:
 
                    selected = 'tag:null:' + repo.EMPTY_CHANGESET
 
                    tags.append((selected, 'null'))
 
            else:
 
                if 'master' in repo.branches:
 
                    selected = 'branch:master:%s' % repo.branches['master']
 
                else:
 
                    k, v = repo.branches.items()[0]
 
                    selected = 'branch:%s:%s' % (k, v)
 

	
 
        groups = [(specials, _("Special")),
 
                  (peers, _("Peer branches")),
 
                  (bookmarks, _("Bookmarks")),
 
                  (branches, _("Branches")),
 
                  (tags, _("Tags")),
 
                  ]
 
        return [g for g in groups if g[0]], selected
 

	
 
    def _get_is_allowed_change_status(self, pull_request):
 
        if pull_request.is_closed():
 
            return False
 

	
 
        owner = self.authuser.user_id == pull_request.user_id
 
        reviewer = self.authuser.user_id in [x.user_id for x in
 
                                                   pull_request.reviewers]
 
        reviewer = PullRequestReviewers.query() \
 
            .filter(PullRequestReviewers.pull_request == pull_request) \
 
            .filter(PullRequestReviewers.user_id == self.authuser.user_id) \
 
            .count() != 0
 

	
 
        return self.authuser.admin or owner or reviewer
 

	
 
    @LoginRequired()
 
    @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
 
                                   'repository.admin')
 
    def show_all(self, repo_name):
 
        c.from_ = request.GET.get('from_') or ''
 
        c.closed = request.GET.get('closed') or ''
 
        c.pull_requests = PullRequestModel().get_all(repo_name, from_=c.from_, closed=c.closed)
 
        c.repo_name = repo_name
 
        p = safe_int(request.GET.get('page', 1), 1)
 

	
 
        c.pullrequests_pager = Page(c.pull_requests, page=p, items_per_page=100)
 

	
 
        return render('/pullrequests/pullrequest_show_all.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    def show_my(self):
 
        c.closed = request.GET.get('closed') or ''
 

	
 
        def _filter(pr):
 
            s = sorted(pr, key=lambda o: o.created_on, reverse=True)
 
            if not c.closed:
kallithea/model/changeset_status.py
Show inline comments
 
@@ -77,56 +77,56 @@ class ChangesetStatusModel(BaseModel):
 
            return ChangesetStatus.STATUS_APPROVED
 

	
 
        if any(st and st.status == ChangesetStatus.STATUS_REJECTED for st in statuses):
 
            return ChangesetStatus.STATUS_REJECTED
 

	
 
        return ChangesetStatus.STATUS_UNDER_REVIEW
 

	
 
    def calculate_pull_request_result(self, pull_request):
 
        """
 
        Return a tuple (reviewers, pending reviewers, pull request status)
 
        Only approve and reject counts as valid votes.
 
        """
 

	
 
        # collect latest votes from all voters
 
        cs_statuses = dict()
 
        for st in reversed(self.get_statuses(pull_request.org_repo,
 
                                             pull_request=pull_request,
 
                                             with_revisions=True)):
 
            cs_statuses[st.author.username] = st
 

	
 
        # collect votes from official reviewers
 
        pull_request_reviewers = []
 
        pull_request_pending_reviewers = []
 
        relevant_statuses = []
 
        for r in pull_request.reviewers:
 
            st = cs_statuses.get(r.user.username)
 
        for user in pull_request.get_reviewer_users():
 
            st = cs_statuses.get(user.username)
 
            relevant_statuses.append(st)
 
            if not st or st.status in (ChangesetStatus.STATUS_NOT_REVIEWED,
 
                                       ChangesetStatus.STATUS_UNDER_REVIEW):
 
                st = None
 
                pull_request_pending_reviewers.append(r.user)
 
            pull_request_reviewers.append((r.user, st))
 
                pull_request_pending_reviewers.append(user)
 
            pull_request_reviewers.append((user, st))
 

	
 
        result = self._calculate_status(relevant_statuses)
 

	
 
        return (pull_request_reviewers,
 
                pull_request_pending_reviewers,
 
                result)
 

	
 
    def get_statuses(self, repo, revision=None, pull_request=None,
 
                     with_revisions=False):
 
        q = self._get_status_query(repo, revision, pull_request,
 
                                   with_revisions)
 
        q = q.options(joinedload('author'))
 
        return q.all()
 

	
 
    def get_status(self, repo, revision=None, pull_request=None, as_str=True):
 
        """
 
        Returns latest status of changeset for given revision or for given
 
        pull request. Statuses are versioned inside a table itself and
 
        version == 0 is always the current one
 

	
 
        :param repo:
 
        :param revision: 40char hash or None
 
        :param pull_request: pull_request reference
 
        :param as_str: return status as string not object
kallithea/model/comment.py
Show inline comments
 
@@ -118,49 +118,49 @@ class ChangesetCommentsModel(BaseModel):
 
            _org_ref_type, org_ref_name, _org_rev = comment.pull_request.org_ref.split(':')
 
            threading = ['%s-pr-%s@%s' % (pull_request.other_repo.repo_name,
 
                                          pull_request.pull_request_id,
 
                                          h.canonical_hostname())]
 
            if line_no: # TODO: url to file _and_ line number
 
                threading.append('%s-pr-%s-line-%s@%s' % (pull_request.other_repo.repo_name,
 
                                                          pull_request.pull_request_id, line_no,
 
                                                          h.canonical_hostname()))
 
            comment_url = pull_request.url(canonical=True,
 
                anchor='comment-%s' % comment.comment_id)
 
            subj = safe_unicode(
 
                h.link_to('Re pull request %(pr_nice_id)s: %(desc)s %(line)s' % \
 
                          {'desc': desc,
 
                           'pr_nice_id': comment.pull_request.nice_id(),
 
                           'line': line},
 
                          comment_url)
 
            )
 
            # get the current participants of this pull request
 
            recipients = ChangesetComment.get_users(pull_request_id=
 
                                                pull_request.pull_request_id)
 
            # add pull request author
 
            recipients += [pull_request.owner]
 

	
 
            # add the reviewers to notification
 
            recipients += [x.user for x in pull_request.reviewers]
 
            recipients += pull_request.get_reviewer_users()
 

	
 
            #set some variables for email notification
 
            email_kwargs = {
 
                'pr_title': pull_request.title,
 
                'pr_nice_id': pull_request.nice_id(),
 
                'status_change': status_change,
 
                'closing_pr': closing_pr,
 
                'pr_comment_url': comment_url,
 
                'pr_comment_user': user.full_name_and_username,
 
                'pr_target_repo': h.canonical_url('summary_home',
 
                                   repo_name=pull_request.other_repo.repo_name),
 
                'repo_name': pull_request.other_repo.repo_name,
 
                'ref': org_ref_name,
 
                'comment_username': user.username,
 
                'threading': threading,
 
            }
 

	
 
        return subj, body, recipients, notification_type, email_kwargs
 

	
 
    def create(self, text, repo, user, revision=None, pull_request=None,
 
               f_path=None, line_no=None, status_change=None, closing_pr=False,
 
               send_email=True):
 
        """
 
        Creates a new comment for either a changeset or a pull request.
kallithea/model/db.py
Show inline comments
 
@@ -2301,48 +2301,56 @@ class PullRequest(Base, BaseModel):
 
    def revisions(self):
 
        return self._revisions.split(':')
 

	
 
    @revisions.setter
 
    def revisions(self, val):
 
        self._revisions = safe_unicode(':'.join(val))
 

	
 
    @property
 
    def org_ref_parts(self):
 
        return self.org_ref.split(':')
 

	
 
    @property
 
    def other_ref_parts(self):
 
        return self.other_ref.split(':')
 

	
 
    owner = relationship('User')
 
    reviewers = relationship('PullRequestReviewers',
 
                             cascade="all, delete-orphan")
 
    org_repo = relationship('Repository', primaryjoin='PullRequest.org_repo_id==Repository.repo_id')
 
    other_repo = relationship('Repository', primaryjoin='PullRequest.other_repo_id==Repository.repo_id')
 
    statuses = relationship('ChangesetStatus', order_by='ChangesetStatus.changeset_status_id')
 
    comments = relationship('ChangesetComment', order_by='ChangesetComment.comment_id',
 
                             cascade="all, delete-orphan")
 

	
 
    def get_reviewer_users(self):
 
        """Like .reviewers, but actually returning the users"""
 
        return User.query() \
 
            .join(PullRequestReviewers) \
 
            .filter(PullRequestReviewers.pull_request == self) \
 
            .order_by(PullRequestReviewers.pull_requests_reviewers_id) \
 
            .all()
 

	
 
    def is_closed(self):
 
        return self.status == self.STATUS_CLOSED
 

	
 
    def user_review_status(self, user_id):
 
        """Return the user's latest status votes on PR"""
 
        # note: no filtering on repo - that would be redundant
 
        status = ChangesetStatus.query() \
 
            .filter(ChangesetStatus.pull_request == self) \
 
            .filter(ChangesetStatus.user_id == user_id) \
 
            .order_by(ChangesetStatus.version) \
 
            .first()
 
        return str(status.status) if status else ''
 

	
 
    @classmethod
 
    def make_nice_id(cls, pull_request_id):
 
        '''Return pull request id nicely formatted for displaying'''
 
        return '#%s' % pull_request_id
 

	
 
    def nice_id(self):
 
        '''Return the id of this pull request, nicely formatted for displaying'''
 
        return self.make_nice_id(self.pull_request_id)
 

	
 
    def __json__(self):
 
        return dict(
0 comments (0 inline, 0 general)