Fix the delete_old_*() db api functions

This commit fixes issues with the 2 delete_old_* functions from the
db api. Previously they were using a join to get a list of metadata
rows to delete as part of the expiration, however this isn't allowed
because sqlalchemy doesn't know which rows to actually delete. This
switches the internals of the function to use a subquery to get a
list of all ids and use that as the filter on the delete command.
Tests are then added to verify the functionality.

Change-Id: Ibe9384cd2b470a10bbc6030dad2bdbb759fee276

subunit2sql/db/api.py

@@ -848,9 +848,14 @@ def delete_old_runs(expire_age=186, session=None):
     """
     session = session or get_session()
     expire_date = datetime.date.today() - datetime.timedelta(days=expire_age)
-    db_utils.model_query(models.Run, session).filter(
-        models.Run.run_at < expire_date).join(
-            models.RunMetadata).delete(synchronize_session='evaluate')
+
+    # Delete the run_metadata
+    sub_query = session.query(models.Run.id).filter(
+        models.Run.run_at < expire_date).subquery()
+    db_utils.model_query(models.RunMetadata, session).filter(
+        models.RunMetadata.run_id.in_(sub_query)).delete(
+            synchronize_session='fetch')
+    # Delete the runs
     db_utils.model_query(models.Run, session).filter(
         models.Run.run_at < expire_date).delete(synchronize_session='evaluate')
 
@@ -864,9 +869,14 @@ def delete_old_test_runs(expire_age=186, session=None):
     """
     session = session or get_session()
     expire_date = datetime.date.today() - datetime.timedelta(days=expire_age)
-    db_utils.model_query(models.TestRun, session).filter(
-        models.TestRun.start_time < expire_date).join(
-            models.TestRunMetadata).delete(synchronize_session='evaluate')
+
+    # Delete the test run metadata
+    sub_query = session.query(models.TestRun.id).filter(
+        models.TestRun.start_time < expire_date).subquery()
+    db_utils.model_query(models.TestRunMetadata, session).filter(
+        models.TestRunMetadata.test_run_id.in_(sub_query)).delete(
+            synchronize_session='fetch')
+    # Delete the test runs
     db_utils.model_query(models.TestRun, session).filter(
         models.TestRun.start_time < expire_date).delete(
             synchronize_session='evaluate')

subunit2sql/tests/db/test_api.py

@@ -17,6 +17,7 @@ import datetime
 import testscenarios
 
 from subunit2sql.db import api
+from subunit2sql.db import models
 from subunit2sql.tests import base
 from subunit2sql.tests import subunit2sql_fixtures as fixtures
 
@@ -359,3 +360,37 @@ class TestDatabaseAPI(base.TestCase):
             'start_time': timestamp_a,
             'stop_time': timestamp_b,
         }], test_run_dicts)
+
+    def test_delete_old_runs(self):
+        run_a = api.create_run(run_at=datetime.datetime(
+            1914, 6, 28, 10, 45, 0))
+        run_b = api.create_run()
+        api.add_run_metadata({'key': 'value'}, run_b.id)
+        api.add_run_metadata({'key': 'not_so_much_a_value'}, run_a.id)
+        api.delete_old_runs()
+        runs = api.get_all_runs()
+        self.assertEqual(1, len(runs))
+        self.assertEqual(1, api.get_session().query(
+            models.RunMetadata.id).count())
+        self.assertEqual(run_b.id, runs[0].id)
+        self.assertEqual(1, len(api.get_run_metadata(run_b.id)))
+        self.assertEqual(0, len(api.get_run_metadata(run_a.id)))
+
+    def test_delete_old_test_runs(self):
+        run_a = api.create_run()
+        run_b = api.create_run()
+        test = api.create_test('fake_test')
+        test_run_a = api.create_test_run(test.id, run_a.id, 'fail',
+                                         start_time=datetime.datetime(
+                                             1914, 6, 28, 10, 45, 0))
+        test_run_b = api.create_test_run(test.id, run_b.id, 'fail',
+                                         start_time=datetime.datetime.utcnow())
+        api.add_test_run_metadata({'key': 'value'}, test_run_b.id)
+        api.add_test_run_metadata({'key': 'not_so_much_a_value'},
+                                  test_run_a.id)
+        api.delete_old_test_runs()
+        test_runs = api.get_all_test_runs()
+        self.assertEqual(1, len(test_runs))
+        self.assertEqual(test_run_b.id, test_runs[0].id)
+        self.assertEqual(1, len(api.get_test_run_metadata(test_run_b.id)))
+        self.assertEqual(0, len(api.get_test_run_metadata(test_run_a.id)))