Add pingback support (bug 1377305)

zalun · zalun · commit bc6b655713ef · 2017-07-05T11:46:56.000+02:00
Added POST API endpoint `/landings/{landing_id}/update`.
`landing_id` (from path) and `request_id` (from data) have to correspond
with database entry, otherwise 404 is raised.
Modified the schema by adding error and result.
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -17,6 +17,7 @@ services:
       - PHABRICATOR_UNPRIVILEGED_API_KEY=api-123456789
       - TRANSPLANT_URL=https://stub.transplant.example.com
       - DATABASE_URL=sqlite:////db/sqlite.db
+      - HOST_URL=https://lando-api.test
     volumes:
       - ./:/app
       - ./.db/:/db/
diff --git a/landoapi/api/landings.py b/landoapi/api/landings.py
@@ -7,11 +7,10 @@
 """
 from connexion import problem
 from flask import request
+from sqlalchemy.orm.exc import NoResultFound
 from landoapi.models.landing import (
-    Landing,
-    LandingNotCreatedException,
-    LandingNotFoundException,
-    RevisionNotFoundException,
+    Landing, LandingNotCreatedException, RevisionNotFoundException,
+    TRANSPLANT_JOB_FAILED, TRANSPLANT_JOB_LANDED
 )
 
 
@@ -60,9 +59,8 @@ def get_list(revision_id=None, status=None):
 def get(landing_id):
     """ API endpoint at /landings/{landing_id} to return stored Landing.
     """
-    try:
-        landing = Landing.get(landing_id)
-    except LandingNotFoundException:
+    landing = Landing.query.get(landing_id)
+    if not landing:
         return problem(
             404,
             'Landing not found',
@@ -71,3 +69,46 @@ def get(landing_id):
         )
 
     return landing.serialize(), 200
+
+
+def update(landing_id, data):
+    """Update landing on pingback from Transplant.
+
+    data contains following fields:
+    request_id: integer
+        id of the landing request in Transplant
+    tree: string
+        tree name as per treestatus
+    rev: string
+        matching phabricator revision identifier
+    destination: string
+        full url of destination repo
+    trysyntax: string
+        change will be pushed to try or empty string
+    landed: boolean;
+        true when operation was successful
+    error_msg: string
+        error message if landed == false
+        empty string if landed == true
+    result: string
+        revision (sha) of push if landed == true
+        empty string if landed == false
+    """
+    try:
+        landing = Landing.query.filter_by(
+            id=landing_id, request_id=data['request_id']
+        ).one()
+    except NoResultFound:
+        return problem(
+            404,
+            'Landing not found',
+            'The requested Landing does not exist',
+            type='https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404'
+        )
+
+    landing.error = data.get('error_msg', '')
+    landing.result = data.get('result', '')
+    landing.status = TRANSPLANT_JOB_LANDED if data['landed'
+                                                  ] else TRANSPLANT_JOB_FAILED
+    landing.save()
+    return {}, 202
diff --git a/landoapi/models/landing.py b/landoapi/models/landing.py
@@ -1,13 +1,17 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+import os
+
 from landoapi.hgexportbuilder import build_patch_for_revision
 from landoapi.models.storage import db
 from landoapi.phabricator_client import PhabricatorClient
 from landoapi.transplant_client import TransplantClient
 
+TRANSPLANT_JOB_STARTING = 'pending'
 TRANSPLANT_JOB_STARTED = 'started'
-TRANSPLANT_JOB_FINISHED = 'finished'
+TRANSPLANT_JOB_LANDED = 'landed'
+TRANSPLANT_JOB_FAILED = 'failed'
 
 
 class Landing(db.Model):
@@ -17,16 +21,21 @@ class Landing(db.Model):
     request_id = db.Column(db.Integer, unique=True)
     revision_id = db.Column(db.String(30))
     status = db.Column(db.Integer)
+    error = db.Column(db.String(128), default='')
+    result = db.Column(db.String(128))
 
     def __init__(
-        self, request_id=None, revision_id=None, status=TRANSPLANT_JOB_STARTED
+        self,
+        request_id=None,
+        revision_id=None,
+        status=TRANSPLANT_JOB_STARTING
     ):
         self.request_id = request_id
         self.revision_id = revision_id
         self.status = status
 
     @classmethod
-    def create(cls, revision_id, phabricator_api_key=None, save=True):
+    def create(cls, revision_id, phabricator_api_key=None):
         """ Land revision and create a Transplant item in storage. """
         phab = PhabricatorClient(phabricator_api_key)
         revision = phab.get_revision(id=revision_id)
@@ -40,39 +49,34 @@ def create(cls, revision_id, phabricator_api_key=None, save=True):
 
         repo = phab.get_revision_repo(revision)
 
+        # save landing to make sure we've got the callback
+        landing = cls(
+            revision_id=revision_id,
+        ).save()
+
         trans = TransplantClient()
+        callback = '%s/landings/%s/update' % (
+            os.getenv('HOST_URL'), landing.id
+        )
         # The LDAP username used here has to be the username of the patch
         # pusher.
         # FIXME: what value do we use here?
         # FIXME: This client, or the person who pushed the 'Land it!' button?
         request_id = trans.land(
-            'ldap_username@example.com', hgpatch, repo['uri']
+            'ldap_username@example.com', hgpatch, repo['uri'], callback
         )
         if not request_id:
             raise LandingNotCreatedException
 
-        landing = cls(
-            revision_id=revision_id,
-            request_id=request_id,
-            status=TRANSPLANT_JOB_STARTED
-        )
-        if save:
-            landing.save(create=True)
+        landing.request_id = request_id
+        landing.status = TRANSPLANT_JOB_STARTED
+        landing.save()
 
         return landing
 
-    @classmethod
-    def get(cls, landing_id):
-        """ Get Landing object from storage. """
-        landing = cls.query.get(landing_id)
-        if not landing:
-            raise LandingNotFoundException()
-
-        return landing
-
-    def save(self, create=False):
+    def save(self):
         """ Save objects in storage. """
-        if create:
+        if not self.id:
             db.session.add(self)
 
         db.session.commit()
@@ -87,7 +91,9 @@ def serialize(self):
             'id': self.id,
             'revision_id': self.revision_id,
             'request_id': self.request_id,
-            'status': self.status
+            'status': self.status,
+            'error_msg': self.error,
+            'result': self.result
         }
 
 
@@ -96,11 +102,6 @@ class LandingNotCreatedException(Exception):
     pass
 
 
-class LandingNotFoundException(Exception):
-    """ No specific Landing was found in database. """
-    pass
-
-
 class RevisionNotFoundException(Exception):
     """ Phabricator returned 404 for a given revision id. """
 
diff --git a/landoapi/spec/swagger.yml b/landoapi/spec/swagger.yml
@@ -111,6 +111,13 @@ paths:
       responses:
         202:
           description: OK
+          schema:
+            type: object
+            properties:
+              id:
+                type: integer
+                description: |
+                  A newly created Landing id
         404:
           description: Revision does not exist
           schema:
@@ -121,6 +128,60 @@ paths:
           schema:
             allOf:
               - $ref: '#/definitions/Error'
+  /landings/{landing_id}/update:
+    post:
+      operationId: landoapi.api.landings.update
+      description: |
+        Sends request to transplant service and responds with just the
+        status code
+      parameters:
+        - name: data
+          in: body
+          description: |
+            Retrieve status of the landing job
+          required: true
+          schema:
+            type: object
+            required:
+              - request_id
+            properties:
+              request_id:
+                type: integer
+              tree:
+                type: string
+              rev:
+                type: string
+              patch:
+                type: string
+              destination:
+                type: string
+              trysyntax:
+                type: string
+              landed:
+                type: boolean
+              error_msg:
+                type: string
+              result:
+                type: string
+        - name: landing_id
+          in: path
+          type: string
+          description: |
+            The id of the landing to return
+          required: true
+      responses:
+        202:
+          description: OK
+        404:
+          description: Landing does not exist
+          schema:
+            allOf:
+              - $ref: '#/definitions/Error'
+        default:
+          description: Unexpected error
+          schema:
+            allOf:
+              - $ref: '#/definitions/Error'
   /landings/{landing_id}:
     get:
       description: |
@@ -168,6 +229,14 @@ definitions:
         type: integer
         description: |
           The id of the Revision
+      result:
+        type: string
+        description: |
+          revision (sha) of push if landed == true
+      error:
+        type: string
+        description: |
+          Error message if landing failed
   Revision:
     type: object
     properties:
diff --git a/landoapi/transplant_client.py b/landoapi/transplant_client.py
@@ -17,15 +17,15 @@ def __init__(self):
         self.api_url = os.getenv('TRANSPLANT_URL')
 
     @requests_mock.mock()
-    def land(self, ldap_username, hgpatch, tree, request):
+    def land(self, ldap_username, hgpatch, tree, pingback, request):
         """ Sends a push request to Transplant API to land a revision.
 
         Returns request_id received from Transplant API.
         """
         # get the number of Landing objects to create the unique request_id
         sql = text('SELECT COUNT(*) FROM landings')
         result = db.session.execute(sql).fetchone()
-        request_id = result[0] + 1
+        request_id = result[0]
 
         # Connect to stubbed Transplant service
         request.post(
@@ -44,7 +44,7 @@ def land(self, ldap_username, hgpatch, tree, request):
                 'destination': 'destination',
                 'push_bookmark': 'push_bookmark',
                 'commit_descriptions': 'commit_descriptions',
-                'pingback_url': 'http://pingback.url/'
+                'pingback_url': pingback
             }
         )
 
diff --git a/migrations/67151bb74080_add_error_message_and_result_to_landing.py b/migrations/67151bb74080_add_error_message_and_result_to_landing.py
@@ -0,0 +1,33 @@
+"""Add error message and result to Landing
+
+Revision ID: 67151bb74080
+Revises: ae9dd729e66c
+Create Date: 2017-06-29 22:16:56.874034
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = '67151bb74080'
+down_revision = 'ae9dd729e66c'
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column(
+        'landings', sa.Column('error', sa.String(length=128), nullable=True)
+    )
+    op.add_column(
+        'landings', sa.Column('result', sa.String(length=128), nullable=True)
+    )
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('landings', 'result')
+    op.drop_column('landings', 'error')
+    # ### end Alembic commands ###
diff --git a/tests/canned_responses/lando_api/landings.py b/tests/canned_responses/lando_api/landings.py
@@ -7,23 +7,40 @@
         'id': 1,
         'request_id': 1,
         'revision_id': 'D1',
-        'status': 'started'
+        'status': 'started',
+        'error_msg': '',
+        'result': None,
     }, {
         'id': 2,
         'request_id': 2,
         'revision_id': 'D1',
-        'status': 'finished'
+        'status': 'finished',
+        'error_msg': '',
+        'result': None,
     }, {
         'id': 4,
         'request_id': 4,
         'revision_id': 'D1',
-        'status': 'started'
+        'status': 'started',
+        'error_msg': '',
+        'result': None,
     }
 ]
 
 CANNED_LANDING_1 = {
     'id': 1,
     'status': 'started',
     'request_id': 1,
-    'revision_id': 'D1'
+    'revision_id': 'D1',
+    'error_msg': '',
+    'result': None,
+}
+
+CANNED_LANDING_2 = {
+    'id': 2,
+    'status': 'started',
+    'request_id': 2,
+    'revision_id': 'D1',
+    'error_msg': '',
+    'result': None,
 }
diff --git a/tests/test_landings.py b/tests/test_landings.py
