diff --git a/lib.py b/lib.py index 7b37174..a9ecb6b 100755 --- a/lib.py +++ b/lib.py @@ -11,7 +11,7 @@ class Client(object): def __init__(self, url, username, key): self.url = url - self.username = username + self.username = username # Unused self.column_cache = {} self.phid_cache = {} self.session = { @@ -251,4 +251,4 @@ class Client(object): } } return self.post('maniphest.search', params)[ - 'data'] \ No newline at end of file + 'data'] diff --git a/patchforreview_remover.py b/patchforreview_remover.py index 9bcf0ba..184f1fe 100755 --- a/patchforreview_remover.py +++ b/patchforreview_remover.py @@ -12,17 +12,60 @@ class Checker(): self.client = client def check(self, t_id): + """ + Returns true if the Patch-For-Review project should be removed from the Phabricator + task identified 't_id'. + """ phid = self.client.lookupPhid(t_id) return self.phid_check(phid) - def phid_check(self, phid): + def get_change_url(self, raw_comment): + m = re.search(r'https://gerrit(?:-test|)\.wikimedia\.org/r/\d+', raw_comment) + if m: + return m[0] + m = re.search(r'https://gitlab\.wikimedia\.org/repos/.*/-/merge_requests/\d+', raw_comment) + if m: + return m[0] + return None + + def get_operation_type(self, raw_comment, url): + """ + If the operation type can be determined from raw_comment, return + it. It will be the string "open" (first patchset created or merge + request created) or "close" (merge or abandon) + """ + # Gitlab style + if re.search(r"opened " + re.escape(url), raw_comment): + return "open" + + if re.search(r"(merged|closed) " + re.escape(url), raw_comment): + return "close" + + # Gerrit style + if re.search(r"Change \d+ had a related patch set uploaded", raw_comment): + return "open" + + if re.search(r'Change \d+ \*\*(?:merged|abandoned)\*\* by ', raw_comment): + return "close" + + return None + + def phid_check(self, phid) -> bool: + """ + Returns true if the Patch-For-Review project should be removed from the Phabricator + task identified by 'phid'. + """ gerrit_bot_actions = [] + + # Note that transactions are returned in reverse chronological order (most recent first). for transaction in self.client.getTransactions(phid): if re.findall(re.escape('https://github.com/') + r'.+?/pull', str(transaction)): return False if transaction['authorPHID'] == self.gerrit_bot_phid: gerrit_bot_actions.append(transaction) else: + # If someone other than GerritBot adds the Patch-For-Review project, don't + # auto-remove it. if transaction['type'] == 'projects': check = self.project_patch_for_review_phid in str( transaction['fields']) @@ -38,33 +81,42 @@ class Checker(): if len(case['comments']) != 1: return False raw_comment = case['comments'][0]['content']['raw'] - gerrit_patch_id = re.findall( - r'https://gerrit(?:-test|)\.wikimedia\.org/r/.*(\d+)(?:$|\]\])', raw_comment)[0] - merged = re.findall( - r'Change \d+ \*\*(?:merged|abandoned)\*\* by ', - raw_comment) - gerrit_patch_status[gerrit_patch_id].append(not(bool(merged))) + change_url = self.get_change_url(raw_comment) + if change_url: + op = self.get_operation_type(raw_comment, change_url) + + # Append True or False depending on whether the action was to + # open/reopen a change (True) or merge a change (False) + gerrit_patch_status[change_url].append(op in ["open", "reopen"]) for patch in gerrit_patch_status: + # The normal sequence of GerritBot transactions for a Gerrit change is "Change + # \d+ had a related patch set uploaded" (indicated by True in + # gerrit_patch_status) eventually followed by "Change \d+ (merged|abandoned) + # by whoever" (indicated by False in gerrit_patch_status). The transactions + # are returned in reverse order so the opened/merged pattern will appear as + # the reverse of [True, False], which is [False, True]. + # FIXME: This logic can't handle a open/close/reopen/merge situation. if gerrit_patch_status[patch] != [False, True]: return False return True +if __name__ == "__main__": + client = Client.newFromCreds() -client = Client.newFromCreds() - -project_patch_for_review_phid = 'PHID-PROJ-onnxucoedheq3jevknyr' -checker = Checker( - 'PHID-USER-idceizaw6elwiwm5xshb', - project_patch_for_review_phid, - client) -gen = client.getTasksWithProject(project_patch_for_review_phid) -for phid in gen: - if checker.phid_check(phid): - print(client.taskDetails(phid)['id']) - try: - client.removeProjectByPhid(project_patch_for_review_phid, phid) - except BaseException: - continue - time.sleep(10) + gerrit_bot_phid = 'PHID-USER-idceizaw6elwiwm5xshb' + project_patch_for_review_phid = 'PHID-PROJ-onnxucoedheq3jevknyr' + checker = Checker( + gerrit_bot_phid, + project_patch_for_review_phid, + client) + gen = client.getTasksWithProject(project_patch_for_review_phid) + for phid in gen: + if checker.phid_check(phid): + print(client.taskDetails(phid)['id']) + try: + client.removeProjectByPhid(project_patch_for_review_phid, phid) + except BaseException: + continue + time.sleep(10)