Merge pull request #277 from minos-framework/issue-276-ref-resolve-bug

#276 - Fix `Ref.resolve` bug

Author: Sergio García Prado

packages/core/minos-microservice-aggregate/minos/aggregate/entities/refs/models.py

@@ -16,11 +16,6 @@
     SafeUUID,
 )
 
-from dependency_injector.wiring import (
-    Provide,
-    inject,
-)
-
 from minos.common import (
     DataDecoder,
     DataEncoder,
@@ -30,19 +25,10 @@
     SchemaDecoder,
     SchemaEncoder,
 )
-from minos.networks import (
-    BrokerClient,
-    BrokerClientPool,
-    BrokerMessageV1,
-    BrokerMessageV1Payload,
-)
 
 from ...contextvars import (
     IS_REPOSITORY_SERIALIZATION_CONTEXT_VAR,
 )
-from ...exceptions import (
-    RefException,
-)
 
 logger = logging.getLogger(__name__)
 MT = TypeVar("MT", bound=Model)
@@ -53,14 +39,11 @@ class Ref(DeclarativeModel, UUID, Generic[MT]):
 
     data: Union[MT, UUID]
 
-    @inject
-    def __init__(self, data: Union[MT, UUID], *args, broker_pool: BrokerClientPool = Provide["broker_pool"], **kwargs):
+    def __init__(self, data: Union[MT, UUID], *args, **kwargs):
         if not isinstance(data, UUID) and not hasattr(data, "uuid"):
             raise ValueError(f"data must be an {UUID!r} instance or have 'uuid' as one of its fields")
         DeclarativeModel.__init__(self, data, *args, **kwargs)
 
-        self._broker_pool = broker_pool
-
     def __setitem__(self, key: str, value: Any) -> None:
         try:
             return super().__setitem__(key, value)
@@ -81,6 +64,9 @@ def __getitem__(self, item: str) -> Any:
             if item == "uuid":
                 return self.uuid
 
+            if not self.resolved:
+                raise KeyError(f"The reference is not resolved: {self!r}")
+
             try:
                 return self.data[item]
             except Exception:
@@ -106,6 +92,9 @@ def __getattr__(self, item: str) -> Any:
             if item == "data":
                 raise exc
 
+            if not self.resolved:
+                raise AttributeError(f"The reference is not resolved: {self!r}")
+
             try:
                 return getattr(self.data, item)
             except Exception:
@@ -223,24 +212,22 @@ async def resolve(self, force: bool = False, **kwargs) -> None:
         if not force and self.resolved:
             return
 
-        name = self.data_cls.__name__
-
-        message = BrokerMessageV1(f"Get{name}", BrokerMessageV1Payload({"uuid": self.uuid}))
-        async with self._broker_pool.acquire() as broker:
-            await broker.send(message)
-            self.data = await self._get_response(broker)
+        from .resolvers import (
+            RefResolver,
+        )
 
-    @staticmethod
-    async def _get_response(broker: BrokerClient, **kwargs) -> MT:
-        message = await broker.receive(**kwargs)
-        if not message.ok:
-            raise RefException(f"The received message is not ok: {message!r}")
-        return message.content
+        self.data = await RefResolver(**kwargs).resolve(self)
 
     @property
     def resolved(self) -> bool:
         """Check if the instance is already resolved.
 
         :return: ``True`` if resolved or ``False`` otherwise.
         """
-        return not isinstance(self.data, UUID)
+        try:
+            return not isinstance(self.data, UUID)
+        except AttributeError:
+            return False
+
+    def __repr__(self) -> str:
+        return f"{type(self).__name__}({self.data!r})"

packages/core/minos-microservice-aggregate/minos/aggregate/entities/refs/resolvers.py

@@ -10,6 +10,7 @@
 )
 from typing import (
     Any,
+    Union,
 )
 from uuid import (
     UUID,
@@ -69,13 +70,12 @@ async def resolve(self, data: Any, **kwargs) -> Any:
         return RefInjector(data, recovered).build()
 
     async def _query(self, references: dict[str, set[UUID]]) -> dict[UUID, Model]:
+        messages = (
+            BrokerMessageV1(self.build_topic_name(name), BrokerMessageV1Payload({"uuids": uuids}))
+            for name, uuids in references.items()
+        )
         async with self.broker_pool.acquire() as broker:
-            futures = (
-                broker.send(
-                    BrokerMessageV1(f"_Get{simplified_name}Snapshots", BrokerMessageV1Payload({"uuids": uuids}))
-                )
-                for simplified_name, uuids in references.items()
-            )
+            futures = (broker.send(message) for message in messages)
             await gather(*futures)
 
             return {model.uuid: model for model in await self._get_response(broker, len(references))}
@@ -89,3 +89,14 @@ async def _get_response(broker: BrokerClient, count: int, **kwargs) -> Iterable[
             messages.append(message)
 
         return chain(*(message.content for message in messages))
+
+    @staticmethod
+    def build_topic_name(entity: Union[type, str]) -> str:
+        """Build the topic name based on the name of the entity.
+
+        :param entity: The name of the entity to be resolved.
+        :return: The topic name.
+        """
+        if isinstance(entity, type):
+            entity = entity.__name__
+        return f"_Get{entity}Snapshots"

packages/core/minos-microservice-aggregate/minos/aggregate/snapshots/services.py

@@ -63,31 +63,16 @@ def __init__(
 
     @classmethod
     def __get_enroute__(cls, config: MinosConfig) -> dict[str, set[EnrouteDecorator]]:
-        simplified_name = config.service.aggregate.rsplit(".", 1)[-1]
+        from ..entities import (
+            RefResolver,
+        )
+
+        name = config.service.aggregate.rsplit(".", 1)[-1]
         return {
-            cls.__get_one__.__name__: {enroute.broker.command(f"_Get{simplified_name}Snapshot")},
-            cls.__get_many__.__name__: {enroute.broker.command(f"_Get{simplified_name}Snapshots")},
+            cls.__get_many__.__name__: {enroute.broker.command(RefResolver.build_topic_name(name))},
             cls.__synchronize__.__name__: {enroute.periodic.event("* * * * *")},
         }
 
-    async def __get_one__(self, request: Request) -> Response:
-        """Get one ``RootEntity`` instance.
-
-        :param request: The ``Request`` instance that contains the instance identifier.
-        :return: A ``Response`` instance containing the requested instances.
-        """
-        try:
-            content = await request.content(model_type=ModelType.build("Query", {"uuid": UUID}))
-        except Exception as exc:
-            raise ResponseException(f"There was a problem while parsing the given request: {exc!r}")
-
-        try:
-            instance = await self.type_.get(content["uuid"])
-        except Exception as exc:
-            raise ResponseException(f"There was a problem while getting the instance: {exc!r}")
-
-        return Response(instance)
-
     async def __get_many__(self, request: Request) -> Response:
         """Get many ``RootEntity`` instances.
 

packages/core/minos-microservice-aggregate/tests/test_aggregate/test_entities/test_refs/test_models.py

@@ -4,6 +4,9 @@
     Generic,
     Union,
 )
+from unittest.mock import (
+    patch,
+)
 from uuid import (
     UUID,
     uuid4,
@@ -13,18 +16,14 @@
     IS_REPOSITORY_SERIALIZATION_CONTEXT_VAR,
     Ref,
     RefException,
+    RefResolver,
 )
 from minos.common import (
     DeclarativeModel,
     Field,
     Model,
     ModelType,
 )
-from minos.networks import (
-    BrokerMessageV1,
-    BrokerMessageV1Payload,
-    BrokerMessageV1Status,
-)
 from tests.utils import (
     MinosTestCase,
 )
@@ -69,6 +68,13 @@ def test_uuid_getattr(self):
 
         self.assertEqual(uuid, value.uuid)
 
+    def test_uuid_getattr_raises(self):
+        uuid = uuid4()
+        value = Ref(uuid)
+
+        with self.assertRaises(AttributeError):
+            value.color
+
     def test_uuid_setattr(self):
         uuid_1 = uuid4()
         uuid_2 = uuid4()
@@ -84,11 +90,12 @@ def test_uuid_getitem(self):
 
         self.assertEqual(uuid, value["uuid"])
 
-    def test_model_getitem_raises(self):
-        value = Ref(Bar(uuid4(), 1))
+    def test_uuid_getitem_raises(self):
+        uuid = uuid4()
+        value = Ref(uuid)
 
         with self.assertRaises(KeyError):
-            value["year"]
+            value["color"]
 
     def test_uuid_setitem(self):
         uuid_1 = uuid4()
@@ -149,6 +156,12 @@ def test_model_getitem(self):
 
         self.assertEqual(1, value["age"])
 
+    def test_model_getitem_raises(self):
+        value = Ref(Bar(uuid4(), 1))
+
+        with self.assertRaises(KeyError):
+            value["year"]
+
     def test_model_setitem(self):
         value = Ref(Bar(uuid4(), 1))
 
@@ -285,34 +298,25 @@ def test_uuid_from_avro(self):
 
     async def test_resolve(self):
         another = uuid4()
-
-        self.broker_subscriber_builder.with_messages([BrokerMessageV1("", BrokerMessageV1Payload(Bar(another, 1)))])
+        resolved_another = Bar(another, 1)
 
         ref = Foo(another).another  # FIXME: This should not be needed to set the type hint properly
 
         self.assertEqual(ref.data, another)
 
-        await ref.resolve()
-
-        observed = self.broker_publisher.messages
-        self.assertEqual(1, len(observed))
+        with patch.object(RefResolver, "resolve", return_value=resolved_another):
+            await ref.resolve()
 
-        self.assertIsInstance(observed[0], BrokerMessageV1)
-        self.assertEqual("GetBar", observed[0].topic)
-        self.assertEqual({"uuid": another}, observed[0].content)
         self.assertEqual(ref.data, Bar(another, 1))
 
     async def test_resolve_raises(self):
         another = uuid4()
 
-        self.broker_subscriber_builder.with_messages(
-            [BrokerMessageV1("", BrokerMessageV1Payload(status=BrokerMessageV1Status.ERROR))]
-        )
-
         ref = Foo(another).another  # FIXME: This should not be needed to set the type hint properly
 
         with self.assertRaises(RefException):
-            await ref.resolve()
+            with patch.object(RefResolver, "resolve", side_effect=RefException("test")):
+                await ref.resolve()
 
     async def test_resolve_already(self):
         uuid = uuid4()
@@ -340,6 +344,32 @@ def test_avro_uuid(self):
 
         self.assertEqual(ref, Ref.from_avro_bytes(ref.avro_bytes))
 
+    def test_repr_uuid(self):
+        another_uuid = uuid4()
+        ref = Foo(another_uuid).another  # FIXME: This should not be needed to set the type hint properly
+
+        self.assertEqual(f"Ref({another_uuid!r})", repr(ref))
+
+    def test_repr_model(self):
+        another_uuid = uuid4()
+        another = Bar(another_uuid, 1)
+        ref = Foo(another).another  # FIXME: This should not be needed to set the type hint properly
+
+        self.assertEqual(f"Ref({another!r})", repr(ref))
+
+    def test_str_uuid(self):
+        another_uuid = uuid4()
+        ref = Foo(another_uuid).another  # FIXME: This should not be needed to set the type hint properly
+
+        self.assertEqual(str(another_uuid), str(ref))
+
+    def test_str_model(self):
+        another_uuid = uuid4()
+        another = Bar(another_uuid, 1)
+        ref = Foo(another).another  # FIXME: This should not be needed to set the type hint properly
+
+        self.assertEqual(str(another_uuid), str(ref))
+
 
 if __name__ == "__main__":
     unittest.main()

packages/core/minos-microservice-aggregate/tests/test_aggregate/test_entities/test_refs/test_resolvers.py

@@ -62,6 +62,18 @@ async def test_resolve_raises(self):
         with self.assertRaises(RefException):
             await self.resolver.resolve(self.value)
 
+    def test_build_topic_name_str(self):
+        expected = "_GetBarSnapshots"
+        observed = RefResolver.build_topic_name("Bar")
+
+        self.assertEqual(observed, expected)
+
+    def test_build_topic_name_type(self):
+        expected = "_GetBarSnapshots"
+        observed = RefResolver.build_topic_name(Bar)
+
+        self.assertEqual(observed, expected)
+
 
 if __name__ == "__main__":
     unittest.main()

packages/core/minos-microservice-aggregate/tests/test_aggregate/test_snapshots/test_services.py

@@ -9,6 +9,7 @@
 )
 
 from minos.aggregate import (
+    RefResolver,
     RootEntity,
     SnapshotService,
 )
@@ -43,27 +44,12 @@ def setUp(self) -> None:
 
     def test_get_enroute(self):
         expected = {
-            "__get_one__": {BrokerCommandEnrouteDecorator("_GetOrderSnapshot")},
-            "__get_many__": {BrokerCommandEnrouteDecorator("_GetOrderSnapshots")},
+            "__get_many__": {BrokerCommandEnrouteDecorator(RefResolver.build_topic_name("Order"))},
             "__synchronize__": {PeriodicEventEnrouteDecorator("* * * * *")},
         }
         observed = SnapshotService.__get_enroute__(self.config)
         self.assertEqual(expected, observed)
 
-    async def test_get(self):
-        uuid = uuid4()
-        expected = Agg(uuid)
-        with patch.object(RootEntity, "get", return_value=expected):
-            response = await self.service.__get_one__(InMemoryRequest({"uuid": uuid}))
-        self.assertEqual(expected, await response.content())
-
-    async def test_get_raises(self):
-        with self.assertRaises(ResponseException):
-            await self.service.__get_one__(InMemoryRequest())
-        with patch.object(RootEntity, "get", side_effect=ValueError):
-            with self.assertRaises(ResponseException):
-                await self.service.__get_one__(InMemoryRequest({"uuid": uuid4()}))
-
     async def test_get_many(self):
         uuids = [uuid4(), uuid4()]