public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashesh Vashi <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Refactor: Registry Classes
Date: Sat, 19 Jun 2021 11:27:19 +0530
Message-ID: <CAG7mmoxQfhVhGUC2ArMkCPUoWiBusu-z6=0PR8OiZfOMQpnWwg@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmoz=wOOc3QiZDZuSwonFue_sowcZRtZLK_M0o839ZPnJ_Q@mail.gmail.com>
References: <CAG7mmoz=wOOc3QiZDZuSwonFue_sowcZRtZLK_M0o839ZPnJ_Q@mail.gmail.com>
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):
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Refactor: Registry Classes
In-Reply-To: <CAG7mmoxQfhVhGUC2ArMkCPUoWiBusu-z6=0PR8OiZfOMQpnWwg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox