[#642] Inject error_handler instead of explicitly passing it as an …

…argument, split large test into smaller tests

ckanext/datastore/plugin.py

@@ -63,7 +63,7 @@ def configure(self, config):
                      "only database. Permission checks and the creation "
                      "of _table_metadata are skipped.")
         else:
-            self._check_urls_and_permissions(self._log_or_raise)
+            self._check_urls_and_permissions()
 
             self._create_alias_table()
 
@@ -106,24 +106,24 @@ def _log_or_raise(self, message):
         else:
             raise DatastoreException(message)
 
-    def _check_urls_and_permissions(self, error_handler):
+    def _check_urls_and_permissions(self):
         # Make sure that the right permissions are set
         # so that no harmful queries can be made
 
         if self._same_ckan_and_datastore_db():
-            error_handler("CKAN and DataStore database "
-                          "cannot be the same.")
+            self._log_or_raise("CKAN and DataStore database "
+                               "cannot be the same.")
 
         # in legacy mode, the read and write url are ths same (both write url)
         # consequently the same url check and and write privilege check
         # don't make sense
         if not self.legacy_mode:
             if self._same_read_and_write_url():
-                error_handler("The write and read-only database "
-                              "connection urls are the same.")
+                self._log_or_raise("The write and read-only database "
+                                   "connection urls are the same.")
 
             if not self._read_connection_has_correct_privileges():
-                error_handler("The read-only user has write privileges.")
+                self._log_or_raise("The read-only user has write privileges.")
 
     def _is_read_only_database(self):
         '''

ckanext/datastore/tests/test_configure.py

@@ -1,14 +1,10 @@
 import unittest
-from nose.tools import assert_equal
+from nose.tools import raises
 
 import ckanext.datastore.plugin as plugin
 
-# global variable used for callback tests
-msg = ''
-called = 0
 
-
-class TestTypeGetters(unittest.TestCase):
+class TestConfiguration(unittest.TestCase):
     def setUp(self):
         self.p = plugin.DatastorePlugin()
 
@@ -47,69 +43,62 @@ def test_same_ckan_and_datastore_db(self):
         self.p.ckan_url = 'postgresql://u:pass@localhost/ckan'
         assert not self.p._same_ckan_and_datastore_db()
 
-    def test_check_urls_and_permissions(self):
-        global msg
-
-        def handler(message):
-            global msg, called
-            msg = message
-            called += 1
-
-        def true_privileges_mock():
-            return True
 
-        def false_privileges_mock():
-            return False
+class TestCheckUrlsAndPermissions(unittest.TestCase):
+    def setUp(self):
+        self.p = plugin.DatastorePlugin()
 
         self.p.legacy_mode = False
+
+        # initialize URLs
         self.p.ckan_url = 'postgresql://u:pass@localhost/ckan'
         self.p.write_url = 'postgresql://u:pass@localhost/ds'
         self.p.read_url = 'postgresql://u2:pass@localhost/ds'
+
+        # initialize mock for privileges check
+        def true_privileges_mock():
+            return True
         self.p._read_connection_has_correct_privileges = true_privileges_mock
 
-        # all urls are correct
-        self.p._check_urls_and_permissions(handler)
-        assert_equal(msg, '')
-        assert_equal(called, 0)
+        def raise_datastore_exception(message):
+            raise plugin.DatastoreException(message)
+        self.p._log_or_raise = raise_datastore_exception
 
-        # same url for read and write but in legacy mode
-        self.p.legacy_mode = True
-        self.p.read_url = 'postgresql://u:pass@localhost/ds'
-        self.p._check_urls_and_permissions(handler)
-        assert_equal(msg, '')
-        assert_equal(called, 0)
+    def test_everything_correct_does_not_raise(self):
+        self.p._check_urls_and_permissions()
 
-        # same url for read and write
-        self.p.legacy_mode = False
-        self.p._check_urls_and_permissions(handler)
-        assert 'urls are the same' in msg, msg
-        assert_equal(called, 1)
+    @raises(plugin.DatastoreException)
+    def test_raises_when_ckan_and_datastore_db_are_the_same(self):
+        self.p.read_url = 'postgresql://u2:pass@localhost/ckan'
+        self.p.ckan_url = 'postgresql://u:pass@localhost/ckan'
 
-        # same ckan and ds db
-        self.p.ckan_url = 'postgresql://u:pass@localhost/ds'
-        self.p.read_url = 'postgresql://u2:pass@localhost/ds'
-        self.p._check_urls_and_permissions(handler)
-        assert 'cannot be the same' in msg, msg
-        assert_equal(called, 2)
+        self.p._check_urls_and_permissions()
 
-        # have write permissions but in legacy mode
+    @raises(plugin.DatastoreException)
+    def test_raises_when_same_read_and_write_url(self):
+        self.p.read_url = 'postgresql://u:pass@localhost/ds'
+        self.p.write_url = 'postgresql://u:pass@localhost/ds'
+
+        self.p._check_urls_and_permissions()
+
+    def test_same_read_and_write_url_in_legacy_mode(self):
+        self.p.read_url = 'postgresql://u:pass@localhost/ds'
+        self.p.write_url = 'postgresql://u:pass@localhost/ds'
         self.p.legacy_mode = True
-        msg = ''
-        self.p.ckan_url = 'postgresql://u:pass@localhost/ckan'
+
+        self.p._check_urls_and_permissions()
+
+    @raises(plugin.DatastoreException)
+    def test_raises_when_we_have_write_permissions(self):
+        def false_privileges_mock():
+            return False
         self.p._read_connection_has_correct_privileges = false_privileges_mock
-        self.p._check_urls_and_permissions(handler)
-        assert_equal(msg, '')
-        assert_equal(called, 2)
+        self.p._check_urls_and_permissions()
 
-        # have write permissions
-        self.p.legacy_mode = False
-        self.p._check_urls_and_permissions(handler)
-        assert 'user has write privileges' in msg, msg
-        assert_equal(called, 3)
+    def test_have_write_permissions_in_legacy_mode(self):
+        def false_privileges_mock():
+            return False
+        self.p._read_connection_has_correct_privileges = false_privileges_mock
+        self.p.legacy_mode = True
 
-        # everything is wrong
-        self.p.ckan_url = 'postgresql://u:pass@localhost/ds'
-        self.p.write_url = 'postgresql://u:pass@localhost/ds'
-        self.p.read_url = 'postgresql://u:pass@localhost/ds'
-        self.p._check_urls_and_permissions(handler)
-        assert_equal(called, 6)
+        self.p._check_urls_and_permissions()