ISSUE #367

* Increase coverage up to "100%".
* Improve `Lock` API.

Author: Sergio García Prado

packages/core/minos-microservice-aggregate/tests/utils.py

@@ -101,8 +101,11 @@ def __init__(self, key=None, *args, **kwargs):
             key = "fake"
         super().__init__(key, *args, **kwargs)
 
-    async def __aexit__(self, exc_type, exc_val, exc_tb):
-        return
+    async def acquire(self) -> None:
+        """For testing purposes."""
+
+    async def release(self):
+        """For testing purposes."""
 
 
 class FakeLockPool(LockPool):

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

@@ -1,10 +1,12 @@
 from .abc import (
     DatabaseClient,
     DatabaseClientBuilder,
-    DatabaseClientException,
-    IntegrityException,
-    UnableToConnectException,
 )
 from .aiopg import (
     AiopgDatabaseClient,
 )
+from .exceptions import (
+    DatabaseClientException,
+    IntegrityException,
+    UnableToConnectException,
+)

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

@@ -24,68 +24,86 @@
 from ...config import (
     Config,
 )
-from ...exceptions import (
-    MinosException,
-)
 
 
 class DatabaseClient(ABC, BuildableMixin):
-    """TODO"""
+    """Database Client base class."""
 
     @classmethod
     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:
-        """TODO"""
+        """Check if the instance is valid.
+
+        :return: ``True`` if it is valid or ``False`` otherwise.
+        """
 
     async def reset(self, **kwargs) -> None:
-        """TODO"""
+        """Reset the current instance status.
 
-    @property
-    @abstractmethod
-    def notifications(self) -> Queue:
-        """TODO"""
+        :param kwargs: Additional named parameters.
+        :return: This method does not return anything.
+        """
 
     @abstractmethod
     async def execute(self, *args, **kwargs) -> None:
-        """TODO"""
+        """Execute an operation.
+
+        :param args: Additional positional arguments.
+        :param kwargs: Additional named arguments.
+        :return: This method does not return anything.
+        """
 
     async def fetch_one(self, *args, **kwargs) -> Any:
-        """TODO"""
+        """Fetch one value.
+
+        :param args: Additional positional arguments.
+        :param kwargs: Additional named arguments.
+        :return: This method does not return anything.
+        """
         return await self.fetch_all(*args, **kwargs).__anext__()
 
     @abstractmethod
     def fetch_all(self, *args, **kwargs) -> AsyncIterator[Any]:
-        """TODO"""
+        """Fetch all values with an asynchronous iterator.
+
+        :param args: Additional positional arguments.
+        :param kwargs: Additional named arguments.
+        :return: This method does not return anything.
+        """
+
+    @property
+    @abstractmethod
+    def notifications(self) -> Queue:
+        """Get the notifications queue.
+
+        :return: A ``Queue`` instance.
+        """
 
 
 class DatabaseClientBuilder(Builder[DatabaseClient]):
-    """TODO"""
+    """Database Client Builder class."""
 
     def with_name(self, name: str) -> DatabaseClientBuilder:
-        """TODO"""
+        """Set name.
+
+        :param name: The name to be added.
+        :return: This method return the builder instance.
+        """
         self.kwargs["name"] = name
         return self
 
     def with_config(self, config: Config) -> DatabaseClientBuilder:
-        """TODO"""
+        """Set config.
+
+        :param config: The config to be set.
+        :return: This method return the builder instance.
+        """
         database_config = config.get_database_by_name(self.kwargs.get("name"))
         self.kwargs |= database_config
         return self
 
 
 DatabaseClient.set_builder(DatabaseClientBuilder)
-
-
-class DatabaseClientException(MinosException):
-    """TODO"""
-
-
-class UnableToConnectException(DatabaseClientException):
-    """TODO"""
-
-
-class IntegrityException(DatabaseClientException):
-    """TODO"""

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

@@ -28,6 +28,8 @@
 
 from .abc import (
     DatabaseClient,
+)
+from .exceptions import (
     IntegrityException,
     UnableToConnectException,
 )
@@ -138,7 +140,7 @@ async def fetch_all(
         lock: Optional[int] = None,
         **kwargs,
     ) -> AsyncIterator[tuple]:
-        """Submit a SQL query and return an asynchronous iterator.
+        """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
@@ -155,7 +157,7 @@ async def fetch_all(
     async def execute(
         self, operation: Any, parameters: Any = None, *, timeout: Optional[float] = None, lock: Any = None, **kwargs
     ) -> None:
-        """Submit a SQL query.
+        """Execute an operation.
 
         :param operation: Query to be executed.
         :param parameters: Parameters to be projected into the query.
@@ -185,11 +187,11 @@ async def _create_cursor(self, *args, lock: Optional[Hashable] = None, **kwargs)
             )
 
             self._lock = DatabaseLock(self, lock, *args, **kwargs)
-            await self._lock.__aenter__()
+            await self._lock.acquire()
 
     async def _destroy_cursor(self, **kwargs):
         if self._lock is not None:
-            await self._lock.__aexit__(None, None, None)
+            await self._lock.release()
             self._lock = None
 
         if self._cursor is not None:

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

@@ -0,0 +1,15 @@
+from ...exceptions import (
+    MinosException,
+)
+
+
+class DatabaseClientException(MinosException):
+    """Base exception for database client."""
+
+
+class UnableToConnectException(DatabaseClientException):
+    """Exception to be raised when database client is not able to connect to the database."""
+
+
+class IntegrityException(DatabaseClientException):
+    """Exception to be raised when an integrity check is not satisfied."""

packages/core/minos-microservice-common/minos/common/database/locks.py

@@ -19,11 +19,18 @@ def __init__(self, client: DatabaseClient, key: Hashable, *args, **kwargs):
 
         self.client = client
 
-    async def __aenter__(self):
+    async def acquire(self) -> None:
+        """Acquire the lock.
+
+        :return: This method does not return anything.
+        """
         await self.client.execute("select pg_advisory_lock(%(hashed_key)s)", {"hashed_key": self.hashed_key})
-        return self
 
-    async def __aexit__(self, exc_type, exc_val, exc_tb):
+    async def release(self) -> None:
+        """Release the lock.
+
+        :return: This method does not return anything.
+        """
         await self.client.execute("select pg_advisory_unlock(%(hashed_key)s)", {"hashed_key": self.hashed_key})
 
 

packages/core/minos-microservice-common/minos/common/database/pools.py

@@ -62,7 +62,7 @@ async def _create_instance(self) -> Optional[DatabaseClient]:
         try:
             await instance.setup()
         except UnableToConnectException:
-            await sleep(1)
+            await sleep(0.1)
             return None
 
         logger.info(f"Created {instance!r}!")

packages/core/minos-microservice-common/minos/common/locks.py

@@ -4,13 +4,11 @@
 
 from abc import (
     ABC,
+    abstractmethod,
 )
 from collections.abc import (
     Hashable,
 )
-from contextlib import (
-    AbstractAsyncContextManager,
-)
 
 from cached_property import (
     cached_property,
@@ -24,7 +22,7 @@
 )
 
 
-class Lock(AbstractAsyncContextManager):
+class Lock(ABC):
     """Lock base class."""
 
     key: Hashable
@@ -35,6 +33,27 @@ def __init__(self, key: Hashable, *args, **kwargs):
 
         self.key = key
 
+    async def __aenter__(self) -> Lock:
+        await self.acquire()
+        return self
+
+    async def __aexit__(self, exc_type, exc_val, exc_tb):
+        await self.release()
+
+    @abstractmethod
+    async def acquire(self) -> None:
+        """Acquire the lock.
+
+        :return: This method does not return anything.
+        """
+
+    @abstractmethod
+    async def release(self):
+        """Release the lock.
+
+        :return: This method does not return anything.
+        """
+
     @cached_property
     def hashed_key(self) -> int:
         """Get the hashed key.

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

@@ -0,0 +1,45 @@
+import unittest
+from abc import (
+    ABC,
+)
+
+from minos.common import (
+    AiopgDatabaseClient,
+    BuildableMixin,
+    DatabaseClient,
+    DatabaseClientBuilder,
+)
+from tests.utils import (
+    CommonTestCase,
+)
+
+
+class TestDatabaseClient(unittest.IsolatedAsyncioTestCase):
+    def test_abstract(self):
+        self.assertTrue(issubclass(DatabaseClient, (ABC, BuildableMixin)))
+        # noinspection PyUnresolvedReferences
+        self.assertEqual({"notifications", "is_valid", "execute", "fetch_all"}, DatabaseClient.__abstractmethods__)
+
+    def test_get_builder(self):
+        self.assertIsInstance(DatabaseClient.get_builder(), DatabaseClientBuilder)
+
+
+class TestDatabaseClientBuilder(CommonTestCase):
+    def test_with_name(self):
+        builder = DatabaseClientBuilder(AiopgDatabaseClient).with_name("query")
+        self.assertEqual({"name": "query"}, builder.kwargs)
+
+    def test_with_config(self):
+        builder = DatabaseClientBuilder(AiopgDatabaseClient).with_name("query").with_config(self.config)
+        self.assertEqual({"name": "query"} | self.config.get_database_by_name("query"), builder.kwargs)
+
+    def test_build(self):
+        builder = DatabaseClientBuilder(AiopgDatabaseClient).with_name("query").with_config(self.config)
+        client = builder.build()
+
+        self.assertIsInstance(client, AiopgDatabaseClient)
+        self.assertEqual(self.config.get_database_by_name("query")["database"], client.database)
+
+
+if __name__ == "__main__":
+    unittest.main()

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

@@ -54,7 +54,7 @@ def test_from_config(self):
 
     async def test_is_valid_true(self):
         async with AiopgDatabaseClient.from_config(self.config) as client:
-            self.assertTrue(client.is_valid())
+            self.assertTrue(await client.is_valid())
 
     async def test_is_valid_false_not_setup(self):
         client = AiopgDatabaseClient.from_config(self.config)
@@ -132,8 +132,8 @@ async def test_execute(self):
         )
 
     async def test_execute_with_lock(self):
-        with patch.object(DatabaseLock, "__aenter__") as enter_lock_mock:
-            with patch.object(DatabaseLock, "__aexit__") as exit_lock_mock:
+        with patch.object(DatabaseLock, "acquire") as enter_lock_mock:
+            with patch.object(DatabaseLock, "release") as exit_lock_mock:
                 async with AiopgDatabaseClient.from_config(self.config) as client:
                     await client.execute(self.sql, lock="foo")
                     self.assertEqual(1, enter_lock_mock.call_count)