fix(wtsite): repair the dangling combine_into reference

src/osw/wtsite.py

@@ -50,6 +50,31 @@
 }
 
 
+def _combine_into(update: dict, combined: dict) -> None:
+    """Merges update into combined in place, recursing into nested dicts
+
+    A nested dict is merged key by key, so keys only present in combined
+    survive. Any other value replaces what is already there.
+
+    Parameters
+    ----------
+    update
+        The dict to take the new keys and values from.
+    combined
+        The dict to merge into. Modified in place.
+    """
+    for key, value in update.items():
+        target = combined.get(key)
+        if isinstance(value, dict):
+            if not isinstance(target, dict):
+                # nothing to merge with, so start from an empty dict rather
+                # than storing a reference to the one in update
+                target = combined[key] = {}
+            _combine_into(value, target)
+        else:
+            combined[key] = value
+
+
 # Classes
 class WtSite:
     """A wrapper class of mwclient.Site, mainly to provide multi-slot page handling and
@@ -1698,14 +1723,19 @@ def get_value(self, jsonpath):
             res.append(match.value)
         return res
 
+    @staticmethod
     @deprecated("No longer supported")
-    def update_dict(self, combined: dict, update: dict) -> None:
-        for k, v in update.items():
-            if isinstance(v, dict):
-                # todo: fix reference for combine_into
-                wt.combine_into(v, combined.setdefault(k, {}))
-            else:
-                combined[k] = v
+    def update_dict(combined: dict, update: dict) -> None:
+        """Merges update into combined in place, recursing into nested dicts
+
+        Parameters
+        ----------
+        combined
+            The dict to merge into. Modified in place.
+        update
+            The dict to take the new keys and values from.
+        """
+        _combine_into(update, combined)
 
     @deprecated("No longer supported for replace=False")
     def set_value(self, jsonpath_match, value, replace=False):
@@ -1735,10 +1765,11 @@ def set_value(self, jsonpath_match, value, replace=False):
         # else: jsonpath_expr.update(d, value)
         matches = jsonpath_expr.find(d)
         for match in matches:
-            print(match.full_path)
+            # str(match.full_path) raises TypeError because the keys of d are the
+            # list indices, so this cannot be printed or logged as it stands
             # pprint(value)
             if not replace:
-                WtPage.update_dict(match.value, value)
+                _combine_into(value, match.value)
                 value = match.value
             # pprint(value)
             match.full_path.update_or_create(d, value)

tests/test_wtpage_update_dict.py

@@ -0,0 +1,113 @@
+"""Unit tests for WtPage.update_dict and its merge helper.
+
+Regression guard for #15: update_dict called wt.combine_into, a function that
+has never existed in this repository, so every nested dict raised
+AttributeError. It was also declared as an instance method but called as
+WtPage.update_dict(a, b) in set_value, which raised TypeError before the
+missing reference was ever reached.
+"""
+
+from typing import Any, Dict, Union
+
+from osw.wtsite import WtPage, _combine_into
+
+
+class OfflineWtPage(WtPage):
+    """A WtPage that never touches a wiki, copied from tests/test_osl.py."""
+
+    def __init__(self, wtSite: Any = None, title: str = None):
+        self.wtSite = wtSite
+        self.title = title
+        self.exists = True
+        self._original_content = ""
+        self.changed: bool = False
+        self._dict = []
+        self._slots: Dict[str, Union[str, dict]] = {"main": ""}
+        self._slots_changed: Dict[str, bool] = {"main": False}
+        self._content_model: Dict[str, str] = {"main": "wikitext"}
+
+
+def test_flat_values_are_replaced():
+    combined = {"a": 1, "b": 2}
+
+    _combine_into({"b": 3}, combined)
+
+    assert combined == {"a": 1, "b": 3}
+
+
+def test_nested_dicts_are_merged_not_replaced():
+    """The point of the recursion: 'keep' has to survive the merge."""
+    combined = {"outer": {"keep": 1, "change": 2}}
+
+    _combine_into({"outer": {"change": 3, "add": 4}}, combined)
+
+    assert combined == {"outer": {"keep": 1, "change": 3, "add": 4}}
+
+
+def test_merging_recurses_to_any_depth():
+    combined = {"a": {"b": {"c": {"keep": 1}}}}
+
+    _combine_into({"a": {"b": {"c": {"add": 2}}}}, combined)
+
+    assert combined == {"a": {"b": {"c": {"keep": 1, "add": 2}}}}
+
+
+def test_a_dict_replaces_a_scalar():
+    combined = {"a": "scalar"}
+
+    _combine_into({"a": {"b": 1}}, combined)
+
+    assert combined == {"a": {"b": 1}}
+
+
+def test_a_scalar_replaces_a_dict():
+    combined = {"a": {"b": 1}}
+
+    _combine_into({"a": "scalar"}, combined)
+
+    assert combined == {"a": "scalar"}
+
+
+def test_a_new_nested_key_is_copied_not_aliased():
+    """Otherwise editing the result would reach back into the update dict."""
+    update = {"a": {"b": 1}}
+    combined = {}
+
+    _combine_into(update, combined)
+    combined["a"]["b"] = 2
+
+    assert update == {"a": {"b": 1}}
+
+
+def test_keys_absent_from_update_are_untouched():
+    combined = {"a": 1}
+
+    _combine_into({}, combined)
+
+    assert combined == {"a": 1}
+
+
+def test_update_dict_merges_in_place_and_returns_none():
+    combined = {"outer": {"keep": 1}}
+
+    assert WtPage.update_dict(combined, {"outer": {"add": 2}}) is None
+    assert combined == {"outer": {"keep": 1, "add": 2}}
+
+
+def test_set_value_merges_into_the_existing_entry():
+    """set_value(replace=False) is the only caller, and it was broken."""
+    page = OfflineWtPage(title="Test")
+    page._dict = [{"Template": {"keep": "yes", "change": "old"}}]
+
+    page.set_value("$.*.Template", {"change": "new"})
+
+    assert page._dict == [{"Template": {"keep": "yes", "change": "new"}}]
+
+
+def test_set_value_with_replace_discards_the_existing_entry():
+    page = OfflineWtPage(title="Test")
+    page._dict = [{"Template": {"keep": "yes", "change": "old"}}]
+
+    page.set_value("$.*.Template", {"change": "new"}, replace=True)
+
+    assert page._dict == [{"Template": {"change": "new"}}]