Module: check_mk
Branch: master
Commit: 7e457acbf19f77a35363350b81f4696ba77e1e46
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=7e457acbf19f77…
Author: Lars Michelsen <lm(a)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")