Transitive query checks need to be for updates, not just deletes

MySQL doesn't support transitive updates either, so we need to extend the testing to prevent the recent breakage from occurring again
This commit is contained in:
Joseph Schorr 2017-03-20 21:06:37 -04:00
parent 81b01aa263
commit a047257656

View file

@ -858,11 +858,11 @@ class TestDeleteNamespace(ApiTestCase):
self.login(ADMIN_ACCESS_USER) self.login(ADMIN_ACCESS_USER)
# Try to first delete the user. Since they are the sole admin of two orgs, it should fail. # Try to first delete the user. Since they are the sole admin of two orgs, it should fail.
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteResponse(User, expected_code=400) self.deleteResponse(User, expected_code=400)
# Delete the two orgs, checking in between. # Delete the two orgs, checking in between.
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteEmptyResponse(Organization, params=dict(orgname=ORGANIZATION), expected_code=204) self.deleteEmptyResponse(Organization, params=dict(orgname=ORGANIZATION), expected_code=204)
self.deleteResponse(User, expected_code=400) # Should still fail. self.deleteResponse(User, expected_code=400) # Should still fail.
self.deleteEmptyResponse(Organization, params=dict(orgname='library'), expected_code=204) self.deleteEmptyResponse(Organization, params=dict(orgname='library'), expected_code=204)
@ -872,7 +872,7 @@ class TestDeleteNamespace(ApiTestCase):
dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'anotherrepo'], '{}') dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'anotherrepo'], '{}')
# Now delete the user. # Now delete the user.
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteEmptyResponse(User, expected_code=204) self.deleteEmptyResponse(User, expected_code=204)
# Ensure the queue items are gone. # Ensure the queue items are gone.
@ -886,12 +886,12 @@ class TestDeleteNamespace(ApiTestCase):
user = model.user.get_user(PUBLIC_USER) user = model.user.get_user(PUBLIC_USER)
model.user.attach_federated_login(user, 'github', 'something', {}) model.user.attach_federated_login(user, 'github', 'something', {})
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteEmptyResponse(User, expected_code=204) self.deleteEmptyResponse(User, expected_code=204)
def test_delete_prompted_user(self): def test_delete_prompted_user(self):
self.login('randomuser') self.login('randomuser')
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteEmptyResponse(User, expected_code=204) self.deleteEmptyResponse(User, expected_code=204)
@ -2169,15 +2169,19 @@ class TestChangeRepoVisibility(ApiTestCase):
class log_queries(object): class log_queries(object):
def __init__(self, query_filter=None): def __init__(self, query_filters=None):
self.filter = query_filter self.filters = query_filters
def get_queries(self): def get_queries(self):
queries = [q.msg[0] for q in self._handler.queries] queries = [q.msg[0] for q in self._handler.queries]
if self.filter: if not self.filters:
queries = [q for q in queries if re.match(self.filter, q)] return queries
return queries filtered_queries = []
for query_filter in self.filters:
filtered_queries.extend([q for q in queries if re.match(query_filter, q)])
return filtered_queries
def __enter__(self): def __enter__(self):
logger = logging.getLogger('peewee') logger = logging.getLogger('peewee')
@ -2191,15 +2195,16 @@ class log_queries(object):
logger.removeHandler(self._handler) logger.removeHandler(self._handler)
class check_transitive_deletes(log_queries): class check_transitive_modifications(log_queries):
def __init__(self): def __init__(self):
super(check_transitive_deletes, self).__init__(query_filter=r'^DELETE.+IN \(SELECT.+$') filters = [r'^DELETE.+IN \(SELECT.+$', r'^UPDATE.+IN \(SELECT.+$']
super(check_transitive_modifications, self).__init__(query_filters=filters)
def __exit__(self, exc_type, exc_val, exc_tb): def __exit__(self, exc_type, exc_val, exc_tb):
super(check_transitive_deletes, self).__exit__(exc_type, exc_val, exc_tb) super(check_transitive_modifications, self).__exit__(exc_type, exc_val, exc_tb)
queries = self.get_queries() queries = self.get_queries()
if queries: if queries:
raise Exception('Detected transitive deletion in queries: %s' % queries) raise Exception('Detected transitive deletion or update in queries: %s' % queries)
class TestDeleteRepository(ApiTestCase): class TestDeleteRepository(ApiTestCase):
@ -2320,7 +2325,7 @@ class TestDeleteRepository(ApiTestCase):
model.label.create_manifest_label(tag_manifest, 'something', '{"some": "json"}', 'manifest') model.label.create_manifest_label(tag_manifest, 'something', '{"some": "json"}', 'manifest')
# Delete the repository. # Delete the repository.
with check_transitive_deletes(): with check_transitive_modifications():
self.deleteEmptyResponse(Repository, params=dict(repository=self.COMPLEX_REPO)) self.deleteEmptyResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
# Verify the repo was deleted. # Verify the repo was deleted.