From 7fb4066cb7beafb6723af98d4df2f6e8f5056b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B0=B4=E7=90=83=E6=BD=98=20WaterBall?= Date: Fri, 13 Nov 2020 12:25:09 +0800 Subject: [PATCH] Dev/19: ServiceInstanceListSupplierBuilder design basic implementation (#24) * :beer: implement DiscoveryClient design * Add more comments * Modify some functional operation concerning performance * :memo: add typing and docstrings * Make some fields private * Fix naming * pending discussion: next+filter vs list comprehension * :art: Add type checking syntax * improve filter_get_first utils * :sparkles: FixedServiceInstanceListSupplier completed * :sparkles: CachingServiceInstanceListSupplier completed * :art: Add type checking syntax * :art: Add NoneTypeError and some type checking * :beer: implement DiscoveryClient design * Add more comments * Modify some functional operation concerning performance * :memo: add typing and docstrings * Make some fields private * Fix naming * pending discussion: next+filter vs list comprehension * :art: Add type checking syntax * improve filter_get_first utils * :truck: Modify the package layout, rooted from spring_cloud * add utils/validate --- pyproject.toml | 1 + .../client/loadbalancer/supplier/__init__.py | 3 + .../client/loadbalancer/supplier/base.py | 85 +++++++++++++++++++ .../client/loadbalancer/supplier/decorator.py | 43 ++++++++++ .../commons/client/service_instance.py | 15 +++- spring_cloud/commons/exceptions/__init__.py | 3 + spring_cloud/commons/exceptions/primitive.py | 17 ++++ spring_cloud/commons/helpers/__init__.py | 2 + .../commons/helpers/cache/__init__.py | 3 + .../commons/helpers/cache/cache_manager.py | 69 +++++++++++++++ spring_cloud/commons/utils/validate.py | 18 ++++ .../loadbalancer/supplier/decorator_test.py | 23 +++++ .../service_instance_list_supplier_test.py | 37 ++++++++ .../client/loadbalancer/supplier/stubs.py | 10 +++ 14 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 spring_cloud/commons/client/loadbalancer/supplier/__init__.py create mode 100644 spring_cloud/commons/client/loadbalancer/supplier/base.py create mode 100644 spring_cloud/commons/client/loadbalancer/supplier/decorator.py create mode 100644 spring_cloud/commons/exceptions/__init__.py create mode 100644 spring_cloud/commons/exceptions/primitive.py create mode 100644 spring_cloud/commons/helpers/__init__.py create mode 100644 spring_cloud/commons/helpers/cache/__init__.py create mode 100644 spring_cloud/commons/helpers/cache/cache_manager.py create mode 100644 spring_cloud/commons/utils/validate.py create mode 100644 tests/commons/client/loadbalancer/supplier/decorator_test.py create mode 100644 tests/commons/client/loadbalancer/supplier/service_instance_list_supplier_test.py create mode 100644 tests/commons/client/loadbalancer/supplier/stubs.py diff --git a/pyproject.toml b/pyproject.toml index de09a11..beb3e95 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ python = "^3.7" pre-commit = "^2.7.1" pytest = "^6.1.2" pytest-cov = "^2.10.1" +pytest-mock = "^3.3.1" [build-system] requires = ["poetry>=0.12"] diff --git a/spring_cloud/commons/client/loadbalancer/supplier/__init__.py b/spring_cloud/commons/client/loadbalancer/supplier/__init__.py new file mode 100644 index 0000000..c87d4dc --- /dev/null +++ b/spring_cloud/commons/client/loadbalancer/supplier/__init__.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- +from .base import * +from .decorator import * diff --git a/spring_cloud/commons/client/loadbalancer/supplier/base.py b/spring_cloud/commons/client/loadbalancer/supplier/base.py new file mode 100644 index 0000000..a63318a --- /dev/null +++ b/spring_cloud/commons/client/loadbalancer/supplier/base.py @@ -0,0 +1,85 @@ +# -*- coding: utf-8 -*- +""" +Since the load-balancer is responsible for choosing one instance +per service request from a list of instances. We need a ServiceInstanceListSupplier for +each service to decouple the source of the instances from load-balancers. +""" +# standard library +from abc import ABC, abstractmethod +from typing import List + +# scip plugin +from spring_cloud.commons.client import ServiceInstance +from spring_cloud.commons.client.discovery import DiscoveryClient + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +class ServiceInstanceListSupplier(ABC): + """ + Non-Reactive version of ServiceInstanceListSupplier. + (Spring Cloud implement the supplier in the reactive way, means that + its supplier returns an Observable which broadcasts the instances on every change.) + + We may consider to adopt reactive programming in the future. + """ + + @property + @abstractmethod + def service_id(self) -> str: + """ + :return: (str) the service's id + """ + pass + + @abstractmethod + def get(self, request=None) -> List[ServiceInstance]: + """ + :param request (opt) TODO not sure will we need this, + this extension was designed by spring-cloud. + :return: (*ServiceInstance) a list of instances + """ + pass + + +class FixedServiceInstanceListSupplier(ServiceInstanceListSupplier): + """ + A supplier that is initialized with fixed instances. (i.e. they won't be changed) + """ + + def __init__(self, service_id: str, instances: List[ServiceInstance]): + """ + :param service_id: (str) + :param instances: (*ServiceInstance) + """ + self._service_id = service_id + self._instances = instances + + def get(self, request=None) -> List[ServiceInstance]: + return self._instances + + @property + def service_id(self) -> str: + return self._service_id + + +class DiscoveryClientServiceInstanceListSupplier(ServiceInstanceListSupplier): + """ + The adapter delegating to discovery client for querying instances + """ + + def __init__(self, service_id: str, discovery_client: DiscoveryClient): + """ + :param service_id: (str) + :param discovery_client: (DiscoveryClient) + """ + self.__service_id = service_id + self.__delegate = discovery_client + + @property + def service_id(self) -> str: + return self.__service_id + + def get(self, request=None) -> List[ServiceInstance]: + return self.__delegate.get_instances(self.service_id) diff --git a/spring_cloud/commons/client/loadbalancer/supplier/decorator.py b/spring_cloud/commons/client/loadbalancer/supplier/decorator.py new file mode 100644 index 0000000..8d2e564 --- /dev/null +++ b/spring_cloud/commons/client/loadbalancer/supplier/decorator.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# standard library +from typing import List + +# scip plugin +from spring_cloud.commons.client import ServiceInstance +from spring_cloud.commons.exceptions import NoneTypeError +from spring_cloud.commons.utils import validate + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" +# standard library +from abc import ABC + +# scip plugin +from spring_cloud.commons.helpers import CacheManager + +from .base import ServiceInstanceListSupplier + + +class DelegatingServiceInstanceListSupplier(ServiceInstanceListSupplier, ABC): + """ + An application of decorator pattern that adds behaviors to ServiceInstanceListSupplier. + The decorators should inherit this class. + """ + + def __init__(self, delegate: ServiceInstanceListSupplier): + self.delegate = validate.not_none(delegate) + + @property + def service_id(self) -> str: + return self.delegate.service_id + + +class CachingServiceInstanceListSupplier(DelegatingServiceInstanceListSupplier): + CACHE_NAME = "CacheKey" + + def __init__(self, cache_manager: CacheManager, delegate: ServiceInstanceListSupplier): + super().__init__(delegate) + self.__cache_manager = cache_manager + + def get(self, request=None) -> List[ServiceInstance]: + return self.__cache_manager.get(self.CACHE_NAME).on_cache_miss(self.delegate.get) diff --git a/spring_cloud/commons/client/service_instance.py b/spring_cloud/commons/client/service_instance.py index a7103e6..ad7acfa 100644 --- a/spring_cloud/commons/client/service_instance.py +++ b/spring_cloud/commons/client/service_instance.py @@ -3,7 +3,6 @@ __author__ = "Waterball (johnny850807@gmail.com)" __license__ = "Apache 2.0" - # standard library from abc import ABC, abstractmethod from urllib.parse import urlparse @@ -95,3 +94,17 @@ def uri(self) -> str: @property def scheme(self) -> str: return self._scheme + + def __eq__(self, o: object) -> bool: + if isinstance(o, ServiceInstance): + return ( + self.uri == o.uri + and self.service_id == o.service_id + and self.instance_id == o.instance_id + and self.host == o.host + and self.port == o.port + and self.secure == o.secure + and self.scheme == o.scheme + ) + + return False diff --git a/spring_cloud/commons/exceptions/__init__.py b/spring_cloud/commons/exceptions/__init__.py new file mode 100644 index 0000000..ba20628 --- /dev/null +++ b/spring_cloud/commons/exceptions/__init__.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- + +from .primitive import * diff --git a/spring_cloud/commons/exceptions/primitive.py b/spring_cloud/commons/exceptions/primitive.py new file mode 100644 index 0000000..f2efcd5 --- /dev/null +++ b/spring_cloud/commons/exceptions/primitive.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- +""" +All primitive exceptions are included here +""" + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +class NoneTypeError(Exception): + @staticmethod + def raise_if_none(obj): + if obj is None: + raise NoneTypeError + + def __init__(self, message): + self.message = message diff --git a/spring_cloud/commons/helpers/__init__.py b/spring_cloud/commons/helpers/__init__.py new file mode 100644 index 0000000..da19d81 --- /dev/null +++ b/spring_cloud/commons/helpers/__init__.py @@ -0,0 +1,2 @@ +# -*- coding: utf-8 -*- +from .cache import * diff --git a/spring_cloud/commons/helpers/cache/__init__.py b/spring_cloud/commons/helpers/cache/__init__.py new file mode 100644 index 0000000..4a76e9b --- /dev/null +++ b/spring_cloud/commons/helpers/cache/__init__.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- + +from .cache_manager import * diff --git a/spring_cloud/commons/helpers/cache/cache_manager.py b/spring_cloud/commons/helpers/cache/cache_manager.py new file mode 100644 index 0000000..65a5862 --- /dev/null +++ b/spring_cloud/commons/helpers/cache/cache_manager.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 -*- + +""" +A cache manager wrapper that supports some syntax sugar. + +Usage: + value = cache_manager.get(cache_key) \ + .on_cache_miss(lambda: retrieve_value(key)) +""" + + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + +# standard library +from abc import ABC, abstractmethod + + +class OnCacheMiss: + def __init__(self, cache_manager, key, value): + self.__cache_manager = cache_manager + self.__key = key + self.__value = value + + @abstractmethod + def on_cache_miss(self, cache_miss_func): + """ + :param cache_miss_func: (lambda ()->value) + """ + if not self.__value: + value = cache_miss_func() + self.__cache_manager.put(self.__key, value) + return value + return self.__value + + +class CacheManager(ABC): + """ + Service Provider Interface (SPI) for basic caching. + We might want to extend this class with many features in the future. + (e.g. timeout, evict-and-replacement) + """ + + def get(self, key) -> OnCacheMiss: + value = self.retrieve_value(key) + return OnCacheMiss(self, key, value) + + @abstractmethod + def retrieve_value(self, key): + pass + + @abstractmethod + def put(self, key, value): + pass + + +class NaiveCacheManager(CacheManager): + """ + A very simple cache implementation. + """ + + def __init__(self): + self.cache_dict = {} + + def retrieve_value(self, key): + return self.cache_dict.get(key, None) + + def put(self, key, value): + self.cache_dict[key] = value diff --git a/spring_cloud/commons/utils/validate.py b/spring_cloud/commons/utils/validate.py new file mode 100644 index 0000000..e1e0a8d --- /dev/null +++ b/spring_cloud/commons/utils/validate.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +# scip plugin +from spring_cloud.commons.exceptions.primitive import NoneTypeError + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +def not_none(obj): + if obj: + return obj + raise NoneTypeError + + +def is_instance_of(obj, the_type): + if isinstance(obj, the_type): + return obj + raise TypeError() diff --git a/tests/commons/client/loadbalancer/supplier/decorator_test.py b/tests/commons/client/loadbalancer/supplier/decorator_test.py new file mode 100644 index 0000000..2941758 --- /dev/null +++ b/tests/commons/client/loadbalancer/supplier/decorator_test.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +# standard library +from unittest.mock import Mock + +# scip plugin +from spring_cloud.commons.client.loadbalancer.supplier.decorator import CachingServiceInstanceListSupplier +from spring_cloud.commons.helpers.cache.cache_manager import NaiveCacheManager +from tests.commons.client.loadbalancer.supplier.stubs import INSTANCES + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +class TestCachingServiceInstanceListSupplier: + def setup_class(self): + self.delegate = Mock() + self.delegate.get = Mock(return_value=INSTANCES) + self.supplier = CachingServiceInstanceListSupplier(NaiveCacheManager(), self.delegate) + + def test_Given_cache_When_10_invocations_Then_only_1_cache_miss_and_delegate(self): + for i in range(1, 10): + self.supplier.get() + assert self.delegate.get.call_count == 1 diff --git a/tests/commons/client/loadbalancer/supplier/service_instance_list_supplier_test.py b/tests/commons/client/loadbalancer/supplier/service_instance_list_supplier_test.py new file mode 100644 index 0000000..9917525 --- /dev/null +++ b/tests/commons/client/loadbalancer/supplier/service_instance_list_supplier_test.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# standard library +from unittest.mock import Mock + +# scip plugin +from spring_cloud.commons.client.loadbalancer.supplier import ( + DiscoveryClientServiceInstanceListSupplier, + FixedServiceInstanceListSupplier, +) +from tests.commons.client.loadbalancer.supplier.stubs import INSTANCES, SERVICE_ID + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +class TestFixedServiceInstanceListSupplier: + def setup_class(self): + self.supplier = FixedServiceInstanceListSupplier(SERVICE_ID, INSTANCES) + + def test_get_service_id(self): + assert SERVICE_ID == self.supplier.service_id + + def test_get_instances(self): + assert INSTANCES == self.supplier.get() + + +class TestDiscoveryClientServiceInstanceListSupplier: + def setup_class(self): + self.discovery_client = Mock() + self.discovery_client.get_instances = Mock(return_value=INSTANCES) + self.supplier = DiscoveryClientServiceInstanceListSupplier(SERVICE_ID, self.discovery_client) + + def test_get_service_id(self): + assert SERVICE_ID == self.supplier.service_id + + def test_get_instances(self): + assert INSTANCES == self.supplier.get() diff --git a/tests/commons/client/loadbalancer/supplier/stubs.py b/tests/commons/client/loadbalancer/supplier/stubs.py new file mode 100644 index 0000000..2da8148 --- /dev/null +++ b/tests/commons/client/loadbalancer/supplier/stubs.py @@ -0,0 +1,10 @@ +# -*- coding: utf-8 -*- +# scip plugin +from spring_cloud.commons.client import StaticServiceInstance + +__author__ = "Waterball (johnny850807@gmail.com)" +__license__ = "Apache 2.0" + + +SERVICE_ID = "serviceId" +INSTANCES = [StaticServiceInstance("uri", SERVICE_ID, instance_id) for instance_id in ["1", "2", "3"]]