From 244bc6628f7474549409bb6093f345b5494025f6 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Wed, 27 Mar 2013 15:50:59 +0100 Subject: [PATCH] [#642] Refactor datastore plugin configuration, improve (and fix ;-)) tests --- ckanext/datastore/plugin.py | 25 ++++++++-------- ckanext/datastore/tests/test_configure.py | 35 +++++++++++++++++------ 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/ckanext/datastore/plugin.py b/ckanext/datastore/plugin.py index 10afcfeb192..9d696181d67 100644 --- a/ckanext/datastore/plugin.py +++ b/ckanext/datastore/plugin.py @@ -56,9 +56,12 @@ def configure(self, config): # Make sure that the right permissions are set # so that no harmful queries can be made if not ('debug' in config and config['debug']): + if self._same_read_and_write_url(): + raise DatastoreException("The write and read-only database " + "connection url are the same.") if self._same_ckan_and_datastore_db(): - raise Exception("The write and read-only database " - "connection url are the same.") + raise DatastoreException("CKAN and DataStore database " + "cannot be the same.") if self.legacy_mode: log.warn("Legacy mode active. " "The sql search will not be available.") @@ -67,8 +70,8 @@ def configure(self, config): log.critical("We have write permissions " "on the read-only database.") else: - raise Exception("We have write permissions " - "on the read-only database.") + raise DatastoreException("We have write permissions " + "on the read-only database.") self._create_alias_table() else: log.warn("We detected that CKAN is running on a read " @@ -129,16 +132,8 @@ def _is_read_only_database(self): def _same_ckan_and_datastore_db(self): ''' - Make sure the datastore is on a separate db. Otherwise one could access - all internal tables via the api. - Returns True if the CKAN and DataStore db are the same ''' - - if not self.legacy_mode: - if self.write_url == self.read_url: - return True - if self._get_db_from_url(self.ckan_url) == self._get_db_from_url(self.read_url): return True return False @@ -146,6 +141,12 @@ def _same_ckan_and_datastore_db(self): def _get_db_from_url(self, url): return url[url.rindex("@"):] + def _same_read_and_write_url(self): + # in legacy mode, this test can be ignored + if self.legacy_mode: + return True + return self.write_url == self.read_url + def _read_connection_has_correct_privileges(self): ''' Returns True if the right permissions are set for the read only user. diff --git a/ckanext/datastore/tests/test_configure.py b/ckanext/datastore/tests/test_configure.py index eb96c9bdc49..a5f1dd24d21 100644 --- a/ckanext/datastore/tests/test_configure.py +++ b/ckanext/datastore/tests/test_configure.py @@ -24,16 +24,33 @@ def test_set_legacy_mode(self): assert self.p.write_url == 'foo' assert self.p.read_url == 'foo' - def test_check_separate_db(self): + def test_check_separate_write_and_read_if_not_legacy(self): + self.p.legacy_mode = True + self.p.write_url = 'postgresql://u:pass@localhost/ds' + self.p.read_url = 'postgresql://u:pass@localhost/ds' + assert self.p._same_read_and_write_url() + + self.p.legacy_mode = False + + assert not self.p.legacy_mode + + self.p.write_url = 'postgresql://u:pass@localhost/ds' + self.p.read_url = 'postgresql://u:pass@localhost/ds' + assert self.p._same_read_and_write_url() + + self.p.write_url = 'postgresql://u:pass@localhost/ds' + self.p.read_url = 'postgresql://u2:pass@localhost/ds' + assert not self.p._same_read_and_write_url() + + def test_same_ckan_and_datastore_db(self): + self.p.write_url = 'postgresql://u:pass@localhost/ckan' + self.p.read_url = 'postgresql://u:pass@localhost/ckan' + self.p.ckan_url = 'postgresql://u:pass@localhost/ckan' + + assert self.p._same_ckan_and_datastore_db() + self.p.write_url = 'postgresql://u:pass@localhost/dt' self.p.read_url = 'postgresql://u:pass@localhost/dt' self.p.ckan_url = 'postgresql://u:pass@localhost/ckan' - self.p.legacy_mode = True - try: - self.p._check_separate_db() - except Exception: - self.fail("_check_separate_db raise Exception unexpectedly!") - - self.p.legacy_mode = False - self.assertRaises(Exception, self.p._check_separate_db) + assert not self.p._same_ckan_and_datastore_db()