From a04725765612a613bf715343aaae627c4efa88d2 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Mar 2017 21:06:37 -0400 Subject: [PATCH] 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 --- test/test_api_usage.py | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/test/test_api_usage.py b/test/test_api_usage.py index c29a1c622..fe8d56bd1 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -858,11 +858,11 @@ class TestDeleteNamespace(ApiTestCase): self.login(ADMIN_ACCESS_USER) # 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) # 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.deleteResponse(User, expected_code=400) # Should still fail. 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'], '{}') # Now delete the user. - with check_transitive_deletes(): + with check_transitive_modifications(): self.deleteEmptyResponse(User, expected_code=204) # Ensure the queue items are gone. @@ -886,12 +886,12 @@ class TestDeleteNamespace(ApiTestCase): user = model.user.get_user(PUBLIC_USER) model.user.attach_federated_login(user, 'github', 'something', {}) - with check_transitive_deletes(): + with check_transitive_modifications(): self.deleteEmptyResponse(User, expected_code=204) def test_delete_prompted_user(self): self.login('randomuser') - with check_transitive_deletes(): + with check_transitive_modifications(): self.deleteEmptyResponse(User, expected_code=204) @@ -2169,15 +2169,19 @@ class TestChangeRepoVisibility(ApiTestCase): class log_queries(object): - def __init__(self, query_filter=None): - self.filter = query_filter + def __init__(self, query_filters=None): + self.filters = query_filters 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)] + if not self.filters: + 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): logger = logging.getLogger('peewee') @@ -2191,15 +2195,16 @@ class log_queries(object): logger.removeHandler(self._handler) -class check_transitive_deletes(log_queries): +class check_transitive_modifications(log_queries): 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): - 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() 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): @@ -2320,7 +2325,7 @@ class TestDeleteRepository(ApiTestCase): model.label.create_manifest_label(tag_manifest, 'something', '{"some": "json"}', 'manifest') # Delete the repository. - with check_transitive_deletes(): + with check_transitive_modifications(): self.deleteEmptyResponse(Repository, params=dict(repository=self.COMPLEX_REPO)) # Verify the repo was deleted.