[IMP] runbot: cleanup and improve hook

When getting pull info, the alive state can be determined easily,
meaning that this field can join the "_compute_branch_infos" familly

Hook was catching some changes made on pr and was conditionnaly updating some fields
and triggering some other operations conditionaly depending on the action flag.
All of the information needed to update the pull info should always be present in the
 payload body, meaning that all fields can be updated at once in case some hook was missed,
 and additionnal operation can be triggered based on fields changes.

runbot/controllers/hook.py

@@ -12,9 +12,8 @@ _logger = logging.getLogger(__name__)
 
 class Hook(http.Controller):
 
-    @http.route(['/runbot/hook/<int:remote_id>', '/runbot/hook/org'], type='http', auth="public", website=True, csrf=False)
-    def hook(self, remote_id=None, **post):
-        _logger.info('Hook on %s', remote_id)
+    @http.route(['/runbot/hook/<int:remote_id>'], type='http', auth="public", website=True, csrf=False)
+    def hook(self, remote_id=None, **_post):
         event = request.httprequest.headers.get("X-Github-Event")
         payload = json.loads(request.params.get('payload', '{}'))
         if remote_id is None:
@@ -32,40 +31,19 @@ class Hook(http.Controller):
         remote = request.env['runbot.remote'].sudo().browse([remote_id])
 
         # force update of dependencies too in case a hook is lost
-        if not payload or event == 'push' or (event == 'pull_request' and payload.get('action') in ('synchronize', 'opened', 'reopened')):
+        if not payload or event == 'push':
             remote.repo_id.set_hook_time(time.time())
         elif event == 'pull_request':
             pr_number = payload.get('pull_request', {}).get('number', '')
             branch = request.env['runbot.branch'].sudo().search([('remote_id', '=', remote.id), ('name', '=', pr_number)])
-            if payload and payload.get('action', '') == 'edited' and 'base' in payload.get('changes'):
-                # handle PR that have been re-targeted
-                branch._compute_branch_infos(payload.get('pull_request', {}))
-                _logger.info('Retargeting %s to %s', branch.name, branch.target_branch_name)
-                base = request.env['runbot.bundle'].search([
-                    ('name', '=', branch.target_branch_name),
-                    ('is_base', '=', True),
-                    ('project_id', '=', branch.remote_id.repo_id.project_id.id)
-                ])
-                if base and branch.bundle_id.defined_base_id != base:
-                    _logger.info('Changing base of bundle %s to %s(%s)', branch.bundle_id, base.name, base.id)
-                    branch.bundle_id.defined_base_id = base.id
-                    branch.bundle_id._force()
-            elif payload.get('action') in ('ready_for_review', 'converted_to_draft'):
-                init_draft = branch.draft
-                branch._compute_branch_infos(payload.get('pull_request', {}))
-                if branch.draft != init_draft:
-                    branch.bundle_id._force()
-            elif payload.get('action') in ('deleted', 'closed'):
-                _logger.info('Closing pr %s', branch.name)
-                branch.alive = False
-            else:
-                _logger.info('Ignoring unsupported pull request operation %s %s', event, payload.get('action', ''))
+            branch.recompute_infos(payload.get('pull_request', {}))
+            if payload.get('action') in ('synchronize', 'opened', 'reopened'):
+                remote.repo_id.set_hook_time(time.time())
+            # remaining recurrent actions: labeled, review_requested, review_request_removed
         elif event == 'delete':
             if payload.get('ref_type') == 'branch':
                 branch_ref = payload.get('ref')
-                _logger.info('Hook for branch deletion %s in repo %s', branch_ref, remote.repo_id.name)
+                _logger.info('Branch %s in repo %s was deleted', branch_ref, remote.repo_id.name)
                 branch = request.env['runbot.branch'].sudo().search([('remote_id', '=', remote.id), ('name', '=', branch_ref)])
                 branch.alive = False
-        else:
-            _logger.info('Ignoring unsupported hook %s %s', event, payload.get('action', ''))
         return ""

runbot/models/branch.py

@@ -104,6 +104,7 @@ class Branch(models.Model):
                 if pi:
                     try:
                         branch.draft = pi.get('draft', False)
+                        branch.alive = pi.get('state', False) != 'closed'
                         branch.target_branch_name = pi['base']['ref']
                         branch.pull_head_name = pi['head']['label']
                         pull_head_repo_name = False
@@ -199,9 +200,26 @@ class Branch(models.Model):
             self.name
         )
 
-    def recompute_infos(self):
+    def recompute_infos(self, payload=None):
         """ public method to recompute infos on demand """
-        self._compute_branch_infos()
+        was_draft = self.draft
+        was_alive = self.alive
+        init_target_branch_name = self.target_branch_name
+        self._compute_branch_infos(payload)
+        if self.target_branch_name != init_target_branch_name:
+            _logger.info('retargeting %s to %s', self.name, self.target_branch_name)
+            base = self.env['runbot.bundle'].search([
+                ('name', '=', self.target_branch_name),
+                ('is_base', '=', True),
+                ('project_id', '=', self.remote_id.repo_id.project_id.id)
+            ])
+            if base and self.bundle_id.defined_base_id != base:
+                _logger.info('Changing base of bundle %s to %s(%s)', self.bundle_id, base.name, base.id)
+                self.bundle_id.defined_base_id = base.id
+                self.bundle_id._force()
+
+        if (not self.draft and was_draft) or (self.alive and not was_alive) or (self.target_branch_name != init_target_branch_name and self.alive):
+            self.bundle_id._force()
 
     @api.model
     def match_is_base(self, name):

runbot/models/repo.py

@@ -429,12 +429,14 @@ class Repo(models.Model):
                         'date': dateutil.parser.parse(date[:19]),
                     })
                 branch.head = commit
-                branch.alive = True
-                # Not perfect, il some case a pr can be closed but still visible in repo.
-                # The head wont change but on creation the branch will be set alive even if git into said pr is closed
-                # It is still better to have false open than false closed
+                if not branch.alive:
+                    if branch.is_pr:
+                        _logger.info('Recomputing infos of dead pr %s', branch.name)
+                        branch._compute_branch_infos()
+                    else:
+                        branch.alive = True
 
-                if branch.reference_name and branch.remote_id and branch.remote_id.repo_id._is_branch_forbidden(branch.reference_name ):
+                if branch.reference_name and branch.remote_id and branch.remote_id.repo_id._is_branch_forbidden(branch.reference_name):
                     message = "This branch name is incorrect. Branch name should be prefixed with a valid version"
                     message = branch.remote_id.repo_id.invalid_branch_message or message
                     branch.head._github_status(False, "Branch naming", 'failure', False, message)
@@ -446,7 +448,7 @@ class Repo(models.Model):
                 if bundle.no_build:
                     continue
 
-                if bundle.last_batch.state != 'preparing' and commit not in bundle.last_batch.commit_ids:
+                if bundle.last_batch.state != 'preparing':
                     preparing = self.env['runbot.batch'].create({
                         'last_update': fields.Datetime.now(),
                         'bundle_id': bundle.id,