fix(core): honour per-property settings in the overwrite policy

- rebuild the effective settings when per_property/overwrite is reassigned
- reject per_property combined with 'replace remote' / 'keep existing'
- normalise overwrite=None and the 'none' sentinel to the field default
- report a missing 'model' as a validation error, not an AttributeError
- stop rewriting the schema of an existing page under 'keep existing'

Author: LukasGold

src/osw/core.py

@@ -1337,7 +1337,13 @@ class OverwriteClassParam(OswBaseModel):
 
         @validator("per_property")
         def validate_per_property(cls, per_property, values):
+            if per_property is None:  # nothing to check, the fallback applies
+                return per_property
             model_ = values.get("model")
+            if model_ is None:
+                # 'model' itself did not validate; without it the property names
+                # below cannot be checked at all
+                raise ValueError("'model' is required to validate 'per_property'")
             field_names = list(model_.__fields__.keys())
             keys = per_property.keys()
             if not all(key in field_names for key in keys):
@@ -1348,36 +1354,68 @@ def validate_per_property(cls, per_property, values):
 
             return per_property
 
+        @classmethod
+        def _normalize_overwrite(cls, value):
+            """Replace the two non-policy values by the default setting.
+
+            Neither ``None`` nor the ``none`` sentinel is a policy:
+            ``get_overwrite_setting()`` would hand them to the merge, where they
+            match no branch and silently behave like 'false'.
+            """
+            if value is None or value is AddOverwriteClassOptions.none:
+                return cls.__fields__["overwrite"].get_default()
+            return value
+
         def __setattr__(self, key, value):
             """Called when setting an attribute"""
+            if key == "overwrite":
+                value = self._normalize_overwrite(value)
+            # the effective settings are derived from these three, so any of them
+            # changing has to rebuild them
+            if key not in ("model", "overwrite", "per_property"):
+                super().__setattr__(key, value)
+                return
+            previous = getattr(self, key)
             super().__setattr__(key, value)
-            if key == "per_property":
-                # compare value and self.per_property
-                if value != self.per_property and value is not None:
-                    self._per_property = {
-                        field_name: value.get(field_name, self.overwrite)
-                        for field_name in self.model.__fields__.keys()
-                    }
-            elif key == "overwrite" or key == "model":
-                if self.per_property is not None:
-                    self._per_property = {
-                        field_name: self.per_property.get(field_name, self.overwrite)
-                        for field_name in self.model.__fields__.keys()
-                    }
+            try:
+                self._sync_per_property()
+            except ValueError:
+                # _sync_per_property() rejects before it touches _per_property,
+                # so restoring the field is enough to undo the assignment. Leaving
+                # a rejected value in place would let it take effect later, on the
+                # next assignment that happens to be accepted.
+                super().__setattr__(key, previous)
+                raise
 
         def __init__(self, **data):
             """Called after validation. Sets the fallback for every property that
             has not been specified in per_property."""
             super().__init__(**data)
-            per_property_ = {}
-            if self.per_property is not None:
-                per_property_ = self.per_property
+            # routed through __setattr__, which normalizes and rebuilds
+            self.overwrite = self.overwrite
+            # todo: from class definition get properties with hidden /
+            #  read_only option  #  those can be safely overwritten - set the to True
+
+        def _sync_per_property(self) -> None:
+            """Rebuild the effective overwrite setting of every model field."""
+            if self.per_property and isinstance(
+                self.overwrite, AddOverwriteClassOptions
+            ):
+                # _apply_overwrite_policy() short-circuits on 'replace remote'
+                # and 'keep existing' before it looks at a single property, so
+                # this combination would discard 'per_property' silently. Check
+                # it here rather than in a validator so that it also holds when
+                # either field is reassigned after construction.
+                raise ValueError(
+                    f"'per_property' cannot be combined with overwrite="
+                    f"'{self.overwrite.value}', which acts on the entity as a "
+                    f"whole. Use an OverwriteOptions value for 'overwrite'."
+                )
+            per_property_ = self.per_property or {}
             self._per_property = {
                 field_name: per_property_.get(field_name, self.overwrite)
                 for field_name in self.model.__fields__.keys()
             }
-            # todo: from class definition get properties with hidden /
-            #  read_only option  #  those can be safely overwritten - set the to True
 
         def get_overwrite_setting(self, property_name: str) -> OverwriteOptions:
             """Returns the fallback overwrite option for the given field name"""
@@ -1773,7 +1811,18 @@ def store_entity_(
                     offline=param.offline,
                 )
             )
-            if len(meta_category_templates.keys()) > 0:
+            # _apply_overwrite_policy() returned the remote page untouched. The
+            # schema regeneration below writes the jsonschema slot regardless of
+            # the policy, which would edit a page the caller asked to keep.
+            kept_existing = (
+                page.exists
+                # mirrors the branch order of _apply_overwrite_policy(), which
+                # tests 'offline is True' before it tests 'keep existing'
+                and param.offline is not True
+                and overwrite_class_param.overwrite
+                == AddOverwriteClassOptions.keep_existing
+            )
+            if not kept_existing and len(meta_category_templates.keys()) > 0:
                 generated_schemas = {}
                 try:
                     jsondata = page.get_slot_content("jsondata")
@@ -1810,7 +1859,7 @@ def store_entity_(
                     )
                 ).aggregated_schema
                 page.set_slot_content("jsonschema", new_schema)
-            if param.offline is False:
+            if param.offline is False and not kept_existing:
                 page.edit(
                     param.edit_comment, bot_edit=param.bot_edit
                 )  # will set page.changed if the content of the page has changed