[checkmk-commits] Reduce direct access to Request.vars, only html.unstash_vars() left.

Module: check_mk Branch: master Commit: 3f32a8b60d38d761dbf4a4df0af242618eb995fb URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=3f32a8b60d38d7… Author: Sven Panne <sp(a)mathias-kettner.de> Date: Mon Jan 7 15:02:48 2019 +0100 Reduce direct access to Request.vars, only html.unstash_vars() left. Change-Id: Iea1d5786e8ae56a1dad01a2225f2e7f53a09af6d --- cmk/gui/crash_reporting.py | 2 +- cmk/gui/htmllib.py | 26 +++++++++------------- cmk/gui/http.py | 4 ++-- cmk/gui/login.py | 2 +- cmk/gui/plugins/views/availability.py | 6 ++--- cmk/gui/plugins/wato/utils/__init__.py | 2 +- cmk/gui/watolib.py | 2 +- .../cmk/gui/test_htmllib_TransactionManager.py | 8 +++---- 8 files changed, 22 insertions(+), 30 deletions(-) diff --git a/cmk/gui/crash_reporting.py b/cmk/gui/crash_reporting.py index ff3e86c..98ac397 100644 --- a/cmk/gui/crash_reporting.py +++ b/cmk/gui/crash_reporting.py @@ -476,7 +476,7 @@ def create_crash_dump_info_file(tar, what): "page": html.myfile + ".py", "vars": { key: "***" if value in ["password", "_password"] else value - for key, value in html.request.vars.iteritems() + for key, value in html.request.all_vars() }, "username": config.user.id, "user_agent": html.request.user_agent, diff --git a/cmk/gui/htmllib.py b/cmk/gui/htmllib.py index 6d43700..2d4996d 100644 --- a/cmk/gui/htmllib.py +++ b/cmk/gui/htmllib.py @@ -1131,10 +1131,6 @@ class html(HTMLGenerator): return self.request.var_utf8(varname, deflt) # TODO: Refactor call sites to html.request.* - def all_vars(self): - return self.request.all_vars() - - # TODO: Refactor call sites to html.request.* def all_varnames_with_prefix(self, prefix): return self.request.all_varnames_with_prefix(prefix) @@ -1163,7 +1159,7 @@ class html(HTMLGenerator): self.request.del_all_vars(prefix) def stash_vars(self): - self._var_stash.append(self.request.vars.copy()) + self._var_stash.append(dict(self.request.all_vars())) def unstash_vars(self): self.request.vars = self._var_stash.pop() @@ -1250,7 +1246,7 @@ class html(HTMLGenerator): raise MKUserError("request", _("Failed to parse JSON request: '%s': %s") % (json_request, e)) - for key, val in self.all_vars().items(): + for key, val in self.request.all_vars(): if key not in ["request", "output_format"] + exclude_vars: request[key] = val.decode("utf-8") @@ -1586,8 +1582,8 @@ class html(HTMLGenerator): # [('varname1', value1), ('varname2', value2) ] def makeuri(self, addvars, remove_prefix=None, filename=None, delvars=None): new_vars = [nv[0] for nv in addvars] - vars_ = [(v, self.var(v)) - for v in self.request.vars + vars_ = [(v, val) + for v, val in self.request.all_vars() if v[0] != "_" and v not in new_vars and (not delvars or v not in delvars)] if remove_prefix is not None: vars_ = [i for i in vars_ if not i[0].startswith(remove_prefix)] @@ -1940,10 +1936,9 @@ class html(HTMLGenerator): add_action_vars = args.get("add_action_vars", False) if varlist is not None: for var in varlist: - value = self.request.vars.get(var, "") - self.hidden_field(var, value) + self.hidden_field(var, self.request.var(var, "")) else: # add *all* get variables, that are not set by any input! - for var in self.request.vars: + for var, _val in self.request.all_vars(): if var not in self.form_vars and \ (var[0] != "_" or add_action_vars): # and var != "filled_in": self.hidden_field(var, self.get_unicode_input(var)) @@ -2176,7 +2171,7 @@ class html(HTMLGenerator): # Model error = self.user_errors.get(varname) - value = self.request.vars.get(varname, default_value) + value = self.request.var(varname, default_value) if not value: value = "" if error: @@ -2434,7 +2429,7 @@ class html(HTMLGenerator): self.open_center() self.open_div(class_="really") self.write_text(msg) - # FIXME: When this confirms another form, use the form name from self.request.vars() + # FIXME: When this confirms another form, use the form name from self.request.all_vars() self.begin_form("confirm", method=method, action=action, add_transid=add_transid) self.hidden_fields(add_action_vars=True) self.button("_do_confirm", _("Yes!"), "really") @@ -2914,12 +2909,11 @@ hy self.end_foldable_container() def debug_vars(self, prefix=None, hide_with_mouse=True, vars_=None): - if not vars_: - vars_ = self.request.vars + it = self.request.all_vars() if vars_ is None else vars_.iteritems() hover = "this.style.display=\'none\';" self.open_table(class_=["debug_vars"], onmouseover=hover if hide_with_mouse else None) self.tr(self.render_th(_("POST / GET Variables"), colspan="2")) - for name, value in sorted(vars_.items()): + for name, value in sorted(it): if name in ["_password", "password"]: value = "***" if not prefix or name.startswith(prefix): diff --git a/cmk/gui/http.py b/cmk/gui/http.py index 46453c6..c7c4cf4 100644 --- a/cmk/gui/http.py +++ b/cmk/gui/http.py @@ -60,7 +60,7 @@ class Request(object): values = [(k, vs) for k, vs in self._wrequest.values.lists() if _valid_varname(k)] # Last occurrence takes precedence, making appending to current URL simpler - # TODO: Some code *directly* accesses and modifies vars, remove that! + # TODO: html.unstash_vars() *directly* modifies vars, remove that! self.vars = {k: vs[-1] for k, vs in values} self._listvars = {k: vs for k, vs in values if len(vs) > 1} @@ -165,7 +165,7 @@ class Request(object): return val def all_vars(self): - return self.vars + return self.vars.iteritems() def all_varnames_with_prefix(self, prefix): for varname in self.vars: diff --git a/cmk/gui/login.py b/cmk/gui/login.py index bc186f4..6ddf4ec 100644 --- a/cmk/gui/login.py +++ b/cmk/gui/login.py @@ -211,7 +211,7 @@ def renew_cookie(cookie_name, username): if (html.myfile != 'logout' and not html.has_var('_ajaxid')) \ and cookie_name == auth_cookie_name(): auth_logger.debug( - "Renewing auth cookie (%s.py, vars: %r)" % (html.myfile, html.request.vars)) + "Renewing auth cookie (%s.py, vars: %r)" % (html.myfile, dict(html.request.all_vars()))) renew_auth_session(username) diff --git a/cmk/gui/plugins/views/availability.py b/cmk/gui/plugins/views/availability.py index a55dc0c..c5266f5 100644 --- a/cmk/gui/plugins/views/availability.py +++ b/cmk/gui/plugins/views/availability.py @@ -81,10 +81,8 @@ def get_availability_options_from_url(what): def render_availability_options(what): if html.var("_reset"): config.user.save_file("avoptions", {}) - for varname in html.request.vars.keys(): - if varname.startswith("avo_"): - html.del_var(varname) - html.del_var("avoptions") + html.request.del_all_vars("avo_") + html.del_var("avoptions") avoptions = availability.get_default_avoptions() diff --git a/cmk/gui/plugins/wato/utils/__init__.py b/cmk/gui/plugins/wato/utils/__init__.py index 71c78f9..1ced1c3a 100644 --- a/cmk/gui/plugins/wato/utils/__init__.py +++ b/cmk/gui/plugins/wato/utils/__init__.py @@ -928,7 +928,7 @@ class TimeperiodValuespec(ValueSpec): def render_input(self, varprefix, value): # The display mode differs when the valuespec is activated - vars_copy = html.request.vars.copy() + vars_copy = dict(html.request.all_vars()) # The timeperiod mode can be set by either the GUI switch or by the value itself # GUI switch overrules the information stored in the value diff --git a/cmk/gui/watolib.py b/cmk/gui/watolib.py index 2697e23..f185a29 100644 --- a/cmk/gui/watolib.py +++ b/cmk/gui/watolib.py @@ -2691,7 +2691,7 @@ class SearchFolder(BaseFolder): url_vars = [("host_search", "1")] + add_vars - for varname, value in html.all_vars().items(): + for varname, value in html.request.all_vars(): if varname.startswith("host_search_") \ or varname.startswith("_change"): url_vars.append((varname, value)) diff --git a/tests/unit/cmk/gui/test_htmllib_TransactionManager.py b/tests/unit/cmk/gui/test_htmllib_TransactionManager.py index 1de0fd7..10f8fa0 100644 --- a/tests/unit/cmk/gui/test_htmllib_TransactionManager.py +++ b/tests/unit/cmk/gui/test_htmllib_TransactionManager.py @@ -49,7 +49,7 @@ def test_transaction_valid(tm, transid, ignore_transids, result, monkeypatch, is assert tm._ignore_transids == True if transid is not None: - tm._request.vars["_transid"] = transid + tm._request.set_var("_transid", transid) assert tm._request.has_var("_transid") assert tm._request.var("_transid") == transid @@ -63,7 +63,7 @@ def test_transaction_valid(tm, transid, ignore_transids, result, monkeypatch, is def test_is_transaction(tm): assert not tm.is_transaction() - tm._request.vars["_transid"] = "123" + tm._request.set_var("_transid", "123") assert tm.is_transaction() @@ -72,7 +72,7 @@ def test_check_transaction_invalid(tm, monkeypatch): def test_check_transaction_valid(tm, monkeypatch, mocker): valid_transid = "%d/abc" % time.time() - tm._request.vars["_transid"] = valid_transid + tm._request.set_var("_transid", valid_transid) monkeypatch.setattr(tm, "_load_transids", lambda: [valid_transid]) @@ -83,7 +83,7 @@ def test_check_transaction_valid(tm, monkeypatch, mocker): def test_check_transaction_automation(tm, monkeypatch, mocker): tm.ignore() - tm._request.vars["_transid"] = "-1" + tm._request.set_var("_transid", "-1") invalidate = mocker.patch.object(tm, "_invalidate") assert tm.check_transaction() == True