[checkmk-commits] Refactored page registration to base on standard class registry

Lars Michelsen lm at mathias-kettner.de
Mon Apr 8 09:20:24 CEST 2019


Module: check_mk
Branch: master
Commit: 7e457acbf19f77a35363350b81f4696ba77e1e46
URL:    http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=7e457acbf19f77a35363350b81f4696ba77e1e46

Author: Lars Michelsen <lm at mathias-kettner.de>
Date:   Sat Apr  6 14:24:07 2019 +0200

Refactored page registration to base on standard class registry

* All pages are registered using the previously existing
  register methods which act as compatibility layer for now
* Add test for the new direct registration of classes to
  page_registry

Next step is to replace all register_page_handler() calls

Change-Id: I1b0d340df6a9a02fa982940ce9d74aefb30b2974

---

 cmk/gui/pages.py                   | 118 ++++++++++++++++++++++++++-----------
 tests/unit/cmk/gui/test_pages.py   |  23 ++++++--
 tests/unit/cmk/gui/test_sidebar.py |  12 ----
 3 files changed, 101 insertions(+), 52 deletions(-)

diff --git a/cmk/gui/pages.py b/cmk/gui/pages.py
index 4672ad4..774cde3 100644
--- a/cmk/gui/pages.py
+++ b/cmk/gui/pages.py
@@ -26,57 +26,36 @@
 
 import abc
 import json
+import inspect
+import cmk.utils.plugin_registry
 from cmk.gui.globals import html
 import cmk.gui.config as config
 from cmk.gui.exceptions import MKException
 from cmk.gui.log import logger
 
-_pages = {}
 
+class Page(object):
+    __metaclass__ = abc.ABCMeta
 
-def register(path):
-    """Register a function to be called when the given URL is called.
-
-    In case you need to register some callable like staticmethods or
-    classmethods, you will have to use register_page_handler() directly
-    because this decorator can not deal with them.
-
-    It is essentially a decorator that calls register_page_handler().
-    """
-
-    def wrap(page_func):
-        if isinstance(page_func, (staticmethod, classmethod)):
-            raise NotImplementedError()
-
-        register_page_handler(path, page_func)
-        return page_func
-
-    return wrap
-
-
-def register_page_handler(path, page_func):
-    """Register a function to be called when the given URL is called."""
-    _pages[path] = page_func
-
+    @classmethod
+    #TODO: Use when we are using python3 abc.abstractmethod
+    def ident(cls):
+        raise NotImplementedError()
 
-def get_page_handler(name, dflt=None):
-    """Returns either the page handler registered for the given name or None
+    def handle_page(self):
+        self.page()
 
-    In case dflt is given it returns dflt instead of None when there is no
-    page handler for the requested name."""
-    return _pages.get(name, dflt)
+    @abc.abstractmethod
+    def page(self):
+        """Override this to implement the page functionality"""
+        raise NotImplementedError()
 
 
 # TODO: Clean up implicit _from_vars() procotocol
-class AjaxPage(object):
+class AjaxPage(Page):
     """Generic page handler that wraps page() calls into AJAX respones"""
     __metaclass__ = abc.ABCMeta
 
-    @abc.abstractmethod
-    def page(self):
-        """Override this to implement the page functionality"""
-        raise NotImplementedError()
-
     def __init__(self):
         super(AjaxPage, self).__init__()
         self._from_vars()
@@ -105,3 +84,70 @@ class AjaxPage(object):
             response = {"result_code": 1, "result": "%s" % e}
 
         html.write(json.dumps(response))
+
+
+class PageRegistry(cmk.utils.plugin_registry.ClassRegistry):
+    def plugin_base_class(self):
+        return Page
+
+    def plugin_name(self, plugin_class):
+        return plugin_class.ident()
+
+    def register_page(self, path):
+        def wrap(plugin_class):
+            if not inspect.isclass(plugin_class):
+                raise NotImplementedError()
+
+            plugin_class._ident = path
+            plugin_class.ident = classmethod(lambda cls: cls._ident)
+
+            self.register(plugin_class)
+            return plugin_class
+
+        return wrap
+
+
+page_registry = PageRegistry()
+
+
+# TODO: Refactor all call sites to sub classes of Page() and change the
+# registration to page_registry.register("path")
+def register(path):
+    """Register a function to be called when the given URL is called.
+
+    In case you need to register some callable like staticmethods or
+    classmethods, you will have to use register_page_handler() directly
+    because this decorator can not deal with them.
+
+    It is essentially a decorator that calls register_page_handler().
+    """
+
+    def wrap(wrapped_callable):
+        cls_name = "PageClass%s" % path.title().replace(":", "")
+        LegacyPageClass = type(cls_name, (Page,), {
+            "_wrapped_callable": (wrapped_callable,),
+            "page": lambda self: self._wrapped_callable[0]()
+        })
+
+        page_registry.register_page(path)(LegacyPageClass)
+        return lambda: LegacyPageClass().handle_page()
+
+    return wrap
+
+
+# TODO: replace all call sites by directly calling page_registry.register_page("path")
+def register_page_handler(path, page_func):
+    """Register a function to be called when the given URL is called."""
+    wrap = register(path)
+    return wrap(page_func)
+
+
+def get_page_handler(name, dflt=None):
+    """Returns either the page handler registered for the given name or None
+
+    In case dflt is given it returns dflt instead of None when there is no
+    page handler for the requested name."""
+    handle_class = page_registry.get(name, dflt)
+    if handle_class is None:
+        return None
+    return lambda: handle_class().handle_page()
diff --git a/tests/unit/cmk/gui/test_pages.py b/tests/unit/cmk/gui/test_pages.py
index 9bac39c..adb6e4a 100644
--- a/tests/unit/cmk/gui/test_pages.py
+++ b/tests/unit/cmk/gui/test_pages.py
@@ -6,7 +6,7 @@ import cmk.gui.pages
 
 @pytest.mark.usefixtures("load_plugins")
 def test_registered_pages():
-    assert sorted(cmk.gui.pages._pages) == sorted([
+    assert sorted(cmk.gui.pages.page_registry.keys()) == sorted([
         'add_bookmark',
         'ajax_activation_state',
         'ajax_add_visual',
@@ -147,10 +147,10 @@ def test_registered_pages():
 
 
 def test_pages_register(monkeypatch, capsys):
-    monkeypatch.setattr(cmk.gui.pages, "_pages", {})
+    monkeypatch.setattr(cmk.gui.pages, "page_registry", cmk.gui.pages.PageRegistry())
 
     @cmk.gui.pages.register("123handler")
-    def page_handler():
+    def page_handler():  # pylint: disable=unused-variable
         sys.stdout.write("123")
 
     handler = cmk.gui.pages.get_page_handler("123handler")
@@ -161,7 +161,7 @@ def test_pages_register(monkeypatch, capsys):
 
 
 def test_pages_register_handler(monkeypatch, capsys):
-    monkeypatch.setattr(cmk.gui.pages, "_pages", {})
+    monkeypatch.setattr(cmk.gui.pages, "page_registry", cmk.gui.pages.PageRegistry())
 
     class PageClass(object):
         def handle_page(self):
@@ -174,3 +174,18 @@ def test_pages_register_handler(monkeypatch, capsys):
 
     handler()
     assert capsys.readouterr()[0] == "234"
+
+
+def test_page_registry_register_page(monkeypatch, capsys):
+    page_registry = cmk.gui.pages.PageRegistry()
+
+    @page_registry.register_page("234handler")
+    class PageClass(cmk.gui.pages.Page):
+        def page(self):
+            sys.stdout.write("234")
+
+    handler = page_registry.get("234handler")
+    assert handler == PageClass
+
+    handler().handle_page()
+    assert capsys.readouterr()[0] == "234"
diff --git a/tests/unit/cmk/gui/test_sidebar.py b/tests/unit/cmk/gui/test_sidebar.py
index e13e1f0..7e7b310 100644
--- a/tests/unit/cmk/gui/test_sidebar.py
+++ b/tests/unit/cmk/gui/test_sidebar.py
@@ -216,10 +216,6 @@ def test_save_user_config_allowed(mocker, monkeypatch):
     save_user_file_mock.assert_called_once_with("sidebar", {"fold": True, "snapins": []})
 
 
-def test_ajax_fold_page():
-    assert cmk.gui.pages.get_page_handler("sidebar_fold") == sidebar.ajax_fold
-
-
 @pytest.mark.parametrize("origin_state,fold_var,set_state", [
     (False, "yes", True),
     (True, "", False),
@@ -247,10 +243,6 @@ def test_ajax_fold(register_builtin_html, mocker, origin_state, fold_var, set_st
     })
 
 
-def test_ajax_openclose_page():
-    assert cmk.gui.pages.get_page_handler("sidebar_openclose") == sidebar.ajax_openclose
-
-
 @pytest.mark.parametrize("origin_state,set_state", [
     ("open", "closed"),
     ("closed", "open"),
@@ -295,10 +287,6 @@ def test_ajax_openclose_close(register_builtin_html, mocker, origin_state, set_s
     })
 
 
-def test_move_snapin_page():
-    assert cmk.gui.pages.get_page_handler("sidebar_move_snapin") == sidebar.move_snapin
-
-
 def test_move_snapin_not_permitted(monkeypatch, mocker, register_builtin_html):
     monkeypatch.setattr(config.user, "may", lambda x: x != "general.configure_sidebar")
     m_load = mocker.patch.object(sidebar.UserSidebarConfig, "_load")



More information about the checkmk-commits mailing list