[#642] Refactor datastore plugin configuration, improve (and fix ;-))…

… tests

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 "
@@ -128,23 +131,21 @@ 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
 
     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.

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()