ISSUE #367

* Improve `DatabaseClient` class.

Author: Sergio García Prado

packages/core/minos-microservice-common/minos/common/database/clients/abc.py

@@ -33,28 +33,41 @@ class DatabaseClient(ABC, BuildableMixin):
     def _from_config(cls, config: Config, name: Optional[str] = None, **kwargs) -> DatabaseClient:
         return super()._from_config(config, **config.get_database_by_name(name), **kwargs)
 
-    @abstractmethod
     async def is_valid(self, **kwargs) -> bool:
         """Check if the instance is valid.
 
         :return: ``True`` if it is valid or ``False`` otherwise.
         """
+        return await self._is_valid(**kwargs)
+
+    @abstractmethod
+    async def _is_valid(self, **kwargs) -> bool:
+        raise NotImplementedError
 
     async def reset(self, **kwargs) -> None:
         """Reset the current instance status.
 
         :param kwargs: Additional named parameters.
         :return: This method does not return anything.
         """
+        return await self._reset(**kwargs)
 
     @abstractmethod
+    async def _reset(self, **kwargs) -> None:
+        raise NotImplementedError
+
     async def execute(self, *args, **kwargs) -> None:
         """Execute an operation.
 
         :param args: Additional positional arguments.
         :param kwargs: Additional named arguments.
         :return: This method does not return anything.
         """
+        await self._execute(*args, **kwargs)
+
+    @abstractmethod
+    async def _execute(self, *args, **kwargs) -> None:
+        raise NotImplementedError
 
     async def fetch_one(self, *args, **kwargs) -> Any:
         """Fetch one value.
@@ -65,22 +78,30 @@ async def fetch_one(self, *args, **kwargs) -> Any:
         """
         return await self.fetch_all(*args, **kwargs).__anext__()
 
-    @abstractmethod
     def fetch_all(self, *args, **kwargs) -> AsyncIterator[Any]:
         """Fetch all values with an asynchronous iterator.
 
-        :param args: Additional positional arguments.
         :param kwargs: Additional named arguments.
         :return: This method does not return anything.
         """
+        return self._fetch_all(*args, **kwargs)
 
-    @property
     @abstractmethod
+    def _fetch_all(self, *args, **kwargs) -> AsyncIterator[Any]:
+        raise NotImplementedError
+
+    @property
     def notifications(self) -> Queue:
         """Get the notifications queue.
 
         :return: A ``Queue`` instance.
         """
+        return self._notifications
+
+    @property
+    @abstractmethod
+    def _notifications(self) -> Queue:
+        raise NotImplementedError
 
 
 class DatabaseClientBuilder(Builder[DatabaseClient]):

packages/core/minos-microservice-common/minos/common/database/clients/aiopg.py

@@ -108,11 +108,7 @@ async def _close_connection(self):
         self._connection = None
         logger.debug(f"Destroyed {self.database!r} database connection identified by {id(self._connection)}!")
 
-    async def is_valid(self) -> bool:
-        """Check if the instance is valid.
-
-        :return: ``True`` if it is valid or ``False`` otherwise.
-        """
+    async def _is_valid(self) -> bool:
         if self._connection is None:
             return False
 
@@ -124,49 +120,26 @@ async def is_valid(self) -> bool:
 
         return not self._connection.closed
 
-    async def reset(self, **kwargs) -> None:
-        """Reset the current instance status.
-
-        :param kwargs: Additional named parameters.
-        :return: This method does not return anything.
-        """
+    async def _reset(self, **kwargs) -> None:
         await self._destroy_cursor(**kwargs)
 
     # noinspection PyUnusedLocal
-    async def fetch_all(
+    async def _fetch_all(
         self,
         *args,
         timeout: Optional[float] = None,
         lock: Optional[int] = None,
         **kwargs,
     ) -> AsyncIterator[tuple]:
-        """Fetch all values with an asynchronous iterator.
-
-        :param timeout: An optional timeout.
-        :param lock: Optional key to perform the query with locking. If not set, the query is performed without any
-            lock.
-        :param kwargs: Additional named arguments.
-        :return: This method does not return anything.
-        """
         await self._create_cursor()
 
         async for row in self._cursor:
             yield row
 
     # noinspection PyUnusedLocal
-    async def execute(
+    async def _execute(
         self, operation: Any, parameters: Any = None, *, timeout: Optional[float] = None, lock: Any = None, **kwargs
     ) -> None:
-        """Execute an operation.
-
-        :param operation: Query to be executed.
-        :param parameters: Parameters to be projected into the query.
-        :param timeout: An optional timeout.
-        :param lock: Optional key to perform the query with locking. If not set, the query is performed without any
-            lock.
-        :param kwargs: Additional named arguments.
-        :return: This method does not return anything.
-        """
         await self._create_cursor(lock=lock)
         try:
             await self._cursor.execute(operation=operation, parameters=parameters, timeout=timeout)
@@ -216,11 +189,7 @@ def cursor(self) -> Optional[Cursor]:
         return self._cursor
 
     @property
-    def notifications(self) -> ClosableQueue:
-        """Get the notifications queue.
-
-        :return: A ``ClosableQueue`` instance.
-        """
+    def _notifications(self) -> ClosableQueue:
         return self._connection.notifies
 
     @property

packages/core/minos-microservice-common/tests/test_common/test_database/test_clients/test_abc.py

@@ -2,6 +2,20 @@
 from abc import (
     ABC,
 )
+from asyncio import (
+    Queue,
+)
+from typing import (
+    Any,
+    AsyncIterator,
+)
+from unittest.mock import (
+    AsyncMock,
+    MagicMock,
+    PropertyMock,
+    call,
+    patch,
+)
 
 from minos.common import (
     AiopgDatabaseClient,
@@ -11,18 +25,95 @@
 )
 from tests.utils import (
     CommonTestCase,
+    FakeAsyncIterator,
 )
 
 
+class _DatabaseClient(DatabaseClient):
+    """For testing purposes."""
+
+    async def _is_valid(self, **kwargs) -> bool:
+        """For testing purposes."""
+
+    async def _reset(self, **kwargs) -> None:
+        """For testing purposes."""
+
+    async def _execute(self, *args, **kwargs) -> None:
+        """For testing purposes."""
+
+    def _fetch_all(self, *args, **kwargs) -> AsyncIterator[Any]:
+        """For testing purposes."""
+
+    # noinspection PyPropertyDefinition
+    @property
+    def _notifications(self) -> Queue:
+        """For testing purposes."""
+
+
 class TestDatabaseClient(unittest.IsolatedAsyncioTestCase):
     def test_abstract(self):
         self.assertTrue(issubclass(DatabaseClient, (ABC, BuildableMixin)))
+        expected = {"_notifications", "_is_valid", "_execute", "_fetch_all", "_reset"}
         # noinspection PyUnresolvedReferences
-        self.assertEqual({"notifications", "is_valid", "execute", "fetch_all"}, DatabaseClient.__abstractmethods__)
+        self.assertEqual(expected, DatabaseClient.__abstractmethods__)
 
     def test_get_builder(self):
         self.assertIsInstance(DatabaseClient.get_builder(), DatabaseClientBuilder)
 
+    async def test_is_valid(self):
+        mock = AsyncMock(side_effect=[True, False])
+        client = _DatabaseClient()
+        client._is_valid = mock
+
+        self.assertEqual(True, await client.is_valid())
+        self.assertEqual(False, await client.is_valid())
+
+        self.assertEqual([call(), call()], mock.call_args_list)
+
+    async def test_reset(self):
+        mock = AsyncMock()
+        client = _DatabaseClient()
+        client._reset = mock
+
+        await client.reset()
+
+        self.assertEqual([call()], mock.call_args_list)
+
+    async def test_execute(self):
+        mock = AsyncMock()
+        client = _DatabaseClient()
+        client._execute = mock
+
+        await client.execute("foo")
+
+        self.assertEqual([call("foo")], mock.call_args_list)
+
+    async def test_fetch_all(self):
+        mock = MagicMock(return_value=FakeAsyncIterator(["one", "two"]))
+        client = _DatabaseClient()
+        client._fetch_all = mock
+
+        self.assertEqual(["one", "two"], [v async for v in client.fetch_all()])
+
+        self.assertEqual([call()], mock.call_args_list)
+
+    async def test_fetch_one(self):
+        mock = MagicMock(return_value=FakeAsyncIterator(["one", "two"]))
+        client = _DatabaseClient()
+        client._fetch_all = mock
+
+        self.assertEqual("one", await client.fetch_one())
+
+        self.assertEqual([call()], mock.call_args_list)
+
+    async def test_notifications(self):
+        expected = Queue()
+        client = _DatabaseClient()
+        with patch.object(_DatabaseClient, "_notifications", new_callable=PropertyMock, return_value=expected) as mock:
+            self.assertEqual(expected, client.notifications)
+
+        self.assertEqual([call()], mock.call_args_list)
+
 
 class TestDatabaseClientBuilder(CommonTestCase):
     def test_with_name(self):