fix: config_section() returns a leaf; 3.6.0 changelog

caldav/config.py

@@ -11,15 +11,21 @@
 
 log = logging.getLogger("caldav")
 
-"""
-This configuration parsing code was just copied from my plann library (and will be removed from there at some point in the future).  Test coverage is poor as for now.
-"""
+## The config file format and its parsing belong to calendaring-config.  What
+## is left here is caldav's own: mapping a section to DAVClient keyword
+## arguments, resolving feature profiles, and the order of the sources
+## (explicit arguments, test server, environment, file).
+##
+## TODO: the public expand_config_section, config_section, read_config and
+## extract_conn_params_from_section are kept for plann, and should be
+## deprecated (with warnings) once plann gets its calendars from
+## get_calendars() (https://github.com/PyCalendar/plann/issues/84).
+##
+## TODO: verify that we have test code for all kinds of corner cases
+##
+## TODO: verify that the config format is well enough documented
 
-## This is being moved from my plann library.  The code itself will be introduced into caldav 2.0, but proper test code and documentation will come in a later release (2.1?)
 
-
-## TODO TODO TODO - write test code for all the corner cases
-## TODO TODO TODO - write documentation of config format
 def expand_config_section(config, section="default", blacklist=None):
     """
     The leaf sections that ``section`` expands to, as a list.
@@ -43,13 +49,13 @@ def expand_config_section(config, section="default", blacklist=None):
 
 
 def config_section(config, section="default"):
-    if section in config and "inherits" in config[section]:
-        ret = config_section(config, config[section]["inherits"])
-    else:
-        ret = {}
-    if section in config:
-        ret.update(config[section])
-    return ret
+    """Section ``section`` with what it ``inherits`` copied in, or ``{}`` when
+    there is no such section.  ``inherits`` and the parents' ``contains`` and
+    ``disable`` are left out; the section's own are kept.  The rules are
+    calendaring-config's ``merged_section``."""
+    if section not in config:
+        return {}
+    return calendaring_config.merged_section(config, section)
 
 
 def read_config(fn, interactive_error=False):
@@ -356,6 +362,9 @@ def _caldav_connections(
     return [params for params in found if params]
 
 
+## TODO: the test server selection below, get_test_setting and
+## get_all_test_servers are test tooling, not config; move them to
+## caldav/testing.py (planned for 3.6.1).
 def _get_test_server_config(
     name: str | None, environment: bool, config_file: str | None = None
 ) -> dict[str, Any] | None:
@@ -616,7 +625,12 @@ def get_all_test_servers(
 
     result: dict[str, dict[str, Any]] = {}
     for section_name in cfg:
-        section_data = config_section(cfg, section_name)
+        try:
+            section_data = config_section(cfg, section_name)
+        except calendaring_config.ConfigError as e:
+            ## A broken section that is not a test server must not hide the others
+            log.warning(f"config section {section_name} skipped: {e}")
+            continue
         if get_test_setting(section_data, "test_allowed"):
             conn_params = extract_conn_params_from_section(section_data, str(path), section_name)
             if conn_params:

docs/source/configfile.rst

@@ -186,7 +186,10 @@ sections share the same server or credentials:
         calendar_name: Tasks
 
 Inheritance is recursive: a section can inherit from a section that itself
-inherits from another.
+inherits from another.  A parent's ``contains`` and ``disable`` are not
+inherited.  Inheriting from a missing section, a cycle of ``inherits``, or a
+parent of another kind (a ``caldav_*`` section inheriting from a
+``kind: jmap`` one) is an error.
 
 Meta-sections (collections)
 ============================

pyproject.toml

@@ -85,7 +85,7 @@ dependencies = [
   "dnspython",
   "python-dateutil",
   "PyYAML",
-  "calendaring-config>=0.3", ## reads the config file, see issue 786
+  "calendaring-config>=0.5", ## reads the config file, see issue 786
 ]
 dynamic = ["version"]
 

tests/test_caldav_unit.py

@@ -4550,6 +4550,91 @@ def test_inherits_recursive(self):
         assert result["caldav_username"] == "alice"
         assert result["caldav_password"] == "secret"
 
+    def test_result_is_a_leaf(self):
+        """plann hands the result to extract_conn_params_from_section, which
+        refused it as long as the inherits key was left in"""
+        from caldav.config import config_section, extract_conn_params_from_section
+
+        config = {
+            "base": {"caldav_url": "https://dav.example.com/", "contains": ["x"]},
+            "default": {"inherits": "base", "caldav_user": "bob"},
+            "x": {},
+        }
+        result = config_section(config, "default")
+        assert "inherits" not in result and "contains" not in result
+        assert extract_conn_params_from_section(result) == {
+            "url": "https://dav.example.com/",
+            "username": "bob",
+        }
+
+    def test_test_server_with_inherits(self, tmp_path):
+        import json
+
+        from caldav.config import get_all_test_servers
+
+        config_file = tmp_path / "calendar.conf"
+        config_file.write_text(
+            json.dumps(
+                {
+                    "base": {"caldav_url": "https://dav.example.com/"},
+                    "srv": {"inherits": "base", "test_allowed": True},
+                }
+            )
+        )
+        assert get_all_test_servers(str(config_file))["srv"]["url"] == "https://dav.example.com/"
+
+    def test_broken_section_does_not_hide_test_servers(self, tmp_path):
+        import json
+
+        from caldav.config import get_all_test_servers
+
+        config_file = tmp_path / "calendar.conf"
+        config_file.write_text(
+            json.dumps(
+                {
+                    "srv": {"caldav_url": "https://dav.example.com/", "test_allowed": True},
+                    "broken": {"inherits": "nosuch"},
+                    "a": {"inherits": "b"},
+                    "b": {"inherits": "a"},
+                }
+            )
+        )
+        assert list(get_all_test_servers(str(config_file))) == ["srv"]
+
+    def test_own_contains_and_disable_are_kept(self):
+        """only the parents' contains and disable are dropped"""
+        from caldav.config import config_section
+
+        config = {"meta": {"contains": ["a"], "disable": True}, "a": {}}
+        assert config_section(config, "meta") == {"contains": ["a"], "disable": True}
+
+    def test_parent_disable_is_not_inherited(self):
+        from caldav.config import config_section
+
+        config = {
+            "base": {"caldav_url": "https://dav.example.com/", "disable": True},
+            "child": {"inherits": "base"},
+        }
+        assert config_section(config, "child") == {"caldav_url": "https://dav.example.com/"}
+
+    def test_kind_mismatch_is_refused(self):
+        import calendaring_config
+        import pytest
+
+        from caldav.config import config_section
+
+        config = {
+            "jmap": {"kind": "jmap", "url": "https://jmap.example.com/"},
+            "child": {"inherits": "jmap", "caldav_url": "https://dav.example.com/"},
+        }
+        with pytest.raises(calendaring_config.ConfigError):
+            config_section(config, "child")
+
+    def test_missing_section_is_empty(self):
+        from caldav.config import config_section
+
+        assert config_section({"a": {}}, "nosuch") == {}
+
 
 class TestExpandEnvVars:
     """Unit tests for caldav.config.expand_env_vars."""