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

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")