From 21c62e67d558f971169d328681652539f1a9e7df Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Sun, 28 Jan 2024 15:53:56 +0200 Subject: [PATCH] fix: log pollution is possible when long values saved to options --- Makefile | 2 +- ckanext/editable_config/model/option.py | 21 ++++++++++------ ckanext/editable_config/plugin.py | 6 ++++- ckanext/editable_config/shared.py | 24 ++++++++++++++----- ckanext/editable_config/tests/conftest.py | 7 ------ .../tests/logic/test_action.py | 10 ++++---- .../tests/model/test_option.py | 2 +- .../editable_config/tests/test_fixtures.py | 2 +- ckanext/editable_config/tests/test_shared.py | 2 +- pyproject.toml | 7 +++++- setup.cfg | 6 +++++ 11 files changed, 58 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 787e9b3..c548ee0 100644 --- a/Makefile +++ b/Makefile @@ -6,4 +6,4 @@ help: changelog: ## compile changelog - git changelog -c conventional -o CHANGELOG.md + git changelog diff --git a/ckanext/editable_config/model/option.py b/ckanext/editable_config/model/option.py index e0324c7..6e9a186 100644 --- a/ckanext/editable_config/model/option.py +++ b/ckanext/editable_config/model/option.py @@ -4,6 +4,7 @@ from typing import Any, cast import sqlalchemy as sa +from sqlalchemy.orm import Mapped from typing_extensions import Self import ckan.plugins.toolkit as tk @@ -13,13 +14,20 @@ from ckanext.editable_config import shared -class Option(tk.BaseModel): - __tablename__ = "editable_config_option" +class Option(tk.BaseModel): # pyright: ignore[reportUntypedBaseClass] + __table__ = sa.Table( + "editable_config_option", + tk.BaseModel.metadata, + sa.Column("key", sa.Text, primary_key=True), + sa.Column("value", sa.Text, nullable=False), + sa.Column("updated_at", sa.DateTime, nullable=False), + sa.Column("prev_value", sa.Text, nullable=False), + ) - key = sa.Column(sa.Text, primary_key=True) - value = sa.Column(sa.Text, nullable=False) - updated_at = sa.Column(sa.DateTime, nullable=False) - prev_value = sa.Column(sa.Text, nullable=False) + key: Mapped[str] + value: Mapped[str] + updated_at: Mapped[datetime] + prev_value: Mapped[str] @classmethod def get(cls, key: str) -> Self | None: @@ -32,7 +40,6 @@ def get(cls, key: str) -> Self | None: @classmethod def set(cls, key: str, value: Any) -> Self: """Create/update an option.""" - option: Self safe_value = shared.value_as_string(key, value) if option := cls.get(key): diff --git a/ckanext/editable_config/plugin.py b/ckanext/editable_config/plugin.py index 619200b..5db91ec 100644 --- a/ckanext/editable_config/plugin.py +++ b/ckanext/editable_config/plugin.py @@ -68,7 +68,11 @@ def configure(self, config_: CKANConfig): ) return - inspector = sa.inspect(model.meta.engine) + engine = model.meta.engine + if not engine: + return + + inspector = sa.inspect(engine) self._editable_config_enabled = inspector.has_table("editable_config_option") if not self._editable_config_enabled: log.critical( diff --git a/ckanext/editable_config/shared.py b/ckanext/editable_config/shared.py index 4735bce..ade03df 100644 --- a/ckanext/editable_config/shared.py +++ b/ckanext/editable_config/shared.py @@ -27,6 +27,18 @@ class OptionDict(TypedDict): prev_value: str +def shorten_for_log(value: Any, width: int = 80, placeholder: str = "...") -> str: + """Prepare value for logging. + + Trasforms value into string and truncates it to specified length, appending + placeholder if result was reduced in size. + """ + result = str(value) + if len(result) > width + len(placeholder): + result = result[:width] + placeholder + return result + + def get_declaration(key: str) -> DeclaredOption[Any] | None: """Return existing declaration or None.""" if key in cd: @@ -77,7 +89,7 @@ def convert_core_overrides(names: Iterable[str]): ) options = {op.key: op.value for op in q} - log.debug("Convert core overrides into editable config: %s", options) + log.debug("Convert core overrides into editable config: %s", list(options)) change( {"ignore_auth": True}, { @@ -168,8 +180,8 @@ def _apply_changes(self) -> int: log.debug( "Change %s from %s to %s", option.key, - tk.config[option.key], - option.value, + shorten_for_log(tk.config[option.key]), + shorten_for_log(option.value), ) tk.config[option.key] = option.value count += 1 @@ -190,8 +202,8 @@ def _remove_keys(self, keys: Collection[str] | None) -> int: log.debug( "Reset %s from %s to %s", key, - tk.config[key], - src_conf[key], + shorten_for_log(tk.config[key]), + shorten_for_log(src_conf[key]), ) tk.config[key] = src_conf[key] @@ -200,7 +212,7 @@ def _remove_keys(self, keys: Collection[str] | None) -> int: log.debug( "Remove %s with value %s", key, - tk.config[key], + shorten_for_log(tk.config[key]), ) tk.config.pop(key) else: diff --git a/ckanext/editable_config/tests/conftest.py b/ckanext/editable_config/tests/conftest.py index 146beda..ae1e78b 100644 --- a/ckanext/editable_config/tests/conftest.py +++ b/ckanext/editable_config/tests/conftest.py @@ -9,7 +9,6 @@ from ckan import model from ckan.tests import factories from ckan.tests.helpers import call_action - from ckanext.editable_config.model import Option from ckanext.editable_config.shared import apply_config_overrides @@ -20,12 +19,6 @@ def reset_last_check(): apply_config_overrides._last_check = None -@pytest.fixture(scope="session") -def reset_db_once(reset_db, migrate_db_for): - reset_db() - migrate_db_for("editable_config") - - @pytest.fixture() def clean_db(reset_db, migrate_db_for): reset_db() diff --git a/ckanext/editable_config/tests/logic/test_action.py b/ckanext/editable_config/tests/logic/test_action.py index b51a7a8..e3fe304 100644 --- a/ckanext/editable_config/tests/logic/test_action.py +++ b/ckanext/editable_config/tests/logic/test_action.py @@ -7,7 +7,7 @@ @pytest.mark.ckan_config(config.WHITELIST, ["ckan.site_title", "ckan.site_description"]) -@pytest.mark.usefixtures("with_plugins", "non_clean_db") +@pytest.mark.usefixtures("with_plugins", "clean_db") class TestList: def test_default(self): result = call_action("editable_config_list") @@ -18,7 +18,7 @@ def test_pattern(self): assert set(result) == {"ckan.site_title"} -@pytest.mark.usefixtures("with_plugins", "non_clean_db", "with_autoclean") +@pytest.mark.usefixtures("with_plugins", "clean_db", "with_autoclean") class TestUpdate: def test_empty(self): result = call_action("editable_config_update") @@ -50,7 +50,7 @@ def test_one_per_group(self, faker, option_factory, ckan_config): assert ckan_config[reset_option["key"]] is None -@pytest.mark.usefixtures("with_plugins", "non_clean_db", "with_autoclean") +@pytest.mark.usefixtures("with_plugins", "clean_db", "with_autoclean") class TestChange: def test_undeclared(self, option_factory, faker): """Undeclared options are not allowed.""" @@ -98,7 +98,7 @@ def test_additional_validators(self): ) -@pytest.mark.usefixtures("with_plugins", "non_clean_db", "with_autoclean") +@pytest.mark.usefixtures("with_plugins", "clean_db", "with_autoclean") class TestRevert: def test_revert_missing(self): with pytest.raises(tk.ObjectNotFound): @@ -114,7 +114,7 @@ def test_revert_initial(self, ckan_config, faker, option_factory): assert result["ckan.site_title"]["prev_value"] == updated -@pytest.mark.usefixtures("with_plugins", "non_clean_db", "with_autoclean") +@pytest.mark.usefixtures("with_plugins", "clean_db", "with_autoclean") class TestReset: def test_reset_missing(self): with pytest.raises(tk.ObjectNotFound): diff --git a/ckanext/editable_config/tests/model/test_option.py b/ckanext/editable_config/tests/model/test_option.py index 672bf52..99f21f9 100644 --- a/ckanext/editable_config/tests/model/test_option.py +++ b/ckanext/editable_config/tests/model/test_option.py @@ -8,7 +8,7 @@ from ckanext.editable_config.model import Option -@pytest.mark.usefixtures("with_plugins", "non_clean_db") +@pytest.mark.usefixtures("with_plugins", "clean_db") class TestOption: def test_get(self, faker, option_factory): """Option.get returns exition option or None.""" diff --git a/ckanext/editable_config/tests/test_fixtures.py b/ckanext/editable_config/tests/test_fixtures.py index efa01e5..35225a1 100644 --- a/ckanext/editable_config/tests/test_fixtures.py +++ b/ckanext/editable_config/tests/test_fixtures.py @@ -7,7 +7,7 @@ import ckan.plugins.toolkit as tk -@pytest.mark.usefixtures("with_plugins", "non_clean_db") +@pytest.mark.usefixtures("with_plugins", "clean_db") class TestOptionFactory: def test_key_and_value_required(self, option_factory): """OptionFactory requires key and value.""" diff --git a/ckanext/editable_config/tests/test_shared.py b/ckanext/editable_config/tests/test_shared.py index 60941f5..454b207 100644 --- a/ckanext/editable_config/tests/test_shared.py +++ b/ckanext/editable_config/tests/test_shared.py @@ -22,7 +22,7 @@ def test_value_as_string(): assert shared.value_as_string("ckan.plugins", ["hello", "world"]) == "hello world" -@pytest.mark.usefixtures("with_plugins", "non_clean_db") +@pytest.mark.usefixtures("with_plugins", "clean_db") class TestUpdater: def test_apply_new_updates(self, faker, ckan_config, freezer, autoclean_option): """New updates are applied.""" diff --git a/pyproject.toml b/pyproject.toml index cb58eeb..bb8912b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,6 +59,11 @@ filterwarnings = [ "ignore::DeprecationWarning", ] +[tool.git-changelog] +output = "CHANGELOG.md" +convention = "conventional" +parse-trailers = true + [tool.pyright] pythonVersion = "3.8" include = ["ckanext"] @@ -76,7 +81,7 @@ reportFunctionMemberAccess = true # non-standard member accesses for functions reportMissingImports = true reportMissingModuleSource = true reportMissingTypeStubs = false -reportImportCycles = true +reportImportCycles = false reportUnusedImport = true reportUnusedClass = true reportUnusedFunction = true diff --git a/setup.cfg b/setup.cfg index ca15373..35eecae 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,6 +30,12 @@ babel.extractors = ckan = ckan.lib.extract:extract_ckan [options.extras_require] +test = + pytest-ckan + +dev = + %(test)s + git-changelog [extract_messages] keywords = translate isPlural