Merge pull request #1088 from coreos-inc/norecursivequeries

Remove remaining recursive queries on repo delete and add test

data/database.py

@@ -406,7 +406,7 @@ class Repository(BaseModel):
     # These models don't need to use transitive deletes, because the referenced objects
     # are cleaned up directly
     skip_transitive_deletes = {RepositoryTag, RepositoryBuild, RepositoryBuildTrigger, BlobUpload,
-                               Image}
+                               Image, TagManifest, DerivedStorageForImage}
 
     delete_instance_filtered(self, Repository, delete_nullable, skip_transitive_deletes)
 

test/test_api_usage.py

@@ -2,12 +2,14 @@
 
 import unittest
 import datetime
+import logging
+import re
 import json as py_json
 
 from urllib import urlencode
 from urlparse import urlparse, urlunparse, parse_qs
 
-from playhouse.test_utils import assert_query_count
+from playhouse.test_utils import assert_query_count, _QueryLogHandler
 
 from endpoints.api import api_bp, api
 from endpoints.building import PreparedBuild
@@ -1490,6 +1492,42 @@ class TestChangeRepoVisibility(ApiTestCase):
     self.assertEquals(False, json['is_public'])
 
 
+
+
+class log_queries(object):
+  def __init__(self, query_filter=None):
+    self.filter = query_filter
+
+  def get_queries(self):
+    queries = [q.msg[0] for q in self._handler.queries]
+    if self.filter:
+      queries = [q for q in queries if re.match(self.filter, q)]
+
+    return queries
+
+  def __enter__(self):
+    logger = logging.getLogger('peewee')
+    self._handler = _QueryLogHandler()
+    logger.setLevel(logging.DEBUG)
+    logger.addHandler(self._handler)
+    return self
+
+  def __exit__(self, exc_type, exc_val, exc_tb):
+    logger = logging.getLogger('peewee')
+    logger.removeHandler(self._handler)
+
+
+class check_transitive_deletes(log_queries):
+  def __init__(self):
+    super(check_transitive_deletes, self).__init__(query_filter=r'^DELETE.+IN \(SELECT.+$')
+
+  def __exit__(self, exc_type, exc_val, exc_tb):
+    super(check_transitive_deletes, self).__exit__(exc_type, exc_val, exc_tb)
+    queries = self.get_queries()
+    if queries:
+      raise Exception('Detected transitive deletion in queries: %s' % queries)
+
+
 class TestDeleteRepository(ApiTestCase):
   SIMPLE_REPO = ADMIN_ACCESS_USER + '/simple'
   COMPLEX_REPO = ADMIN_ACCESS_USER + '/complex'
@@ -1573,7 +1611,8 @@ class TestDeleteRepository(ApiTestCase):
                                  date=datetime.datetime.now() - datetime.timedelta(days=5), count=6)
 
     # Delete the repository.
-    self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
+    with check_transitive_deletes():
+      self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
 
     # Verify the repo was deleted.
     self.getResponse(Repository,

test/test_gc.py

@@ -121,7 +121,7 @@ class TestGarbageCollection(unittest.TestCase):
     """
     # Delete all existing repos.
     for repo in database.Repository.select():
-      repo.delete_instance(recursive=True)
+      model.repository.purge_repository(repo.namespace_user.username, repo.name)
 
     # Change the time machine expiration on the namespace.
     (database.User.update(removed_tag_expiration_s=1000000000)