diff --git a/base_external_dbsource/README.rst b/base_external_dbsource/README.rst index f77bb396b72..0147fe2dff8 100644 --- a/base_external_dbsource/README.rst +++ b/base_external_dbsource/README.rst @@ -50,6 +50,7 @@ Known issues / Roadmap * Add a ConnectionEnvironment that allows for the reuse of connections * Message box should be displayed instead of error in ``connection_test`` * Remove old api compatibility layers (v11) +* Implement better CRUD handling Bug Tracker diff --git a/base_external_dbsource/models/base_external_dbsource.py b/base_external_dbsource/models/base_external_dbsource.py index 822f24deb44..fa7e9ca0d5d 100644 --- a/base_external_dbsource/models/base_external_dbsource.py +++ b/base_external_dbsource/models/base_external_dbsource.py @@ -203,6 +203,7 @@ def remote_browse(self, record_ids, *args, **kwargs): (iter) Iterator of record mappings that match the ID. """ + assert self.current_table method = self._get_adapter_method('remote_browse') return method(record_ids, *args, **kwargs) @@ -221,6 +222,7 @@ def remote_create(self, vals, *args, **kwargs): (mapping) A mapping of the record that was created. """ + assert self.current_table method = self._get_adapter_method('remote_create') return method(vals, *args, **kwargs) @@ -239,6 +241,7 @@ def remote_delete(self, record_ids, *args, **kwargs): (iter) Iterator of bools indicating delete status. """ + assert self.current_table method = self._get_adapter_method('remote_delete') return method(record_ids, *args, **kwargs) @@ -257,6 +260,7 @@ def remote_search(self, query, *args, **kwargs): (iter) Iterator of record mappings that match query. """ + assert self.current_table method = self._get_adapter_method('remote_search') return method(query, *args, **kwargs) @@ -275,6 +279,7 @@ def remote_update(self, record_ids, vals, *args, **kwargs): (iter) Iterator of record mappings that were updated. """ + assert self.current_table method = self._get_adapter_method('remote_update') return method(record_ids, vals, *args, **kwargs) diff --git a/base_external_dbsource/tests/test_base_external_dbsource.py b/base_external_dbsource/tests/test_base_external_dbsource.py index 57319356d7e..11517a6f1a4 100644 --- a/base_external_dbsource/tests/test_base_external_dbsource.py +++ b/base_external_dbsource/tests/test_base_external_dbsource.py @@ -14,6 +14,22 @@ def setUp(self): super(TestBaseExternalDbsource, self).setUp() self.dbsource = self.env.ref('base_external_dbsource.demo_postgre') + def _test_adapter_method( + self, method_name, side_effect=None, return_value=None, + create=False, *args, **kwargs + ): + if args is None: + args = [] + adapter = '%s_postgresql' % method_name + with mock.patch.object(self.dbsource, + adapter, create=create) as adapter: + if side_effect is not None: + adapter.side_effect = side_effect + elif return_value is not None: + adapter.return_value = return_value + res = getattr(self.dbsource, method_name)(*args, **kwargs) + return res, adapter + def test_conn_string_full(self): """ It should add password if string interpolation not detected """ self.dbsource.conn_string = 'User=Derp;' @@ -32,7 +48,6 @@ def test_connection_success(self): def test_connection_fail(self): """ It should raise for failed/invalid connection """ - # Connection without connection_string with mock.patch.object(self.dbsource, 'connection_open') as conn: conn.side_effect = Exception with self.assertRaises(ConnectionFailedError): @@ -49,12 +64,9 @@ def test_connection_open_calls_close(self): def test_connection_close(self): """ It should call adapter's close method """ - with mock.patch.object( - self.dbsource, 'connection_close_postgresql', autospec=True, - ) as close: - expect = mock.MagicMock() - self.dbsource.connection_close(expect) - close.assert_called_once_with(expect) + args = [mock.MagicMock()] + res, adapter = self._test_adapter_method('connection_close') + adapter.assert_called_once_with(args[0]) def test_execute_asserts_query_arg(self): """ It should raise a TypeError if query and sqlquery not in args """ @@ -64,33 +76,83 @@ def test_execute_asserts_query_arg(self): def test_execute_calls_adapter(self): """ It should call the adapter methods with proper args """ expect = ('query', 'execute', 'metadata') - with mock.patch.object( - self.dbsource, 'execute_postgresql', autospec=True, - ) as psql: - psql.return_value = 'rows', 'cols' - self.dbsource.execute(*expect) - psql.assert_called_once_with(*expect) + return_value = 'rows', 'cols' + res, adapter = self._test_adapter_method( + 'execute', expect, return_value=return_value, + ) + adapter.assert_called_once_with(*expect) def test_execute_return(self): """ It should return rows if not metadata """ - with mock.patch.object( - self.dbsource, 'execute_postgresql', autospec=True, - ) as psql: - psql.return_value = 'rows', 'cols' - res = self.dbsource.execute(True, True, False) - self.assertEqual(res, 'rows') + expect = (True, True, False) + return_value = 'rows', 'cols' + res, adapter = self._test_adapter_method( + 'execute', expect, return_value=return_value, + ) + self.assertEqual(res, return_value[1]) def test_execute_return_metadata(self): """ It should return rows and cols if metadata """ - with mock.patch.object( - self.dbsource, 'execute_postgresql', autospec=True, - ) as psql: - psql.return_value = 'rows', 'cols' - res = self.dbsource.execute(True, True, True) - self.assertDictEqual( - res, - {'rows': 'rows', 'cols': 'cols'}, - ) + expect = (True, True, False) + return_value = 'rows', 'cols' + res, adapter = self._test_adapter_method( + 'execute', expect, return_value=return_value, + ) + self.assertDictEqual( + res, + {'rows': return_value[0], + 'cols': return_value[1]}, + ) + + def test_remote_browse(self): + """ It should call the adapter method with proper args """ + args = [1], 'args' + kwargs = {'kwargs': True} + res, adapter = self._test_adapter_method( + 'remote_browse', create=True, *args, **kwargs + ) + adapter.assert_called_once_with(*args, **kwargs) + self.assertEqual(res, adapter()) + + def test_remote_create(self): + """ It should call the adapter method with proper args """ + args = {'val': 'Value'}, 'args' + kwargs = {'kwargs': True} + res, adapter = self._test_adapter_method( + 'remote_create', create=True, *args, **kwargs + ) + adapter.assert_called_once_with(*args, **kwargs) + self.assertEqual(res, adapter()) + + def test_remote_delete(self): + """ It should call the adapter method with proper args """ + args = [1], 'args' + kwargs = {'kwargs': True} + res, adapter = self._test_adapter_method( + 'remote_delete', create=True, *args, **kwargs + ) + adapter.assert_called_once_with(*args, **kwargs) + self.assertEqual(res, adapter()) + + def test_remote_search(self): + """ It should call the adapter method with proper args """ + args = {'search': 'query'}, 'args' + kwargs = {'kwargs': True} + res, adapter = self._test_adapter_method( + 'remote_search', create=True, *args, **kwargs + ) + adapter.assert_called_once_with(*args, **kwargs) + self.assertEqual(res, adapter()) + + def test_remote_update(self): + """ It should call the adapter method with proper args """ + args = [1], {'vals': 'Value'}, 'args' + kwargs = {'kwargs': True} + res, adapter = self._test_adapter_method( + 'remote_update', create=True, *args, **kwargs + ) + adapter.assert_called_once_with(*args, **kwargs) + self.assertEqual(res, adapter()) # Postgres