fix: log pollution is possible when long values saved to options

Makefile

@@ -6,4 +6,4 @@ help:
 
 
 changelog:  ## compile changelog
-	git changelog -c conventional -o CHANGELOG.md
+	git changelog

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

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(

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:

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

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

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

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

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

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

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