Merge pull request #308 from minos-framework/issue-150-refactor-broker-builder-classes

#150 - Improve `BrokerSubscriberBuilder`

Author: Sergio García Prado

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

@@ -4,12 +4,14 @@
 
 from abc import (
     ABC,
-    abstractmethod,
 )
 from typing import (
     Any,
     Generic,
+    Optional,
     TypeVar,
+    Union,
+    get_args,
 )
 
 from .config import (
@@ -25,25 +27,50 @@
 class Builder(SetupMixin, ABC, Generic[Instance]):
     """Builder class."""
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, instance_cls: Optional[type[Instance]] = None, *args, **kwargs):
+
         super().__init__(*args, **kwargs)
+        if instance_cls is None:
+            instance_cls = self._get_cls()
+
         self.kwargs = dict()
+        self.instance_cls = instance_cls
+
+    def _get_cls(self) -> Optional[type]:
+        # noinspection PyUnresolvedReferences
+        bases = self.__orig_bases__
+
+        instance_cls = get_args(next((base for base in bases if len(get_args(base))), None))[0]
+
+        if not isinstance(instance_cls, type):
+            return None
+
+        return instance_cls
 
     def copy(self: type[B]) -> B:
         """Get a copy of the instance.
 
-        :return: A ``BrokerSubscriberBuilder`` instance.
+        :return: A ``Builder`` instance.
         """
-        return self.new().with_kwargs(self.kwargs)
+        return self.new().with_cls(self.instance_cls).with_kwargs(self.kwargs)
 
     @classmethod
     def new(cls: type[B]) -> B:
         """Get a new instance.
 
-        :return: A ``BrokerSubscriberBuilder`` instance.
+        :return: A ``Builder`` instance.
         """
         return cls()
 
+    def with_cls(self: B, cls: type) -> B:
+        """Set class to be built.
+
+        :param cls: The class to be set.
+        :return: This method return the builder instance.
+        """
+        self.instance_cls = cls
+        return self
+
     def with_kwargs(self: B, kwargs: dict[str, Any]) -> B:
         """Set kwargs.
 
@@ -62,12 +89,18 @@ def with_config(self: B, config: Config) -> B:
         """
         return self
 
-    @abstractmethod
     def build(self) -> Instance:
         """Build the instance.
 
-        :return: A ``BrokerSubscriber`` instance.
+        :return: A ``Instance`` instance.
         """
+        return self.instance_cls(**self.kwargs)
+
+    def __eq__(self, other: Any) -> bool:
+        return isinstance(other, type(self)) and self.instance_cls == other.instance_cls and self.kwargs == other.kwargs
+
+    def __repr__(self) -> str:
+        return f"{type(self).__name__}({self.instance_cls.__name__}, {self.kwargs!r})"
 
 
 Ins = TypeVar("Ins", bound="BuildableMixin")
@@ -76,28 +109,38 @@ def build(self) -> Instance:
 class BuildableMixin(SetupMixin):
     """Buildable Mixin class."""
 
-    _builder_cls: type[Builder[Ins]]
+    _builder: Union[Builder[Ins], type[Builder[Ins]]] = Builder
 
     @classmethod
-    def _from_config(cls: type[Ins], config: Config, **kwargs) -> Ins:
-        return cls.get_builder().new().with_config(config).with_kwargs(kwargs).build()
+    def _from_config(cls, config: Config, **kwargs):
+        return cls.get_builder().with_config(config).with_kwargs(kwargs).build()
 
     @classmethod
-    def set_builder(cls: type[Ins], builder: type[Builder[Ins]]) -> None:
+    def set_builder(cls: type[Ins], builder: Union[Builder[Ins], type[Builder[Ins]]]) -> None:
         """Set a builder class.
 
         :param builder: The builder class to be set.
         :return: This method does not return anything.
         """
-        cls._builder_cls = builder
+        if not isinstance(builder, Builder) and not (isinstance(builder, type) and issubclass(builder, Builder)):
+            raise ValueError(f"Given builder value is invalid: {builder!r}")
+
+        cls._builder = builder
 
     @classmethod
-    def get_builder(cls) -> type[Builder[Ins]]:
+    def get_builder(cls) -> Builder[Ins]:
         """Get the builder class.
 
-        :return: A ``Builder`` subclass.
+        :return: A ``Builder`` instance.
         """
-        return cls._builder_cls
+        builder = cls._builder
+
+        if isinstance(builder, Builder):
+            builder = builder.copy()
+        else:
+            builder = builder.new()
+
+        return builder.with_cls(cls)
 
 
 B = TypeVar("B", bound=Builder)

packages/core/minos-microservice-common/minos/common/config/v2.py

@@ -11,6 +11,7 @@
 from typing import (
     TYPE_CHECKING,
     Any,
+    Union,
 )
 
 from ..exceptions import (
@@ -39,7 +40,7 @@ def _version(self) -> int:
     def _get_name(self) -> str:
         return self.get_by_key("name")
 
-    def _get_injections(self) -> list[type[InjectableMixin]]:
+    def _get_injections(self) -> list[Union[InjectableMixin, type[InjectableMixin]]]:
         from ..builders import (
             BuildableMixin,
         )
@@ -82,15 +83,15 @@ def _get_injections(self) -> list[type[InjectableMixin]]:
             if (
                 not issubclass(type_, InjectableMixin)
                 and issubclass(type_, BuildableMixin)
-                and issubclass((builder_type := type_.get_builder()), InjectableMixin)
+                and isinstance((builder_type := type_.get_builder()), InjectableMixin)
             ):
                 type_ = builder_type
-
-            if not issubclass(type_, InjectableMixin):
+            elif not issubclass(type_, InjectableMixin):
                 raise MinosConfigException(f"{type_!r} must be subclass of {InjectableMixin!r}.")
 
             ans.append(type_)
 
+        # noinspection PyTypeChecker
         return ans
 
     def _get_databases(self) -> dict[str, dict[str, Any]]:

packages/core/minos-microservice-common/tests/test_common/test_builders.py

@@ -1,61 +1,152 @@
 import unittest
-from abc import (
-    ABC,
-)
 from typing import (
-    Any,
+    Generic,
+    TypeVar,
 )
 
 from minos.common import (
+    BuildableMixin,
     Builder,
     Config,
-    SetupMixin,
 )
 from tests.utils import (
     CONFIG_FILE_PATH,
 )
 
 
-class _Builder(Builder[dict[str, Any]]):
-    def build(self) -> dict[str, Any]:
-        """For testing purposes."""
-        return self.kwargs
-
-
 class TestBuilder(unittest.TestCase):
-    def test_abstract(self):
-        self.assertTrue(issubclass(Builder, (ABC, SetupMixin)))
-        # noinspection PyUnresolvedReferences
-        self.assertEqual({"build"}, Builder.__abstractmethods__)
-
     def test_new(self):
-        builder = _Builder.new()
-        self.assertIsInstance(builder, _Builder)
+        builder = Builder.new().with_cls(dict)
+        self.assertIsInstance(builder, Builder)
         self.assertEqual(dict(), builder.kwargs)
 
     def test_copy(self):
-        base = _Builder.new().with_kwargs({"one": "two"})
+        base = Builder.new().with_cls(dict).with_kwargs({"one": "two"})
         builder = base.copy()
         self.assertNotEqual(id(base), id(builder))
-        self.assertIsInstance(builder, _Builder)
+        self.assertIsInstance(builder, Builder)
         self.assertEqual({"one": "two"}, builder.kwargs)
+        self.assertEqual(dict, builder.instance_cls)
 
     def test_with_kwargs(self):
-        builder = _Builder().with_kwargs({"foo": "bar"})
-        self.assertIsInstance(builder, _Builder)
+        builder = Builder().with_cls(dict).with_kwargs({"foo": "bar"})
+        self.assertIsInstance(builder, Builder)
         self.assertEqual({"foo": "bar"}, builder.kwargs)
 
     def test_with_config(self):
         config = Config(CONFIG_FILE_PATH)
-        builder = _Builder().with_config(config)
-        self.assertIsInstance(builder, _Builder)
+        builder = Builder().with_cls(dict).with_config(config)
+        self.assertIsInstance(builder, Builder)
         self.assertEqual(dict(), builder.kwargs)
 
     def test_build(self):
-        builder = _Builder().with_kwargs({"one": "two"})
-        self.assertIsInstance(builder, _Builder)
+        builder = Builder().with_cls(dict).with_kwargs({"one": "two"})
+        self.assertIsInstance(builder, Builder)
         self.assertEqual({"one": "two"}, builder.build())
 
+    def test_str(self):
+        builder = Builder().with_cls(dict).with_kwargs({"one": "two"})
+
+        self.assertEqual("Builder(dict, {'one': 'two'})", repr(builder))
+
+    def test_cmp(self):
+        base = Builder().with_cls(dict).with_kwargs({"one": "two"})
+
+        one = Builder().with_cls(dict).with_kwargs({"one": "two"})
+        self.assertEqual(base, one)
+
+        two = Builder().with_cls(int).with_kwargs({"one": "two"})
+        self.assertNotEqual(base, two)
+
+        three = Builder().with_cls(dict).with_kwargs({"three": "four"})
+        self.assertNotEqual(base, three)
+
+    def test_instance_cls_from_generic(self):
+        class _Builder(Builder):
+            """For Testing purposes."""
+
+        class _Builder2(Builder[int]):
+            """For Testing purposes."""
+
+        class _Builder3(_Builder2):
+            """For Testing purposes."""
+
+        T = TypeVar("T")
+
+        class _Builder4(_Builder2, Generic[T]):
+            """For Testing purposes."""
+
+        class _Builder5(_Builder4[float]):
+            """For Testing purposes."""
+
+        self.assertEqual(None, _Builder().instance_cls)
+        self.assertEqual(int, _Builder2().instance_cls)
+        self.assertEqual(int, _Builder3().instance_cls)
+        self.assertEqual(None, _Builder4().instance_cls)
+        self.assertEqual(float, _Builder5().instance_cls)
+
+
+class TestBuildableMixin(unittest.TestCase):
+    def test_get_builder_default(self):
+        class _Foo(BuildableMixin):
+            """For Testing purposes."""
+
+        self.assertEqual(Builder().with_cls(_Foo), _Foo.get_builder())
+
+    def test_get_builder_custom_type(self):
+        class _Foo(BuildableMixin):
+            """For Testing purposes."""
+
+        class _Builder(Builder):
+            """For Testing purposes."""
+
+        _Foo.set_builder(_Builder)
+
+        self.assertEqual(_Builder().with_cls(_Foo), _Foo.get_builder())
+
+    def test_get_builder_custom_instance(self):
+        class _Foo(BuildableMixin):
+            """For Testing purposes."""
+
+        class _Builder(Builder):
+            """For Testing purposes."""
+
+        _Foo.set_builder(_Builder().with_kwargs({"foo": "bar"}))
+
+        self.assertEqual(_Builder().with_cls(_Foo).with_kwargs({"foo": "bar"}), _Foo.get_builder())
+
+    def test_set_builder_raises(self):
+        class _Foo(BuildableMixin):
+            """For Testing purposes."""
+
+        with self.assertRaises(ValueError):
+            # noinspection PyTypeChecker
+            _Foo.set_builder(int)
+
+    def test_from_config(self):
+        class _Foo(BuildableMixin):
+            """For Testing purposes."""
+
+            # noinspection PyShadowingNames
+            def __init__(self, config):
+                super().__init__()
+                self.config = config
+
+        class _Builder(Builder):
+            """For Testing purposes."""
+
+            def with_config(self, config: Config):
+                """For Testing purposes."""
+                self.kwargs["config"] = config
+                return super().with_config(config)
+
+        _Foo.set_builder(_Builder)
+
+        config = Config(CONFIG_FILE_PATH)
+        foo = _Foo.from_config(config)
+
+        self.assertEqual(config, foo.config)
+
 
 if __name__ == "__main__":
     unittest.main()

packages/core/minos-microservice-common/tests/test_common/test_config/test_v2/test_base.py

@@ -65,7 +65,7 @@ def test_injections(self):
             FakeBrokerClientPool,
             FakeHttpConnector,
             FakeBrokerPublisher,
-            FakeBrokerSubscriberBuilder,
+            FakeBrokerSubscriberBuilder(FakeBrokerSubscriber),
             FakeEventRepository,
             FakeSnapshotRepository,
             FakeTransactionRepository,

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

@@ -132,9 +132,6 @@ class FakeBrokerPublisher(BuildableMixin):
 class FakeBrokerPublisherBuilder(Builder[FakeBrokerPublisher]):
     """For testing purposes."""
 
-    def build(self) -> FakeBrokerPublisher:
-        return FakeBrokerPublisher()
-
 
 FakeBrokerPublisher.set_builder(FakeBrokerPublisherBuilder)
 
@@ -147,9 +144,6 @@ class FakeBrokerSubscriber(BuildableMixin):
 class FakeBrokerSubscriberBuilder(Builder[FakeBrokerSubscriber]):
     """For testing purposes."""
 
-    def build(self) -> FakeBrokerSubscriber:
-        return FakeBrokerSubscriber()
-
 
 FakeBrokerSubscriber.set_builder(FakeBrokerSubscriberBuilder)