public inbox for [email protected]help / color / mirror / Atom feed
Refactor: Registry Classes 7+ messages / 3 participants [nested] [flat]
* Refactor: Registry Classes @ 2021-06-18 20:09 Ashesh Vashi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Ashesh Vashi @ 2021-06-18 20:09 UTC (permalink / raw) To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers Hi Akshay, We do have a couple of classes, which does automatic registration of the base classes, and which creates single-ton objects for these base classes, when needed. I would be working on a patch sooner, which will be using similar functionality for loading the multi-factor authentication. I realized - it will be a duplicate code at three places for the same functionalities. Hence - I worked on refactoring this registry class. Please find the patch for the same. -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company <http://www.enterprisedb.com; *http://www.linkedin.com/in/asheshvashi* <http://www.linkedin.com/in/asheshvashi; Attachments: [application/octet-stream] refactor_registry_class.patch (18.2K, 3-refactor_registry_class.patch) download | inline diff: diff --git a/web/pgadmin/authenticate/__init__.py b/web/pgadmin/authenticate/__init__.py index d328b0d24..101d57656 100644 --- a/web/pgadmin/authenticate/__init__.py +++ b/web/pgadmin/authenticate/__init__.py @@ -223,7 +223,7 @@ def get_auth_sources(type): if type in auth_sources: return auth_sources[type] - auth_source = AuthSourceRegistry.create(type) + auth_source = AuthSourceRegistry.get(type) if auth_source is not None: auth_sources[type] = auth_source @@ -236,7 +236,7 @@ def init_app(app): auth_sources = dict() setattr(app, '_pgadmin_auth_sources', auth_sources) - AuthSourceRegistry.load_auth_sources() + AuthSourceRegistry.load_modules(app) return auth_sources diff --git a/web/pgadmin/authenticate/registry.py b/web/pgadmin/authenticate/registry.py index 7d797e658..fdf9b400e 100644 --- a/web/pgadmin/authenticate/registry.py +++ b/web/pgadmin/authenticate/registry.py @@ -10,56 +10,9 @@ """External Authentication Registry.""" -from flask_babelex import gettext -from abc import ABCMeta +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class AuthSourceRegistry(ABCMeta): - registry = None - auth_sources = dict() - - def __init__(self, name, bases, d): - - # Register this type of auth_sources, based on the module name - # Avoid registering the BaseAuthentication itself - - AuthSourceRegistry.registry[_decorate_cls_name(d['__module__'])] = self - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in AuthSourceRegistry.auth_sources: - return AuthSourceRegistry.auth_sources[name] - - if name in AuthSourceRegistry.registry: - AuthSourceRegistry.auth_sources[name] = \ - (AuthSourceRegistry.registry[name])(**kwargs) - return AuthSourceRegistry.auth_sources[name] - - raise NotImplementedError( - gettext( - "Authentication source '{0}' has not been implemented." - ).format(name) - ) - - @classmethod - def load_auth_sources(cls): - # Initialize the registry only if it has not yet been initialized - if AuthSourceRegistry.registry is None: - AuthSourceRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +AuthSourceRegistry = CreateRegistryMetaClass( + 'AuthSourceRegistry', __package__, decorate_as_module=True +) diff --git a/web/pgadmin/utils/driver/__init__.py b/web/pgadmin/utils/driver/__init__.py index 5c6d6945a..d7748c839 100644 --- a/web/pgadmin/utils/driver/__init__.py +++ b/web/pgadmin/utils/driver/__init__.py @@ -12,32 +12,19 @@ from flask import current_app from .registry import DriverRegistry -def get_driver(type, app=None): - if app is not None: - DriverRegistry.load_drivers() - - drivers = getattr(app or current_app, '_pgadmin_server_drivers', None) - - if drivers is None or not isinstance(drivers, dict): - drivers = dict() +def get_driver(_type, app=None): - if type in drivers: - return drivers[type] - - driver = DriverRegistry.create(type) - - if driver is not None: - drivers[type] = driver - setattr(app or current_app, '_pgadmin_server_drivers', drivers) + if app is not None: + DriverRegistry.load_modules(app) - return driver + return DriverRegistry.get(_type) def init_app(app): drivers = dict() setattr(app, '_pgadmin_server_drivers', drivers) - DriverRegistry.load_drivers() + DriverRegistry.load_modules(app) return drivers diff --git a/web/pgadmin/utils/driver/registry.py b/web/pgadmin/utils/driver/registry.py index ae535429b..c82b6707b 100644 --- a/web/pgadmin/utils/driver/registry.py +++ b/web/pgadmin/utils/driver/registry.py @@ -10,79 +10,9 @@ from abc import ABCMeta from flask_babelex import gettext +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class DriverRegistry(ABCMeta): - """ - class DriverRegistry(object) - Every Driver will be registered automatically by its module name. - - This uses factory pattern to genreate driver object based on its name - automatically. - - Class-level Methods: - ----------- ------- - * __init__(...) - - It will be used to register type of drivers. You don't need to call - this function explicitly. This will be automatically executed, whenever - we create class and inherit from BaseDriver, it will register it as - available driver in DriverRegistry. Because - the __metaclass__ for - BaseDriver is set it to DriverRegistry, and it will create new instance - of this DriverRegistry per class. - - * create(type, *args, **kwargs) - - Create type of driver object for this server, from the available - driver list (if available, or raise exception). - - * load_drivers(): - - Use this function from init_app(...) to load all available drivers in - the registry. - """ - registry = None - drivers = dict() - - def __init__(self, name, bases, d): - - # Register this type of driver, based on the module name - # Avoid registering the BaseDriver itself - - if name != 'BaseDriver': - DriverRegistry.registry[_decorate_cls_name(d['__module__'])] = self - - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in DriverRegistry.drivers: - return DriverRegistry.drivers[name] - - if name in DriverRegistry.registry: - DriverRegistry.drivers[name] = \ - (DriverRegistry.registry[name])(**kwargs) - return DriverRegistry.drivers[name] - - raise NotImplementedError( - gettext("Driver '{0}' has not been implemented.").format(name) - ) - - @classmethod - def load_drivers(cls): - # Initialize the registry only if it has not yet been initialized - if DriverRegistry.registry is None: - DriverRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +DriverRegistry = CreateRegistryMetaClass( + 'DriverRegistry', __package__, decorate_as_module=True +) diff --git a/web/pgadmin/utils/dynamic_registry/__init__.py b/web/pgadmin/utils/dynamic_registry/__init__.py new file mode 100644 index 000000000..9bca14510 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/__init__.py @@ -0,0 +1,105 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +######################################################################### + +"""This file contains functions for creating dynamic registry meta class.""" + +from abc import ABCMeta + + +# Constructor +def _constructor(self, name, bases, kwargs): + + # Register this type of auth_sources, based on the module name + # Avoid registering the BaseAuthentication itself + cls = self.__class__ + entry = self._decorate_cls_name(name, kwargs) + + if cls._registry is None: + cls._registry = dict() + else: + if entry in cls._registry: + raise RuntimeError(( + '{} class is already been registered with {} ' + '(package: {})' + ).format(entry, cls._name_, cls._package_)) + + if cls._initialized is True: + cls._registry[entry] = self + cls._initialized = True + + ABCMeta.__init__(self, name, bases, kwargs) + + +@classmethod +def _get(cls, name, **kwargs): + + if name in cls._objects: + return cls._objects[name] + + if cls._registry is not None and name in cls._registry: + cls._objects[name] = (cls._registry[name])(**kwargs) + + return cls._objects[name] + + raise NotImplementedError( + "{} '{}' has not been implemented.".format(cls.__name__, name) + ) + + +@classmethod +def _load_modules(cls, app=None): + # Initialize the registry only if it has not yet been initialized + if cls._registry is None: + cls._registry = dict() + + from importlib import import_module + from werkzeug.utils import find_modules + + for module_name in find_modules(cls._package_, True): + module = import_module(module_name) + + if 'init_app' in module.__dict__.keys(): + module.__dict__['init_app'](app) + + +def _get_module_name(self, name, kwargs): + module_name = kwargs['__module__'] + length = len(self._package_) + 1 + + if len(module_name) > length and module_name.startswith(self._package_): + return module_name[length:] + + return module_name + +def _get_class_name(self, name, kwargs): + return name + + +def CreateRegistryMetaClass(name, package, decorate_as_module=True): + + class_params = { + # constructor + "__init__": _constructor, + + # Class members + "_registry": None, + "_objects": dict(), + "_package_": package, + + # Member functions + "get": _get, + "load_modules": _load_modules, + '_name_': name, + '_decorate_cls_name': _get_module_name + if decorate_as_module is True else _get_class_name, + '_initialized': False, + } + + # Creating class dynamically + return type(package + "." + name, (ABCMeta, ), class_params) diff --git a/web/pgadmin/utils/dynamic_registry/tests/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/__init__.py new file mode 100644 index 000000000..89190b316 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/__init__.py @@ -0,0 +1,8 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py new file mode 100644 index 000000000..c5765418e --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py @@ -0,0 +1,101 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass +from .registry import TestModuleRegistry, TestNamedRegistry, TestNameBase +from .test1 import TestModule1 + + +def test_load_modules_based_registry(): + TestModuleRegistry.load_modules() + + if len(TestModuleRegistry._registry) != 2: + return "Length of the registry should have been 2" + + if 'test1' not in TestModuleRegistry._registry: + return "'test1' is not found in the registry" + + if 'test2' not in TestModuleRegistry._registry: + return "'test2' is not found in the registry" + + obj_test1_1 = TestModuleRegistry.get('test1') + obj_test1_2 = TestModuleRegistry.get('test1') + obj_test2_1 = TestModuleRegistry.get('test2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if isinstance(obj_test1_1, TestModule1) is False: + return "Registry created wrong object" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for " \ + "different classes" + + +def test_load_classname_registry(): + TestNamedRegistry.load_modules() + + if 'ClassTestName1' not in TestNamedRegistry._registry: + return "'ClassTestName1' is not found in the registry" + + if 'ClassTestName2' not in TestNamedRegistry._registry: + return "'ClassTestName2' is not found in the registry" + + obj_test1_1 = TestNamedRegistry.get('ClassTestName1') + obj_test1_2 = TestNamedRegistry.get('ClassTestName1') + obj_test2_1 = TestNamedRegistry.get('ClassTestName2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for different classes" + + try: + class ClassTestName1(TestNameBase): + pass + + return "Expected an runtime error when using the same classname" + except RuntimeError: + pass + + +def test_empty_registry(): + + EmptyModuleRegistry = CreateRegistryMetaClass( + 'EmptyModuleRegistry', __package__, decorate_as_module=True + ) + + if EmptyModuleRegistry._registry is not None: + return "Registry was supposed to be None" + + if EmptyModuleRegistry._initialized is not False: + return "Registry initialized flag should be false" + + +def test_create_base_class(): + + RegistryWithBaseClass = CreateRegistryMetaClass( + 'RegistryWithBaseClass', __package__, decorate_as_module=False + ) + + @six.add_metaclass(RegistryWithBaseClass) + class TestBase(object): + pass + + failed = False + + registry = RegistryWithBaseClass._registry + + if registry is None or len(registry) != 0: + return "Registry was not supposed to be None, and empty" + + if RegistryWithBaseClass._initialized is False: + return "Registry initialized flag should not be true" diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py new file mode 100644 index 000000000..2fb530ff9 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py @@ -0,0 +1,38 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass + + +TestModuleRegistry = CreateRegistryMetaClass( + 'TestModuleRegistry', __package__, decorate_as_module=True +) + + +TestNamedRegistry = CreateRegistryMetaClass( + 'TestRegistry', __package__, decorate_as_module=False +) + + [email protected]_metaclass(TestModuleRegistry) +class TestModuleBase(object): + pass + + [email protected]_metaclass(TestNamedRegistry) +class TestNameBase(object): + pass + + +class ClassTestName1(TestNameBase): + pass + + +class ClassTestName2(TestNameBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py new file mode 100644 index 000000000..656318fdf --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py @@ -0,0 +1,14 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from .registry import TestModuleBase #, TestModuleRegistry + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule1(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py new file mode 100644 index 000000000..96c9ae538 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py @@ -0,0 +1,14 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from .registry import TestModuleBase #, TestModuleRegistry + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule2(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py new file mode 100644 index 000000000..472aa2e69 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py @@ -0,0 +1,39 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from pgadmin.utils.route import BaseTestGenerator +from .registry import test_load_modules_based_registry, test_empty_registry, \ + test_create_base_class, test_load_classname_registry + + +class TestDynamicRegistry(BaseTestGenerator): + + scenarios = [ + ( + "Check empty registry", + dict(test=test_empty_registry), + ), + ( + 'Load the registry based on the modules', + dict(test=test_load_modules_based_registry), + ), + ( + 'Load the registry based on the name of the classes', + dict(test=test_load_classname_registry), + ), + ( + "When created a base class registry is initialized", + dict(test=test_create_base_class), + ), + ] + + def runTest(self): + error = self.test() + + if error is not None: + self.fail(error) ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-19 05:57 Ashesh Vashi <[email protected]> parent: Ashesh Vashi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Ashesh Vashi @ 2021-06-19 05:57 UTC (permalink / raw) To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi <[email protected]> wrote: > Hi Akshay, > > We do have a couple of classes, which does automatic registration of the > base classes, > and which creates single-ton objects for these base classes, when needed. > > I would be working on a patch sooner, which will be using similar > functionality for loading > the multi-factor authentication. > > I realized - it will be a duplicate code at three places for the same > functionalities. > Hence - I worked on refactoring this registry class. > > Please find the patch for the same. > Found issues - some test files were using the old function 'Driver.load_drivers(...)'. They're fixed now. -- Thanks, Ashesh > > -- > > Thanks & Regards, > > Ashesh Vashi > EnterpriseDB INDIA: Enterprise PostgreSQL Company > <http://www.enterprisedb.com; > > > *http://www.linkedin.com/in/asheshvashi* > <http://www.linkedin.com/in/asheshvashi; > Attachments: [application/octet-stream] refactor_registry_class_v2.patch (19.7K, 3-refactor_registry_class_v2.patch) download | inline diff: diff --git a/web/pgadmin/authenticate/__init__.py b/web/pgadmin/authenticate/__init__.py index d328b0d24..101d57656 100644 --- a/web/pgadmin/authenticate/__init__.py +++ b/web/pgadmin/authenticate/__init__.py @@ -223,7 +223,7 @@ def get_auth_sources(type): if type in auth_sources: return auth_sources[type] - auth_source = AuthSourceRegistry.create(type) + auth_source = AuthSourceRegistry.get(type) if auth_source is not None: auth_sources[type] = auth_source @@ -236,7 +236,7 @@ def init_app(app): auth_sources = dict() setattr(app, '_pgadmin_auth_sources', auth_sources) - AuthSourceRegistry.load_auth_sources() + AuthSourceRegistry.load_modules(app) return auth_sources diff --git a/web/pgadmin/authenticate/registry.py b/web/pgadmin/authenticate/registry.py index 7d797e658..fdf9b400e 100644 --- a/web/pgadmin/authenticate/registry.py +++ b/web/pgadmin/authenticate/registry.py @@ -10,56 +10,9 @@ """External Authentication Registry.""" -from flask_babelex import gettext -from abc import ABCMeta +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class AuthSourceRegistry(ABCMeta): - registry = None - auth_sources = dict() - - def __init__(self, name, bases, d): - - # Register this type of auth_sources, based on the module name - # Avoid registering the BaseAuthentication itself - - AuthSourceRegistry.registry[_decorate_cls_name(d['__module__'])] = self - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in AuthSourceRegistry.auth_sources: - return AuthSourceRegistry.auth_sources[name] - - if name in AuthSourceRegistry.registry: - AuthSourceRegistry.auth_sources[name] = \ - (AuthSourceRegistry.registry[name])(**kwargs) - return AuthSourceRegistry.auth_sources[name] - - raise NotImplementedError( - gettext( - "Authentication source '{0}' has not been implemented." - ).format(name) - ) - - @classmethod - def load_auth_sources(cls): - # Initialize the registry only if it has not yet been initialized - if AuthSourceRegistry.registry is None: - AuthSourceRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +AuthSourceRegistry = CreateRegistryMetaClass( + 'AuthSourceRegistry', __package__, decorate_as_module=True +) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py index 7c9d8f423..4a6c2664b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py @@ -14,7 +14,7 @@ from regression.python_test_utils.template_helper import file_as_template from pgadmin.utils.route import BaseTestGenerator from regression.python_test_utils import test_utils -DriverRegistry.load_drivers() +DriverRegistry.load_modules() class TestColumnForeignKeyGetConstraintCols(BaseTestGenerator): diff --git a/web/pgadmin/utils/driver/__init__.py b/web/pgadmin/utils/driver/__init__.py index 5c6d6945a..d7748c839 100644 --- a/web/pgadmin/utils/driver/__init__.py +++ b/web/pgadmin/utils/driver/__init__.py @@ -12,32 +12,19 @@ from flask import current_app from .registry import DriverRegistry -def get_driver(type, app=None): - if app is not None: - DriverRegistry.load_drivers() - - drivers = getattr(app or current_app, '_pgadmin_server_drivers', None) - - if drivers is None or not isinstance(drivers, dict): - drivers = dict() +def get_driver(_type, app=None): - if type in drivers: - return drivers[type] - - driver = DriverRegistry.create(type) - - if driver is not None: - drivers[type] = driver - setattr(app or current_app, '_pgadmin_server_drivers', drivers) + if app is not None: + DriverRegistry.load_modules(app) - return driver + return DriverRegistry.get(_type) def init_app(app): drivers = dict() setattr(app, '_pgadmin_server_drivers', drivers) - DriverRegistry.load_drivers() + DriverRegistry.load_modules(app) return drivers diff --git a/web/pgadmin/utils/driver/registry.py b/web/pgadmin/utils/driver/registry.py index ae535429b..c82b6707b 100644 --- a/web/pgadmin/utils/driver/registry.py +++ b/web/pgadmin/utils/driver/registry.py @@ -10,79 +10,9 @@ from abc import ABCMeta from flask_babelex import gettext +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class DriverRegistry(ABCMeta): - """ - class DriverRegistry(object) - Every Driver will be registered automatically by its module name. - - This uses factory pattern to genreate driver object based on its name - automatically. - - Class-level Methods: - ----------- ------- - * __init__(...) - - It will be used to register type of drivers. You don't need to call - this function explicitly. This will be automatically executed, whenever - we create class and inherit from BaseDriver, it will register it as - available driver in DriverRegistry. Because - the __metaclass__ for - BaseDriver is set it to DriverRegistry, and it will create new instance - of this DriverRegistry per class. - - * create(type, *args, **kwargs) - - Create type of driver object for this server, from the available - driver list (if available, or raise exception). - - * load_drivers(): - - Use this function from init_app(...) to load all available drivers in - the registry. - """ - registry = None - drivers = dict() - - def __init__(self, name, bases, d): - - # Register this type of driver, based on the module name - # Avoid registering the BaseDriver itself - - if name != 'BaseDriver': - DriverRegistry.registry[_decorate_cls_name(d['__module__'])] = self - - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in DriverRegistry.drivers: - return DriverRegistry.drivers[name] - - if name in DriverRegistry.registry: - DriverRegistry.drivers[name] = \ - (DriverRegistry.registry[name])(**kwargs) - return DriverRegistry.drivers[name] - - raise NotImplementedError( - gettext("Driver '{0}' has not been implemented.").format(name) - ) - - @classmethod - def load_drivers(cls): - # Initialize the registry only if it has not yet been initialized - if DriverRegistry.registry is None: - DriverRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +DriverRegistry = CreateRegistryMetaClass( + 'DriverRegistry', __package__, decorate_as_module=True +) diff --git a/web/pgadmin/utils/dynamic_registry/__init__.py b/web/pgadmin/utils/dynamic_registry/__init__.py new file mode 100644 index 000000000..9bca14510 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/__init__.py @@ -0,0 +1,105 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +######################################################################### + +"""This file contains functions for creating dynamic registry meta class.""" + +from abc import ABCMeta + + +# Constructor +def _constructor(self, name, bases, kwargs): + + # Register this type of auth_sources, based on the module name + # Avoid registering the BaseAuthentication itself + cls = self.__class__ + entry = self._decorate_cls_name(name, kwargs) + + if cls._registry is None: + cls._registry = dict() + else: + if entry in cls._registry: + raise RuntimeError(( + '{} class is already been registered with {} ' + '(package: {})' + ).format(entry, cls._name_, cls._package_)) + + if cls._initialized is True: + cls._registry[entry] = self + cls._initialized = True + + ABCMeta.__init__(self, name, bases, kwargs) + + +@classmethod +def _get(cls, name, **kwargs): + + if name in cls._objects: + return cls._objects[name] + + if cls._registry is not None and name in cls._registry: + cls._objects[name] = (cls._registry[name])(**kwargs) + + return cls._objects[name] + + raise NotImplementedError( + "{} '{}' has not been implemented.".format(cls.__name__, name) + ) + + +@classmethod +def _load_modules(cls, app=None): + # Initialize the registry only if it has not yet been initialized + if cls._registry is None: + cls._registry = dict() + + from importlib import import_module + from werkzeug.utils import find_modules + + for module_name in find_modules(cls._package_, True): + module = import_module(module_name) + + if 'init_app' in module.__dict__.keys(): + module.__dict__['init_app'](app) + + +def _get_module_name(self, name, kwargs): + module_name = kwargs['__module__'] + length = len(self._package_) + 1 + + if len(module_name) > length and module_name.startswith(self._package_): + return module_name[length:] + + return module_name + +def _get_class_name(self, name, kwargs): + return name + + +def CreateRegistryMetaClass(name, package, decorate_as_module=True): + + class_params = { + # constructor + "__init__": _constructor, + + # Class members + "_registry": None, + "_objects": dict(), + "_package_": package, + + # Member functions + "get": _get, + "load_modules": _load_modules, + '_name_': name, + '_decorate_cls_name': _get_module_name + if decorate_as_module is True else _get_class_name, + '_initialized': False, + } + + # Creating class dynamically + return type(package + "." + name, (ABCMeta, ), class_params) diff --git a/web/pgadmin/utils/dynamic_registry/tests/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/__init__.py new file mode 100644 index 000000000..89190b316 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/__init__.py @@ -0,0 +1,8 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py new file mode 100644 index 000000000..c5765418e --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py @@ -0,0 +1,101 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass +from .registry import TestModuleRegistry, TestNamedRegistry, TestNameBase +from .test1 import TestModule1 + + +def test_load_modules_based_registry(): + TestModuleRegistry.load_modules() + + if len(TestModuleRegistry._registry) != 2: + return "Length of the registry should have been 2" + + if 'test1' not in TestModuleRegistry._registry: + return "'test1' is not found in the registry" + + if 'test2' not in TestModuleRegistry._registry: + return "'test2' is not found in the registry" + + obj_test1_1 = TestModuleRegistry.get('test1') + obj_test1_2 = TestModuleRegistry.get('test1') + obj_test2_1 = TestModuleRegistry.get('test2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if isinstance(obj_test1_1, TestModule1) is False: + return "Registry created wrong object" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for " \ + "different classes" + + +def test_load_classname_registry(): + TestNamedRegistry.load_modules() + + if 'ClassTestName1' not in TestNamedRegistry._registry: + return "'ClassTestName1' is not found in the registry" + + if 'ClassTestName2' not in TestNamedRegistry._registry: + return "'ClassTestName2' is not found in the registry" + + obj_test1_1 = TestNamedRegistry.get('ClassTestName1') + obj_test1_2 = TestNamedRegistry.get('ClassTestName1') + obj_test2_1 = TestNamedRegistry.get('ClassTestName2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for different classes" + + try: + class ClassTestName1(TestNameBase): + pass + + return "Expected an runtime error when using the same classname" + except RuntimeError: + pass + + +def test_empty_registry(): + + EmptyModuleRegistry = CreateRegistryMetaClass( + 'EmptyModuleRegistry', __package__, decorate_as_module=True + ) + + if EmptyModuleRegistry._registry is not None: + return "Registry was supposed to be None" + + if EmptyModuleRegistry._initialized is not False: + return "Registry initialized flag should be false" + + +def test_create_base_class(): + + RegistryWithBaseClass = CreateRegistryMetaClass( + 'RegistryWithBaseClass', __package__, decorate_as_module=False + ) + + @six.add_metaclass(RegistryWithBaseClass) + class TestBase(object): + pass + + failed = False + + registry = RegistryWithBaseClass._registry + + if registry is None or len(registry) != 0: + return "Registry was not supposed to be None, and empty" + + if RegistryWithBaseClass._initialized is False: + return "Registry initialized flag should not be true" diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py new file mode 100644 index 000000000..2fb530ff9 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py @@ -0,0 +1,38 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import CreateRegistryMetaClass + + +TestModuleRegistry = CreateRegistryMetaClass( + 'TestModuleRegistry', __package__, decorate_as_module=True +) + + +TestNamedRegistry = CreateRegistryMetaClass( + 'TestRegistry', __package__, decorate_as_module=False +) + + [email protected]_metaclass(TestModuleRegistry) +class TestModuleBase(object): + pass + + [email protected]_metaclass(TestNamedRegistry) +class TestNameBase(object): + pass + + +class ClassTestName1(TestNameBase): + pass + + +class ClassTestName2(TestNameBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py new file mode 100644 index 000000000..656318fdf --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py @@ -0,0 +1,14 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from .registry import TestModuleBase #, TestModuleRegistry + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule1(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py new file mode 100644 index 000000000..96c9ae538 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py @@ -0,0 +1,14 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from .registry import TestModuleBase #, TestModuleRegistry + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule2(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py new file mode 100644 index 000000000..472aa2e69 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py @@ -0,0 +1,39 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from pgadmin.utils.route import BaseTestGenerator +from .registry import test_load_modules_based_registry, test_empty_registry, \ + test_create_base_class, test_load_classname_registry + + +class TestDynamicRegistry(BaseTestGenerator): + + scenarios = [ + ( + "Check empty registry", + dict(test=test_empty_registry), + ), + ( + 'Load the registry based on the modules', + dict(test=test_load_modules_based_registry), + ), + ( + 'Load the registry based on the name of the classes', + dict(test=test_load_classname_registry), + ), + ( + "When created a base class registry is initialized", + dict(test=test_create_base_class), + ), + ] + + def runTest(self): + error = self.test() + + if error is not None: + self.fail(error) diff --git a/web/regression/python_test_utils/sql_template_test_base.py b/web/regression/python_test_utils/sql_template_test_base.py index e3a381675..2b9fe76b0 100644 --- a/web/regression/python_test_utils/sql_template_test_base.py +++ b/web/regression/python_test_utils/sql_template_test_base.py @@ -15,7 +15,7 @@ from pgadmin.utils.driver import DriverRegistry from pgadmin.utils.versioned_template_loader \ import get_version_mapping_directories -DriverRegistry.load_drivers() +DriverRegistry.load_modules() class SQLTemplateTestBase(BaseTestGenerator): ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-23 07:52 Akshay Joshi <[email protected]> parent: Ashesh Vashi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Akshay Joshi @ 2021-06-23 07:52 UTC (permalink / raw) To: Ashesh Vashi <[email protected]>; +Cc: pgadmin-hackers Hi Ashesh Following are the review comments: - Fixed PEP8 issues. - In "dynamic_registry/__init__.py" decorator @classmethod used for " *_get*" and "*_load_modules*" methods which are actually outside of the class. Even constructor also outside of the class. - Remove unused imports from "driver/registry.py" - Fixed sonarqube issues in "dynamic_registry/tests/registry/__init__.py" On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi <[email protected]> wrote: > On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi < > [email protected]> wrote: > >> Hi Akshay, >> >> We do have a couple of classes, which does automatic registration of the >> base classes, >> and which creates single-ton objects for these base classes, when needed. >> >> I would be working on a patch sooner, which will be using similar >> functionality for loading >> the multi-factor authentication. >> >> I realized - it will be a duplicate code at three places for the same >> functionalities. >> Hence - I worked on refactoring this registry class. >> >> Please find the patch for the same. >> > Found issues - some test files were using the old function > 'Driver.load_drivers(...)'. > They're fixed now. > > -- Thanks, Ashesh > >> >> -- >> >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL Company >> <http://www.enterprisedb.com; >> >> >> *http://www.linkedin.com/in/asheshvashi* >> <http://www.linkedin.com/in/asheshvashi; >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres <http://edbpostgres.com>* *Mobile: +91 976-788-8246* ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-23 14:24 Ashesh Vashi <[email protected]> parent: Akshay Joshi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Ashesh Vashi @ 2021-06-23 14:24 UTC (permalink / raw) To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi <[email protected]> wrote: > Hi Ashesh > > Following are the review comments: > > - Fixed PEP8 issues. > > Done. > > - In "dynamic_registry/__init__.py" decorator @classmethod used for " > *_get*" and "*_load_modules*" methods which are actually outside of > the class. Even constructor also outside of the class. > > 'create_registry_metaclass' is not a class, but a method to create the dynamic classes. If I move these methods in 'create_registry_metaclass' method, SonarQube raises issues about complexity of the functions, hence - they're best kept outside of that method. > > - Remove unused imports from "driver/registry.py" > > Done > > - > - Fixed sonarqube issues in > "dynamic_registry/tests/registry/__init__.py" > > Done As discussed, SonarQube is not able to understand that the result object is a class, and not an object, hence - showing linter issues. I've disabled them in those lines by adding the comment '# NOSNAR' at the end. -- Thanks, Ashesh > > On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi < > [email protected]> wrote: > >> On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi < >> [email protected]> wrote: >> >>> Hi Akshay, >>> >>> We do have a couple of classes, which does automatic registration of the >>> base classes, >>> and which creates single-ton objects for these base classes, when needed. >>> >>> I would be working on a patch sooner, which will be using similar >>> functionality for loading >>> the multi-factor authentication. >>> >>> I realized - it will be a duplicate code at three places for the same >>> functionalities. >>> Hence - I worked on refactoring this registry class. >>> >>> Please find the patch for the same. >>> >> Found issues - some test files were using the old function >> 'Driver.load_drivers(...)'. >> They're fixed now. >> >> -- Thanks, Ashesh >> >>> >>> -- >>> >>> Thanks & Regards, >>> >>> Ashesh Vashi >>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>> <http://www.enterprisedb.com; >>> >>> >>> *http://www.linkedin.com/in/asheshvashi* >>> <http://www.linkedin.com/in/asheshvashi; >>> >> > > -- > *Thanks & Regards* > *Akshay Joshi* > *pgAdmin Hacker | Principal Software Architect* > *EDB Postgres <http://edbpostgres.com>* > > *Mobile: +91 976-788-8246* > Attachments: [application/octet-stream] refactor_registry_class_v3.patch (19.7K, 3-refactor_registry_class_v3.patch) download | inline diff: diff --git a/web/pgadmin/authenticate/__init__.py b/web/pgadmin/authenticate/__init__.py index d328b0d24..101d57656 100644 --- a/web/pgadmin/authenticate/__init__.py +++ b/web/pgadmin/authenticate/__init__.py @@ -223,7 +223,7 @@ def get_auth_sources(type): if type in auth_sources: return auth_sources[type] - auth_source = AuthSourceRegistry.create(type) + auth_source = AuthSourceRegistry.get(type) if auth_source is not None: auth_sources[type] = auth_source @@ -236,7 +236,7 @@ def init_app(app): auth_sources = dict() setattr(app, '_pgadmin_auth_sources', auth_sources) - AuthSourceRegistry.load_auth_sources() + AuthSourceRegistry.load_modules(app) return auth_sources diff --git a/web/pgadmin/authenticate/registry.py b/web/pgadmin/authenticate/registry.py index 7d797e658..84c5fd07a 100644 --- a/web/pgadmin/authenticate/registry.py +++ b/web/pgadmin/authenticate/registry.py @@ -10,56 +10,9 @@ """External Authentication Registry.""" -from flask_babelex import gettext -from abc import ABCMeta +from pgadmin.utils.dynamic_registry import create_registry_metaclass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class AuthSourceRegistry(ABCMeta): - registry = None - auth_sources = dict() - - def __init__(self, name, bases, d): - - # Register this type of auth_sources, based on the module name - # Avoid registering the BaseAuthentication itself - - AuthSourceRegistry.registry[_decorate_cls_name(d['__module__'])] = self - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in AuthSourceRegistry.auth_sources: - return AuthSourceRegistry.auth_sources[name] - - if name in AuthSourceRegistry.registry: - AuthSourceRegistry.auth_sources[name] = \ - (AuthSourceRegistry.registry[name])(**kwargs) - return AuthSourceRegistry.auth_sources[name] - - raise NotImplementedError( - gettext( - "Authentication source '{0}' has not been implemented." - ).format(name) - ) - - @classmethod - def load_auth_sources(cls): - # Initialize the registry only if it has not yet been initialized - if AuthSourceRegistry.registry is None: - AuthSourceRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +AuthSourceRegistry = create_registry_metaclass( + "AuthSourceRegistry", __package__, decorate_as_module=True +) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py index 7c9d8f423..4a6c2664b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/foreign_key/sql/tests/test_foreign_key_properties.py @@ -14,7 +14,7 @@ from regression.python_test_utils.template_helper import file_as_template from pgadmin.utils.route import BaseTestGenerator from regression.python_test_utils import test_utils -DriverRegistry.load_drivers() +DriverRegistry.load_modules() class TestColumnForeignKeyGetConstraintCols(BaseTestGenerator): diff --git a/web/pgadmin/utils/driver/__init__.py b/web/pgadmin/utils/driver/__init__.py index 5c6d6945a..d7748c839 100644 --- a/web/pgadmin/utils/driver/__init__.py +++ b/web/pgadmin/utils/driver/__init__.py @@ -12,32 +12,19 @@ from flask import current_app from .registry import DriverRegistry -def get_driver(type, app=None): - if app is not None: - DriverRegistry.load_drivers() - - drivers = getattr(app or current_app, '_pgadmin_server_drivers', None) - - if drivers is None or not isinstance(drivers, dict): - drivers = dict() +def get_driver(_type, app=None): - if type in drivers: - return drivers[type] - - driver = DriverRegistry.create(type) - - if driver is not None: - drivers[type] = driver - setattr(app or current_app, '_pgadmin_server_drivers', drivers) + if app is not None: + DriverRegistry.load_modules(app) - return driver + return DriverRegistry.get(_type) def init_app(app): drivers = dict() setattr(app, '_pgadmin_server_drivers', drivers) - DriverRegistry.load_drivers() + DriverRegistry.load_modules(app) return drivers diff --git a/web/pgadmin/utils/driver/registry.py b/web/pgadmin/utils/driver/registry.py index ae535429b..7ee5ee89b 100644 --- a/web/pgadmin/utils/driver/registry.py +++ b/web/pgadmin/utils/driver/registry.py @@ -9,80 +9,9 @@ from abc import ABCMeta -from flask_babelex import gettext +from pgadmin.utils.dynamic_registry import create_registry_metaclass -def _decorate_cls_name(module_name): - length = len(__package__) + 1 - - if len(module_name) > length and module_name.startswith(__package__): - return module_name[length:] - - return module_name - - -class DriverRegistry(ABCMeta): - """ - class DriverRegistry(object) - Every Driver will be registered automatically by its module name. - - This uses factory pattern to genreate driver object based on its name - automatically. - - Class-level Methods: - ----------- ------- - * __init__(...) - - It will be used to register type of drivers. You don't need to call - this function explicitly. This will be automatically executed, whenever - we create class and inherit from BaseDriver, it will register it as - available driver in DriverRegistry. Because - the __metaclass__ for - BaseDriver is set it to DriverRegistry, and it will create new instance - of this DriverRegistry per class. - - * create(type, *args, **kwargs) - - Create type of driver object for this server, from the available - driver list (if available, or raise exception). - - * load_drivers(): - - Use this function from init_app(...) to load all available drivers in - the registry. - """ - registry = None - drivers = dict() - - def __init__(self, name, bases, d): - - # Register this type of driver, based on the module name - # Avoid registering the BaseDriver itself - - if name != 'BaseDriver': - DriverRegistry.registry[_decorate_cls_name(d['__module__'])] = self - - ABCMeta.__init__(self, name, bases, d) - - @classmethod - def create(cls, name, **kwargs): - - if name in DriverRegistry.drivers: - return DriverRegistry.drivers[name] - - if name in DriverRegistry.registry: - DriverRegistry.drivers[name] = \ - (DriverRegistry.registry[name])(**kwargs) - return DriverRegistry.drivers[name] - - raise NotImplementedError( - gettext("Driver '{0}' has not been implemented.").format(name) - ) - - @classmethod - def load_drivers(cls): - # Initialize the registry only if it has not yet been initialized - if DriverRegistry.registry is None: - DriverRegistry.registry = dict() - - from importlib import import_module - from werkzeug.utils import find_modules - - for module_name in find_modules(__package__, True): - import_module(module_name) +DriverRegistry = create_registry_metaclass( + "DriverRegistry", __package__, decorate_as_module=True +) diff --git a/web/pgadmin/utils/dynamic_registry/__init__.py b/web/pgadmin/utils/dynamic_registry/__init__.py new file mode 100644 index 000000000..dabde3a95 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/__init__.py @@ -0,0 +1,104 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +######################################################################### + +"""This file contains functions for creating dynamic registry meta class.""" + +from abc import ABCMeta + + +# Constructor +def __constructor(self, name, bases, kwargs): + # Register this type of auth_sources, based on the module name + # Avoid registering the BaseAuthentication itself + cls = self.__class__ + entry = self._decorate_cls_name(name, kwargs) + + if cls._registry is None: + cls._registry = dict() + else: + if entry in cls._registry: + raise RuntimeError(( + "{} class is already been registered with {} " + "(package: {})" + ).format(entry, cls._name_, cls._package_)) + + if cls._initialized is True: + cls._registry[entry] = self + cls._initialized = True + + ABCMeta.__init__(self, name, bases, kwargs) + + +@classmethod +def __get(cls, name, **kwargs): + if name in cls._objects: + return cls._objects[name] + + if cls._registry is not None and name in cls._registry: + cls._objects[name] = (cls._registry[name])(**kwargs) + + return cls._objects[name] + + raise NotImplementedError( + "{} '{}' has not been implemented.".format(cls.__name__, name) + ) + + +@classmethod +def __load_modules(cls, app=None): + # Initialize the registry only if it has not yet been initialized + if cls._registry is None: + cls._registry = dict() + + from importlib import import_module + from werkzeug.utils import find_modules + + for module_name in find_modules(cls._package_, True): + module = import_module(module_name) + + if "init_app" in module.__dict__.keys(): + module.__dict__["init_app"](app) + + +def __get_module_name(self, name, kwargs): + module_name = kwargs["__module__"] + length = len(self._package_) + 1 + + if len(module_name) > length and module_name.startswith(self._package_): + return module_name[length:] + + return module_name + + +def __get_class_name(self, name, kwargs): + return name + + +def create_registry_metaclass(name, package, decorate_as_module=True): + + class_params = { + # constructor + "__init__": __constructor, + + # Class members + "_registry": None, + "_objects": dict(), + "_package_": package, + + # Member functions + "get": __get, + "load_modules": __load_modules, + "_name_": name, + "_decorate_cls_name": __get_module_name + if decorate_as_module is True else __get_class_name, + "_initialized": False, + } + + # Creating class dynamically + return type(package + "." + name, (ABCMeta, ), class_params) diff --git a/web/pgadmin/utils/dynamic_registry/tests/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/__init__.py new file mode 100644 index 000000000..89190b316 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/__init__.py @@ -0,0 +1,8 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py new file mode 100644 index 000000000..d6455b67f --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/__init__.py @@ -0,0 +1,99 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import create_registry_metaclass +from .registry import TestModuleRegistry, TestNamedRegistry, TestNameBase +from .test1 import TestModule1 + + +def test_load_modules_based_registry(): + TestModuleRegistry.load_modules() + + if len(TestModuleRegistry._registry) != 2: + return "Length of the registry should have been 2" + + if 'test1' not in TestModuleRegistry._registry: + return "'test1' is not found in the registry" + + if 'test2' not in TestModuleRegistry._registry: + return "'test2' is not found in the registry" + + obj_test1_1 = TestModuleRegistry.get('test1') + obj_test1_2 = TestModuleRegistry.get('test1') + obj_test2_1 = TestModuleRegistry.get('test2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if isinstance(obj_test1_1, TestModule1) is False: + return "Registry created wrong object" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for " \ + "different classes" + + +def test_load_classname_registry(): + TestNamedRegistry.load_modules() + + if 'ClassTestName1' not in TestNamedRegistry._registry: + return "'ClassTestName1' is not found in the registry" + + if 'ClassTestName2' not in TestNamedRegistry._registry: + return "'ClassTestName2' is not found in the registry" + + obj_test1_1 = TestNamedRegistry.get('ClassTestName1') + obj_test1_2 = TestNamedRegistry.get('ClassTestName1') + obj_test2_1 = TestNamedRegistry.get('ClassTestName2') + + if id(obj_test1_1) != id(obj_test1_2): + return "Registry has created two separate instances" + + if id(obj_test1_1) == id(obj_test2_1): + return "Registry should have created a separate instances for " \ + "different classes" + + try: + class ClassTestName1(TestNameBase): # NOSONAR + pass + + return "Expected an runtime error when using the same classname" + except RuntimeError: + pass + + +def test_empty_registry(): + + EmptyModuleRegistry = create_registry_metaclass( # NOSONAR + "EmptyModuleRegistry", __package__, decorate_as_module=True + ) + + if EmptyModuleRegistry._registry is not None: + return "Registry was supposed to be None" + + if EmptyModuleRegistry._initialized is not False: + return "Registry initialized flag should be false" + + +def test_create_base_class(): + RegistryWithBaseClass = create_registry_metaclass( # NOSONAR + 'RegistryWithBaseClass', __package__, decorate_as_module=False + ) + + @six.add_metaclass(RegistryWithBaseClass) + class TestBase(object): + pass + + registry = RegistryWithBaseClass._registry + + if registry is None or len(registry) != 0: + return "Registry was not supposed to be None, and empty" + + if RegistryWithBaseClass._initialized is False: + return "Registry initialized flag should not be true" diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py new file mode 100644 index 000000000..ac83beebc --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/registry.py @@ -0,0 +1,38 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +import six +from pgadmin.utils.dynamic_registry import create_registry_metaclass + + +TestModuleRegistry = create_registry_metaclass( + 'TestModuleRegistry', __package__, decorate_as_module=True +) + + +TestNamedRegistry = create_registry_metaclass( + 'TestRegistry', __package__, decorate_as_module=False +) + + [email protected]_metaclass(TestModuleRegistry) +class TestModuleBase(object): + pass + + [email protected]_metaclass(TestNamedRegistry) +class TestNameBase(object): + pass + + +class ClassTestName1(TestNameBase): + pass + + +class ClassTestName2(TestNameBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py new file mode 100644 index 000000000..8f8f25db0 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test1.py @@ -0,0 +1,15 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from .registry import TestModuleBase + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule1(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py new file mode 100644 index 000000000..c9b045895 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/registry/test2.py @@ -0,0 +1,14 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from .registry import TestModuleBase + + +# This class will be registered with TestModuleRegistry (registry) +class TestModule2(TestModuleBase): + pass diff --git a/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py new file mode 100644 index 000000000..52136b069 --- /dev/null +++ b/web/pgadmin/utils/dynamic_registry/tests/test_dynamic_registry.py @@ -0,0 +1,39 @@ +####################################################################### +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## +from pgadmin.utils.route import BaseTestGenerator +from .registry import test_load_modules_based_registry, test_empty_registry, \ + test_create_base_class, test_load_classname_registry + + +class TestDynamicRegistry(BaseTestGenerator): + + scenarios = [ + ( + "Check empty registry", + dict(test=test_empty_registry), + ), + ( + 'Load the registry based on the modules', + dict(test=test_load_modules_based_registry), + ), + ( + 'Load the registry based on the name of the classes', + dict(test=test_load_classname_registry), + ), + ( + "When created a base class registry is initialized", + dict(test=test_create_base_class), + ), + ] + + def runTest(self): + error = self.test() + + if error is not None: + self.fail(error) diff --git a/web/regression/python_test_utils/sql_template_test_base.py b/web/regression/python_test_utils/sql_template_test_base.py index e3a381675..2b9fe76b0 100644 --- a/web/regression/python_test_utils/sql_template_test_base.py +++ b/web/regression/python_test_utils/sql_template_test_base.py @@ -15,7 +15,7 @@ from pgadmin.utils.driver import DriverRegistry from pgadmin.utils.versioned_template_loader \ import get_version_mapping_directories -DriverRegistry.load_drivers() +DriverRegistry.load_modules() class SQLTemplateTestBase(BaseTestGenerator): ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-24 06:00 Akshay Joshi <[email protected]> parent: Ashesh Vashi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Akshay Joshi @ 2021-06-24 06:00 UTC (permalink / raw) To: Ashesh Vashi <[email protected]>; +Cc: pgadmin-hackers Thanks, the patch applied. On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi <[email protected]> wrote: > On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi < > [email protected]> wrote: > >> Hi Ashesh >> >> Following are the review comments: >> >> - Fixed PEP8 issues. >> >> Done. > >> >> - In "dynamic_registry/__init__.py" decorator @classmethod used for " >> *_get*" and "*_load_modules*" methods which are actually outside of >> the class. Even constructor also outside of the class. >> >> 'create_registry_metaclass' is not a class, but a method to create the > dynamic classes. > If I move these methods in 'create_registry_metaclass' method, SonarQube > raises issues about complexity of the functions, hence - they're best kept > outside of that method. > >> >> - Remove unused imports from "driver/registry.py" >> >> Done > >> >> - >> - Fixed sonarqube issues in >> "dynamic_registry/tests/registry/__init__.py" >> >> Done > As discussed, SonarQube is not able to understand that the result object > is a class, and not an object, hence - showing linter issues. > I've disabled them in those lines by adding the comment '# NOSNAR' at the > end. > > -- Thanks, Ashesh > >> >> On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi < >> [email protected]> wrote: >> >>> On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi < >>> [email protected]> wrote: >>> >>>> Hi Akshay, >>>> >>>> We do have a couple of classes, which does automatic registration of >>>> the base classes, >>>> and which creates single-ton objects for these base classes, when >>>> needed. >>>> >>>> I would be working on a patch sooner, which will be using similar >>>> functionality for loading >>>> the multi-factor authentication. >>>> >>>> I realized - it will be a duplicate code at three places for the same >>>> functionalities. >>>> Hence - I worked on refactoring this registry class. >>>> >>>> Please find the patch for the same. >>>> >>> Found issues - some test files were using the old function >>> 'Driver.load_drivers(...)'. >>> They're fixed now. >>> >>> -- Thanks, Ashesh >>> >>>> >>>> -- >>>> >>>> Thanks & Regards, >>>> >>>> Ashesh Vashi >>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>> <http://www.enterprisedb.com; >>>> >>>> >>>> *http://www.linkedin.com/in/asheshvashi* >>>> <http://www.linkedin.com/in/asheshvashi; >>>> >>> >> >> -- >> *Thanks & Regards* >> *Akshay Joshi* >> *pgAdmin Hacker | Principal Software Architect* >> *EDB Postgres <http://edbpostgres.com>* >> >> *Mobile: +91 976-788-8246* >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres <http://edbpostgres.com>* *Mobile: +91 976-788-8246* ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-24 10:21 Khushboo Vashi <[email protected]> parent: Akshay Joshi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Khushboo Vashi @ 2021-06-24 10:21 UTC (permalink / raw) To: Akshay Joshi <[email protected]>; +Cc: Ashesh Vashi <[email protected]>; pgadmin-hackers Hi, This patch introduced the server mode api test case failure, please find the attached patch to fix those as well as some of the old issues in the server mode. Patch by: Ashesh Vashi Thanks, Khushboo On Thu, Jun 24, 2021 at 11:31 AM Akshay Joshi <[email protected]> wrote: > Thanks, the patch applied. > > On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi < > [email protected]> wrote: > >> On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi < >> [email protected]> wrote: >> >>> Hi Ashesh >>> >>> Following are the review comments: >>> >>> - Fixed PEP8 issues. >>> >>> Done. >> >>> >>> - In "dynamic_registry/__init__.py" decorator @classmethod used for " >>> *_get*" and "*_load_modules*" methods which are actually outside of >>> the class. Even constructor also outside of the class. >>> >>> 'create_registry_metaclass' is not a class, but a method to create the >> dynamic classes. >> If I move these methods in 'create_registry_metaclass' method, SonarQube >> raises issues about complexity of the functions, hence - they're best kept >> outside of that method. >> >>> >>> - Remove unused imports from "driver/registry.py" >>> >>> Done >> >>> >>> - >>> - Fixed sonarqube issues in >>> "dynamic_registry/tests/registry/__init__.py" >>> >>> Done >> As discussed, SonarQube is not able to understand that the result object >> is a class, and not an object, hence - showing linter issues. >> I've disabled them in those lines by adding the comment '# NOSNAR' at the >> end. >> >> -- Thanks, Ashesh >> >>> >>> On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi < >>> [email protected]> wrote: >>> >>>> On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi < >>>> [email protected]> wrote: >>>> >>>>> Hi Akshay, >>>>> >>>>> We do have a couple of classes, which does automatic registration of >>>>> the base classes, >>>>> and which creates single-ton objects for these base classes, when >>>>> needed. >>>>> >>>>> I would be working on a patch sooner, which will be using similar >>>>> functionality for loading >>>>> the multi-factor authentication. >>>>> >>>>> I realized - it will be a duplicate code at three places for the same >>>>> functionalities. >>>>> Hence - I worked on refactoring this registry class. >>>>> >>>>> Please find the patch for the same. >>>>> >>>> Found issues - some test files were using the old function >>>> 'Driver.load_drivers(...)'. >>>> They're fixed now. >>>> >>>> -- Thanks, Ashesh >>>> >>>>> >>>>> -- >>>>> >>>>> Thanks & Regards, >>>>> >>>>> Ashesh Vashi >>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>>> <http://www.enterprisedb.com; >>>>> >>>>> >>>>> *http://www.linkedin.com/in/asheshvashi* >>>>> <http://www.linkedin.com/in/asheshvashi; >>>>> >>>> >>> >>> -- >>> *Thanks & Regards* >>> *Akshay Joshi* >>> *pgAdmin Hacker | Principal Software Architect* >>> *EDB Postgres <http://edbpostgres.com>* >>> >>> *Mobile: +91 976-788-8246* >>> >> > > -- > *Thanks & Regards* > *Akshay Joshi* > *pgAdmin Hacker | Principal Software Architect* > *EDB Postgres <http://edbpostgres.com>* > > *Mobile: +91 976-788-8246* > Attachments: [application/octet-stream] test_cases_server_mode.patch (6.2K, 3-test_cases_server_mode.patch) download | inline diff: diff --git a/web/pgadmin/browser/tests/test_change_password.py b/web/pgadmin/browser/tests/test_change_password.py index ffd499068..2d09c26bd 100644 --- a/web/pgadmin/browser/tests/test_change_password.py +++ b/web/pgadmin/browser/tests/test_change_password.py @@ -62,7 +62,7 @@ class ChangePasswordTestCase(BaseTestGenerator): new_password_confirm=( config_data['pgAdmin4_login_credentials'] ['new_password']), - respdata='Invalid password')), + respdata='Incorrect username or password')), # This test case checks for valid password ('TestCase for Changing Valid_Password', dict( diff --git a/web/pgadmin/browser/tests/test_kerberos_with_mocking.py b/web/pgadmin/browser/tests/test_kerberos_with_mocking.py index 6b61dc1d0..e67ced8c4 100644 --- a/web/pgadmin/browser/tests/test_kerberos_with_mocking.py +++ b/web/pgadmin/browser/tests/test_kerberos_with_mocking.py @@ -101,7 +101,7 @@ class KerberosLoginMockTestCase(BaseTestGenerator): del_crads = delCrads() - AuthSourceRegistry.registry['kerberos'].negotiate_start = MagicMock( + AuthSourceRegistry._registry['kerberos'].negotiate_start = MagicMock( return_value=[True, del_crads]) return del_crads diff --git a/web/pgadmin/browser/tests/test_ldap_with_mocking.py b/web/pgadmin/browser/tests/test_ldap_with_mocking.py index 30df7729e..92f5c70c7 100644 --- a/web/pgadmin/browser/tests/test_ldap_with_mocking.py +++ b/web/pgadmin/browser/tests/test_ldap_with_mocking.py @@ -57,13 +57,13 @@ class LDAPLoginMockTestCase(BaseTestGenerator): app_config.LDAP_BIND_USER = None app_config.LDAP_BIND_PASSWORD = None - @patch.object(AuthSourceRegistry.registry['ldap'], 'connect', + @patch.object(AuthSourceRegistry._registry['ldap'], 'connect', return_value=[True, "Done"]) - @patch.object(AuthSourceRegistry.registry['ldap'], 'search_ldap_user', + @patch.object(AuthSourceRegistry._registry['ldap'], 'search_ldap_user', return_value=[True, '']) def runTest(self, conn_mock_obj, search_mock_obj): """This function checks ldap login functionality.""" - AuthSourceRegistry.registry['ldap'].dedicated_user = False + AuthSourceRegistry._registry['ldap'].dedicated_user = False res = self.tester.login(self.username, self.password, True) respdata = 'Gravatar image for %s' % self.username self.assertTrue(respdata in res.data.decode('utf8')) diff --git a/web/pgadmin/browser/tests/test_login.py b/web/pgadmin/browser/tests/test_login.py index c7816156d..743d0099c 100644 --- a/web/pgadmin/browser/tests/test_login.py +++ b/web/pgadmin/browser/tests/test_login.py @@ -29,7 +29,7 @@ class LoginTestCase(BaseTestGenerator): ['login_username']), password=str(uuid.uuid4())[4:8], is_gravtar_image_check=False, - respdata='Invalid password')), + respdata='Incorrect username or password')), # This test case validates the empty password field ('Empty_Password', dict( @@ -45,13 +45,13 @@ class LoginTestCase(BaseTestGenerator): config_data['pgAdmin4_login_credentials'] ['login_password']), is_gravtar_image_check=False, - respdata='Email not provided')), + respdata='Email/Username is not valid')), # This test case validates empty email and password ('Empty_Credentials', dict( email='', password='', is_gravtar_image_check=False, - respdata='Email not provided')), + respdata='Email/Username is not valid')), # This test case validates the invalid/incorrect email id ('Invalid_Email', dict( @@ -60,14 +60,14 @@ class LoginTestCase(BaseTestGenerator): config_data['pgAdmin4_login_credentials'] ['login_password']), is_gravtar_image_check=False, - respdata='Specified user does not exist')), + respdata='Incorrect username or password')), # This test case validates invalid email and password ('Invalid_Credentials', dict( email=str(uuid.uuid4())[1:8] + '@xyz.com', password=str(uuid.uuid4())[4:8], is_gravtar_image_check=False, - respdata='Specified user does not exist')), + respdata='Incorrect username or password')), # This test case validates the valid/correct credentials and allow user # to login pgAdmin 4 @@ -106,8 +106,6 @@ class LoginTestCase(BaseTestGenerator): if self.is_gravtar_image_check: if app_config.SHOW_GRAVATAR_IMAGE: self.assertTrue(self.respdata in res.data.decode('utf8')) - else: - print(self.respdata_without_gravtar in res.data.decode('utf8')) else: self.assertTrue(self.respdata in res.data.decode('utf8')) diff --git a/web/pgadmin/browser/tests/test_reset_password.py b/web/pgadmin/browser/tests/test_reset_password.py index 1ce7ea69c..17af7c0b6 100644 --- a/web/pgadmin/browser/tests/test_reset_password.py +++ b/web/pgadmin/browser/tests/test_reset_password.py @@ -29,7 +29,7 @@ class ResetPasswordTestCase(BaseTestGenerator): # This test case validates the invalid/incorrect email field ('TestCase for Validating Invalid_Email', dict( email=str(uuid.uuid4())[1:8] + '@xyz.com', - respdata='Specified user does not exist')), + respdata='Incorrect username or password')), # This test case validates the valid email id ('TestCase for Validating Valid_Email', dict( diff --git a/web/regression/python_test_utils/csrf_test_client.py b/web/regression/python_test_utils/csrf_test_client.py index ca4120e18..5e5259014 100644 --- a/web/regression/python_test_utils/csrf_test_client.py +++ b/web/regression/python_test_utils/csrf_test_client.py @@ -76,6 +76,10 @@ class TestClient(testing.FlaskClient): b' value="([^"]*)">', res.data ) + if m is None: + # When login through Kerberos, we won't find the CSRF + return None + return m.group(1).decode("utf-8") def generate_csrf_token(self, *args, **kwargs): ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Refactor: Registry Classes @ 2021-06-24 14:32 Akshay Joshi <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Akshay Joshi @ 2021-06-24 14:32 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Ashesh Vashi <[email protected]>; pgadmin-hackers Thanks, the patch applied. On Thu, Jun 24, 2021 at 3:51 PM Khushboo Vashi < [email protected]> wrote: > Hi, > > This patch introduced the server mode api test case failure, please find > the attached patch to fix those as well as some of the old issues in the > server mode. > Patch by: Ashesh Vashi > > Thanks, > Khushboo > > > On Thu, Jun 24, 2021 at 11:31 AM Akshay Joshi < > [email protected]> wrote: > >> Thanks, the patch applied. >> >> On Wed, Jun 23, 2021 at 7:54 PM Ashesh Vashi < >> [email protected]> wrote: >> >>> On Wed, Jun 23, 2021 at 1:22 PM Akshay Joshi < >>> [email protected]> wrote: >>> >>>> Hi Ashesh >>>> >>>> Following are the review comments: >>>> >>>> - Fixed PEP8 issues. >>>> >>>> Done. >>> >>>> >>>> - In "dynamic_registry/__init__.py" decorator @classmethod used for >>>> "*_get*" and "*_load_modules*" methods which are actually outside >>>> of the class. Even constructor also outside of the class. >>>> >>>> 'create_registry_metaclass' is not a class, but a method to create the >>> dynamic classes. >>> If I move these methods in 'create_registry_metaclass' method, SonarQube >>> raises issues about complexity of the functions, hence - they're best kept >>> outside of that method. >>> >>>> >>>> - Remove unused imports from "driver/registry.py" >>>> >>>> Done >>> >>>> >>>> - >>>> - Fixed sonarqube issues in >>>> "dynamic_registry/tests/registry/__init__.py" >>>> >>>> Done >>> As discussed, SonarQube is not able to understand that the result object >>> is a class, and not an object, hence - showing linter issues. >>> I've disabled them in those lines by adding the comment '# NOSNAR' at >>> the end. >>> >>> -- Thanks, Ashesh >>> >>>> >>>> On Sat, Jun 19, 2021 at 11:27 AM Ashesh Vashi < >>>> [email protected]> wrote: >>>> >>>>> On Sat, Jun 19, 2021 at 1:39 AM Ashesh Vashi < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Akshay, >>>>>> >>>>>> We do have a couple of classes, which does automatic registration of >>>>>> the base classes, >>>>>> and which creates single-ton objects for these base classes, when >>>>>> needed. >>>>>> >>>>>> I would be working on a patch sooner, which will be using similar >>>>>> functionality for loading >>>>>> the multi-factor authentication. >>>>>> >>>>>> I realized - it will be a duplicate code at three places for the same >>>>>> functionalities. >>>>>> Hence - I worked on refactoring this registry class. >>>>>> >>>>>> Please find the patch for the same. >>>>>> >>>>> Found issues - some test files were using the old function >>>>> 'Driver.load_drivers(...)'. >>>>> They're fixed now. >>>>> >>>>> -- Thanks, Ashesh >>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> Thanks & Regards, >>>>>> >>>>>> Ashesh Vashi >>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>>>> <http://www.enterprisedb.com; >>>>>> >>>>>> >>>>>> *http://www.linkedin.com/in/asheshvashi* >>>>>> <http://www.linkedin.com/in/asheshvashi; >>>>>> >>>>> >>>> >>>> -- >>>> *Thanks & Regards* >>>> *Akshay Joshi* >>>> *pgAdmin Hacker | Principal Software Architect* >>>> *EDB Postgres <http://edbpostgres.com>* >>>> >>>> *Mobile: +91 976-788-8246* >>>> >>> >> >> -- >> *Thanks & Regards* >> *Akshay Joshi* >> *pgAdmin Hacker | Principal Software Architect* >> *EDB Postgres <http://edbpostgres.com>* >> >> *Mobile: +91 976-788-8246* >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres <http://edbpostgres.com>* *Mobile: +91 976-788-8246* ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2021-06-24 14:32 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-06-18 20:09 Refactor: Registry Classes Ashesh Vashi <[email protected]> 2021-06-19 05:57 ` Ashesh Vashi <[email protected]> 2021-06-23 07:52 ` Akshay Joshi <[email protected]> 2021-06-23 14:24 ` Ashesh Vashi <[email protected]> 2021-06-24 06:00 ` Akshay Joshi <[email protected]> 2021-06-24 10:21 ` Khushboo Vashi <[email protected]> 2021-06-24 14:32 ` Akshay Joshi <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox