[LNT] r308695 - api: Rework json serialization, add roundtrip test

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 20 16:14:22 PDT 2017


Author: matze
Date: Thu Jul 20 16:14:22 2017
New Revision: 308695

URL: http://llvm.org/viewvc/llvm-project?rev=308695&view=rev
Log:
api: Rework json serialization, add roundtrip test

- Move a bunch of object->dict logic from api.py to testsuitedb.py into
  the __json__() methods.
- Explicitely mention all fields/members in the __json__() methods and
  leave out some odd/unnecessary ones:
    - Generally leave out parent pointers (i.e. samples do not have
      their run_id, runs not their machine_id)
    - Leave out simple_run_id, import_from
- Change response to 'machine/XXX' api to return a
  'machine' element at the top level instead of a 'machines' array with
  1 element.
- Inject the LNT jsonencoder in restful output_json() callback so we
  don't need to manually call jsonify() everywhere.
- Add a roundtrip test that downloads a run through the API, deletes it,
  reuploads it, downloads it again and checks whether it is identical.

Added:
    lnt/trunk/tests/server/ui/test_api_roundtrip.py
Modified:
    lnt/trunk/lnt/server/db/testsuitedb.py
    lnt/trunk/lnt/server/ui/api.py
    lnt/trunk/tests/server/ui/test_api.py
    lnt/trunk/tests/server/ui/test_api_modify.py

Modified: lnt/trunk/lnt/server/db/testsuitedb.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/db/testsuitedb.py?rev=308695&r1=308694&r2=308695&view=diff
==============================================================================
--- lnt/trunk/lnt/server/db/testsuitedb.py (original)
+++ lnt/trunk/lnt/server/db/testsuitedb.py Thu Jul 20 16:14:22 2017
@@ -22,11 +22,12 @@ import lnt.testing.profile.profile as pr
 import lnt
 
 
-def strip(obj):
-    """Give back a dict without sqlalchemy stuff."""
-    new_dict = dict(obj)
-    new_dict.pop('_sa_instance_state', None)
-    return new_dict
+def _dict_update_abort_on_duplicates(base_dict, to_merge):
+    '''This behaves like base_dict.update(to_merge) but asserts that none
+    of the keys in to_merge is present in base_dict yet.'''
+    for key, value in to_merge.items():
+        assert base_dict.get(key, None) is None
+        base_dict[key] = value
 
 
 _sample_type_to_sql = {
@@ -101,6 +102,15 @@ class TestSuiteDB(object):
             def set_field(self, field, value):
                 return setattr(self, field.name, value)
 
+            def get_fields(self):
+                result = dict()
+                for field in self.fields:
+                    value = self.get_field(field)
+                    if value is None:
+                        continue
+                    result[field.name] = value
+                return result
+
         db_key_name = self.test_suite.db_key_name
 
         class Machine(self.base, ParameterizedMixin):
@@ -187,8 +197,12 @@ class TestSuiteDB(object):
                 return closest_run
 
             def __json__(self):
-                # {u'name': self.name, u'MachineID': self.id}
-                return strip(self.__dict__)
+                result = dict()
+                result['name'] = self.name
+                result['id'] = self.id
+                _dict_update_abort_on_duplicates(result, self.get_fields())
+                _dict_update_abort_on_duplicates(result, self.parameters)
+                return result
 
         class Order(self.base, ParameterizedMixin):
             __tablename__ = db_key_name + '_Order'
@@ -282,12 +296,12 @@ class TestSuiteDB(object):
                            tuple(convert_field(b.get_field(item))
                                  for item in self.fields))
 
-            def __json__(self):
-                order = dict((item.name, self.get_field(item))
-                             for item in self.fields)
-                order[u'id'] = self.id
-                order[u'name'] = self.as_ordered_string()
-                return strip(order)
+            def __json__(self, include_id=True):
+                result = {}
+                if include_id:
+                    result['id'] = self.id
+                _dict_update_abort_on_duplicates(result, self.get_fields())
+                return result
 
         class Run(self.base, ParameterizedMixin):
             __tablename__ = db_key_name + '_Run'
@@ -346,10 +360,24 @@ class TestSuiteDB(object):
             def parameters(self, data):
                 self.parameters_data = json.dumps(sorted(data.items()))
 
-            def __json__(self):
-                self.machine
-                self.order
-                return strip(self.__dict__)
+            def __json__(self, flatten_order=True):
+                result = {
+                    'id': self.id,
+                    'start_time': self.start_time,
+                    'end_time': self.end_time,
+                }
+                # Leave out: machine_id, simple_run_id, imported_from
+                if flatten_order:
+                    _dict_update_abort_on_duplicates(result,
+                        self.order.__json__(include_id=False))
+                    result['order_by'] = \
+                        ', '.join([f.name for f in self.order.fields])
+                    result['order_id'] = self.order_id
+                else:
+                    result['order_id'] = self.order_id
+                _dict_update_abort_on_duplicates(result, self.get_fields())
+                _dict_update_abort_on_duplicates(result, self.parameters)
+                return result
 
         Machine.runs = relation(Run, back_populates='machine',
                                 cascade="all, delete-orphan")
@@ -369,8 +397,11 @@ class TestSuiteDB(object):
                 return '%s_%s%r' % (db_key_name, self.__class__.__name__,
                                     (self.name,))
 
-            def __json__(self):
-                return strip(self.__dict__)
+            def __json__(self, include_id=True):
+                result = {'name': self.name}
+                if include_id:
+                    result['id'] = self.id
+                return result
 
         class Profile(self.base):
             __tablename__ = db_key_name + '_Profile'
@@ -503,8 +534,19 @@ class TestSuiteDB(object):
                     db_key_name, self.__class__.__name__,
                     self.run, self.test, fields)
 
-            def __json__(self):
-                return strip(self.__dict__)
+            def __json__(self, flatten_test=False, include_id=True):
+                result = {}
+                if include_id:
+                    result['id'] = self.id
+                # Leave out: run_id
+                # TODO: What about profile/profile_id?
+                if flatten_test:
+                    _dict_update_abort_on_duplicates(result,
+                        self.test.__json__(include_id=False))
+                else:
+                    result['test_id'] = self.test_id
+                _dict_update_abort_on_duplicates(result, self.get_fields())
+                return result
 
         Run.samples = relation(Sample, back_populates='run',
                                cascade="all, delete-orphan")
@@ -553,13 +595,17 @@ class TestSuiteDB(object):
                                      self.test, self.machine, self.field))
 
             def __json__(self):
-                self.machine
-                self.test
-                self.field
-                self.run
-                self.start_order
-                self.end_order
-                return strip(self.__dict__)
+                return {
+                    'id': self.id,
+                    'old_value': self.old_value,
+                    'new_value': self.new_value,
+                    'start_order_id': self.start_order_id,
+                    'end_order_id': self.end_order_id,
+                    'test_id': self.test_id,
+                    'machine_id': self.machine_id,
+                    'field_id': self.field_id,
+                    'run_id': self.run_id,
+                }
 
         Machine.fieldchanges = relation(FieldChange, back_populates='machine',
                                         cascade="all, delete-orphan")
@@ -595,7 +641,12 @@ class TestSuiteDB(object):
                                                "<Deleted>")
 
             def __json__(self):
-                return strip(self.__dict__)
+                return {
+                    'id': self.id,
+                    'title': self.title,
+                    'bug': self.bug,
+                    'state': self.state,
+                }
 
         class RegressionIndicator(self.base, ParameterizedMixin):
             """"""
@@ -620,9 +671,11 @@ class TestSuiteDB(object):
                                      self.field_change))
 
             def __json__(self):
-                return {u'RegressionIndicatorID': self.id,
-                        u'Regression': self.regression,
-                        u'FieldChange': self.field_change}
+                return {
+                    'RegressionIndicatorID': self.id,
+                    'Regression': self.regression,
+                    'FieldChange': self.field_change
+                }
 
         FieldChange.regression_indicators = \
             relation(RegressionIndicator, back_populates='field_change',
@@ -834,6 +887,10 @@ class TestSuiteDB(object):
 
         # Added by REST API, we will replace as well.
         run_parameters.pop('order_by', None)
+        run_parameters.pop('order_id', None)
+        run_parameters.pop('machine_id', None)
+        run_parameters.pop('imported_from', None)
+        run_parameters.pop('simple_run_id', None)
 
         # Find the order record.
         order, inserted = self._getOrCreateOrder(run_parameters)

Modified: lnt/trunk/lnt/server/ui/api.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/lnt/server/ui/api.py?rev=308695&r1=308694&r2=308695&view=diff
==============================================================================
--- lnt/trunk/lnt/server/ui/api.py (original)
+++ lnt/trunk/lnt/server/ui/api.py Thu Jul 20 16:14:22 2017
@@ -1,7 +1,7 @@
 import lnt.util.ImportData
 import sqlalchemy
-from flask import current_app, g, Response, stream_with_context
-from flask import jsonify
+from flask import current_app, g, Response, make_response, stream_with_context
+from flask import json, jsonify
 from flask import request
 from flask_restful import Resource, abort
 from sqlalchemy.orm import joinedload
@@ -85,39 +85,11 @@ class Machines(Resource):
     @staticmethod
     def get():
         ts = request.get_testsuite()
-        machine_infos = ts.query(ts.Machine).all()
+        machines = ts.query(ts.Machine).all()
 
-        serializable_machines = []
-
-        for machine in machine_infos:
-            serializable_machines.append(common_machine_format(machine))
-
-        machines = common_fields_factory()
-        machines['machines'] = serializable_machines
-        return jsonify(machines)
-
-
-def common_run_format(run):
-    serializable_run = run.__json__()
-    del serializable_run['order']
-    # Replace orders with text order.
-
-    # Embed the parameters and order right into the run dict.
-    serializable_run.update(run.parameters)
-    serializable_run.update(dict((item.name, run.order.get_field(item))
-                                 for item in run.order.fields))
-    serializable_run['order_by'] = ', '.join([f.name for f in run.order.fields])
-    del serializable_run['machine']
-    del serializable_run['parameters_data']
-    return serializable_run
-
-
-def common_machine_format(machine):
-    serializable_machine = machine.__json__()
-    # Embed the parameters and order right into the run dict.
-    serializable_machine.update(machine.parameters)
-    del serializable_machine['parameters_data']
-    return serializable_machine
+        result = common_fields_factory()
+        result['machines'] = machines
+        return result
 
 
 class Machine(Resource):
@@ -137,8 +109,6 @@ class Machine(Resource):
     def get(machine_id):
         ts = request.get_testsuite()
         this_machine = Machine._get_machine(machine_id)
-        machine = common_fields_factory()
-        machine['machines'] = [common_machine_format(this_machine)]
         machine_runs = ts.query(ts.Run) \
             .join(ts.Machine) \
             .join(ts.Order) \
@@ -146,12 +116,12 @@ class Machine(Resource):
             .options(joinedload('order')) \
             .all()
 
-        runs = []
-        for run in machine_runs:
-            runs.append(common_run_format(run))
-        machine['runs'] = runs
+        runs = [run.__json__(flatten_order=True) for run in machine_runs]
 
-        return jsonify(machine)
+        result = common_fields_factory()
+        result['machine'] = this_machine
+        result['runs'] = runs
+        return result
 
     @staticmethod
     @requires_auth_token
@@ -230,24 +200,24 @@ class Run(Resource):
         except sqlalchemy.orm.exc.NoResultFound:
             abort(404, msg="Did not find run " + str(run_id))
 
-        full_run['run'] = common_run_format(run)
-        full_run['machine'] = common_machine_format(run.machine)
-
         to_get = [ts.Sample.id, ts.Sample.run_id, ts.Test.name]
         for f in ts.sample_fields:
             to_get.append(f.column)
 
-        q = ts.query(*to_get) \
+        sample_query = ts.query(*to_get) \
             .join(ts.Test) \
-            .join(ts.Run) \
-            .join(ts.Order) \
-            .filter(ts.Sample.run_id == run_id)
+            .filter(ts.Sample.run_id == run_id) \
+            .all()
+        # TODO: Handle multiple samples for a single test?
 
         # noinspection PyProtectedMember
-        ret = [sample._asdict() for sample in q.all()]
+        samples = [row._asdict() for row in sample_query]
 
-        full_run['tests'] = ret
-        return jsonify(full_run)
+        result = common_fields_factory()
+        result['run'] = run
+        result['machine'] = run.machine
+        result['tests'] = samples
+        return result
 
     @staticmethod
     @requires_auth_token
@@ -258,7 +228,6 @@ class Run(Resource):
             abort(404, msg="Did not find run " + str(run_id))
         ts.delete(run)
         ts.commit()
-        return
 
 
 class Runs(Resource):
@@ -272,7 +241,7 @@ class Runs(Resource):
         db = request.get_db()
         data = request.data
         result = lnt.util.ImportData.import_from_string(current_app.old_config,
-                                                        g.db_name, db, g.testsuite_name, data)
+            g.db_name, db, g.testsuite_name, data)
 
         new_url = ('%sapi/db_%s/v4/%s/runs/%s' %
                    (request.url_root, g.db_name, g.testsuite_name,
@@ -294,9 +263,9 @@ class Order(Resource):
             order = ts.query(ts.Order).filter(ts.Order.id == order_id).one()
         except NoResultFound:
             abort(404, message="Invalid order.")
-        order_output = common_fields_factory()
-        order_output['orders'] = [order]
-        return jsonify(order_output)
+        result = common_fields_factory()
+        result['orders'] = [order]
+        return result
 
 
 class SampleData(Resource):
@@ -306,12 +275,14 @@ class SampleData(Resource):
     def get(sample_id):
         ts = request.get_testsuite()
         try:
-            sample = ts.query(ts.Sample).filter(ts.Sample.id == sample_id).one()
+            sample = ts.query(ts.Sample) \
+                .filter(ts.Sample.id == sample_id) \
+                .one()
         except NoResultFound:
             abort(404, message="Invalid order.")
-        sample_output = common_fields_factory()
-        sample_output['samples'] = [{k: v for k, v in sample.__json__().items() if v is not None}]
-        return jsonify(sample_output)
+        result = common_fields_factory()
+        result['samples'] = [sample]
+        return result
 
 
 class SamplesData(Resource):
@@ -343,12 +314,12 @@ class SamplesData(Resource):
             .join(ts.Run) \
             .join(ts.Order) \
             .filter(ts.Sample.run_id.in_(run_ids))
-        output_samples = common_fields_factory()
+        result = common_fields_factory()
         # noinspection PyProtectedMember
-        output_samples['samples'] = [{k: v for k, v in sample.items() if v is not None}
-                                     for sample in [sample._asdict() for sample in q.all()]]
+        result['samples'] = [{k: v for k, v in sample.items() if v is not None}
+                             for sample in [sample._asdict() for sample in q.all()]]
 
-        return output_samples
+        return result
 
 
 class Graph(Resource):
@@ -446,6 +417,14 @@ def ts_path(path):
 
 
 def load_api_resources(api):
+    @api.representation('application/json')
+    def output_json(data, code, headers=None):
+        '''Override output_json() to use LNT json encoder'''
+        resp = make_response(json.dumps(data), code)
+        if headers is not None:
+            resp.headers.extend(headers)
+        return resp
+
     api.add_resource(Machines, ts_path("machines"), ts_path("machines/"))
     api.add_resource(Machine, ts_path("machines/<machine_id>"))
     api.add_resource(Runs, ts_path("runs"), ts_path("runs/"))

Modified: lnt/trunk/tests/server/ui/test_api.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/ui/test_api.py?rev=308695&r1=308694&r2=308695&view=diff
==============================================================================
--- lnt/trunk/tests/server/ui/test_api.py (original)
+++ lnt/trunk/tests/server/ui/test_api.py Thu Jul 20 16:14:22 2017
@@ -34,11 +34,9 @@ machines_expected_response = [{u'hardwar
                                u'name': u'machine3'}]
 
 order_expected_response = {u'llvm_project_revision': u'154331',
-                           u'id': 1,
-                           u'name': u'154331'}
+                           u'id': 1}
 
-sample_expected_response = {u'run_id': 1,
-                            u'id': 1,
+sample_expected_response = {u'id': 1,
                             u'execution_time': 0.0003,
                             u'test_id': 1,
                             u'compile_time': 0.007}
@@ -60,7 +58,6 @@ graph_data2 = [[[152293], 10.0,
 possible_run_keys = {u'__report_version__',
                      u'inferred_run_order',
                      u'test_suite_revision',
-                     u'simple_run_id',
                      u'cc_alt_src_revision',
                      u'USE_REFERENCE_OUTPUT',
                      u'cc_as_version',
@@ -88,7 +85,6 @@ possible_run_keys = {u'__report_version_
                      u'cc_version',
                      u'OPTFLAGS',
                      u'cc_ld_version',
-                     u'imported_from',
                      u'TARGET_LLVMGCC',
                      u'LLI_OPTFLAGS',
                      u'cc_target',
@@ -97,7 +93,6 @@ possible_run_keys = {u'__report_version_
                      u'end_time',
                      u'TEST',
                      u'LLC_OPTFLAGS',
-                     u'machine_id',
                      u'cc_exec_hash',
                      u'llvm_project_revision'
                      }
@@ -152,14 +147,11 @@ class JSONAPITester(unittest.TestCase):
         # Machine + properties + run information.
         j = check_json(client, 'api/db_default/v4/nts/machines/1')
         self._check_response_is_well_formed(j)
-        self.assertEqual(len(j['machines']), 1)
-        for machine in j['machines']:
-            self.assertSetEqual(set(machine.keys()), possible_machine_keys)
-        expected = {"hardware": "x86_64", "os": "Darwin 11.3.0", "id": 1}
-        self.assertDictContainsSubset(expected, j['machines'][0])
+        self.assertEqual(j['machine'], machines_expected_response[0])
 
         self.assertEqual(len(j['runs']), 2)
-        self.assertSetEqual(set(j['runs'][0].keys()), possible_run_keys)
+        for run in j['runs']:
+          self.assertSetEqual(set(run.keys()), possible_run_keys)
 
         # Invalid machine ids are 404.
         check_json(client, 'api/db_default/v4/nts/machines/99', expected_code=404)
@@ -171,9 +163,7 @@ class JSONAPITester(unittest.TestCase):
         j = check_json(client, 'api/db_default/v4/nts/runs/1')
         self._check_response_is_well_formed(j)
         expected = {"end_time": "2012-04-11T16:28:58",
-                    "order_id": 1,
                     "start_time": "2012-04-11T16:28:23",
-                    "machine_id": 1,
                     "id": 1,
                     "llvm_project_revision": u'154331'}
         self.assertDictContainsSubset(expected, j['run'])

Modified: lnt/trunk/tests/server/ui/test_api_modify.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/ui/test_api_modify.py?rev=308695&r1=308694&r2=308695&view=diff
==============================================================================
--- lnt/trunk/tests/server/ui/test_api_modify.py (original)
+++ lnt/trunk/tests/server/ui/test_api_modify.py Thu Jul 20 16:14:22 2017
@@ -36,7 +36,7 @@ class JSONAPIDeleteTester(unittest.TestC
 
         # Make sure the environment is as expected.
         j = check_json(client, 'api/db_default/v4/nts/machines/1')
-        self.assertEqual(j['machines'][0]['name'], 'localhost__clang_DEV__x86_64')
+        self.assertEqual(j['machine']['name'], 'localhost__clang_DEV__x86_64')
 
         data = {
             'action': 'rename',
@@ -55,7 +55,7 @@ class JSONAPIDeleteTester(unittest.TestC
 
         # Machine should be renamed now.
         j = check_json(client, 'api/db_default/v4/nts/machines/1')
-        self.assertEqual(j['machines'][0]['name'], 'new_machine_name')
+        self.assertEqual(j['machine']['name'], 'new_machine_name')
 
     def test_01_delete_run(self):
         """Check /runs/n can be deleted."""

Added: lnt/trunk/tests/server/ui/test_api_roundtrip.py
URL: http://llvm.org/viewvc/llvm-project/lnt/trunk/tests/server/ui/test_api_roundtrip.py?rev=308695&view=auto
==============================================================================
--- lnt/trunk/tests/server/ui/test_api_roundtrip.py (added)
+++ lnt/trunk/tests/server/ui/test_api_roundtrip.py Thu Jul 20 16:14:22 2017
@@ -0,0 +1,61 @@
+# Check that the LNT REST JSON API is working.
+# create temporary instance
+# RUN: rm -rf %t.instance
+# RUN: python %{shared_inputs}/create_temp_instance.py \
+# RUN:     %s %{shared_inputs}/SmallInstance \
+# RUN:     %t.instance %S/Inputs/V4Pages_extra_records.sql
+#
+# RUN: python %s %t.instance %{shared_inputs}
+
+import json
+import logging
+import sys
+import unittest
+
+import lnt.server.db.migrate
+import lnt.server.ui.app
+from V4Pages import check_json
+
+logging.basicConfig(level=logging.DEBUG)
+
+
+class JSONAPIDeleteTester(unittest.TestCase):
+    """Test the REST api."""
+    def setUp(self):
+        """Bind to the LNT test instance."""
+        _, instance_path, shared_inputs = sys.argv
+        app = lnt.server.ui.app.App.create_standalone(instance_path)
+        app.testing = True
+        self.client = app.test_client()
+        self.shared_inputs = shared_inputs
+
+    def test_roundtrip(self):
+        """Check /runs GET, POST roundtrip"""
+        client = self.client
+
+        # Download originl
+        original = check_json(client, 'api/db_default/v4/nts/runs/2')
+
+        # Remove the run
+        resp = client.delete('api/db_default/v4/nts/runs/2',
+                             headers={'AuthToken': 'test_token'})
+        self.assertEqual(resp.status_code, 200)
+
+        # Post it back
+        resp = client.post('api/db_default/v4/nts/runs',
+                           data=json.dumps(original),
+                           headers={'AuthToken': 'test_token'})
+        self.assertEqual(resp.status_code, 301)
+        new_location = resp.headers['Location']
+
+        # Download new data
+        reimported = check_json(client, new_location)
+
+        # The 'id' field may be the different, the rest must be the same.
+        reimported['run']['id'] = original['run']['id']
+        self.assertEqual(original, reimported)
+
+
+if __name__ == '__main__':
+    unittest.TestLoader.sortTestMethodsUsing = lambda _, x, y: cmp(x, y)
+    unittest.main(argv=[sys.argv[0], ])




More information about the llvm-commits mailing list