Merge pull request #55 from dancysoft/master

patchforreview_remover.py: Support both Gerrit and Gitlab

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']
+            'data']

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()
+    gerrit_bot_phid = 'PHID-USER-idceizaw6elwiwm5xshb'
-
+    project_patch_for_review_phid = 'PHID-PROJ-onnxucoedheq3jevknyr'
-project_patch_for_review_phid = 'PHID-PROJ-onnxucoedheq3jevknyr'
+    checker = Checker(
-checker = Checker(
+        gerrit_bot_phid,
-    'PHID-USER-idceizaw6elwiwm5xshb',
+        project_patch_for_review_phid,
-    project_patch_for_review_phid,
+        client)
-    client)
+    gen = client.getTasksWithProject(project_patch_for_review_phid)
-gen = client.getTasksWithProject(project_patch_for_review_phid)
+    for phid in gen:
-for phid in gen:
+        if checker.phid_check(phid):
-    if checker.phid_check(phid):
+            print(client.taskDetails(phid)['id'])
-        print(client.taskDetails(phid)['id'])
+            try:
-        try:
+                client.removeProjectByPhid(project_patch_for_review_phid, phid)
-            client.removeProjectByPhid(project_patch_for_review_phid, phid)
+            except BaseException:
-        except BaseException:
+                continue
-            continue
+            time.sleep(10)
-        time.sleep(10)