From 217b1017b46295340bc79cee050a91c45c0d379d Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Tue, 15 Sep 2026 16:36:56 +0200 Subject: [PATCH 01/20] chore(user-sync): scaffold module, example and tracking --- examples/user_sync.py | 22 ++++++ src/osw/tools/__init__.py | 1 + src/osw/tools/user_sync/__init__.py | 22 ++++++ src/osw/tools/user_sync/config.py | 104 +++++++++++++++++++++++++++ src/osw/tools/user_sync/sync.py | 46 ++++++++++++ tests/tools/test_user_sync_config.py | 48 +++++++++++++ 6 files changed, 243 insertions(+) create mode 100644 examples/user_sync.py create mode 100644 src/osw/tools/__init__.py create mode 100644 src/osw/tools/user_sync/__init__.py create mode 100644 src/osw/tools/user_sync/config.py create mode 100644 src/osw/tools/user_sync/sync.py create mode 100644 tests/tools/test_user_sync_config.py diff --git a/examples/user_sync.py b/examples/user_sync.py new file mode 100644 index 00000000..3d9807e8 --- /dev/null +++ b/examples/user_sync.py @@ -0,0 +1,22 @@ +"""Create or update OSW User items from MediaWiki accounts and ORCID. + +Usage: + uv run python examples/user_sync.py --domain llm4eln.semos.dev --dry-run +""" + +import dotenv + +from osw.express import OswExpress +from osw.tools.user_sync import config_from_args, run_user_sync + + +def main() -> None: + dotenv.load_dotenv() + config = config_from_args() + osw = OswExpress(domain=config.domain, cred_filepath=config.cred_filepath) + report = run_user_sync(config, osw=osw) + print(report.summary()) + + +if __name__ == "__main__": + main() diff --git a/src/osw/tools/__init__.py b/src/osw/tools/__init__.py new file mode 100644 index 00000000..406eb4bd --- /dev/null +++ b/src/osw/tools/__init__.py @@ -0,0 +1 @@ +"""Maintenance and automation tools built on top of the osw core library.""" diff --git a/src/osw/tools/user_sync/__init__.py b/src/osw/tools/user_sync/__init__.py new file mode 100644 index 00000000..606890f8 --- /dev/null +++ b/src/osw/tools/user_sync/__init__.py @@ -0,0 +1,22 @@ +"""Sync OSW User items from MediaWiki accounts and public ORCID data.""" + +from __future__ import annotations + +from .config import ( + ORGANIZATION_CATEGORY, + USER_CATEGORY, + SyncConfig, + build_arg_parser, + config_from_args, +) +from .sync import SyncReport, run_user_sync + +__all__ = [ + "ORGANIZATION_CATEGORY", + "USER_CATEGORY", + "SyncConfig", + "SyncReport", + "build_arg_parser", + "config_from_args", + "run_user_sync", +] diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py new file mode 100644 index 00000000..5a8f8d4f --- /dev/null +++ b/src/osw/tools/user_sync/config.py @@ -0,0 +1,104 @@ +"""Configuration for the user-item sync tool.""" + +from __future__ import annotations + +import argparse +from dataclasses import dataclass, field +from typing import List, Optional + +# Category page titles of the target item types (see opensemantic.base.v1). +USER_CATEGORY = "Category:OSWd9aa0bca9b0040d8af6f5c091bf9eec7" +ORGANIZATION_CATEGORY = "Category:OSW1969007d5acf40539642877659a02c23" + +# Public ORCID API used to enrich ORCID users. +ORCID_API_BASE_DEFAULT = "https://pub.orcid.org/v3.0" + +# MediaWiki group whose members are excluded from the sync (bots). +BOT_GROUP = "bot" + + +@dataclass +class SyncConfig: + """Runtime options for a single sync run.""" + + domain: Optional[str] = None + cred_filepath: Optional[str] = None + dry_run: bool = False + assume_yes: bool = False + limit: Optional[int] = None + include_non_orcid: bool = True + exclude_bot_group: bool = True + create_redirects: bool = True + link_organizations: bool = True + orcid_api_base: str = ORCID_API_BASE_DEFAULT + excluded_groups: List[str] = field(default_factory=lambda: [BOT_GROUP]) + + +def build_arg_parser() -> argparse.ArgumentParser: + """Build the argument parser used by the example wrapper.""" + parser = argparse.ArgumentParser( + description="Create or update OSW User items from MediaWiki and ORCID.", + ) + parser.add_argument("--domain", help="Target OSL domain, e.g. llm4eln.semos.dev.") + parser.add_argument( + "--cred-filepath", + dest="cred_filepath", + help="Path to accounts.pwd.yaml (defaults to the working directory).", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Show the preview only; do not write anything.", + ) + parser.add_argument( + "--yes", + dest="assume_yes", + action="store_true", + help="Non-interactive: apply new items and gap-fills, keep existing on conflicts.", + ) + parser.add_argument( + "--limit", + type=int, + help="Process at most this many MediaWiki users (for testing).", + ) + parser.add_argument( + "--no-redirects", + dest="create_redirects", + action="store_false", + help="Do not create User: redirect pages.", + ) + parser.add_argument( + "--no-organizations", + dest="link_organizations", + action="store_false", + help="Do not resolve or link ORCID affiliations to Organization items.", + ) + parser.add_argument( + "--include-non-orcid", + dest="include_non_orcid", + action="store_true", + default=True, + help="Also sync non-bot accounts without an ORCID username (default).", + ) + parser.add_argument( + "--orcid-only", + dest="include_non_orcid", + action="store_false", + help="Sync only accounts whose username is an ORCID iD.", + ) + return parser + + +def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig: + """Parse command-line arguments into a SyncConfig.""" + args = build_arg_parser().parse_args(argv) + return SyncConfig( + domain=args.domain, + cred_filepath=args.cred_filepath, + dry_run=args.dry_run, + assume_yes=args.assume_yes, + limit=args.limit, + include_non_orcid=args.include_non_orcid, + create_redirects=args.create_redirects, + link_organizations=args.link_organizations, + ) diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py new file mode 100644 index 00000000..8001e235 --- /dev/null +++ b/src/osw/tools/user_sync/sync.py @@ -0,0 +1,46 @@ +"""Orchestrator for the user-item sync tool. + +Later phases implement enumeration, ORCID enrichment, reconciliation, the +interactive layer and the store step. This module currently defines the report +type and the public entry point. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Dict, List, Optional + +from osw.core import OSW + +from .config import SyncConfig + + +@dataclass +class SyncReport: + """Outcome of a sync run.""" + + created: List[str] = field(default_factory=list) + updated: List[str] = field(default_factory=list) + skipped: List[str] = field(default_factory=list) + failed: Dict[str, str] = field(default_factory=dict) + redirects_created: List[str] = field(default_factory=list) + organizations: List[str] = field(default_factory=list) + + def summary(self) -> str: + """One-line human-readable summary of the run.""" + return ( + f"created={len(self.created)} updated={len(self.updated)} " + f"skipped={len(self.skipped)} failed={len(self.failed)} " + f"redirects={len(self.redirects_created)} orgs={len(self.organizations)}" + ) + + +def run_user_sync(config: SyncConfig, osw: Optional[OSW] = None) -> SyncReport: + """Create or update OSW User items from MediaWiki accounts and ORCID data. + + Args: + config: Runtime options for the run. + osw: An authenticated OSW/OswExpress connection. Required for any wiki + access; the example wrapper builds one from ``config``. + """ + raise NotImplementedError("Implemented incrementally in phases 1 to 6.") diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py new file mode 100644 index 00000000..38f1c417 --- /dev/null +++ b/tests/tools/test_user_sync_config.py @@ -0,0 +1,48 @@ +"""Unit tests for the user-sync configuration and public surface.""" + +from osw.tools.user_sync import ( + ORGANIZATION_CATEGORY, + USER_CATEGORY, + SyncConfig, + SyncReport, + config_from_args, + run_user_sync, +) + + +def test_defaults(): + cfg = SyncConfig() + assert cfg.include_non_orcid is True + assert cfg.create_redirects is True + assert cfg.link_organizations is True + assert cfg.excluded_groups == ["bot"] + assert USER_CATEGORY.startswith("Category:OSW") + assert ORGANIZATION_CATEGORY.startswith("Category:OSW") + + +def test_config_from_args_parses_flags(): + cfg = config_from_args([ + "--domain", + "llm4eln.semos.dev", + "--dry-run", + "--limit", + "5", + "--orcid-only", + ]) + assert cfg.domain == "llm4eln.semos.dev" + assert cfg.dry_run is True + assert cfg.limit == 5 + assert cfg.include_non_orcid is False + + +def test_report_summary(): + report = SyncReport(created=["Item:OSW1"], updated=["Item:OSW2"]) + assert "created=1" in report.summary() + assert "updated=1" in report.summary() + + +def test_run_user_sync_is_stub(): + import pytest + + with pytest.raises(NotImplementedError): + run_user_sync(SyncConfig()) From 90bf3daf68639af2a1f133472ced70d0c8d2af71 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 10:17:43 +0200 Subject: [PATCH 02/20] feat(user-sync): enumerate and classify mediawiki users --- src/osw/tools/user_sync/sources.py | 108 ++++++++++++++++++++++++++ tests/tools/test_user_sync_sources.py | 104 +++++++++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 src/osw/tools/user_sync/sources.py create mode 100644 tests/tools/test_user_sync_sources.py diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py new file mode 100644 index 00000000..9a2b6975 --- /dev/null +++ b/src/osw/tools/user_sync/sources.py @@ -0,0 +1,108 @@ +"""External data sources for the user-item sync tool. + +This module wraps the MediaWiki ``allusers`` API. The ORCID public-API client is +added in a later phase. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass, field +from typing import Any, Dict, Iterable, Iterator, List, Optional, Sequence + +# An ORCID iD is 16 digits in groups of four; the last character may be an X. +ORCID_RE = re.compile(r"^\d{4}-\d{4}-\d{4}-\d{3}[\dX]$") + +_ALLUSERS_PROPS = "registration|editcount|groups|centralids|blockinfo" + + +def is_orcid_username(name: str) -> bool: + """True if a MediaWiki username is an ORCID iD (whitelisted ORCID login).""" + return bool(ORCID_RE.match(name or "")) + + +@dataclass +class MwUser: + """A MediaWiki account and the metadata exposed by the ``allusers`` API.""" + + name: str + userid: Optional[int] = None + registration: Optional[str] = None + editcount: int = 0 + groups: List[str] = field(default_factory=list) + blocked: bool = False + + @property + def is_orcid(self) -> bool: + return is_orcid_username(self.name) + + @property + def orcid_id(self) -> Optional[str]: + """The bare ORCID iD when the username is one, else None.""" + return self.name if self.is_orcid else None + + @classmethod + def from_api(cls, raw: Dict[str, Any]) -> MwUser: + return cls( + name=raw["name"], + userid=raw.get("userid"), + registration=raw.get("registration") or None, + editcount=int(raw.get("editcount") or 0), + groups=list(raw.get("groups") or []), + blocked="blockid" in raw, + ) + + +def _iter_allusers(site: Any, batch: Any = "max") -> Iterator[Dict[str, Any]]: + """Yield raw ``allusers`` records, following API continuation.""" + params: Dict[str, Any] = { + "list": "allusers", + "auprop": _ALLUSERS_PROPS, + "aulimit": batch, + } + while True: + resp = site.api("query", **params) + yield from resp.get("query", {}).get("allusers", []) + cont = resp.get("continue") + if not cont: + break + params.update(cont) + + +def enumerate_mw_users( + site: Any, + excluded_groups: Sequence[str] = ("bot",), + include_non_orcid: bool = True, + limit: Optional[int] = None, + batch: Any = "max", +) -> List[MwUser]: + """List MediaWiki accounts to sync. + + Args: + site: An object exposing ``api("query", ...)`` (an mwclient Site). + excluded_groups: Accounts in any of these groups are skipped (bots). + include_non_orcid: If False, keep only ORCID-username accounts. + limit: Keep at most this many accounts after filtering. + batch: ``aulimit`` value passed to the API. + """ + excluded = set(excluded_groups) + users: List[MwUser] = [] + for raw in _iter_allusers(site, batch=batch): + user = MwUser.from_api(raw) + if excluded.intersection(user.groups): + continue + if not include_non_orcid and not user.is_orcid: + continue + users.append(user) + if limit is not None and len(users) >= limit: + break + return users + + +def partition_by_orcid(users: Iterable[MwUser]) -> Dict[str, List[MwUser]]: + """Split users into ``{"orcid": [...], "other": [...]}``.""" + orcid: List[MwUser] = [] + other: List[MwUser] = [] + for user in users: + (orcid if user.is_orcid else other).append(user) + return {"orcid": orcid, "other": other} diff --git a/tests/tools/test_user_sync_sources.py b/tests/tools/test_user_sync_sources.py new file mode 100644 index 00000000..9045aac1 --- /dev/null +++ b/tests/tools/test_user_sync_sources.py @@ -0,0 +1,104 @@ +"""Unit tests for MediaWiki user enumeration and classification.""" + +from osw.tools.user_sync.sources import ( + MwUser, + enumerate_mw_users, + is_orcid_username, + partition_by_orcid, +) + + +class FakeSite: + """Mimics mwclient Site.api for list=allusers, with continuation.""" + + def __init__(self, pages): + self._pages = pages + self.calls = 0 + + def api(self, action, **params): + assert action == "query" + assert params["list"] == "allusers" + page = self._pages[self.calls] + self.calls += 1 + return page + + +def _page(users, cont=None): + resp = {"query": {"allusers": users}} + if cont is not None: + resp["continue"] = cont + return resp + + +def test_is_orcid_username(): + assert is_orcid_username("0000-0002-6374-9831") + assert is_orcid_username("0000-0003-0930-082X") + assert not is_orcid_username("Alice") + assert not is_orcid_username("") + + +def test_from_api_classifies_orcid_and_blocked(): + u = MwUser.from_api({ + "name": "0000-0002-6374-9831", + "userid": 16, + "registration": "2026-01-21T08:31:14Z", + "editcount": 37, + "groups": ["*", "user", "autoconfirmed"], + }) + assert u.is_orcid is True + assert u.orcid_id == "0000-0002-6374-9831" + assert u.editcount == 37 + assert u.blocked is False + + +def test_enumerate_follows_continuation_and_excludes_bots(): + site = FakeSite([ + _page( + [ + {"name": "0000-0002-6374-9831", "userid": 16, "groups": ["user"]}, + {"name": "SyncBot", "userid": 2, "groups": ["user", "bot"]}, + ], + cont={"aufrom": "M"}, + ), + _page([ + {"name": "Maintainer", "userid": 3, "groups": ["sysop"]}, + ]), + ]) + users = enumerate_mw_users(site) + names = [u.name for u in users] + assert names == ["0000-0002-6374-9831", "Maintainer"] # bot dropped + assert site.calls == 2 # continuation followed + + +def test_enumerate_orcid_only_and_limit(): + site = FakeSite([ + _page([ + {"name": "0000-0002-6374-9831", "groups": ["user"]}, + {"name": "Alice", "groups": ["user"]}, + {"name": "0000-0003-0930-082X", "groups": ["user"]}, + ]) + ]) + orcid_only = enumerate_mw_users(site, include_non_orcid=False) + assert [u.name for u in orcid_only] == [ + "0000-0002-6374-9831", + "0000-0003-0930-082X", + ] + + site2 = FakeSite([ + _page([ + {"name": "0000-0002-6374-9831", "groups": ["user"]}, + {"name": "Alice", "groups": ["user"]}, + ]) + ]) + limited = enumerate_mw_users(site2, limit=1) + assert len(limited) == 1 + + +def test_partition_by_orcid(): + users = [ + MwUser(name="0000-0002-6374-9831"), + MwUser(name="Alice"), + ] + parts = partition_by_orcid(users) + assert [u.name for u in parts["orcid"]] == ["0000-0002-6374-9831"] + assert [u.name for u in parts["other"]] == ["Alice"] From 5e40525b826713d37af47602d0542f957fba0e94 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 10:25:36 +0200 Subject: [PATCH 03/20] feat(user-sync): add orcid public api client --- src/osw/tools/user_sync/sources.py | 147 ++++++++++++++++++++++++++++ tests/tools/test_user_sync_orcid.py | 147 ++++++++++++++++++++++++++++ 2 files changed, 294 insertions(+) create mode 100644 tests/tools/test_user_sync_orcid.py diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py index 9a2b6975..32de437f 100644 --- a/src/osw/tools/user_sync/sources.py +++ b/src/osw/tools/user_sync/sources.py @@ -7,9 +7,14 @@ from __future__ import annotations import re +import time from dataclasses import dataclass, field from typing import Any, Dict, Iterable, Iterator, List, Optional, Sequence +import requests + +from .config import ORCID_API_BASE_DEFAULT + # An ORCID iD is 16 digits in groups of four; the last character may be an X. ORCID_RE = re.compile(r"^\d{4}-\d{4}-\d{4}-\d{3}[\dX]$") @@ -106,3 +111,145 @@ def partition_by_orcid(users: Iterable[MwUser]) -> Dict[str, List[MwUser]]: for user in users: (orcid if user.is_orcid else other).append(user) return {"orcid": orcid, "other": other} + + +class OrcidRateLimitError(RuntimeError): + """Raised when the ORCID public API keeps returning HTTP 429.""" + + +@dataclass +class OrcidAffiliation: + """An employment entry from an ORCID record.""" + + organization: str + ror_id: Optional[str] = None + department: Optional[str] = None + role: Optional[str] = None + + +@dataclass +class OrcidProfile: + """The subset of a public ORCID record that maps to an OSW User item.""" + + orcid_id: str + given_names: Optional[str] = None + family_name: Optional[str] = None + credit_name: Optional[str] = None + other_names: List[str] = field(default_factory=list) + emails: List[str] = field(default_factory=list) + urls: List[str] = field(default_factory=list) + affiliations: List[OrcidAffiliation] = field(default_factory=list) + + @property + def orcid_uri(self) -> str: + return f"https://orcid.org/{self.orcid_id}" + + @property + def display_name(self) -> Optional[str]: + """Best full name: credit name, else given + family, else None.""" + if self.credit_name: + return self.credit_name + parts = [p for p in (self.given_names, self.family_name) if p] + return " ".join(parts) or None + + +def _value(node: Any, *keys: str) -> Optional[Any]: + """Safely walk nested dicts, returning None on any missing key.""" + cur = node + for key in keys: + if not isinstance(cur, dict): + return None + cur = cur.get(key) + return cur + + +def _parse_affiliations(activities: Any) -> List[OrcidAffiliation]: + affiliations: List[OrcidAffiliation] = [] + groups = _value(activities, "employments", "affiliation-group") or [] + for group in groups: + for summary in group.get("summaries") or []: + emp = summary.get("employment-summary") or {} + org = emp.get("organization") or {} + name = org.get("name") + if not name: + continue + ror = None + disamb = org.get("disambiguated-organization") or {} + if (disamb.get("disambiguation-source") or "").upper() == "ROR": + ror = disamb.get("disambiguated-organization-identifier") + affiliations.append( + OrcidAffiliation( + organization=name, + ror_id=ror, + department=emp.get("department-name"), + role=emp.get("role-title"), + ) + ) + return affiliations + + +def parse_orcid_record(data: Dict[str, Any], orcid_id: str) -> OrcidProfile: + """Parse an ORCID ``/record`` JSON payload into an OrcidProfile (pure).""" + person = data.get("person") or {} + other_names = [ + n.get("content") + for n in _value(person, "other-names", "other-name") or [] + if n.get("content") + ] + emails = [ + e.get("email") + for e in _value(person, "emails", "email") or [] + if e.get("email") + ] + urls = [ + _value(u, "url", "value") + for u in _value(person, "researcher-urls", "researcher-url") or [] + if _value(u, "url", "value") + ] + return OrcidProfile( + orcid_id=orcid_id, + given_names=_value(person, "name", "given-names", "value"), + family_name=_value(person, "name", "family-name", "value"), + credit_name=_value(person, "name", "credit-name", "value"), + other_names=other_names, + emails=emails, + urls=urls, + affiliations=_parse_affiliations(data.get("activities-summary") or {}), + ) + + +def fetch_orcid_record( + orcid_id: str, + session: Optional[requests.Session] = None, + base: str = ORCID_API_BASE_DEFAULT, + timeout: float = 30.0, + max_retries: int = 2, + cache: Optional[Dict[str, Optional[OrcidProfile]]] = None, +) -> Optional[OrcidProfile]: + """Fetch and parse a public ORCID record. + + Returns None when the record does not exist (HTTP 404). Retries on HTTP 429 + honoring ``Retry-After`` and raises OrcidRateLimitError if still limited. + """ + if cache is not None and orcid_id in cache: + return cache[orcid_id] + sess = session or requests.Session() + url = f"{base}/{orcid_id}/record" + headers = {"Accept": "application/json"} + result: Optional[OrcidProfile] = None + for attempt in range(max_retries + 1): + resp = sess.get(url, headers=headers, timeout=timeout) + if resp.status_code == 404: + result = None + break + if resp.status_code == 429: + if attempt >= max_retries: + raise OrcidRateLimitError(f"ORCID rate limit for {orcid_id}") + time.sleep(float(resp.headers.get("Retry-After", 1))) + continue + resp.raise_for_status() + result = parse_orcid_record(resp.json(), orcid_id) + break + if cache is not None: + cache[orcid_id] = result + return result diff --git a/tests/tools/test_user_sync_orcid.py b/tests/tools/test_user_sync_orcid.py new file mode 100644 index 00000000..158a3db2 --- /dev/null +++ b/tests/tools/test_user_sync_orcid.py @@ -0,0 +1,147 @@ +"""Unit tests for the ORCID public-API client.""" + +import pytest +import requests + +from osw.tools.user_sync import sources +from osw.tools.user_sync.sources import ( + OrcidProfile, + OrcidRateLimitError, + fetch_orcid_record, + parse_orcid_record, +) + +ORCID_ID = "0000-0002-1825-0097" + +RECORD = { + "person": { + "name": { + "given-names": {"value": "Jane"}, + "family-name": {"value": "Doe"}, + "credit-name": {"value": "Jane A. Doe"}, + }, + "other-names": {"other-name": [{"content": "J. Doe"}]}, + "emails": {"email": [{"email": "jane@example.org"}]}, + "researcher-urls": { + "researcher-url": [{"url": {"value": "https://jane.example.org"}}] + }, + }, + "activities-summary": { + "employments": { + "affiliation-group": [ + { + "summaries": [ + { + "employment-summary": { + "department-name": "Physics", + "role-title": "Researcher", + "organization": { + "name": "Example University", + "disambiguated-organization": { + "disambiguated-organization-identifier": ( + "https://ror.org/01abc2345" + ), + "disambiguation-source": "ROR", + }, + }, + } + } + ] + } + ] + } + }, +} + + +class FakeResponse: + def __init__(self, status_code=200, json_data=None, headers=None): + self.status_code = status_code + self._json = json_data + self.headers = headers or {} + + def json(self): + return self._json + + def raise_for_status(self): + if self.status_code >= 400: + raise requests.HTTPError(f"status {self.status_code}") + + +class FakeSession: + def __init__(self, responses): + self._responses = list(responses) + self.calls = 0 + + def get(self, url, headers=None, timeout=None): + self.calls += 1 + return self._responses.pop(0) + + +def test_parse_record_full(): + p = parse_orcid_record(RECORD, ORCID_ID) + assert p.given_names == "Jane" + assert p.family_name == "Doe" + assert p.credit_name == "Jane A. Doe" + assert p.display_name == "Jane A. Doe" + assert p.other_names == ["J. Doe"] + assert p.emails == ["jane@example.org"] + assert p.urls == ["https://jane.example.org"] + assert p.orcid_uri == f"https://orcid.org/{ORCID_ID}" + assert len(p.affiliations) == 1 + aff = p.affiliations[0] + assert aff.organization == "Example University" + assert aff.ror_id == "https://ror.org/01abc2345" + assert aff.department == "Physics" + assert aff.role == "Researcher" + + +def test_parse_record_sparse(): + p = parse_orcid_record({"person": {}}, ORCID_ID) + assert p.given_names is None + assert p.display_name is None + assert p.emails == [] + assert p.affiliations == [] + + +def test_display_name_falls_back_to_given_family(): + p = OrcidProfile(orcid_id=ORCID_ID, given_names="Jane", family_name="Doe") + assert p.display_name == "Jane Doe" + + +def test_fetch_404_returns_none(): + session = FakeSession([FakeResponse(status_code=404)]) + assert fetch_orcid_record(ORCID_ID, session=session) is None + + +def test_fetch_200_parses(): + session = FakeSession([FakeResponse(status_code=200, json_data=RECORD)]) + p = fetch_orcid_record(ORCID_ID, session=session) + assert p.family_name == "Doe" + + +def test_fetch_retries_on_429(monkeypatch): + monkeypatch.setattr(sources.time, "sleep", lambda *_: None) + session = FakeSession([ + FakeResponse(status_code=429, headers={"Retry-After": "0"}), + FakeResponse(status_code=200, json_data=RECORD), + ]) + p = fetch_orcid_record(ORCID_ID, session=session) + assert p.given_names == "Jane" + assert session.calls == 2 + + +def test_fetch_raises_when_rate_limited(monkeypatch): + monkeypatch.setattr(sources.time, "sleep", lambda *_: None) + session = FakeSession([FakeResponse(status_code=429) for _ in range(3)]) + with pytest.raises(OrcidRateLimitError): + fetch_orcid_record(ORCID_ID, session=session, max_retries=2) + + +def test_fetch_uses_cache(): + session = FakeSession([FakeResponse(status_code=200, json_data=RECORD)]) + cache = {} + first = fetch_orcid_record(ORCID_ID, session=session, cache=cache) + second = fetch_orcid_record(ORCID_ID, session=session, cache=cache) + assert first is second + assert session.calls == 1 From 59b56109baf330e20aae1fc277c1381435fc6230 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 10:33:27 +0200 Subject: [PATCH 04/20] feat(user-sync): map sources to user and organization items --- src/osw/tools/user_sync/existing.py | 61 +++++++++++ src/osw/tools/user_sync/mapping.py | 146 +++++++++++++++++++++++++ tests/tools/test_user_sync_existing.py | 59 ++++++++++ tests/tools/test_user_sync_mapping.py | 112 +++++++++++++++++++ 4 files changed, 378 insertions(+) create mode 100644 src/osw/tools/user_sync/existing.py create mode 100644 src/osw/tools/user_sync/mapping.py create mode 100644 tests/tools/test_user_sync_existing.py create mode 100644 tests/tools/test_user_sync_mapping.py diff --git a/src/osw/tools/user_sync/existing.py b/src/osw/tools/user_sync/existing.py new file mode 100644 index 00000000..e9e9ac59 --- /dev/null +++ b/src/osw/tools/user_sync/existing.py @@ -0,0 +1,61 @@ +"""Load existing OSW User items so the sync can update in place. + +Matches are keyed on the ``username`` field, so accounts already stored under a +different uuid are updated rather than duplicated. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional + +from .config import USER_CATEGORY + + +@dataclass +class ExistingUsers: + """Existing User items indexed by their MediaWiki username.""" + + by_username: Dict[str, Any] = field(default_factory=dict) + + def get(self, username: str) -> Optional[Any]: + return self.by_username.get(username) + + +def _as_entity_list(result: Any) -> List[Any]: + """Normalize load_entity output (single, list, or LoadEntityResult).""" + if result is None: + return [] + entities = getattr(result, "entities", None) + if entities is not None: + return list(entities) + if isinstance(result, list): + return result + return [result] + + +def load_existing_users( + osw: Any, + category: str = USER_CATEGORY, + limit: Optional[int] = None, +) -> ExistingUsers: + """Query and load all existing User items, indexed by username. + + Args: + osw: An authenticated OSW connection exposing ``query_instances`` and + ``load_entity``. + category: The User category page title. + limit: Load at most this many items (for testing). + """ + titles = osw.query_instances(category=category) + if limit is not None: + titles = titles[:limit] + if not titles: + return ExistingUsers() + entities = _as_entity_list(osw.load_entity(titles)) + by_username: Dict[str, Any] = {} + for entity in entities: + username = getattr(entity, "username", None) + if username: + by_username[username] = entity + return ExistingUsers(by_username=by_username) diff --git a/src/osw/tools/user_sync/mapping.py b/src/osw/tools/user_sync/mapping.py new file mode 100644 index 00000000..e734c142 --- /dev/null +++ b/src/osw/tools/user_sync/mapping.py @@ -0,0 +1,146 @@ +"""Map MediaWiki accounts and ORCID profiles to proposed OSW items. + +These functions are pure: they compute the field values and deterministic ids of +the items to store, without instantiating the pydantic models (which requires an +open OSW connection) or touching the network. The store step builds the actual +``User`` / ``Organization`` entities from these proposals. +""" + +from __future__ import annotations + +import uuid +from dataclasses import dataclass, field +from typing import List, Optional, Tuple + +from osw.data.import_utility import uuid_to_full_page_title + +from .config import ORGANIZATION_CATEGORY, USER_CATEGORY +from .sources import MwUser, OrcidAffiliation, OrcidProfile + + +def _category_namespace(category: str) -> uuid.UUID: + """Derive the uuid5 namespace of a category from its page title.""" + return uuid.UUID(category.split("OSW")[-1]) + + +USER_NAMESPACE = _category_namespace(USER_CATEGORY) +ORGANIZATION_NAMESPACE = _category_namespace(ORGANIZATION_CATEGORY) + + +@dataclass +class ProposedOrganization: + """An Organization item derived from an ORCID affiliation.""" + + uuid: uuid.UUID + name: str + ror_id: Optional[str] = None + + @property + def full_page_title(self) -> str: + return uuid_to_full_page_title(self.uuid) + + +@dataclass +class ProposedUser: + """A User item derived from a MediaWiki account and optional ORCID data.""" + + uuid: uuid.UUID + username: str + first_name: str + surname: str + label: str + orcid: Optional[str] = None + emails: List[str] = field(default_factory=list) + websites: List[str] = field(default_factory=list) + organizations: List[str] = field(default_factory=list) + placeholder_name: bool = False + + @property + def full_page_title(self) -> str: + return uuid_to_full_page_title(self.uuid) + + +def normalize_org_name(name: str) -> str: + """Collapse whitespace and lowercase, for a stable org identity key.""" + return " ".join(name.lower().split()) + + +def user_uuid(mw_user: MwUser) -> uuid.UUID: + """Deterministic uuid5: keyed on ORCID iD for ORCID users, else username.""" + key = mw_user.orcid_id or mw_user.name + return uuid.uuid5(USER_NAMESPACE, key) + + +def organization_uuid(affiliation: OrcidAffiliation) -> uuid.UUID: + """Deterministic uuid5: keyed on ROR id when present, else the org name.""" + key = affiliation.ror_id or normalize_org_name(affiliation.organization) + return uuid.uuid5(ORGANIZATION_NAMESPACE, key) + + +def map_organization(affiliation: OrcidAffiliation) -> ProposedOrganization: + return ProposedOrganization( + uuid=organization_uuid(affiliation), + name=affiliation.organization, + ror_id=affiliation.ror_id, + ) + + +def _derive_names( + profile: Optional[OrcidProfile], username: str +) -> Tuple[str, str, str, bool]: + """Return (first_name, surname, label, is_placeholder). + + Prefers ORCID given/family names, then a split of the display name, then a + username-based placeholder when no name information is available. + """ + given = profile.given_names if profile else None + family = profile.family_name if profile else None + display = profile.display_name if profile else None + if given and family: + return given, family, display or f"{given} {family}", False + if display: + parts = display.split() + first = given or parts[0] + surname = family or (parts[-1] if len(parts) > 1 else parts[0]) + return first, surname, display, False + if given or family: + first = given or family + surname = family or given + return first, surname, f"{first} {surname}", False + return username, username, username, True + + +def map_user( + mw_user: MwUser, + profile: Optional[OrcidProfile] = None, + link_organizations: bool = True, +) -> Tuple[ProposedUser, List[ProposedOrganization]]: + """Build a ProposedUser (and any linked organizations) from the sources.""" + orcid_uri = f"https://orcid.org/{mw_user.orcid_id}" if mw_user.orcid_id else None + first, surname, label, placeholder = _derive_names(profile, mw_user.name) + + organizations: List[ProposedOrganization] = [] + org_titles: List[str] = [] + if link_organizations and profile: + seen = set() + for affiliation in profile.affiliations: + org = map_organization(affiliation) + if org.uuid in seen: + continue + seen.add(org.uuid) + organizations.append(org) + org_titles.append(org.full_page_title) + + proposed = ProposedUser( + uuid=user_uuid(mw_user), + username=mw_user.name, + first_name=first, + surname=surname, + label=label, + orcid=orcid_uri, + emails=list(profile.emails) if profile else [], + websites=list(profile.urls) if profile else [], + organizations=org_titles, + placeholder_name=placeholder, + ) + return proposed, organizations diff --git a/tests/tools/test_user_sync_existing.py b/tests/tools/test_user_sync_existing.py new file mode 100644 index 00000000..d584866b --- /dev/null +++ b/tests/tools/test_user_sync_existing.py @@ -0,0 +1,59 @@ +"""Unit tests for loading and indexing existing User items.""" + +from types import SimpleNamespace + +from osw.tools.user_sync.existing import load_existing_users + + +class FakeOsw: + def __init__(self, titles, entities): + self._titles = titles + self._entities = entities + self.loaded = None + + def query_instances(self, category): + assert category.startswith("Category:OSW") + return list(self._titles) + + def load_entity(self, titles): + self.loaded = list(titles) + return [self._entities[t] for t in titles] + + +def test_load_indexes_by_username(): + entities = { + "Item:OSW1": SimpleNamespace(username="0000-0002-6374-9831"), + "Item:OSW2": SimpleNamespace(username="Alice"), + } + osw = FakeOsw(["Item:OSW1", "Item:OSW2"], entities) + existing = load_existing_users(osw) + assert set(existing.by_username) == {"0000-0002-6374-9831", "Alice"} + assert existing.get("Alice") is entities["Item:OSW2"] + assert existing.get("missing") is None + + +def test_load_empty(): + osw = FakeOsw([], {}) + existing = load_existing_users(osw) + assert existing.by_username == {} + + +def test_load_skips_entities_without_username(): + entities = { + "Item:OSW1": SimpleNamespace(username=None), + "Item:OSW2": SimpleNamespace(username="Bob"), + } + osw = FakeOsw(["Item:OSW1", "Item:OSW2"], entities) + existing = load_existing_users(osw) + assert set(existing.by_username) == {"Bob"} + + +def test_load_respects_limit(): + entities = { + "Item:OSW1": SimpleNamespace(username="A"), + "Item:OSW2": SimpleNamespace(username="B"), + } + osw = FakeOsw(["Item:OSW1", "Item:OSW2"], entities) + existing = load_existing_users(osw, limit=1) + assert osw.loaded == ["Item:OSW1"] + assert set(existing.by_username) == {"A"} diff --git a/tests/tools/test_user_sync_mapping.py b/tests/tools/test_user_sync_mapping.py new file mode 100644 index 00000000..8c86abef --- /dev/null +++ b/tests/tools/test_user_sync_mapping.py @@ -0,0 +1,112 @@ +"""Unit tests for mapping sources to proposed User/Organization items.""" + +import uuid + +from osw.tools.user_sync import mapping +from osw.tools.user_sync.mapping import ( + ProposedUser, + map_organization, + map_user, + normalize_org_name, + organization_uuid, + user_uuid, +) +from osw.tools.user_sync.sources import MwUser, OrcidAffiliation, OrcidProfile + +ORCID = "0000-0002-6374-9831" + + +def _profile(**kw): + base = dict(orcid_id=ORCID, given_names="Lukas", family_name="Koschmieder") + base.update(kw) + return OrcidProfile(**base) + + +def test_map_orcid_user_full_profile(): + mw = MwUser(name=ORCID) + profile = _profile( + emails=["l@example.org"], + urls=["https://example.org"], + affiliations=[OrcidAffiliation(organization="Example University")], + ) + proposed, orgs = map_user(mw, profile) + assert isinstance(proposed, ProposedUser) + assert proposed.username == ORCID + assert proposed.orcid == f"https://orcid.org/{ORCID}" + assert proposed.first_name == "Lukas" + assert proposed.surname == "Koschmieder" + assert proposed.label == "Lukas Koschmieder" + assert proposed.emails == ["l@example.org"] + assert proposed.websites == ["https://example.org"] + assert proposed.placeholder_name is False + assert len(orgs) == 1 + assert proposed.organizations == [orgs[0].full_page_title] + assert proposed.full_page_title.startswith("Item:OSW") + + +def test_map_orcid_user_without_profile_uses_placeholder_names(): + mw = MwUser(name=ORCID) + proposed, orgs = map_user(mw, None) + assert proposed.orcid == f"https://orcid.org/{ORCID}" + assert proposed.first_name == ORCID + assert proposed.surname == ORCID + assert proposed.placeholder_name is True + assert orgs == [] + + +def test_map_non_orcid_user(): + mw = MwUser(name="Alice") + proposed, _orgs = map_user(mw, None) + assert proposed.orcid is None + assert proposed.placeholder_name is True + assert proposed.uuid == uuid.uuid5(mapping.USER_NAMESPACE, "Alice") + + +def test_user_uuid_is_deterministic_and_orcid_keyed(): + mw = MwUser(name=ORCID) + expected = uuid.uuid5(mapping.USER_NAMESPACE, ORCID) + assert user_uuid(mw) == expected + assert user_uuid(mw) == user_uuid(MwUser(name=ORCID)) + + +def test_credit_name_split_when_no_given_family(): + mw = MwUser(name=ORCID) + profile = OrcidProfile( + orcid_id=ORCID, given_names=None, family_name=None, credit_name="Jane A. Doe" + ) + proposed, _ = map_user(mw, profile) + assert proposed.first_name == "Jane" + assert proposed.surname == "Doe" + assert proposed.label == "Jane A. Doe" + assert proposed.placeholder_name is False + + +def test_organization_uuid_prefers_ror_over_name(): + ror = OrcidAffiliation(organization="Example U", ror_id="https://ror.org/01") + named = OrcidAffiliation(organization="Example U") + assert organization_uuid(ror) != organization_uuid(named) + assert organization_uuid(named) == uuid.uuid5( + mapping.ORGANIZATION_NAMESPACE, normalize_org_name("Example U") + ) + assert map_organization(ror).ror_id == "https://ror.org/01" + + +def test_duplicate_affiliations_deduped(): + mw = MwUser(name=ORCID) + profile = _profile( + affiliations=[ + OrcidAffiliation(organization="Example University"), + OrcidAffiliation(organization="example university"), + ] + ) + proposed, orgs = map_user(mw, profile) + assert len(orgs) == 1 + assert len(proposed.organizations) == 1 + + +def test_link_organizations_disabled(): + mw = MwUser(name=ORCID) + profile = _profile(affiliations=[OrcidAffiliation(organization="Example U")]) + proposed, orgs = map_user(mw, profile, link_organizations=False) + assert orgs == [] + assert proposed.organizations == [] From 54711e96f36c74ac125a4411b7b2cf7b33d23d04 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 10:39:41 +0200 Subject: [PATCH 05/20] feat(user-sync): add reconciliation engine --- src/osw/tools/user_sync/reconcile.py | 161 ++++++++++++++++++++++++ tests/tools/test_user_sync_reconcile.py | 137 ++++++++++++++++++++ 2 files changed, 298 insertions(+) create mode 100644 src/osw/tools/user_sync/reconcile.py create mode 100644 tests/tools/test_user_sync_reconcile.py diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py new file mode 100644 index 00000000..3218448f --- /dev/null +++ b/src/osw/tools/user_sync/reconcile.py @@ -0,0 +1,161 @@ +"""Reconcile proposed User items against the ones already stored. + +Pure logic: given proposed users and an index of existing items, classify each +into NEW, GAP_FILL, CONFLICT or UNCHANGED and record the per-field differences. +The interactive layer consumes this plan; nothing here does IO. +""" + +from __future__ import annotations + +from collections import Counter +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional + +from .existing import ExistingUsers +from .mapping import ProposedUser + +NEW = "new" +GAP_FILL = "gap_fill" +CONFLICT = "conflict" +UNCHANGED = "unchanged" + +# username is the match key and is never reconciled as a value. +SCALAR_FIELDS = ("label", "first_name", "surname", "orcid") +SET_FIELDS = ("emails", "websites", "organizations") +URL_FIELDS = ("orcid", "websites") +RECONCILED_FIELDS = SCALAR_FIELDS + SET_FIELDS + + +def _is_empty(value: Any) -> bool: + if value is None or value == "": + return True + return isinstance(value, (list, set, tuple)) and len(value) == 0 + + +def _norm_url(value: Any) -> Optional[str]: + return None if value is None else str(value).rstrip("/") + + +def _norm_scalar(name: str, value: Any) -> Optional[str]: + if value is None: + return None + return _norm_url(value) if name in URL_FIELDS else str(value) + + +def _norm_set(name: str, value: Any) -> set: + if not value: + return set() + if name in URL_FIELDS: + return {_norm_url(x) for x in value} + return {str(x) for x in value} + + +def _existing_label(entity: Any) -> Optional[str]: + for label in getattr(entity, "label", None) or []: + text = getattr(label, "text", None) + if text: + return text + return None + + +def existing_fields(entity: Any) -> Dict[str, Any]: + """Normalized comparable field view of a loaded User entity.""" + return { + "label": _existing_label(entity), + "first_name": getattr(entity, "first_name", None), + "surname": getattr(entity, "surname", None), + "orcid": _norm_scalar("orcid", getattr(entity, "orcid", None)), + "emails": _norm_set("emails", getattr(entity, "email", None)), + "websites": _norm_set("websites", getattr(entity, "website", None)), + "organizations": _norm_set( + "organizations", getattr(entity, "organization", None) + ), + } + + +def proposed_fields(proposed: ProposedUser) -> Dict[str, Any]: + """Normalized comparable field view of a proposed user.""" + return { + "label": proposed.label, + "first_name": proposed.first_name, + "surname": proposed.surname, + "orcid": _norm_scalar("orcid", proposed.orcid), + "emails": _norm_set("emails", proposed.emails), + "websites": _norm_set("websites", proposed.websites), + "organizations": _norm_set("organizations", proposed.organizations), + } + + +@dataclass +class FieldDiff: + """A single field that would change on an existing item.""" + + name: str + existing: Any + proposed: Any + status: str # GAP_FILL or CONFLICT + + +@dataclass +class UserChange: + """The reconciliation outcome for one proposed user.""" + + proposed: ProposedUser + existing: Optional[Any] + category: str + diffs: List[FieldDiff] = field(default_factory=list) + + +def _compare(existing_val: Any, proposed_val: Any) -> Optional[str]: + """Per-field status, or None when there is nothing to propose.""" + if _is_empty(proposed_val): + return None + if _is_empty(existing_val): + return GAP_FILL + return UNCHANGED if existing_val == proposed_val else CONFLICT + + +def reconcile_user(proposed: ProposedUser, existing: Optional[Any]) -> UserChange: + if existing is None: + return UserChange(proposed=proposed, existing=None, category=NEW) + ef = existing_fields(existing) + pf = proposed_fields(proposed) + diffs: List[FieldDiff] = [] + has_conflict = has_gap = False + for name in RECONCILED_FIELDS: + status = _compare(ef[name], pf[name]) + if status in (GAP_FILL, CONFLICT): + diffs.append(FieldDiff(name, ef[name], pf[name], status)) + has_conflict = has_conflict or status == CONFLICT + has_gap = has_gap or status == GAP_FILL + category = CONFLICT if has_conflict else GAP_FILL if has_gap else UNCHANGED + return UserChange( + proposed=proposed, existing=existing, category=category, diffs=diffs + ) + + +@dataclass +class ReconcilePlan: + """All per-user reconciliation outcomes for a run.""" + + changes: List[UserChange] = field(default_factory=list) + + def by_category(self, category: str) -> List[UserChange]: + return [c for c in self.changes if c.category == category] + + @property + def conflicts(self) -> List[UserChange]: + return self.by_category(CONFLICT) + + def counts(self) -> Dict[str, int]: + counter = Counter(c.category for c in self.changes) + return { + cat: counter.get(cat, 0) for cat in (NEW, GAP_FILL, CONFLICT, UNCHANGED) + } + + +def reconcile( + proposed_users: List[ProposedUser], existing: ExistingUsers +) -> ReconcilePlan: + changes = [reconcile_user(p, existing.get(p.username)) for p in proposed_users] + return ReconcilePlan(changes=changes) diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py new file mode 100644 index 00000000..3d353984 --- /dev/null +++ b/tests/tools/test_user_sync_reconcile.py @@ -0,0 +1,137 @@ +"""Unit tests for the reconciliation engine.""" + +import uuid +from types import SimpleNamespace + +from osw.tools.user_sync.existing import ExistingUsers +from osw.tools.user_sync.mapping import ProposedUser +from osw.tools.user_sync.reconcile import ( + CONFLICT, + GAP_FILL, + NEW, + UNCHANGED, + reconcile, + reconcile_user, +) + +ORCID = "0000-0002-6374-9831" + + +def _proposed(**kw): + base = dict( + uuid=uuid.uuid4(), + username=ORCID, + first_name="Lukas", + surname="Koschmieder", + label="Lukas Koschmieder", + orcid=f"https://orcid.org/{ORCID}", + emails=[], + websites=[], + organizations=[], + ) + base.update(kw) + return ProposedUser(**base) + + +def _entity(**kw): + base = dict( + label="Lukas Koschmieder", + first_name="Lukas", + surname="Koschmieder", + orcid=f"https://orcid.org/{ORCID}", + email=None, + website=None, + organization=None, + ) + base.update(kw) + return SimpleNamespace( + label=[SimpleNamespace(text=base["label"])] if base["label"] else [], + first_name=base["first_name"], + surname=base["surname"], + orcid=base["orcid"], + email=set(base["email"] or []), + website=set(base["website"] or []), + organization=set(base["organization"] or []), + ) + + +def test_new_when_no_existing(): + change = reconcile_user(_proposed(), None) + assert change.category == NEW + assert change.diffs == [] + + +def test_unchanged_when_equal(): + change = reconcile_user(_proposed(), _entity()) + assert change.category == UNCHANGED + assert change.diffs == [] + + +def test_gap_fill_for_missing_field(): + proposed = _proposed(emails=["l@example.org"]) + change = reconcile_user(proposed, _entity(email=None)) + assert change.category == GAP_FILL + assert [d.name for d in change.diffs] == ["emails"] + assert change.diffs[0].status == GAP_FILL + + +def test_conflict_on_differing_value(): + change = reconcile_user(_proposed(surname="Koschmieder"), _entity(surname="Kosch")) + assert change.category == CONFLICT + assert change.diffs[0].name == "surname" + assert change.diffs[0].existing == "Kosch" + assert change.diffs[0].proposed == "Koschmieder" + + +def test_conflict_dominates_gap_fill(): + proposed = _proposed(surname="Koschmieder", emails=["l@example.org"]) + change = reconcile_user(proposed, _entity(surname="Kosch", email=None)) + assert change.category == CONFLICT + names = {d.name for d in change.diffs} + assert names == {"surname", "emails"} + + +def test_empty_proposal_does_not_clear_existing(): + change = reconcile_user(_proposed(emails=[]), _entity(email=["keep@example.org"])) + assert change.category == UNCHANGED + assert change.diffs == [] + + +def test_orcid_trailing_slash_not_a_conflict(): + change = reconcile_user( + _proposed(orcid=f"https://orcid.org/{ORCID}"), + _entity(orcid=f"https://orcid.org/{ORCID}/"), + ) + assert change.category == UNCHANGED + + +def test_set_order_independent(): + change = reconcile_user( + _proposed(organizations=["Item:OSWb", "Item:OSWa"]), + _entity(organization=["Item:OSWa", "Item:OSWb"]), + ) + assert change.category == UNCHANGED + + +def test_missing_label_is_gap_fill(): + change = reconcile_user(_proposed(label="Lukas Koschmieder"), _entity(label=None)) + assert change.category == GAP_FILL + assert change.diffs[0].name == "label" + + +def test_reconcile_plan_counts(): + proposed = [ + _proposed(username="new-user"), + _proposed(username=ORCID), + _proposed(username="conflict-user", surname="New"), + ] + existing = ExistingUsers( + by_username={ + ORCID: _entity(), + "conflict-user": _entity(surname="Old"), + } + ) + plan = reconcile(proposed, existing) + counts = plan.counts() + assert counts == {NEW: 1, GAP_FILL: 0, CONFLICT: 1, UNCHANGED: 1} + assert [c.proposed.username for c in plan.conflicts] == ["conflict-user"] From e0c54db9845447a62de544c4651e7d512a6ead74 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 12:21:50 +0200 Subject: [PATCH 06/20] feat(user-sync): add interactive preview and resolution --- src/osw/tools/user_sync/interactive.py | 199 ++++++++++++++++++++++ tests/tools/test_user_sync_interactive.py | 196 +++++++++++++++++++++ 2 files changed, 395 insertions(+) create mode 100644 src/osw/tools/user_sync/interactive.py create mode 100644 tests/tools/test_user_sync_interactive.py diff --git a/src/osw/tools/user_sync/interactive.py b/src/osw/tools/user_sync/interactive.py new file mode 100644 index 00000000..e244ed3d --- /dev/null +++ b/src/osw/tools/user_sync/interactive.py @@ -0,0 +1,199 @@ +"""Interactive preview and conflict resolution for the sync. + +Rendering is pure (returns strings). All terminal IO goes through ``Prompter``, +which wraps ``input`` / ``print`` so tests can script answers and capture output. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any, Callable, List, Optional, Sequence, Set + +from .mapping import ProposedUser +from .reconcile import ( + CONFLICT, + GAP_FILL, + NEW, + RECONCILED_FIELDS, + UNCHANGED, + ReconcilePlan, + UserChange, + proposed_fields, +) + +_MARKERS = {NEW: "+", GAP_FILL: "~", CONFLICT: "!", UNCHANGED: "="} + + +def _is_empty(value: Any) -> bool: + if value is None or value == "": + return True + return isinstance(value, (list, set, tuple)) and len(value) == 0 + + +def nonempty_fields(proposed: ProposedUser) -> Set[str]: + """Reconciled field names that carry a value on the proposed user.""" + pf = proposed_fields(proposed) + return {name for name in RECONCILED_FIELDS if not _is_empty(pf[name])} + + +def render_preview(plan: ReconcilePlan) -> List[str]: + """Human-readable preview: a header line plus one row per user.""" + counts = plan.counts() + lines = [ + f"Users: {len(plan.changes)} | " + f"new={counts[NEW]} gap_fill={counts[GAP_FILL]} " + f"conflict={counts[CONFLICT]} unchanged={counts[UNCHANGED]}" + ] + for change in plan.changes: + marker = _MARKERS[change.category] + detail = "" + if change.diffs: + detail = " (" + ", ".join(d.name for d in change.diffs) + ")" + placeholder = " [placeholder name]" if change.proposed.placeholder_name else "" + lines.append( + f" {marker} {change.proposed.username} {change.category}{detail}{placeholder}" + ) + return lines + + +def render_conflict(change: UserChange) -> List[str]: + """Field-by-field display of one user's differences.""" + lines = [f"User {change.proposed.username}:"] + for diff in change.diffs: + lines.append( + f" {diff.name} [{diff.status}]: " + f"existing={diff.existing!r} -> proposed={diff.proposed!r}" + ) + return lines + + +class Prompter: + """Thin IO seam over input/print for testable prompting.""" + + def __init__( + self, + input_fn: Callable[[str], str] = input, + output_fn: Callable[[str], None] = print, + ): + self._input = input_fn + self._output = output_fn + + def write(self, text: str = "") -> None: + self._output(text) + + def ask(self, prompt: str, choices: Sequence[str]) -> str: + keys = "/".join(choices) + while True: + answer = self._input(f"{prompt} [{keys}]: ").strip().lower() + if answer in choices: + return answer + self.write(f"Please choose one of: {keys}") + + def confirm(self, prompt: str, default: bool = False) -> bool: + suffix = "[Y/n]" if default else "[y/N]" + answer = self._input(f"{prompt} {suffix}: ").strip().lower() + if not answer: + return default + return answer in ("y", "yes") + + +@dataclass +class ResolvedChange: + """A user change with the decided action and fields to write.""" + + change: UserChange + action: str # "create" | "update" | "skip" + apply_fields: Set[str] = field(default_factory=set) + + +@dataclass +class Resolution: + """The outcome of resolving a plan: what to write and whether to proceed.""" + + resolved: List[ResolvedChange] = field(default_factory=list) + accepted: bool = False + + def creates(self) -> List[ResolvedChange]: + return [r for r in self.resolved if r.action == "create"] + + def updates(self) -> List[ResolvedChange]: + return [r for r in self.resolved if r.action == "update"] + + +def _review_user(change: UserChange, prompter: Prompter) -> Set[str]: + """Resolve one conflicting user's fields, returning accepted conflict fields.""" + for line in render_conflict(change): + prompter.write(line) + conflict_diffs = [d for d in change.diffs if d.status == CONFLICT] + choice = prompter.ask( + "Take all new / Keep all existing / Field-by-field?", ("a", "k", "f") + ) + if choice == "a": + return {d.name for d in conflict_diffs} + if choice == "k": + return set() + accepted: Set[str] = set() + for diff in conflict_diffs: + if prompter.ask(f"{diff.name}: keep or take new?", ("k", "n")) == "n": + accepted.add(diff.name) + return accepted + + +def resolve_plan( + plan: ReconcilePlan, + prompter: Prompter, + dry_run: bool = False, + assume_yes: bool = False, + summary_lines: Optional[Sequence[str]] = None, +) -> Resolution: + """Preview the plan and resolve conflicts into a write decision.""" + for line in render_preview(plan): + prompter.write(line) + for line in summary_lines or []: + prompter.write(line) + + conflict_mode: Optional[str] = None + if plan.conflicts and not dry_run: + if assume_yes: + conflict_mode = "keep" + else: + choice = prompter.ask( + "Conflicts found: Apply all / Keep existing / Review each / Abort?", + ("a", "k", "r", "x"), + ) + if choice == "x": + return Resolution(resolved=[], accepted=False) + conflict_mode = {"a": "all", "k": "keep", "r": "review"}[choice] + + resolved: List[ResolvedChange] = [] + for change in plan.changes: + if change.category == NEW: + resolved.append( + ResolvedChange(change, "create", nonempty_fields(change.proposed)) + ) + elif change.category == UNCHANGED: + resolved.append(ResolvedChange(change, "skip")) + elif change.category == GAP_FILL: + resolved.append( + ResolvedChange(change, "update", {d.name for d in change.diffs}) + ) + else: # CONFLICT + accepted = {d.name for d in change.diffs if d.status == GAP_FILL} + if conflict_mode == "all": + accepted |= {d.name for d in change.diffs if d.status == CONFLICT} + elif conflict_mode == "review": + accepted |= _review_user(change, prompter) + action = "update" if accepted else "skip" + resolved.append(ResolvedChange(change, action, accepted)) + + if dry_run: + accepted_run = False + elif assume_yes: + accepted_run = True + else: + n_create = sum(1 for r in resolved if r.action == "create") + n_update = sum(1 for r in resolved if r.action == "update") + accepted_run = prompter.confirm( + f"Proceed to create {n_create} and update {n_update} user items?" + ) + return Resolution(resolved=resolved, accepted=accepted_run) diff --git a/tests/tools/test_user_sync_interactive.py b/tests/tools/test_user_sync_interactive.py new file mode 100644 index 00000000..912a72ee --- /dev/null +++ b/tests/tools/test_user_sync_interactive.py @@ -0,0 +1,196 @@ +"""Unit tests for the interactive preview and conflict resolution.""" + +import uuid + +from osw.tools.user_sync.interactive import ( + Prompter, + nonempty_fields, + render_preview, + resolve_plan, +) +from osw.tools.user_sync.mapping import ProposedUser +from osw.tools.user_sync.reconcile import ( + CONFLICT, + GAP_FILL, + NEW, + UNCHANGED, + FieldDiff, + ReconcilePlan, + UserChange, +) + + +class FakeIO: + def __init__(self, answers): + self.answers = list(answers) + self.outputs = [] + + def input(self, prompt=""): + return self.answers.pop(0) + + def output(self, text=""): + self.outputs.append(text) + + +def _proposed(username="u", **kw): + base = dict( + uuid=uuid.uuid4(), + username=username, + first_name="F", + surname="S", + label="F S", + orcid=None, + emails=[], + websites=[], + organizations=[], + ) + base.update(kw) + return ProposedUser(**base) + + +def _change(category, username="u", diffs=None, placeholder=False): + proposed = _proposed(username=username) + proposed.placeholder_name = placeholder + return UserChange( + proposed=proposed, + existing=None if category == NEW else object(), + category=category, + diffs=diffs or [], + ) + + +def _prompter(answers): + io = FakeIO(answers) + return Prompter(io.input, io.output), io + + +def test_render_preview(): + plan = ReconcilePlan(changes=[_change(NEW, "alice", placeholder=True)]) + lines = render_preview(plan) + assert lines[0].startswith("Users: 1") + assert "new=1" in lines[0] + assert "+ alice new" in lines[1] + assert "[placeholder name]" in lines[1] + + +def test_nonempty_fields(): + proposed = _proposed(orcid="https://orcid.org/x", emails=["e@x.org"]) + assert nonempty_fields(proposed) == { + "label", + "first_name", + "surname", + "orcid", + "emails", + } + + +def _mixed_plan(): + return ReconcilePlan( + changes=[ + _change(NEW, "new1"), + _change(GAP_FILL, "gap1", [FieldDiff("emails", set(), {"e"}, GAP_FILL)]), + _change(CONFLICT, "con1", [FieldDiff("surname", "Old", "New", CONFLICT)]), + _change( + CONFLICT, + "mix1", + [ + FieldDiff("emails", set(), {"e"}, GAP_FILL), + FieldDiff("surname", "Old", "New", CONFLICT), + ], + ), + _change(UNCHANGED, "same1"), + ] + ) + + +def test_resolve_keep_existing(): + prompter, _ = _prompter(["k", "y"]) + res = resolve_plan(_mixed_plan(), prompter) + assert [r.change.proposed.username for r in res.creates()] == ["new1"] + updates = {r.change.proposed.username: r.apply_fields for r in res.updates()} + assert updates == {"gap1": {"emails"}, "mix1": {"emails"}} + skipped = {r.change.proposed.username for r in res.resolved if r.action == "skip"} + assert skipped == {"con1", "same1"} + assert res.accepted is True + + +def test_resolve_apply_all(): + prompter, _ = _prompter(["a", "y"]) + res = resolve_plan(_mixed_plan(), prompter) + updates = {r.change.proposed.username: r.apply_fields for r in res.updates()} + assert updates["con1"] == {"surname"} + assert updates["mix1"] == {"emails", "surname"} + + +def test_resolve_abort(): + prompter, _ = _prompter(["x"]) + res = resolve_plan(_mixed_plan(), prompter) + assert res.resolved == [] + assert res.accepted is False + + +def test_resolve_dry_run_makes_no_writes_and_no_prompts(): + prompter, io = _prompter([]) # no answers available + res = resolve_plan(_mixed_plan(), prompter, dry_run=True) + assert res.accepted is False + assert io.answers == [] # nothing was asked + + +def test_resolve_assume_yes_keeps_conflicts(): + prompter, io = _prompter([]) + res = resolve_plan(_mixed_plan(), prompter, assume_yes=True) + assert res.accepted is True + updates = {r.change.proposed.username: r.apply_fields for r in res.updates()} + assert updates == {"gap1": {"emails"}, "mix1": {"emails"}} + assert io.answers == [] + + +def test_resolve_review_field_by_field(): + plan = ReconcilePlan( + changes=[ + _change( + CONFLICT, + "con1", + [ + FieldDiff("surname", "Old", "New", CONFLICT), + FieldDiff("label", "L1", "L2", CONFLICT), + ], + ) + ] + ) + prompter, _ = _prompter(["r", "f", "n", "k", "y"]) + res = resolve_plan(plan, prompter) + assert res.updates()[0].apply_fields == {"surname"} + assert res.accepted is True + + +def test_resolve_review_take_all_for_user(): + plan = ReconcilePlan( + changes=[ + _change( + CONFLICT, + "con1", + [ + FieldDiff("surname", "Old", "New", CONFLICT), + FieldDiff("label", "L1", "L2", CONFLICT), + ], + ) + ] + ) + prompter, _ = _prompter(["r", "a", "y"]) + res = resolve_plan(plan, prompter) + assert res.updates()[0].apply_fields == {"surname", "label"} + + +def test_final_confirm_no(): + plan = ReconcilePlan(changes=[_change(NEW, "new1")]) + prompter, _ = _prompter(["n"]) + res = resolve_plan(plan, prompter) + assert res.accepted is False + assert res.creates()[0].change.proposed.username == "new1" + + +def test_prompter_reprompts_on_invalid(): + prompter, io = _prompter(["z", "k"]) + assert prompter.ask("q", ("k", "n")) == "k" + assert any("Please choose one of" in o for o in io.outputs) From a2a9d8c524d77e4f8266bf210c022db2b4a85263 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 14:46:26 +0200 Subject: [PATCH 07/20] feat(user-sync): store items, redirects, orgs and verify --- src/osw/tools/user_sync/build.py | 65 +++++++++ src/osw/tools/user_sync/redirects.py | 35 +++++ src/osw/tools/user_sync/sync.py | 172 ++++++++++++++++++++++-- tests/tools/test_user_sync_config.py | 4 +- tests/tools/test_user_sync_redirects.py | 69 ++++++++++ tests/tools/test_user_sync_sync.py | 107 +++++++++++++++ 6 files changed, 436 insertions(+), 16 deletions(-) create mode 100644 src/osw/tools/user_sync/build.py create mode 100644 src/osw/tools/user_sync/redirects.py create mode 100644 tests/tools/test_user_sync_redirects.py create mode 100644 tests/tools/test_user_sync_sync.py diff --git a/src/osw/tools/user_sync/build.py b/src/osw/tools/user_sync/build.py new file mode 100644 index 00000000..24d7fcc8 --- /dev/null +++ b/src/osw/tools/user_sync/build.py @@ -0,0 +1,65 @@ +"""Build pydantic User/Organization entities from proposals. + +Must run within an open OSW connection (the models resolve linked entities on +construction). Organization references are passed as a list of page-title IRIs, +which oold stores in the entity's ``__iris__`` map; the reference attribute is +never read back here, as that would trigger a network resolve. +""" + +from __future__ import annotations + +from typing import Any, List, Set + +from opensemantic.base.v1 import Organization, User + +from .mapping import ProposedOrganization, ProposedUser + +# The concrete DisplayName type used by the label field. +_DISPLAY_NAME = User.__fields__["label"].type_ + + +def _labels(text: str) -> List[Any]: + return [_DISPLAY_NAME(text=text)] + + +def build_organization(proposed: ProposedOrganization) -> Organization: + return Organization(uuid=proposed.uuid, label=_labels(proposed.name)) + + +def build_user(proposed: ProposedUser) -> User: + """Build a full User entity for creation.""" + args: dict = { + "uuid": proposed.uuid, + "username": proposed.username, + "first_name": proposed.first_name, + "surname": proposed.surname, + "label": _labels(proposed.label), + } + if proposed.orcid: + args["orcid"] = proposed.orcid + if proposed.emails: + args["email"] = set(proposed.emails) + if proposed.websites: + args["website"] = set(proposed.websites) + if proposed.organizations: + args["organization"] = list(proposed.organizations) + return User(**args) + + +def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> Any: + """Apply the accepted fields onto a loaded existing entity, in place.""" + if "label" in apply_fields: + entity.label = _labels(proposed.label) + if "first_name" in apply_fields: + entity.first_name = proposed.first_name + if "surname" in apply_fields: + entity.surname = proposed.surname + if "orcid" in apply_fields: + entity.orcid = proposed.orcid + if "emails" in apply_fields: + entity.email = set(proposed.emails) + if "websites" in apply_fields: + entity.website = set(proposed.websites) + if "organizations" in apply_fields: + entity.__iris__["organization"] = list(proposed.organizations) + return entity diff --git a/src/osw/tools/user_sync/redirects.py b/src/osw/tools/user_sync/redirects.py new file mode 100644 index 00000000..7d85b45b --- /dev/null +++ b/src/osw/tools/user_sync/redirects.py @@ -0,0 +1,35 @@ +"""Create ``User:`` redirect pages pointing at the user item. + +An existing user page with real content is never overwritten; only missing +pages or pages that are already redirects are (re)written. +""" + +from __future__ import annotations + +from typing import Any, Optional + +from osw.wtsite import WtPage + + +def redirect_wikitext(item_title: str) -> str: + return f"#REDIRECT [[{item_title}]]" + + +def is_redirect(content: str) -> bool: + return content.lstrip().upper().startswith("#REDIRECT") + + +def ensure_redirect(osw: Any, username: str, item_title: str) -> Optional[str]: + """Create or refresh ``User:`` -> item redirect. + + Returns the page title if written, or None if skipped (real page exists). + """ + title = f"User:{username}" + page = WtPage(wtSite=osw.site, title=title) + if getattr(page, "exists", False): + content = page.get_content() or "" + if content.strip() and not is_redirect(content): + return None + page.set_content(redirect_wikitext(item_title)) + page.edit(comment="user-sync: redirect to user item") + return title diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index 8001e235..e541368e 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -1,18 +1,30 @@ """Orchestrator for the user-item sync tool. -Later phases implement enumeration, ORCID enrichment, reconciliation, the -interactive layer and the store step. This module currently defines the report -type and the public entry point. +Ties the sources, mapping, reconciliation, interactive and store steps into one +run: enumerate MediaWiki users, enrich ORCID users, reconcile against existing +items, preview and confirm, then store items, redirects and organizations and +verify. """ from __future__ import annotations from dataclasses import dataclass, field -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from osw.core import OSW +from .build import apply_update, build_organization, build_user from .config import SyncConfig +from .existing import load_existing_users +from .interactive import Prompter, Resolution, resolve_plan +from .mapping import ProposedOrganization, map_user +from .reconcile import reconcile +from .redirects import ensure_redirect +from .sources import ( + OrcidRateLimitError, + enumerate_mw_users, + fetch_orcid_record, +) @dataclass @@ -25,22 +37,154 @@ class SyncReport: failed: Dict[str, str] = field(default_factory=dict) redirects_created: List[str] = field(default_factory=list) organizations: List[str] = field(default_factory=list) + accepted: bool = False def summary(self) -> str: - """One-line human-readable summary of the run.""" + state = "applied" if self.accepted else "no changes written" return ( - f"created={len(self.created)} updated={len(self.updated)} " + f"{state}: created={len(self.created)} updated={len(self.updated)} " f"skipped={len(self.skipped)} failed={len(self.failed)} " f"redirects={len(self.redirects_created)} orgs={len(self.organizations)}" ) -def run_user_sync(config: SyncConfig, osw: Optional[OSW] = None) -> SyncReport: - """Create or update OSW User items from MediaWiki accounts and ORCID data. +def _build_proposals(config: SyncConfig, osw: Any, session: Any): + """Enumerate MediaWiki users, enrich ORCID users and map to proposals.""" + excluded = tuple(config.excluded_groups) if config.exclude_bot_group else () + mw_users = enumerate_mw_users( + osw.mw_site, + excluded_groups=excluded, + include_non_orcid=config.include_non_orcid, + limit=config.limit, + ) + cache: Dict[str, Any] = {} + proposed_users = [] + org_map: Dict[Any, ProposedOrganization] = {} + for mw_user in mw_users: + profile = None + if mw_user.is_orcid: + try: + profile = fetch_orcid_record( + mw_user.orcid_id, + session=session, + base=config.orcid_api_base, + cache=cache, + ) + except OrcidRateLimitError: + profile = None + proposed, orgs = map_user( + mw_user, profile, link_organizations=config.link_organizations + ) + proposed_users.append(proposed) + for org in orgs: + org_map.setdefault(org.uuid, org) + return proposed_users, org_map + + +def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: + if not org_map: + return + entities = [build_organization(org) for org in org_map.values()] + try: + osw.store_entity( + OSW.StoreEntityParam(entities=entities, edit_comment="user-sync: orgs") + ) + report.organizations = [org.full_page_title for org in org_map.values()] + except Exception as exc: # pragma: no cover - network failure path + report.failed["organizations"] = str(exc) + + +def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: + entities = [] + for resolved in resolution.resolved: + proposed = resolved.change.proposed + if resolved.action == "create": + entities.append(build_user(proposed)) + report.created.append(proposed.full_page_title) + elif resolved.action == "update": + entities.append( + apply_update(resolved.change.existing, proposed, resolved.apply_fields) + ) + report.updated.append(proposed.full_page_title) + else: + report.skipped.append(proposed.full_page_title) + if entities: + osw.store_entity( + OSW.StoreEntityParam( + entities=entities, overwrite=True, edit_comment="user-sync" + ) + ) + + +def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None: + for resolved in resolution.resolved: + if resolved.action not in ("create", "update"): + continue + proposed = resolved.change.proposed + try: + written = ensure_redirect(osw, proposed.username, proposed.full_page_title) + if written: + report.redirects_created.append(written) + except Exception as exc: # pragma: no cover - network failure path + report.failed[f"redirect:{proposed.username}"] = str(exc) + + +def _verify(osw: Any, report: SyncReport) -> None: + titles = report.created + report.updated + if not titles: + return + try: + loaded = osw.load_entity(titles) + loaded_list = loaded if isinstance(loaded, list) else [loaded] + found = {getattr(e, "username", None) for e in loaded_list} + missing = [ + t + for t, e in zip(titles, loaded_list) + if getattr(e, "username", None) is None + ] + if missing or None in found: + report.failed["verify"] = f"missing username on {missing}" + except Exception as exc: # pragma: no cover - network failure path + report.failed["verify"] = str(exc) + + +def run_user_sync( + config: SyncConfig, + osw: Optional[OSW] = None, + prompter: Optional[Prompter] = None, + session: Any = None, +) -> SyncReport: + """Create or update OSW User items from MediaWiki accounts and ORCID data.""" + if osw is None: + raise ValueError("run_user_sync requires an authenticated OSW connection") + prompter = prompter or Prompter() + + proposed_users, org_map = _build_proposals(config, osw, session) + existing = load_existing_users(osw) + plan = reconcile(proposed_users, existing) + + summary_lines = [] + if config.link_organizations and org_map: + summary_lines.append(f"Organizations to ensure: {len(org_map)}") + if config.create_redirects: + summary_lines.append("Redirects: User: -> item for created/updated") + + resolution = resolve_plan( + plan, + prompter, + dry_run=config.dry_run, + assume_yes=config.assume_yes, + summary_lines=summary_lines, + ) + + report = SyncReport(accepted=resolution.accepted) + if not resolution.accepted: + return report - Args: - config: Runtime options for the run. - osw: An authenticated OSW/OswExpress connection. Required for any wiki - access; the example wrapper builds one from ``config``. - """ - raise NotImplementedError("Implemented incrementally in phases 1 to 6.") + if config.link_organizations: + _store_organizations(osw, org_map, report) + _store_users(osw, resolution, report) + if config.create_redirects: + _create_redirects(osw, resolution, report) + _verify(osw, report) + return report diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py index 38f1c417..48ff9ee2 100644 --- a/tests/tools/test_user_sync_config.py +++ b/tests/tools/test_user_sync_config.py @@ -41,8 +41,8 @@ def test_report_summary(): assert "updated=1" in report.summary() -def test_run_user_sync_is_stub(): +def test_run_user_sync_requires_connection(): import pytest - with pytest.raises(NotImplementedError): + with pytest.raises(ValueError, match="connection"): run_user_sync(SyncConfig()) diff --git a/tests/tools/test_user_sync_redirects.py b/tests/tools/test_user_sync_redirects.py new file mode 100644 index 00000000..5d933f62 --- /dev/null +++ b/tests/tools/test_user_sync_redirects.py @@ -0,0 +1,69 @@ +"""Unit tests for redirect page handling.""" + +from osw.tools.user_sync import redirects +from osw.tools.user_sync.redirects import ( + ensure_redirect, + is_redirect, + redirect_wikitext, +) + + +def test_redirect_wikitext(): + assert redirect_wikitext("Item:OSW123") == "#REDIRECT [[Item:OSW123]]" + + +def test_is_redirect(): + assert is_redirect("#REDIRECT [[Item:OSW1]]") + assert is_redirect(" #redirect [[x]]") + assert not is_redirect("Some real user page content") + + +class FakeWtPage: + exists_map: dict = {} + content_map: dict = {} + + def __init__(self, wtSite=None, title=None, do_init=True): + self.title = title + self.exists = self.exists_map.get(title, False) + self._content = self.content_map.get(title, "") + self.written = None + self.edited = False + + def get_content(self): + return self._content + + def set_content(self, content): + self.written = content + + def edit(self, comment=None): + self.edited = True + + +class FakeOsw: + site = object() + + +def _patch(monkeypatch, exists_map, content_map): + FakeWtPage.exists_map = exists_map + FakeWtPage.content_map = content_map + monkeypatch.setattr(redirects, "WtPage", FakeWtPage) + + +def test_ensure_redirect_creates_missing(monkeypatch): + _patch(monkeypatch, {}, {}) + written = ensure_redirect(FakeOsw(), "0000-0002-6374-9831", "Item:OSW1") + assert written == "User:0000-0002-6374-9831" + + +def test_ensure_redirect_overwrites_existing_redirect(monkeypatch): + title = "User:alice" + _patch(monkeypatch, {title: True}, {title: "#REDIRECT [[Item:OSWold]]"}) + written = ensure_redirect(FakeOsw(), "alice", "Item:OSWnew") + assert written == title + + +def test_ensure_redirect_skips_real_page(monkeypatch): + title = "User:bob" + _patch(monkeypatch, {title: True}, {title: "This is my real user page."}) + written = ensure_redirect(FakeOsw(), "bob", "Item:OSW1") + assert written is None diff --git a/tests/tools/test_user_sync_sync.py b/tests/tools/test_user_sync_sync.py new file mode 100644 index 00000000..aca5ef98 --- /dev/null +++ b/tests/tools/test_user_sync_sync.py @@ -0,0 +1,107 @@ +"""Unit tests for the orchestrator (dry-run path, no live models).""" + +from osw.tools.user_sync.config import SyncConfig +from osw.tools.user_sync.interactive import Prompter +from osw.tools.user_sync.sync import SyncReport, run_user_sync + +ORCID = "0000-0002-6374-9831" + +ORCID_RECORD = { + "person": { + "name": { + "given-names": {"value": "Lukas"}, + "family-name": {"value": "Koschmieder"}, + } + } +} + + +class FakeMwSite: + def api(self, action, **params): + assert action == "query" and params["list"] == "allusers" + return { + "query": { + "allusers": [ + {"name": ORCID, "groups": ["user"]}, + {"name": "SyncBot", "groups": ["user", "bot"]}, + ] + } + } + + +class FakeOsw: + def __init__(self): + self.mw_site = FakeMwSite() + self.store_calls = 0 + + def query_instances(self, category): + return [] + + def load_entity(self, titles): + return [] + + def store_entity(self, param): + self.store_calls += 1 + + +class FakeResponse: + status_code = 200 + headers: dict = {} + + def json(self): + return ORCID_RECORD + + def raise_for_status(self): + pass + + +class FakeSession: + def __init__(self): + self.calls = 0 + + def get(self, url, headers=None, timeout=None): + self.calls += 1 + return FakeResponse() + + +class CapturingIO: + def __init__(self): + self.outputs = [] + + def input(self, prompt=""): # pragma: no cover - not used in dry-run + raise AssertionError("dry-run must not prompt") + + def output(self, text=""): + self.outputs.append(text) + + +def test_summary_reads_accepted_state(): + assert SyncReport(accepted=False).summary().startswith("no changes written") + assert SyncReport(accepted=True, created=["x"]).summary().startswith("applied") + + +def test_dry_run_previews_without_writing(): + osw = FakeOsw() + io = CapturingIO() + session = FakeSession() + report = run_user_sync( + SyncConfig(domain="d", dry_run=True), + osw=osw, + prompter=Prompter(io.input, io.output), + session=session, + ) + assert report.accepted is False + assert osw.store_calls == 0 # nothing written + assert session.calls == 1 # ORCID user enriched, bot skipped + joined = "\n".join(io.outputs) + assert ORCID in joined + assert "new=1" in joined # bot excluded, only the ORCID user + + +def test_requires_connection(): + try: + run_user_sync(SyncConfig()) + except ValueError as exc: + assert "connection" in str(exc) + else: # pragma: no cover + raise AssertionError("expected ValueError") From f2d011d61bf4dfe1ea445cdfc78bd3f6b69af520 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 15:24:46 +0200 Subject: [PATCH 08/20] feat(user-sync): exclude system accounts and complete idempotent sync --- src/osw/tools/user_sync/config.py | 15 +++++++-- src/osw/tools/user_sync/reconcile.py | 22 +++++++++++-- src/osw/tools/user_sync/redirects.py | 12 ++++--- src/osw/tools/user_sync/sources.py | 25 +++++++++++++- src/osw/tools/user_sync/sync.py | 43 ++++++++++++++++--------- tests/tools/test_user_sync_config.py | 9 +++++- tests/tools/test_user_sync_reconcile.py | 23 +++++++++++++ tests/tools/test_user_sync_redirects.py | 11 +++++-- tests/tools/test_user_sync_sources.py | 16 +++++++++ 9 files changed, 146 insertions(+), 30 deletions(-) diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py index 5a8f8d4f..73c8e80e 100644 --- a/src/osw/tools/user_sync/config.py +++ b/src/osw/tools/user_sync/config.py @@ -13,8 +13,9 @@ # Public ORCID API used to enrich ORCID users. ORCID_API_BASE_DEFAULT = "https://pub.orcid.org/v3.0" -# MediaWiki group whose members are excluded from the sync (bots). -BOT_GROUP = "bot" +# MediaWiki groups whose members are treated as bot/system accounts and skipped. +# All are standard MediaWiki groups, so this is not a per-instance skip-list. +SYSTEM_GROUPS = ["bot", "sysop", "bureaucrat", "interface-admin"] @dataclass @@ -28,10 +29,11 @@ class SyncConfig: limit: Optional[int] = None include_non_orcid: bool = True exclude_bot_group: bool = True + exclude_system_usernames: bool = True create_redirects: bool = True link_organizations: bool = True orcid_api_base: str = ORCID_API_BASE_DEFAULT - excluded_groups: List[str] = field(default_factory=lambda: [BOT_GROUP]) + excluded_groups: List[str] = field(default_factory=lambda: list(SYSTEM_GROUPS)) def build_arg_parser() -> argparse.ArgumentParser: @@ -86,6 +88,12 @@ def build_arg_parser() -> argparse.ArgumentParser: action="store_false", help="Sync only accounts whose username is an ORCID iD.", ) + parser.add_argument( + "--include-system", + dest="exclude_system_usernames", + action="store_false", + help="Do not skip MediaWiki reserved system accounts (Maintenance script etc.).", + ) return parser @@ -99,6 +107,7 @@ def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig: assume_yes=args.assume_yes, limit=args.limit, include_non_orcid=args.include_non_orcid, + exclude_system_usernames=args.exclude_system_usernames, create_redirects=args.create_redirects, link_organizations=args.link_organizations, ) diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py index 3218448f..479473ad 100644 --- a/src/osw/tools/user_sync/reconcile.py +++ b/src/osw/tools/user_sync/reconcile.py @@ -58,6 +58,24 @@ def _existing_label(entity: Any) -> Optional[str]: return None +def _existing_orgs(entity: Any) -> Any: + """Read organization page-title refs without resolving the relation. + + Loaded entities keep relation targets as IRIs in ``__iris__``; reading the + attribute itself would make oold resolve (and fail on) the linked items. + """ + iris = getattr(entity, "__iris__", None) + if isinstance(iris, dict): + value = iris.get("organization") + if value is None: + return None + return value if isinstance(value, (list, tuple, set)) else [value] + try: + return getattr(entity, "organization", None) + except Exception: # pragma: no cover - defensive + return None + + def existing_fields(entity: Any) -> Dict[str, Any]: """Normalized comparable field view of a loaded User entity.""" return { @@ -67,9 +85,7 @@ def existing_fields(entity: Any) -> Dict[str, Any]: "orcid": _norm_scalar("orcid", getattr(entity, "orcid", None)), "emails": _norm_set("emails", getattr(entity, "email", None)), "websites": _norm_set("websites", getattr(entity, "website", None)), - "organizations": _norm_set( - "organizations", getattr(entity, "organization", None) - ), + "organizations": _norm_set("organizations", _existing_orgs(entity)), } diff --git a/src/osw/tools/user_sync/redirects.py b/src/osw/tools/user_sync/redirects.py index 7d85b45b..4f2cae79 100644 --- a/src/osw/tools/user_sync/redirects.py +++ b/src/osw/tools/user_sync/redirects.py @@ -22,14 +22,18 @@ def is_redirect(content: str) -> bool: def ensure_redirect(osw: Any, username: str, item_title: str) -> Optional[str]: """Create or refresh ``User:`` -> item redirect. - Returns the page title if written, or None if skipped (real page exists). + Returns the page title if written, or None if skipped (a real page already + exists, or the redirect is already correct). """ title = f"User:{username}" + target = redirect_wikitext(item_title) page = WtPage(wtSite=osw.site, title=title) if getattr(page, "exists", False): - content = page.get_content() or "" + content = page.get_slot_content("main") or "" if content.strip() and not is_redirect(content): - return None - page.set_content(redirect_wikitext(item_title)) + return None # do not clobber a real user page + if content.strip() == target: + return None # already correct + page.set_slot_content("main", target) page.edit(comment="user-sync: redirect to user item") return title diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py index 32de437f..7e40b137 100644 --- a/src/osw/tools/user_sync/sources.py +++ b/src/osw/tools/user_sync/sources.py @@ -20,6 +20,24 @@ _ALLUSERS_PROPS = "registration|editcount|groups|centralids|blockinfo" +# MediaWiki default reserved system usernames. These ship with MediaWiki and are +# the same across instances, so skipping them is not a per-instance skip-list. +RESERVED_USERNAMES = frozenset({ + "MediaWiki default", + "Maintenance script", + "Conversion script", + "Template namespace initialisation script", + "ScriptImporter", + "Delete page script", + "Move page script", + "Command line script", + "Unknown user", + "MediaWiki message delivery", + "Flow talk page manager", + "Abuse filter", + "New user message", +}) + def is_orcid_username(name: str) -> bool: """True if a MediaWiki username is an ORCID iD (whitelisted ORCID login).""" @@ -77,6 +95,7 @@ def _iter_allusers(site: Any, batch: Any = "max") -> Iterator[Dict[str, Any]]: def enumerate_mw_users( site: Any, excluded_groups: Sequence[str] = ("bot",), + excluded_usernames: Sequence[str] = (), include_non_orcid: bool = True, limit: Optional[int] = None, batch: Any = "max", @@ -85,15 +104,19 @@ def enumerate_mw_users( Args: site: An object exposing ``api("query", ...)`` (an mwclient Site). - excluded_groups: Accounts in any of these groups are skipped (bots). + excluded_groups: Accounts in any of these groups are skipped (bots, admins). + excluded_usernames: Exact usernames to skip (reserved system accounts). include_non_orcid: If False, keep only ORCID-username accounts. limit: Keep at most this many accounts after filtering. batch: ``aulimit`` value passed to the API. """ excluded = set(excluded_groups) + excluded_names = set(excluded_usernames) users: List[MwUser] = [] for raw in _iter_allusers(site, batch=batch): user = MwUser.from_api(raw) + if user.name in excluded_names: + continue if excluded.intersection(user.groups): continue if not include_non_orcid and not user.is_orcid: diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index e541368e..69611cd6 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -21,6 +21,7 @@ from .reconcile import reconcile from .redirects import ensure_redirect from .sources import ( + RESERVED_USERNAMES, OrcidRateLimitError, enumerate_mw_users, fetch_orcid_record, @@ -51,9 +52,11 @@ def summary(self) -> str: def _build_proposals(config: SyncConfig, osw: Any, session: Any): """Enumerate MediaWiki users, enrich ORCID users and map to proposals.""" excluded = tuple(config.excluded_groups) if config.exclude_bot_group else () + reserved = RESERVED_USERNAMES if config.exclude_system_usernames else () mw_users = enumerate_mw_users( osw.mw_site, excluded_groups=excluded, + excluded_usernames=reserved, include_non_orcid=config.include_non_orcid, limit=config.limit, ) @@ -98,28 +101,36 @@ def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: entities = [] for resolved in resolution.resolved: proposed = resolved.change.proposed - if resolved.action == "create": - entities.append(build_user(proposed)) - report.created.append(proposed.full_page_title) - elif resolved.action == "update": - entities.append( - apply_update(resolved.change.existing, proposed, resolved.apply_fields) - ) - report.updated.append(proposed.full_page_title) - else: - report.skipped.append(proposed.full_page_title) + try: + if resolved.action == "create": + entities.append(build_user(proposed)) + report.created.append(proposed.full_page_title) + elif resolved.action == "update": + entities.append( + apply_update( + resolved.change.existing, proposed, resolved.apply_fields + ) + ) + report.updated.append(proposed.full_page_title) + else: + report.skipped.append(proposed.full_page_title) + except Exception as exc: # pragma: no cover - build failure path + report.failed[f"build:{proposed.username}"] = str(exc) if entities: - osw.store_entity( - OSW.StoreEntityParam( - entities=entities, overwrite=True, edit_comment="user-sync" + try: + osw.store_entity( + OSW.StoreEntityParam( + entities=entities, overwrite=True, edit_comment="user-sync" + ) ) - ) + except Exception as exc: # pragma: no cover - network failure path + report.failed["store"] = str(exc) def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None: + # Every in-scope user has an item (created or pre-existing), so ensure the + # redirect for all of them; a re-run repairs any missing redirect. for resolved in resolution.resolved: - if resolved.action not in ("create", "update"): - continue proposed = resolved.change.proposed try: written = ensure_redirect(osw, proposed.username, proposed.full_page_title) diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py index 48ff9ee2..b923768c 100644 --- a/tests/tools/test_user_sync_config.py +++ b/tests/tools/test_user_sync_config.py @@ -15,11 +15,18 @@ def test_defaults(): assert cfg.include_non_orcid is True assert cfg.create_redirects is True assert cfg.link_organizations is True - assert cfg.excluded_groups == ["bot"] + assert cfg.exclude_system_usernames is True + assert "bot" in cfg.excluded_groups + assert "sysop" in cfg.excluded_groups assert USER_CATEGORY.startswith("Category:OSW") assert ORGANIZATION_CATEGORY.startswith("Category:OSW") +def test_include_system_flag(): + cfg = config_from_args(["--include-system"]) + assert cfg.exclude_system_usernames is False + + def test_config_from_args_parses_flags(): cfg = config_from_args([ "--domain", diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py index 3d353984..7f637876 100644 --- a/tests/tools/test_user_sync_reconcile.py +++ b/tests/tools/test_user_sync_reconcile.py @@ -119,6 +119,29 @@ def test_missing_label_is_gap_fill(): assert change.diffs[0].name == "label" +class _OrgTrap: + """A loaded-entity stand-in whose organization attribute must never be read.""" + + __iris__ = {"organization": ["Item:OSWa", "Item:OSWb"]} + label = [SimpleNamespace(text="X Y")] + first_name = "X" + surname = "Y" + orcid = None + email: set = set() + website: set = set() + + @property + def organization(self): + raise RuntimeError("relation must not be resolved during reconcile") + + +def test_existing_orgs_read_from_iris_without_resolving(): + from osw.tools.user_sync.reconcile import existing_fields + + ef = existing_fields(_OrgTrap()) + assert ef["organizations"] == {"Item:OSWa", "Item:OSWb"} + + def test_reconcile_plan_counts(): proposed = [ _proposed(username="new-user"), diff --git a/tests/tools/test_user_sync_redirects.py b/tests/tools/test_user_sync_redirects.py index 5d933f62..1c1f36df 100644 --- a/tests/tools/test_user_sync_redirects.py +++ b/tests/tools/test_user_sync_redirects.py @@ -29,10 +29,10 @@ def __init__(self, wtSite=None, title=None, do_init=True): self.written = None self.edited = False - def get_content(self): + def get_slot_content(self, slot_key): return self._content - def set_content(self, content): + def set_slot_content(self, slot_key, content): self.written = content def edit(self, comment=None): @@ -67,3 +67,10 @@ def test_ensure_redirect_skips_real_page(monkeypatch): _patch(monkeypatch, {title: True}, {title: "This is my real user page."}) written = ensure_redirect(FakeOsw(), "bob", "Item:OSW1") assert written is None + + +def test_ensure_redirect_skips_when_already_correct(monkeypatch): + title = "User:carol" + _patch(monkeypatch, {title: True}, {title: "#REDIRECT [[Item:OSW1]]"}) + written = ensure_redirect(FakeOsw(), "carol", "Item:OSW1") + assert written is None diff --git a/tests/tools/test_user_sync_sources.py b/tests/tools/test_user_sync_sources.py index 9045aac1..9bc134ec 100644 --- a/tests/tools/test_user_sync_sources.py +++ b/tests/tools/test_user_sync_sources.py @@ -94,6 +94,22 @@ def test_enumerate_orcid_only_and_limit(): assert len(limited) == 1 +def test_enumerate_excludes_privileged_groups_and_reserved_names(): + site = FakeSite([ + _page([ + {"name": "0000-0002-6374-9831", "groups": ["user"]}, + {"name": "Admin", "groups": ["user", "sysop", "bureaucrat"]}, + {"name": "Maintenance script", "groups": ["user"]}, + ]) + ]) + users = enumerate_mw_users( + site, + excluded_groups=("bot", "sysop", "bureaucrat"), + excluded_usernames={"Maintenance script"}, + ) + assert [u.name for u in users] == ["0000-0002-6374-9831"] + + def test_partition_by_orcid(): users = [ MwUser(name="0000-0002-6374-9831"), From 830d12072f9acfcbbaa265e5b75e6db38ca0a0d1 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Wed, 16 Sep 2026 15:25:05 +0200 Subject: [PATCH 09/20] docs(user-sync): add user sync guide and finalize example --- docs/tools/user-sync.md | 57 +++++++++++++++++++++++++++++++++++++++++ examples/user_sync.py | 6 +++++ zensical.toml | 3 +++ 3 files changed, 66 insertions(+) create mode 100644 docs/tools/user-sync.md diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md new file mode 100644 index 00000000..0bcc273d --- /dev/null +++ b/docs/tools/user-sync.md @@ -0,0 +1,57 @@ +# User Item Sync + +Create or update OSW `User` items from the MediaWiki accounts of an OSL +instance, enriching ORCID users from the public ORCID API. The run is +idempotent and previews every change before writing. + +## When to use it + +OSL instances gain accounts from manual creation and from whitelisted ORCID +single sign-on (whose username is the ORCID iD). Those accounts have no +semantic `User` item until this tool reconciles them. + +## Run it + +```bash +uv run python examples/user_sync.py --domain your-instance.example.org --dry-run +``` + +Credentials are read from `accounts.pwd.yaml` (or `--cred-filepath`), the same +mechanism as `OswExpress`. Start with `--dry-run` to preview, then run without +it to write after confirming at the prompt. + +### Options + +| Flag | Effect | +| --- | --- | +| `--domain` | Target OSL domain. | +| `--cred-filepath` | Path to `accounts.pwd.yaml`. | +| `--dry-run` | Preview only; never writes. | +| `--yes` | Non-interactive: create and gap-fill, keep existing on conflicts. | +| `--limit N` | Process at most N accounts (testing). | +| `--orcid-only` | Only accounts whose username is an ORCID iD. | +| `--include-system` | Do not skip MediaWiki reserved system accounts. | +| `--no-redirects` | Do not create `User:` redirect pages. | +| `--no-organizations` | Do not link ORCID affiliations to Organization items. | + +## What it does + +1. Enumerate MediaWiki accounts, skipping bots and privileged groups + (`bot`, `sysop`, `bureaucrat`, `interface-admin`) and MediaWiki reserved + system usernames. +2. Enrich ORCID users from `https://pub.orcid.org/v3.0/` (names, public + email, websites, employment or affiliation). +3. Map each account to a proposed `User` item with a deterministic id + (uuid5 on the ORCID iD, else the username), so re-runs are idempotent. +4. Reconcile against existing items by `username` into NEW, GAP_FILL, CONFLICT + or UNCHANGED, then preview and resolve conflicts interactively. +5. Store items, create `User:` redirects to each item, ensure linked + Organization items, and verify by reloading. + +## Notes + +- MediaWiki does not expose other users' email or real name, so ORCID is the + only source of rich data. Non-ORCID accounts get username-based placeholder + names, flagged in the preview. +- On a first accepted run the library autofetches schemas and may regenerate + `src/osw/model/entity.py`; restore it if you do not want that change. diff --git a/examples/user_sync.py b/examples/user_sync.py index 3d9807e8..e5468495 100644 --- a/examples/user_sync.py +++ b/examples/user_sync.py @@ -16,6 +16,12 @@ def main() -> None: osw = OswExpress(domain=config.domain, cred_filepath=config.cred_filepath) report = run_user_sync(config, osw=osw) print(report.summary()) + for title in report.created: + print(f" created: {title}") + for title in report.updated: + print(f" updated: {title}") + for key, error in report.failed.items(): + print(f" FAILED {key}: {error}") if __name__ == "__main__": diff --git a/zensical.toml b/zensical.toml index 50218f41..1abfb91d 100644 --- a/zensical.toml +++ b/zensical.toml @@ -22,6 +22,9 @@ nav = [ { "Controllers" = "api/controller.md" }, { "Model" = "api/model.md" }, ]}, + { "Tools" = [ + { "User Sync" = "tools/user-sync.md" }, + ]}, { "Development" = "dev.md" }, ] From 31c2f82bae2f47502557098d4f7d9d82c8466f14 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Thu, 17 Sep 2026 14:52:45 +0200 Subject: [PATCH 10/20] feat(user-sync): opt-in extras, standard email, --auto-apply --- docs/tools/user-sync.md | 36 +++++++++---- src/osw/tools/user_sync/config.py | 64 +++++++++++++++++------ src/osw/tools/user_sync/interactive.py | 6 +-- src/osw/tools/user_sync/sources.py | 4 ++ src/osw/tools/user_sync/sync.py | 3 +- tests/tools/test_user_sync_config.py | 27 +++++++++- tests/tools/test_user_sync_interactive.py | 4 +- tests/tools/test_user_sync_sources.py | 11 ++++ 8 files changed, 123 insertions(+), 32 deletions(-) diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 0bcc273d..262821d6 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -22,31 +22,49 @@ it to write after confirming at the prompt. ### Options +By default the tool syncs both ORCID and MediaWiki-native accounts and writes +only core identity plus email. + | Flag | Effect | | --- | --- | | `--domain` | Target OSL domain. | | `--cred-filepath` | Path to `accounts.pwd.yaml`. | | `--dry-run` | Preview only; never writes. | -| `--yes` | Non-interactive: create and gap-fill, keep existing on conflicts. | +| `--auto-apply` | Non-interactive: apply creates, gap-fills and removals; keep existing on conflicts. | | `--limit N` | Process at most N accounts (testing). | -| `--orcid-only` | Only accounts whose username is an ORCID iD. | +| `--orcid-only` | Only ORCID-username accounts (excludes `--mw-only`). | +| `--mw-only` | Only non-ORCID (MediaWiki-native) accounts (excludes `--orcid-only`). | | `--include-system` | Do not skip MediaWiki reserved system accounts. | | `--no-redirects` | Do not create `User:` redirect pages. | -| `--no-organizations` | Do not link ORCID affiliations to Organization items. | +| `--with-websites` | Also store ORCID researcher URLs (opt-in). | +| `--with-organizations` | Also create and link Organization items from ORCID affiliations (opt-in). | +| `--with-extras` | Enable both websites and organizations. | ## What it does 1. Enumerate MediaWiki accounts, skipping bots and privileged groups (`bot`, `sysop`, `bureaucrat`, `interface-admin`) and MediaWiki reserved system usernames. -2. Enrich ORCID users from `https://pub.orcid.org/v3.0/` (names, public - email, websites, employment or affiliation). +2. Enrich ORCID users from `https://pub.orcid.org/v3.0/`: names and email + always; researcher URLs and affiliations only when their flag is enabled. 3. Map each account to a proposed `User` item with a deterministic id (uuid5 on the ORCID iD, else the username), so re-runs are idempotent. -4. Reconcile against existing items by `username` into NEW, GAP_FILL, CONFLICT - or UNCHANGED, then preview and resolve conflicts interactively. -5. Store items, create `User:` redirects to each item, ensure linked - Organization items, and verify by reloading. +4. Reconcile against existing items by `username` into NEW, GAP_FILL, CONFLICT, + REMOVE or UNCHANGED, then preview; gap-fills and removals apply + automatically, conflicts are resolved interactively. +5. Store items, create `User:` redirects to each item, ensure any + linked Organization items, verify by reloading, and warn about users with no + email. + +## Data written + +| Field | Policy | +| --- | --- | +| `username`, `first_name`, `surname`, `label`, `orcid` | Always (core identity). | +| `email` | Standard: always attempted; missing email is warned, not fatal; never removed. | +| `website` | Opt-in (`--with-websites`); removed from existing items when disabled. | +| `organization` | Opt-in (`--with-organizations`); removed from existing items when disabled. | +| `employment_contract_status` | Never written; removed from existing items (data protection). | ## Notes diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py index 73c8e80e..88d4dbb8 100644 --- a/src/osw/tools/user_sync/config.py +++ b/src/osw/tools/user_sync/config.py @@ -4,7 +4,7 @@ import argparse from dataclasses import dataclass, field -from typing import List, Optional +from typing import List, Optional, Set # Category page titles of the target item types (see opensemantic.base.v1). USER_CATEGORY = "Category:OSWd9aa0bca9b0040d8af6f5c091bf9eec7" @@ -17,6 +17,10 @@ # All are standard MediaWiki groups, so this is not a per-instance skip-list. SYSTEM_GROUPS = ["bot", "sysop", "bureaucrat", "interface-admin"] +# User fields the script must never write and must remove from existing items +# (data protection). +PROTECTED_FIELDS = ("employment_contract_status",) + @dataclass class SyncConfig: @@ -25,16 +29,30 @@ class SyncConfig: domain: Optional[str] = None cred_filepath: Optional[str] = None dry_run: bool = False - assume_yes: bool = False + auto_apply: bool = False limit: Optional[int] = None + include_orcid: bool = True include_non_orcid: bool = True exclude_bot_group: bool = True exclude_system_usernames: bool = True create_redirects: bool = True - link_organizations: bool = True + # Optional enrichment beyond core identity, opt-in (off by default). + # Email is not optional: it is always attempted, and missing emails are + # reported as warnings rather than failing the run. + include_websites: bool = False + link_organizations: bool = False orcid_api_base: str = ORCID_API_BASE_DEFAULT excluded_groups: List[str] = field(default_factory=lambda: list(SYSTEM_GROUPS)) + def enabled_optional_fields(self) -> Set[str]: + """Reconcile field names for the enabled optional data.""" + enabled: Set[str] = set() + if self.include_websites: + enabled.add("websites") + if self.link_organizations: + enabled.add("organizations") + return enabled + def build_arg_parser() -> argparse.ArgumentParser: """Build the argument parser used by the example wrapper.""" @@ -53,10 +71,10 @@ def build_arg_parser() -> argparse.ArgumentParser: help="Show the preview only; do not write anything.", ) parser.add_argument( - "--yes", - dest="assume_yes", + "--auto-apply", + dest="auto_apply", action="store_true", - help="Non-interactive: apply new items and gap-fills, keep existing on conflicts.", + help="Non-interactive: apply creates, gap-fills and removals; keep existing on conflicts.", ) parser.add_argument( "--limit", @@ -70,24 +88,36 @@ def build_arg_parser() -> argparse.ArgumentParser: help="Do not create User: redirect pages.", ) parser.add_argument( - "--no-organizations", - dest="link_organizations", - action="store_false", - help="Do not resolve or link ORCID affiliations to Organization items.", + "--with-websites", + dest="include_websites", + action="store_true", + help="Also store ORCID researcher URLs on the user item (opt-in).", ) parser.add_argument( - "--include-non-orcid", - dest="include_non_orcid", + "--with-organizations", + dest="link_organizations", action="store_true", - default=True, - help="Also sync non-bot accounts without an ORCID username (default).", + help="Also resolve ORCID affiliations to Organization items and link them.", ) parser.add_argument( + "--with-extras", + dest="with_extras", + action="store_true", + help="Enable all optional data: websites and organizations.", + ) + account_scope = parser.add_mutually_exclusive_group() + account_scope.add_argument( "--orcid-only", dest="include_non_orcid", action="store_false", help="Sync only accounts whose username is an ORCID iD.", ) + account_scope.add_argument( + "--mw-only", + dest="include_orcid", + action="store_false", + help="Sync only non-ORCID (MediaWiki-native) accounts.", + ) parser.add_argument( "--include-system", dest="exclude_system_usernames", @@ -104,10 +134,12 @@ def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig: domain=args.domain, cred_filepath=args.cred_filepath, dry_run=args.dry_run, - assume_yes=args.assume_yes, + auto_apply=args.auto_apply, limit=args.limit, + include_orcid=args.include_orcid, include_non_orcid=args.include_non_orcid, exclude_system_usernames=args.exclude_system_usernames, create_redirects=args.create_redirects, - link_organizations=args.link_organizations, + include_websites=args.include_websites or args.with_extras, + link_organizations=args.link_organizations or args.with_extras, ) diff --git a/src/osw/tools/user_sync/interactive.py b/src/osw/tools/user_sync/interactive.py index e244ed3d..0ea05d8d 100644 --- a/src/osw/tools/user_sync/interactive.py +++ b/src/osw/tools/user_sync/interactive.py @@ -143,7 +143,7 @@ def resolve_plan( plan: ReconcilePlan, prompter: Prompter, dry_run: bool = False, - assume_yes: bool = False, + auto_apply: bool = False, summary_lines: Optional[Sequence[str]] = None, ) -> Resolution: """Preview the plan and resolve conflicts into a write decision.""" @@ -154,7 +154,7 @@ def resolve_plan( conflict_mode: Optional[str] = None if plan.conflicts and not dry_run: - if assume_yes: + if auto_apply: conflict_mode = "keep" else: choice = prompter.ask( @@ -188,7 +188,7 @@ def resolve_plan( if dry_run: accepted_run = False - elif assume_yes: + elif auto_apply: accepted_run = True else: n_create = sum(1 for r in resolved if r.action == "create") diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py index 7e40b137..b666aa30 100644 --- a/src/osw/tools/user_sync/sources.py +++ b/src/osw/tools/user_sync/sources.py @@ -96,6 +96,7 @@ def enumerate_mw_users( site: Any, excluded_groups: Sequence[str] = ("bot",), excluded_usernames: Sequence[str] = (), + include_orcid: bool = True, include_non_orcid: bool = True, limit: Optional[int] = None, batch: Any = "max", @@ -106,6 +107,7 @@ def enumerate_mw_users( site: An object exposing ``api("query", ...)`` (an mwclient Site). excluded_groups: Accounts in any of these groups are skipped (bots, admins). excluded_usernames: Exact usernames to skip (reserved system accounts). + include_orcid: If False, skip ORCID-username accounts. include_non_orcid: If False, keep only ORCID-username accounts. limit: Keep at most this many accounts after filtering. batch: ``aulimit`` value passed to the API. @@ -121,6 +123,8 @@ def enumerate_mw_users( continue if not include_non_orcid and not user.is_orcid: continue + if not include_orcid and user.is_orcid: + continue users.append(user) if limit is not None and len(users) >= limit: break diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index 69611cd6..6ad8af82 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -57,6 +57,7 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any): osw.mw_site, excluded_groups=excluded, excluded_usernames=reserved, + include_orcid=config.include_orcid, include_non_orcid=config.include_non_orcid, limit=config.limit, ) @@ -184,7 +185,7 @@ def run_user_sync( plan, prompter, dry_run=config.dry_run, - assume_yes=config.assume_yes, + auto_apply=config.auto_apply, summary_lines=summary_lines, ) diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py index b923768c..32eee0d4 100644 --- a/tests/tools/test_user_sync_config.py +++ b/tests/tools/test_user_sync_config.py @@ -14,7 +14,10 @@ def test_defaults(): cfg = SyncConfig() assert cfg.include_non_orcid is True assert cfg.create_redirects is True - assert cfg.link_organizations is True + # Websites and organizations are opt-in (off by default); email is standard. + assert cfg.include_websites is False + assert cfg.link_organizations is False + assert cfg.enabled_optional_fields() == set() assert cfg.exclude_system_usernames is True assert "bot" in cfg.excluded_groups assert "sysop" in cfg.excluded_groups @@ -27,6 +30,18 @@ def test_include_system_flag(): assert cfg.exclude_system_usernames is False +def test_optional_field_flags(): + cfg = config_from_args(["--with-organizations", "--with-websites"]) + assert cfg.link_organizations is True + assert cfg.include_websites is True + assert cfg.enabled_optional_fields() == {"organizations", "websites"} + + +def test_with_extras_enables_all(): + cfg = config_from_args(["--with-extras"]) + assert cfg.enabled_optional_fields() == {"websites", "organizations"} + + def test_config_from_args_parses_flags(): cfg = config_from_args([ "--domain", @@ -40,6 +55,16 @@ def test_config_from_args_parses_flags(): assert cfg.dry_run is True assert cfg.limit == 5 assert cfg.include_non_orcid is False + assert cfg.include_orcid is True + + +def test_account_scope_defaults_and_mw_only(): + cfg = SyncConfig() + assert cfg.include_orcid is True + assert cfg.include_non_orcid is True # both are standard by default + mw_only = config_from_args(["--mw-only"]) + assert mw_only.include_orcid is False + assert mw_only.include_non_orcid is True def test_report_summary(): diff --git a/tests/tools/test_user_sync_interactive.py b/tests/tools/test_user_sync_interactive.py index 912a72ee..461c41d8 100644 --- a/tests/tools/test_user_sync_interactive.py +++ b/tests/tools/test_user_sync_interactive.py @@ -136,9 +136,9 @@ def test_resolve_dry_run_makes_no_writes_and_no_prompts(): assert io.answers == [] # nothing was asked -def test_resolve_assume_yes_keeps_conflicts(): +def test_resolve_auto_apply_keeps_conflicts(): prompter, io = _prompter([]) - res = resolve_plan(_mixed_plan(), prompter, assume_yes=True) + res = resolve_plan(_mixed_plan(), prompter, auto_apply=True) assert res.accepted is True updates = {r.change.proposed.username: r.apply_fields for r in res.updates()} assert updates == {"gap1": {"emails"}, "mix1": {"emails"}} diff --git a/tests/tools/test_user_sync_sources.py b/tests/tools/test_user_sync_sources.py index 9bc134ec..0895c3cb 100644 --- a/tests/tools/test_user_sync_sources.py +++ b/tests/tools/test_user_sync_sources.py @@ -110,6 +110,17 @@ def test_enumerate_excludes_privileged_groups_and_reserved_names(): assert [u.name for u in users] == ["0000-0002-6374-9831"] +def test_enumerate_mw_only_skips_orcid(): + site = FakeSite([ + _page([ + {"name": "0000-0002-6374-9831", "groups": ["user"]}, + {"name": "Alice", "groups": ["user"]}, + ]) + ]) + users = enumerate_mw_users(site, include_orcid=False) + assert [u.name for u in users] == ["Alice"] + + def test_partition_by_orcid(): users = [ MwUser(name="0000-0002-6374-9831"), From 4e38b292972bb57faaafbb66feeed55a9822cea6 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Thu, 17 Sep 2026 15:41:34 +0200 Subject: [PATCH 11/20] feat(user-sync): gate optional fields and opt-in prune removals --- src/osw/tools/user_sync/config.py | 10 +++++ src/osw/tools/user_sync/interactive.py | 8 +++- src/osw/tools/user_sync/mapping.py | 11 +++-- src/osw/tools/user_sync/reconcile.py | 52 +++++++++++++++++++---- src/osw/tools/user_sync/sync.py | 12 +++++- tests/tools/test_user_sync_config.py | 6 +++ tests/tools/test_user_sync_interactive.py | 36 ++++++++++++++++ tests/tools/test_user_sync_mapping.py | 20 ++++++++- tests/tools/test_user_sync_reconcile.py | 42 ++++++++++++++++++ 9 files changed, 179 insertions(+), 18 deletions(-) diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py index 88d4dbb8..57d25b83 100644 --- a/src/osw/tools/user_sync/config.py +++ b/src/osw/tools/user_sync/config.py @@ -41,6 +41,8 @@ class SyncConfig: # reported as warnings rather than failing the run. include_websites: bool = False link_organizations: bool = False + # Remove disabled optional fields and protected fields from existing items. + prune: bool = False orcid_api_base: str = ORCID_API_BASE_DEFAULT excluded_groups: List[str] = field(default_factory=lambda: list(SYSTEM_GROUPS)) @@ -105,6 +107,13 @@ def build_arg_parser() -> argparse.ArgumentParser: action="store_true", help="Enable all optional data: websites and organizations.", ) + parser.add_argument( + "--prune", + dest="prune", + action="store_true", + help="Remove disabled optional fields and employment_contract_status from " + "existing items (data cleanup). Off by default; a normal run only adds.", + ) account_scope = parser.add_mutually_exclusive_group() account_scope.add_argument( "--orcid-only", @@ -142,4 +151,5 @@ def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig: create_redirects=args.create_redirects, include_websites=args.include_websites or args.with_extras, link_organizations=args.link_organizations or args.with_extras, + prune=args.prune, ) diff --git a/src/osw/tools/user_sync/interactive.py b/src/osw/tools/user_sync/interactive.py index 0ea05d8d..835245ca 100644 --- a/src/osw/tools/user_sync/interactive.py +++ b/src/osw/tools/user_sync/interactive.py @@ -15,6 +15,7 @@ GAP_FILL, NEW, RECONCILED_FIELDS, + REMOVE, UNCHANGED, ReconcilePlan, UserChange, @@ -48,7 +49,10 @@ def render_preview(plan: ReconcilePlan) -> List[str]: marker = _MARKERS[change.category] detail = "" if change.diffs: - detail = " (" + ", ".join(d.name for d in change.diffs) + ")" + names = [ + ("-" + d.name if d.status == REMOVE else d.name) for d in change.diffs + ] + detail = " (" + ", ".join(names) + ")" placeholder = " [placeholder name]" if change.proposed.placeholder_name else "" lines.append( f" {marker} {change.proposed.username} {change.category}{detail}{placeholder}" @@ -178,7 +182,7 @@ def resolve_plan( ResolvedChange(change, "update", {d.name for d in change.diffs}) ) else: # CONFLICT - accepted = {d.name for d in change.diffs if d.status == GAP_FILL} + accepted = {d.name for d in change.diffs if d.status in (GAP_FILL, REMOVE)} if conflict_mode == "all": accepted |= {d.name for d in change.diffs if d.status == CONFLICT} elif conflict_mode == "review": diff --git a/src/osw/tools/user_sync/mapping.py b/src/osw/tools/user_sync/mapping.py index e734c142..b5dcd414 100644 --- a/src/osw/tools/user_sync/mapping.py +++ b/src/osw/tools/user_sync/mapping.py @@ -113,9 +113,14 @@ def _derive_names( def map_user( mw_user: MwUser, profile: Optional[OrcidProfile] = None, - link_organizations: bool = True, + include_websites: bool = False, + link_organizations: bool = False, ) -> Tuple[ProposedUser, List[ProposedOrganization]]: - """Build a ProposedUser (and any linked organizations) from the sources.""" + """Build a ProposedUser (and any linked organizations) from the sources. + + Email and names are always taken from the profile (core). Websites and + organizations are populated only when their flag is enabled. + """ orcid_uri = f"https://orcid.org/{mw_user.orcid_id}" if mw_user.orcid_id else None first, surname, label, placeholder = _derive_names(profile, mw_user.name) @@ -139,7 +144,7 @@ def map_user( label=label, orcid=orcid_uri, emails=list(profile.emails) if profile else [], - websites=list(profile.urls) if profile else [], + websites=list(profile.urls) if (profile and include_websites) else [], organizations=org_titles, placeholder_name=placeholder, ) diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py index 479473ad..5e1c2ae9 100644 --- a/src/osw/tools/user_sync/reconcile.py +++ b/src/osw/tools/user_sync/reconcile.py @@ -1,7 +1,8 @@ """Reconcile proposed User items against the ones already stored. Pure logic: given proposed users and an index of existing items, classify each -into NEW, GAP_FILL, CONFLICT or UNCHANGED and record the per-field differences. +into NEW, GAP_FILL, CONFLICT or UNCHANGED and record the per-field differences +(including REMOVE diffs for disabled optional fields and protected fields). The interactive layer consumes this plan; nothing here does IO. """ @@ -9,14 +10,16 @@ from collections import Counter from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from typing import Any, Dict, FrozenSet, List, Optional +from .config import PROTECTED_FIELDS from .existing import ExistingUsers from .mapping import ProposedUser NEW = "new" GAP_FILL = "gap_fill" CONFLICT = "conflict" +REMOVE = "remove" UNCHANGED = "unchanged" # username is the match key and is never reconciled as a value. @@ -24,6 +27,8 @@ SET_FIELDS = ("emails", "websites", "organizations") URL_FIELDS = ("orcid", "websites") RECONCILED_FIELDS = SCALAR_FIELDS + SET_FIELDS +# Fields that are only written when opted in; removed from existing when disabled. +OPTIONAL_FIELDS = ("websites", "organizations") def _is_empty(value: Any) -> bool: @@ -76,6 +81,12 @@ def _existing_orgs(entity: Any) -> Any: return None +def _existing_has_iri(entity: Any, name: str) -> bool: + """True if the loaded entity carries a relation IRI for ``name``.""" + iris = getattr(entity, "__iris__", None) + return isinstance(iris, dict) and bool(iris.get(name)) + + def existing_fields(entity: Any) -> Dict[str, Any]: """Normalized comparable field view of a loaded User entity.""" return { @@ -109,7 +120,7 @@ class FieldDiff: name: str existing: Any proposed: Any - status: str # GAP_FILL or CONFLICT + status: str # GAP_FILL, CONFLICT or REMOVE @dataclass @@ -131,20 +142,37 @@ def _compare(existing_val: Any, proposed_val: Any) -> Optional[str]: return UNCHANGED if existing_val == proposed_val else CONFLICT -def reconcile_user(proposed: ProposedUser, existing: Optional[Any]) -> UserChange: +def reconcile_user( + proposed: ProposedUser, + existing: Optional[Any], + enabled_fields: FrozenSet[str] = frozenset(), + prune: bool = False, +) -> UserChange: if existing is None: return UserChange(proposed=proposed, existing=None, category=NEW) ef = existing_fields(existing) pf = proposed_fields(proposed) diffs: List[FieldDiff] = [] - has_conflict = has_gap = False + has_conflict = has_change = False for name in RECONCILED_FIELDS: + if name in OPTIONAL_FIELDS and name not in enabled_fields: + # Disabled optional field: remove any existing value only when pruning. + if prune and not _is_empty(ef[name]): + diffs.append(FieldDiff(name, ef[name], None, REMOVE)) + has_change = True + continue status = _compare(ef[name], pf[name]) if status in (GAP_FILL, CONFLICT): diffs.append(FieldDiff(name, ef[name], pf[name], status)) has_conflict = has_conflict or status == CONFLICT - has_gap = has_gap or status == GAP_FILL - category = CONFLICT if has_conflict else GAP_FILL if has_gap else UNCHANGED + has_change = has_change or status == GAP_FILL + # Protected fields are stripped from existing items only when pruning. + if prune: + for name in PROTECTED_FIELDS: + if _existing_has_iri(existing, name): + diffs.append(FieldDiff(name, "set", None, REMOVE)) + has_change = True + category = CONFLICT if has_conflict else GAP_FILL if has_change else UNCHANGED return UserChange( proposed=proposed, existing=existing, category=category, diffs=diffs ) @@ -171,7 +199,13 @@ def counts(self) -> Dict[str, int]: def reconcile( - proposed_users: List[ProposedUser], existing: ExistingUsers + proposed_users: List[ProposedUser], + existing: ExistingUsers, + enabled_fields: FrozenSet[str] = frozenset(), + prune: bool = False, ) -> ReconcilePlan: - changes = [reconcile_user(p, existing.get(p.username)) for p in proposed_users] + changes = [ + reconcile_user(p, existing.get(p.username), enabled_fields, prune) + for p in proposed_users + ] return ReconcilePlan(changes=changes) diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index 6ad8af82..bc704940 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -77,7 +77,10 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any): except OrcidRateLimitError: profile = None proposed, orgs = map_user( - mw_user, profile, link_organizations=config.link_organizations + mw_user, + profile, + include_websites=config.include_websites, + link_organizations=config.link_organizations, ) proposed_users.append(proposed) for org in orgs: @@ -173,7 +176,12 @@ def run_user_sync( proposed_users, org_map = _build_proposals(config, osw, session) existing = load_existing_users(osw) - plan = reconcile(proposed_users, existing) + plan = reconcile( + proposed_users, + existing, + enabled_fields=frozenset(config.enabled_optional_fields()), + prune=config.prune, + ) summary_lines = [] if config.link_organizations and org_map: diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py index 32eee0d4..fcf3cc05 100644 --- a/tests/tools/test_user_sync_config.py +++ b/tests/tools/test_user_sync_config.py @@ -17,6 +17,7 @@ def test_defaults(): # Websites and organizations are opt-in (off by default); email is standard. assert cfg.include_websites is False assert cfg.link_organizations is False + assert cfg.prune is False # removals are opt-in assert cfg.enabled_optional_fields() == set() assert cfg.exclude_system_usernames is True assert "bot" in cfg.excluded_groups @@ -42,6 +43,11 @@ def test_with_extras_enables_all(): assert cfg.enabled_optional_fields() == {"websites", "organizations"} +def test_prune_flag(): + assert config_from_args([]).prune is False + assert config_from_args(["--prune"]).prune is True + + def test_config_from_args_parses_flags(): cfg = config_from_args([ "--domain", diff --git a/tests/tools/test_user_sync_interactive.py b/tests/tools/test_user_sync_interactive.py index 461c41d8..ba0a47e9 100644 --- a/tests/tools/test_user_sync_interactive.py +++ b/tests/tools/test_user_sync_interactive.py @@ -13,6 +13,7 @@ CONFLICT, GAP_FILL, NEW, + REMOVE, UNCHANGED, FieldDiff, ReconcilePlan, @@ -190,6 +191,41 @@ def test_final_confirm_no(): assert res.creates()[0].change.proposed.username == "new1" +def test_removals_auto_apply_and_marked_in_preview(): + plan = ReconcilePlan( + changes=[ + _change( + GAP_FILL, + "u1", + [FieldDiff("organizations", {"x"}, None, REMOVE)], + ) + ] + ) + assert any("-organizations" in ln for ln in render_preview(plan)) + prompter, _ = _prompter(["y"]) # only the final confirm + res = resolve_plan(plan, prompter) + assert res.updates()[0].apply_fields == {"organizations"} + assert res.accepted is True + + +def test_conflict_user_still_auto_applies_removal(): + plan = ReconcilePlan( + changes=[ + _change( + CONFLICT, + "u1", + [ + FieldDiff("surname", "Old", "New", CONFLICT), + FieldDiff("organizations", {"x"}, None, REMOVE), + ], + ) + ] + ) + prompter, _ = _prompter(["k", "y"]) # keep existing on conflict, final yes + res = resolve_plan(plan, prompter) + assert res.updates()[0].apply_fields == {"organizations"} + + def test_prompter_reprompts_on_invalid(): prompter, io = _prompter(["z", "k"]) assert prompter.ask("q", ("k", "n")) == "k" diff --git a/tests/tools/test_user_sync_mapping.py b/tests/tools/test_user_sync_mapping.py index 8c86abef..0b6c72f5 100644 --- a/tests/tools/test_user_sync_mapping.py +++ b/tests/tools/test_user_sync_mapping.py @@ -29,7 +29,9 @@ def test_map_orcid_user_full_profile(): urls=["https://example.org"], affiliations=[OrcidAffiliation(organization="Example University")], ) - proposed, orgs = map_user(mw, profile) + proposed, orgs = map_user( + mw, profile, include_websites=True, link_organizations=True + ) assert isinstance(proposed, ProposedUser) assert proposed.username == ORCID assert proposed.orcid == f"https://orcid.org/{ORCID}" @@ -99,7 +101,7 @@ def test_duplicate_affiliations_deduped(): OrcidAffiliation(organization="example university"), ] ) - proposed, orgs = map_user(mw, profile) + proposed, orgs = map_user(mw, profile, link_organizations=True) assert len(orgs) == 1 assert len(proposed.organizations) == 1 @@ -110,3 +112,17 @@ def test_link_organizations_disabled(): proposed, orgs = map_user(mw, profile, link_organizations=False) assert orgs == [] assert proposed.organizations == [] + + +def test_email_always_included_but_extras_gated_by_default(): + mw = MwUser(name=ORCID) + profile = _profile( + emails=["l@example.org"], + urls=["https://example.org"], + affiliations=[OrcidAffiliation(organization="Example U")], + ) + proposed, orgs = map_user(mw, profile) # defaults: websites/orgs off + assert proposed.emails == ["l@example.org"] + assert proposed.websites == [] + assert proposed.organizations == [] + assert orgs == [] diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py index 7f637876..4a592afd 100644 --- a/tests/tools/test_user_sync_reconcile.py +++ b/tests/tools/test_user_sync_reconcile.py @@ -9,6 +9,7 @@ CONFLICT, GAP_FILL, NEW, + REMOVE, UNCHANGED, reconcile, reconcile_user, @@ -109,10 +110,51 @@ def test_set_order_independent(): change = reconcile_user( _proposed(organizations=["Item:OSWb", "Item:OSWa"]), _entity(organization=["Item:OSWa", "Item:OSWb"]), + enabled_fields=frozenset({"organizations"}), ) assert change.category == UNCHANGED +def test_disabled_optional_field_removed_when_pruning(): + # organizations disabled + existing value + prune -> REMOVE + change = reconcile_user( + _proposed(), _entity(organization=["Item:OSWa"]), prune=True + ) + assert change.category == GAP_FILL + removes = [d for d in change.diffs if d.status == REMOVE] + assert [d.name for d in removes] == ["organizations"] + assert removes[0].proposed is None + + +def test_prune_off_keeps_disabled_and_protected(): + entity = _entity() + entity.__iris__ = { + "organization": ["Item:OSWa"], + "employment_contract_status": "Item:OSWx", + } + change = reconcile_user(_proposed(), entity) # prune off (default) + assert change.category == UNCHANGED + assert change.diffs == [] + + +def test_enabled_optional_field_not_removed(): + change = reconcile_user( + _proposed(organizations=["Item:OSWa"]), + _entity(organization=["Item:OSWa"]), + enabled_fields=frozenset({"organizations"}), + ) + assert change.category == UNCHANGED + + +def test_protected_field_removed_when_pruning(): + entity = _entity() + entity.__iris__ = {"employment_contract_status": "Item:OSWx"} + change = reconcile_user(_proposed(), entity, prune=True) + removes = [d.name for d in change.diffs if d.status == REMOVE] + assert "employment_contract_status" in removes + assert change.category == GAP_FILL + + def test_missing_label_is_gap_fill(): change = reconcile_user(_proposed(label="Lukas Koschmieder"), _entity(label=None)) assert change.category == GAP_FILL From 1899d7f2c070a3bee614602509109bc97ea19f79 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Thu, 17 Sep 2026 15:41:44 +0200 Subject: [PATCH 12/20] docs(user-sync): document prune and opt-in extras --- docs/tools/user-sync.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 262821d6..627faa0a 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -39,6 +39,7 @@ only core identity plus email. | `--with-websites` | Also store ORCID researcher URLs (opt-in). | | `--with-organizations` | Also create and link Organization items from ORCID affiliations (opt-in). | | `--with-extras` | Enable both websites and organizations. | +| `--prune` | Remove disabled optional fields and `employment_contract_status` from existing items (default off; a normal run only adds). | ## What it does @@ -50,8 +51,9 @@ only core identity plus email. 3. Map each account to a proposed `User` item with a deterministic id (uuid5 on the ORCID iD, else the username), so re-runs are idempotent. 4. Reconcile against existing items by `username` into NEW, GAP_FILL, CONFLICT, - REMOVE or UNCHANGED, then preview; gap-fills and removals apply - automatically, conflicts are resolved interactively. + REMOVE or UNCHANGED, then preview; gap-fills apply automatically and conflicts + are resolved interactively. Removals only appear with `--prune`; a normal run + never removes anything. 5. Store items, create `User:` redirects to each item, ensure any linked Organization items, verify by reloading, and warn about users with no email. @@ -62,9 +64,9 @@ only core identity plus email. | --- | --- | | `username`, `first_name`, `surname`, `label`, `orcid` | Always (core identity). | | `email` | Standard: always attempted; missing email is warned, not fatal; never removed. | -| `website` | Opt-in (`--with-websites`); removed from existing items when disabled. | -| `organization` | Opt-in (`--with-organizations`); removed from existing items when disabled. | -| `employment_contract_status` | Never written; removed from existing items (data protection). | +| `website` | Opt-in (`--with-websites`); removed from existing only with `--prune`. | +| `organization` | Opt-in (`--with-organizations`); removed from existing only with `--prune`. | +| `employment_contract_status` | Never written; removed from existing only with `--prune`. | ## Notes From 828f02c1df1ca525dc040f89fbd5d1b5232db4dd Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Thu, 17 Sep 2026 15:51:58 +0200 Subject: [PATCH 13/20] feat(user-sync): strip employment status and warn on missing email --- src/osw/tools/user_sync/build.py | 28 +++++++++-- src/osw/tools/user_sync/sync.py | 17 ++++++- tests/tools/test_user_sync_build.py | 77 +++++++++++++++++++++++++++++ tests/tools/test_user_sync_sync.py | 2 + 4 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 tests/tools/test_user_sync_build.py diff --git a/src/osw/tools/user_sync/build.py b/src/osw/tools/user_sync/build.py index 24d7fcc8..6bf30c9a 100644 --- a/src/osw/tools/user_sync/build.py +++ b/src/osw/tools/user_sync/build.py @@ -12,6 +12,7 @@ from opensemantic.base.v1 import Organization, User +from .config import PROTECTED_FIELDS from .mapping import ProposedOrganization, ProposedUser # The concrete DisplayName type used by the label field. @@ -22,12 +23,20 @@ def _labels(text: str) -> List[Any]: return [_DISPLAY_NAME(text=text)] +def _strip_protected(entity: Any) -> None: + """Remove protected relation defaults (e.g. employment_contract_status).""" + iris = getattr(entity, "__iris__", None) + if isinstance(iris, dict): + for name in PROTECTED_FIELDS: + iris.pop(name, None) + + def build_organization(proposed: ProposedOrganization) -> Organization: return Organization(uuid=proposed.uuid, label=_labels(proposed.name)) def build_user(proposed: ProposedUser) -> User: - """Build a full User entity for creation.""" + """Build a full User entity for creation (never carries protected fields).""" args: dict = { "uuid": proposed.uuid, "username": proposed.username, @@ -43,11 +52,17 @@ def build_user(proposed: ProposedUser) -> User: args["website"] = set(proposed.websites) if proposed.organizations: args["organization"] = list(proposed.organizations) - return User(**args) + entity = User(**args) + _strip_protected(entity) + return entity def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> Any: - """Apply the accepted fields onto a loaded existing entity, in place.""" + """Apply the accepted fields onto a loaded existing entity, in place. + + Fields whose proposal is empty (REMOVE) are cleared; protected fields in + ``apply_fields`` are stripped from the entity's ``__iris__`` map. + """ if "label" in apply_fields: entity.label = _labels(proposed.label) if "first_name" in apply_fields: @@ -61,5 +76,10 @@ def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> if "websites" in apply_fields: entity.website = set(proposed.websites) if "organizations" in apply_fields: - entity.__iris__["organization"] = list(proposed.organizations) + if proposed.organizations: + entity.__iris__["organization"] = list(proposed.organizations) + else: + entity.__iris__.pop("organization", None) + if any(name in apply_fields for name in PROTECTED_FIELDS): + _strip_protected(entity) return entity diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index bc704940..516deaa5 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -38,6 +38,7 @@ class SyncReport: failed: Dict[str, str] = field(default_factory=dict) redirects_created: List[str] = field(default_factory=list) organizations: List[str] = field(default_factory=list) + missing_email: List[str] = field(default_factory=list) accepted: bool = False def summary(self) -> str: @@ -45,7 +46,8 @@ def summary(self) -> str: return ( f"{state}: created={len(self.created)} updated={len(self.updated)} " f"skipped={len(self.skipped)} failed={len(self.failed)} " - f"redirects={len(self.redirects_created)} orgs={len(self.organizations)}" + f"redirects={len(self.redirects_created)} orgs={len(self.organizations)} " + f"missing_email={len(self.missing_email)}" ) @@ -144,6 +146,14 @@ def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> N report.failed[f"redirect:{proposed.username}"] = str(exc) +def _warn_missing_email(prompter: Prompter, missing: List[str]) -> None: + if missing: + prompter.write( + f"WARNING: {len(missing)} user(s) have no ORCID email: " + + ", ".join(missing) + ) + + def _verify(osw: Any, report: SyncReport) -> None: titles = report.created + report.updated if not titles: @@ -175,6 +185,7 @@ def run_user_sync( prompter = prompter or Prompter() proposed_users, org_map = _build_proposals(config, osw, session) + missing_email = [p.username for p in proposed_users if not p.emails] existing = load_existing_users(osw) plan = reconcile( proposed_users, @@ -197,8 +208,9 @@ def run_user_sync( summary_lines=summary_lines, ) - report = SyncReport(accepted=resolution.accepted) + report = SyncReport(accepted=resolution.accepted, missing_email=missing_email) if not resolution.accepted: + _warn_missing_email(prompter, missing_email) return report if config.link_organizations: @@ -207,4 +219,5 @@ def run_user_sync( if config.create_redirects: _create_redirects(osw, resolution, report) _verify(osw, report) + _warn_missing_email(prompter, missing_email) return report diff --git a/tests/tools/test_user_sync_build.py b/tests/tools/test_user_sync_build.py new file mode 100644 index 00000000..5f22adfb --- /dev/null +++ b/tests/tools/test_user_sync_build.py @@ -0,0 +1,77 @@ +"""Unit tests for entity building/updating helpers (fakes; no live models).""" + +import uuid +from types import SimpleNamespace + +from osw.tools.user_sync.build import _strip_protected, apply_update +from osw.tools.user_sync.mapping import ProposedUser + + +def _proposed(**kw): + base = dict( + uuid=uuid.uuid4(), + username="u", + first_name="F", + surname="S", + label="F S", + orcid=None, + emails=[], + websites=[], + organizations=[], + ) + base.update(kw) + return ProposedUser(**base) + + +def _entity(): + return SimpleNamespace( + label=[], + first_name="", + surname="", + orcid=None, + email=set(), + website=set(), + __iris__={ + "organization": ["Item:OSWa"], + "employment_contract_status": "Item:OSWx", + }, + ) + + +def test_strip_protected_removes_only_protected(): + entity = _entity() + _strip_protected(entity) + assert "employment_contract_status" not in entity.__iris__ + assert "organization" in entity.__iris__ + + +def test_apply_update_strips_protected_when_in_fields(): + entity = _entity() + apply_update(entity, _proposed(), {"employment_contract_status"}) + assert "employment_contract_status" not in entity.__iris__ + + +def test_apply_update_keeps_protected_when_not_in_fields(): + entity = _entity() + apply_update(entity, _proposed(surname="New"), {"surname"}) + assert "employment_contract_status" in entity.__iris__ + assert entity.surname == "New" + + +def test_apply_update_clears_organization_when_empty(): + entity = _entity() + apply_update(entity, _proposed(organizations=[]), {"organizations"}) + assert "organization" not in entity.__iris__ + + +def test_apply_update_sets_organization_when_present(): + entity = _entity() + apply_update(entity, _proposed(organizations=["Item:OSWb"]), {"organizations"}) + assert entity.__iris__["organization"] == ["Item:OSWb"] + + +def test_apply_update_clears_website(): + entity = _entity() + entity.website = {"https://x"} + apply_update(entity, _proposed(websites=[]), {"websites"}) + assert entity.website == set() diff --git a/tests/tools/test_user_sync_sync.py b/tests/tools/test_user_sync_sync.py index aca5ef98..017c6c18 100644 --- a/tests/tools/test_user_sync_sync.py +++ b/tests/tools/test_user_sync_sync.py @@ -96,6 +96,8 @@ def test_dry_run_previews_without_writing(): joined = "\n".join(io.outputs) assert ORCID in joined assert "new=1" in joined # bot excluded, only the ORCID user + assert report.missing_email == [ORCID] # record has no email + assert "no ORCID email" in joined def test_requires_connection(): From 87cf85b31cbfec6b80e2e30b92852c359586133c Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Thu, 17 Sep 2026 16:20:06 +0200 Subject: [PATCH 14/20] fix(user-sync): actually remove pruned fields via empty relations and remove_empty --- docs/tools/user-sync.md | 10 ++++++++++ src/osw/tools/user_sync/build.py | 16 ++++++++++------ src/osw/tools/user_sync/sync.py | 12 +++++++++--- tests/tools/test_user_sync_build.py | 10 +++++----- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 627faa0a..1847223d 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -20,6 +20,16 @@ Credentials are read from `accounts.pwd.yaml` (or `--cred-filepath`), the same mechanism as `OswExpress`. Start with `--dry-run` to preview, then run without it to write after confirming at the prompt. +To also remove data no longer wanted (disabled optional fields and +`employment_contract_status`) from existing items, add `--prune`: + +```bash +uv run python examples/user_sync.py --domain your-instance.example.org --prune --dry-run +``` + +Users whose ORCID record exposes no email are listed in a warning at the end of +the run; a missing email never fails the sync. + ### Options By default the tool syncs both ORCID and MediaWiki-native accounts and writes diff --git a/src/osw/tools/user_sync/build.py b/src/osw/tools/user_sync/build.py index 6bf30c9a..7dfd9b69 100644 --- a/src/osw/tools/user_sync/build.py +++ b/src/osw/tools/user_sync/build.py @@ -24,11 +24,17 @@ def _labels(text: str) -> List[Any]: def _strip_protected(entity: Any) -> None: - """Remove protected relation defaults (e.g. employment_contract_status).""" + """Clear protected relations (e.g. employment_contract_status). + + Set to an empty list rather than removed: the model re-applies its default + when the field is absent, and the store only overwrites a relation that is + present as an explicit empty value (with ``remove_empty=False``). + """ iris = getattr(entity, "__iris__", None) if isinstance(iris, dict): for name in PROTECTED_FIELDS: - iris.pop(name, None) + if name in iris: + iris[name] = [] def build_organization(proposed: ProposedOrganization) -> Organization: @@ -76,10 +82,8 @@ def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> if "websites" in apply_fields: entity.website = set(proposed.websites) if "organizations" in apply_fields: - if proposed.organizations: - entity.__iris__["organization"] = list(proposed.organizations) - else: - entity.__iris__.pop("organization", None) + # Empty list both links (non-empty) and clears (empty, on removal). + entity.__iris__["organization"] = list(proposed.organizations) if any(name in apply_fields for name in PROTECTED_FIELDS): _strip_protected(entity) return entity diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index 516deaa5..c8fb7c7f 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -103,7 +103,9 @@ def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: report.failed["organizations"] = str(exc) -def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: +def _store_users( + osw: Any, resolution: Resolution, report: SyncReport, remove_empty: bool = True +) -> None: entities = [] for resolved in resolution.resolved: proposed = resolved.change.proposed @@ -126,7 +128,10 @@ def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: try: osw.store_entity( OSW.StoreEntityParam( - entities=entities, overwrite=True, edit_comment="user-sync" + entities=entities, + overwrite=True, + remove_empty=remove_empty, + edit_comment="user-sync", ) ) except Exception as exc: # pragma: no cover - network failure path @@ -215,7 +220,8 @@ def run_user_sync( if config.link_organizations: _store_organizations(osw, org_map, report) - _store_users(osw, resolution, report) + # When pruning, keep emptied fields in the payload so they overwrite (delete). + _store_users(osw, resolution, report, remove_empty=not config.prune) if config.create_redirects: _create_redirects(osw, resolution, report) _verify(osw, report) diff --git a/tests/tools/test_user_sync_build.py b/tests/tools/test_user_sync_build.py index 5f22adfb..2aba32a8 100644 --- a/tests/tools/test_user_sync_build.py +++ b/tests/tools/test_user_sync_build.py @@ -38,17 +38,17 @@ def _entity(): ) -def test_strip_protected_removes_only_protected(): +def test_strip_protected_empties_only_protected(): entity = _entity() _strip_protected(entity) - assert "employment_contract_status" not in entity.__iris__ - assert "organization" in entity.__iris__ + assert entity.__iris__["employment_contract_status"] == [] + assert entity.__iris__["organization"] == ["Item:OSWa"] def test_apply_update_strips_protected_when_in_fields(): entity = _entity() apply_update(entity, _proposed(), {"employment_contract_status"}) - assert "employment_contract_status" not in entity.__iris__ + assert entity.__iris__["employment_contract_status"] == [] def test_apply_update_keeps_protected_when_not_in_fields(): @@ -61,7 +61,7 @@ def test_apply_update_keeps_protected_when_not_in_fields(): def test_apply_update_clears_organization_when_empty(): entity = _entity() apply_update(entity, _proposed(organizations=[]), {"organizations"}) - assert "organization" not in entity.__iris__ + assert entity.__iris__["organization"] == [] def test_apply_update_sets_organization_when_present(): From 4d6f4401c63a148c924bef89963b8f18bc8d4b0c Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Tue, 3 Nov 2026 15:38:05 +0100 Subject: [PATCH 15/20] fix: botfilter for user sync --- docs/tools/user-sync.md | 4 +++- pyproject.toml | 3 +++ src/osw/tools/user_sync/sources.py | 7 +++++-- tests/tools/test_user_sync_sources.py | 13 +++++++++++++ uv.lock | 20 ++++++++++++++------ 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 1847223d..4499bd4c 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -55,7 +55,9 @@ only core identity plus email. 1. Enumerate MediaWiki accounts, skipping bots and privileged groups (`bot`, `sysop`, `bureaucrat`, `interface-admin`) and MediaWiki reserved - system usernames. + system usernames. Accounts whose username is an exact ORCID iD are exempt + from the group filter (so a person's own admin account is synced); other + accounts, such as `@bot`, are still excluded by their groups. 2. Enrich ORCID users from `https://pub.orcid.org/v3.0/`: names and email always; researcher URLs and affiliations only when their flag is enabled. 3. Map each account to a proposed `User` item with a deterministic id diff --git a/pyproject.toml b/pyproject.toml index 4198c239..b69e0638 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -86,6 +86,9 @@ workflow = [ ] tutorial = ["osw[dataimport]"] all = ["osw[dataimport,DB,UI,S3,wikitext]"] +examples = [ + "python-dotenv>=1.2.4", +] [build-system] requires = ["hatchling"] diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py index b666aa30..63b60eed 100644 --- a/src/osw/tools/user_sync/sources.py +++ b/src/osw/tools/user_sync/sources.py @@ -105,7 +105,10 @@ def enumerate_mw_users( Args: site: An object exposing ``api("query", ...)`` (an mwclient Site). - excluded_groups: Accounts in any of these groups are skipped (bots, admins). + excluded_groups: Accounts in any of these groups are skipped (bots, admins), + unless the username is an exact ORCID iD (a person's own account). + Bot accounts derived from an ORCID (e.g. ``@bot``) are not exact + ORCID iDs and stay excluded. excluded_usernames: Exact usernames to skip (reserved system accounts). include_orcid: If False, skip ORCID-username accounts. include_non_orcid: If False, keep only ORCID-username accounts. @@ -119,7 +122,7 @@ def enumerate_mw_users( user = MwUser.from_api(raw) if user.name in excluded_names: continue - if excluded.intersection(user.groups): + if not user.is_orcid and excluded.intersection(user.groups): continue if not include_non_orcid and not user.is_orcid: continue diff --git a/tests/tools/test_user_sync_sources.py b/tests/tools/test_user_sync_sources.py index 0895c3cb..aa90ff6e 100644 --- a/tests/tools/test_user_sync_sources.py +++ b/tests/tools/test_user_sync_sources.py @@ -110,6 +110,19 @@ def test_enumerate_excludes_privileged_groups_and_reserved_names(): assert [u.name for u in users] == ["0000-0002-6374-9831"] +def test_enumerate_keeps_orcid_accounts_despite_excluded_groups(): + site = FakeSite([ + _page([ + {"name": "0000-0001-7620-1245", "groups": ["user", "bot", "sysop"]}, + {"name": "0000-0001-7620-1245@importer", "groups": ["user", "bot"]}, + {"name": "SomeBot", "groups": ["user", "bot"]}, + {"name": "Admin", "groups": ["user", "sysop"]}, + ]) + ]) + users = enumerate_mw_users(site, excluded_groups=("bot", "sysop")) + assert [u.name for u in users] == ["0000-0001-7620-1245"] + + def test_enumerate_mw_only_skips_orcid(): site = FakeSite([ _page([ diff --git a/uv.lock b/uv.lock index a50885a0..edced417 100644 --- a/uv.lock +++ b/uv.lock @@ -2026,6 +2026,9 @@ db = [ { name = "psycopg2" }, { name = "sqlalchemy" }, ] +examples = [ + { name = "python-dotenv" }, +] s3 = [ { name = "boto3" }, ] @@ -2099,6 +2102,7 @@ requires-dist = [ { name = "pydantic", extras = ["email"], specifier = ">=2.12.0" }, { name = "pyld" }, { name = "pysimplegui", marker = "extra == 'ui'", specifier = ">=6" }, + { name = "python-dotenv", marker = "extra == 'examples'", specifier = ">=1.2.4" }, { name = "pyyaml" }, { name = "rdflib" }, { name = "requests" }, @@ -2107,7 +2111,7 @@ requires-dist = [ { name = "tqdm" }, { name = "typing-extensions" }, ] -provides-extras = ["wikitext", "db", "s3", "dataimport", "ui", "workflow", "tutorial", "all"] +provides-extras = ["wikitext", "db", "s3", "dataimport", "ui", "workflow", "tutorial", "all", "examples"] [package.metadata.requires-dev] dev = [ @@ -2700,6 +2704,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/28/78/9b77ecb4644d1bbea94d29abf78f21c47eca6eb79e9745b702ec0bed2e19/python_discovery-1.4.3-py3-none-any.whl", hash = "sha256:b6e1e4a7d9e3f6948c39746ffe8218225162d738ba39d05ab1d2f6c1cac4878c", size = 33885, upload-time = "2026-07-03T13:21:50.174Z" }, ] +[[package]] +name = "python-dotenv" +version = "1.2.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/74/26/2fbeedb218a787a5eea551c7532cac4e009f83d689dd2faa0d0353473f86/python_dotenv-1.2.4.tar.gz", hash = "sha256:f0d53e69935a851c0dcc78f3ab7aaccd8cabef0b92382b576b824212902873c0", size = 60824, upload-time = "2026-10-01T05:36:10Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/60/d1/38f3a3405989a89ac18390803e70c6ad7c7760da4f9b83cbeca0c44a0c72/python_dotenv-1.2.4-py3-none-any.whl", hash = "sha256:42269a8a5b3fd54ffa6f3d84b18abed50064717576b4ecf03dc4a55d8aa04fdc", size = 23266, upload-time = "2026-10-01T05:36:08.633Z" }, +] + [[package]] name = "python-gitlab" version = "8.4.0" @@ -3924,14 +3937,9 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/07/7c/5848aad6a5b566f5a3cebc77ab787793daec622b79295de243da7b8efae6/zensical-0.0.47-cp310-abi3-macosx_10_12_x86_64.whl", hash = "sha256:4162fb8b62f38e6d9b75688c1fb87e18a1cffef5eee51d2c18b79334b548e973", size = 12789282, upload-time = "2026-07-05T15:50:45.118Z" }, { url = "https://files.pythonhosted.org/packages/ce/08/c2a056ae22eba7063985cc89554ab8e54efb11af8e26f6d606e5bd300ae8/zensical-0.0.47-cp310-abi3-macosx_11_0_arm64.whl", hash = "sha256:cdc2d84f38da809a28402eda5f2b6dbb150e14427f296c5b521e0dfc6a2e8a39", size = 12661193, upload-time = "2026-07-05T15:50:49.138Z" }, { url = "https://files.pythonhosted.org/packages/dc/2b/30dc3262e99bc72e9a6529487fbfca58cf4f81cebfac59ef8e8d757d90eb/zensical-0.0.47-cp310-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:97ed2b21aba5f788fc39d1597d00938d602b4d2724ed599a1dab7548fe4f0025", size = 13044347, upload-time = "2026-07-05T15:50:52.429Z" }, - { url = "https://files.pythonhosted.org/packages/4d/80/296ff21b686cda47c4a3f94c35bf62472291501c34e4f06dfd107cb237da/zensical-0.0.47-cp310-abi3-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:6f6b2de477c45284201e92301f997415f45f78493bd20f479ea9a1c86bbbbcca", size = 13006494, upload-time = "2026-07-05T15:50:55.376Z" }, - { url = "https://files.pythonhosted.org/packages/2d/4e/5aeba02363db89c625cd41c76d2a6e40b07d6a75d869d970ace7accd2bfa/zensical-0.0.47-cp310-abi3-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:2ee29ff819372eaab02ca0f14ac82e804332d898c57c56ffd4d89674f3e5ff71", size = 13379253, upload-time = "2026-07-05T15:50:58.743Z" }, { url = "https://files.pythonhosted.org/packages/fd/dc/e72a1449544aeffa3dbe878c6340bcec98ba4eebabe7a592414064b0ae8d/zensical-0.0.47-cp310-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:1bd94937c48a2e42b5b65b32c5075849937f23cebccaf250d249efa27266e0be", size = 13103356, upload-time = "2026-07-05T15:51:02.247Z" }, { url = "https://files.pythonhosted.org/packages/48/eb/7fb956915cc59ee9f2ddf36ec5b16e8e858823217611e1d766816a661651/zensical-0.0.47-cp310-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:4702605b991bece11494a9bb318d5ba7229f00e5adcc9e6010acd58e9719686b", size = 13220897, upload-time = "2026-07-05T15:51:05.919Z" }, - { url = "https://files.pythonhosted.org/packages/57/6b/944954d60bf1d61de2a7fa2732d0fb2d5deee64567db6088cdd2b5f76973/zensical-0.0.47-cp310-abi3-musllinux_1_2_armv7l.whl", hash = "sha256:77f08ffcc3da9ca2f972330e501927aa7e8e445bfa7b758107edc645acda266e", size = 13280476, upload-time = "2026-07-05T15:51:09.352Z" }, - { url = "https://files.pythonhosted.org/packages/ce/80/4aece9b99eb5809b04fc6577fc50e877c998917293dfd58b5b229616b92b/zensical-0.0.47-cp310-abi3-musllinux_1_2_i686.whl", hash = "sha256:319cf370ecc6d87da69c935c5acd6e1959dd48954766e9f954319bdb0bec5d36", size = 13439392, upload-time = "2026-07-05T15:51:12.564Z" }, { url = "https://files.pythonhosted.org/packages/41/10/5cbffe680605b4fa7c0d6326f07a6adc86854f8b9d61bb1b29bb482d40eb/zensical-0.0.47-cp310-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:59656bf604a8b03eede4ce1a847640bab1129ab86dec2e39e5bd4b808b1802d2", size = 13386534, upload-time = "2026-07-05T15:51:15.892Z" }, - { url = "https://files.pythonhosted.org/packages/d5/06/79c7726b0388da40a96cc01113d872d639da1d8a5f96b5265e46e8472ceb/zensical-0.0.47-cp310-abi3-win32.whl", hash = "sha256:81a13a8bacadedada4847eed4aaa4a3f4ef0e5b78dbb2a2022bc6fb7e7dc9464", size = 12356060, upload-time = "2026-07-05T15:51:19.167Z" }, { url = "https://files.pythonhosted.org/packages/f9/7a/7daaeadfdf7dc2941c8b931f9c6c83396d1da45634cec1a28f8d85a57708/zensical-0.0.47-cp310-abi3-win_amd64.whl", hash = "sha256:944a309be69b11daa8bba46c61fb74f32a98b637f3e7c11135e7cc2a5eccbe32", size = 12601932, upload-time = "2026-07-05T15:51:22.485Z" }, ] From 02570c8f88d6d27c58e1e595bc8bb13e90354fd4 Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Fri, 6 Nov 2026 11:14:33 +0100 Subject: [PATCH 16/20] fix(user-sync): drop python-dotenv, use default accounts.pwd.yaml --- examples/user_sync.py | 10 ++++++---- pyproject.toml | 3 --- uv.lock | 20 ++++++-------------- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/examples/user_sync.py b/examples/user_sync.py index e5468495..9e507d74 100644 --- a/examples/user_sync.py +++ b/examples/user_sync.py @@ -4,16 +4,18 @@ uv run python examples/user_sync.py --domain llm4eln.semos.dev --dry-run """ -import dotenv - +from osw.defaults import paths from osw.express import OswExpress from osw.tools.user_sync import config_from_args, run_user_sync def main() -> None: - dotenv.load_dotenv() config = config_from_args() - osw = OswExpress(domain=config.domain, cred_filepath=config.cred_filepath) + cred_filepath = config.cred_filepath + if cred_filepath is None and paths.cred_filepath.is_file(): + # OswExpress only reads OSW_CRED_FILEPATH; use the default accounts file. + cred_filepath = paths.cred_filepath + osw = OswExpress(domain=config.domain, cred_filepath=cred_filepath) report = run_user_sync(config, osw=osw) print(report.summary()) for title in report.created: diff --git a/pyproject.toml b/pyproject.toml index b69e0638..4198c239 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -86,9 +86,6 @@ workflow = [ ] tutorial = ["osw[dataimport]"] all = ["osw[dataimport,DB,UI,S3,wikitext]"] -examples = [ - "python-dotenv>=1.2.4", -] [build-system] requires = ["hatchling"] diff --git a/uv.lock b/uv.lock index edced417..a50885a0 100644 --- a/uv.lock +++ b/uv.lock @@ -2026,9 +2026,6 @@ db = [ { name = "psycopg2" }, { name = "sqlalchemy" }, ] -examples = [ - { name = "python-dotenv" }, -] s3 = [ { name = "boto3" }, ] @@ -2102,7 +2099,6 @@ requires-dist = [ { name = "pydantic", extras = ["email"], specifier = ">=2.12.0" }, { name = "pyld" }, { name = "pysimplegui", marker = "extra == 'ui'", specifier = ">=6" }, - { name = "python-dotenv", marker = "extra == 'examples'", specifier = ">=1.2.4" }, { name = "pyyaml" }, { name = "rdflib" }, { name = "requests" }, @@ -2111,7 +2107,7 @@ requires-dist = [ { name = "tqdm" }, { name = "typing-extensions" }, ] -provides-extras = ["wikitext", "db", "s3", "dataimport", "ui", "workflow", "tutorial", "all", "examples"] +provides-extras = ["wikitext", "db", "s3", "dataimport", "ui", "workflow", "tutorial", "all"] [package.metadata.requires-dev] dev = [ @@ -2704,15 +2700,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/28/78/9b77ecb4644d1bbea94d29abf78f21c47eca6eb79e9745b702ec0bed2e19/python_discovery-1.4.3-py3-none-any.whl", hash = "sha256:b6e1e4a7d9e3f6948c39746ffe8218225162d738ba39d05ab1d2f6c1cac4878c", size = 33885, upload-time = "2026-07-03T13:21:50.174Z" }, ] -[[package]] -name = "python-dotenv" -version = "1.2.4" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/74/26/2fbeedb218a787a5eea551c7532cac4e009f83d689dd2faa0d0353473f86/python_dotenv-1.2.4.tar.gz", hash = "sha256:f0d53e69935a851c0dcc78f3ab7aaccd8cabef0b92382b576b824212902873c0", size = 60824, upload-time = "2026-10-01T05:36:10Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/60/d1/38f3a3405989a89ac18390803e70c6ad7c7760da4f9b83cbeca0c44a0c72/python_dotenv-1.2.4-py3-none-any.whl", hash = "sha256:42269a8a5b3fd54ffa6f3d84b18abed50064717576b4ecf03dc4a55d8aa04fdc", size = 23266, upload-time = "2026-10-01T05:36:08.633Z" }, -] - [[package]] name = "python-gitlab" version = "8.4.0" @@ -3937,9 +3924,14 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/07/7c/5848aad6a5b566f5a3cebc77ab787793daec622b79295de243da7b8efae6/zensical-0.0.47-cp310-abi3-macosx_10_12_x86_64.whl", hash = "sha256:4162fb8b62f38e6d9b75688c1fb87e18a1cffef5eee51d2c18b79334b548e973", size = 12789282, upload-time = "2026-07-05T15:50:45.118Z" }, { url = "https://files.pythonhosted.org/packages/ce/08/c2a056ae22eba7063985cc89554ab8e54efb11af8e26f6d606e5bd300ae8/zensical-0.0.47-cp310-abi3-macosx_11_0_arm64.whl", hash = "sha256:cdc2d84f38da809a28402eda5f2b6dbb150e14427f296c5b521e0dfc6a2e8a39", size = 12661193, upload-time = "2026-07-05T15:50:49.138Z" }, { url = "https://files.pythonhosted.org/packages/dc/2b/30dc3262e99bc72e9a6529487fbfca58cf4f81cebfac59ef8e8d757d90eb/zensical-0.0.47-cp310-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:97ed2b21aba5f788fc39d1597d00938d602b4d2724ed599a1dab7548fe4f0025", size = 13044347, upload-time = "2026-07-05T15:50:52.429Z" }, + { url = "https://files.pythonhosted.org/packages/4d/80/296ff21b686cda47c4a3f94c35bf62472291501c34e4f06dfd107cb237da/zensical-0.0.47-cp310-abi3-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:6f6b2de477c45284201e92301f997415f45f78493bd20f479ea9a1c86bbbbcca", size = 13006494, upload-time = "2026-07-05T15:50:55.376Z" }, + { url = "https://files.pythonhosted.org/packages/2d/4e/5aeba02363db89c625cd41c76d2a6e40b07d6a75d869d970ace7accd2bfa/zensical-0.0.47-cp310-abi3-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:2ee29ff819372eaab02ca0f14ac82e804332d898c57c56ffd4d89674f3e5ff71", size = 13379253, upload-time = "2026-07-05T15:50:58.743Z" }, { url = "https://files.pythonhosted.org/packages/fd/dc/e72a1449544aeffa3dbe878c6340bcec98ba4eebabe7a592414064b0ae8d/zensical-0.0.47-cp310-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:1bd94937c48a2e42b5b65b32c5075849937f23cebccaf250d249efa27266e0be", size = 13103356, upload-time = "2026-07-05T15:51:02.247Z" }, { url = "https://files.pythonhosted.org/packages/48/eb/7fb956915cc59ee9f2ddf36ec5b16e8e858823217611e1d766816a661651/zensical-0.0.47-cp310-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:4702605b991bece11494a9bb318d5ba7229f00e5adcc9e6010acd58e9719686b", size = 13220897, upload-time = "2026-07-05T15:51:05.919Z" }, + { url = "https://files.pythonhosted.org/packages/57/6b/944954d60bf1d61de2a7fa2732d0fb2d5deee64567db6088cdd2b5f76973/zensical-0.0.47-cp310-abi3-musllinux_1_2_armv7l.whl", hash = "sha256:77f08ffcc3da9ca2f972330e501927aa7e8e445bfa7b758107edc645acda266e", size = 13280476, upload-time = "2026-07-05T15:51:09.352Z" }, + { url = "https://files.pythonhosted.org/packages/ce/80/4aece9b99eb5809b04fc6577fc50e877c998917293dfd58b5b229616b92b/zensical-0.0.47-cp310-abi3-musllinux_1_2_i686.whl", hash = "sha256:319cf370ecc6d87da69c935c5acd6e1959dd48954766e9f954319bdb0bec5d36", size = 13439392, upload-time = "2026-07-05T15:51:12.564Z" }, { url = "https://files.pythonhosted.org/packages/41/10/5cbffe680605b4fa7c0d6326f07a6adc86854f8b9d61bb1b29bb482d40eb/zensical-0.0.47-cp310-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:59656bf604a8b03eede4ce1a847640bab1129ab86dec2e39e5bd4b808b1802d2", size = 13386534, upload-time = "2026-07-05T15:51:15.892Z" }, + { url = "https://files.pythonhosted.org/packages/d5/06/79c7726b0388da40a96cc01113d872d639da1d8a5f96b5265e46e8472ceb/zensical-0.0.47-cp310-abi3-win32.whl", hash = "sha256:81a13a8bacadedada4847eed4aaa4a3f4ef0e5b78dbb2a2022bc6fb7e7dc9464", size = 12356060, upload-time = "2026-07-05T15:51:19.167Z" }, { url = "https://files.pythonhosted.org/packages/f9/7a/7daaeadfdf7dc2941c8b931f9c6c83396d1da45634cec1a28f8d85a57708/zensical-0.0.47-cp310-abi3-win_amd64.whl", hash = "sha256:944a309be69b11daa8bba46c61fb74f32a98b637f3e7c11135e7cc2a5eccbe32", size = 12601932, upload-time = "2026-07-05T15:51:22.485Z" }, ] From bc6b0d642f5697ca8048b4be05fd26c9ad2162f6 Mon Sep 17 00:00:00 2001 From: LukasGold <41064839+LukasGold@users.noreply.github.com> Date: Wed, 11 Nov 2026 16:27:50 +0100 Subject: [PATCH 17/20] fix(user-sync): align with stage and the tools docs - list the page in the existing Tools nav group, describe scripts there - page through all User items and adopt pages at the deterministic title - skip blocked accounts by default, never propose placeholder names - prune via a second jsondata edit; oold 2.3 ignores __iris__ edits - never write employment_contract_status; add store and prune tests --- docs/tools/index.md | 7 +- docs/tools/user-sync.md | 57 ++-- src/osw/tools/user_sync/build.py | 40 +-- src/osw/tools/user_sync/config.py | 14 +- src/osw/tools/user_sync/existing.py | 78 +++++- src/osw/tools/user_sync/prune.py | 38 +++ src/osw/tools/user_sync/reconcile.py | 18 +- src/osw/tools/user_sync/sources.py | 13 +- src/osw/tools/user_sync/sync.py | 68 ++++- tests/tools/test_user_sync_build.py | 44 +-- tests/tools/test_user_sync_config.py | 6 + tests/tools/test_user_sync_existing.py | 112 +++++++- tests/tools/test_user_sync_reconcile.py | 36 +++ tests/tools/test_user_sync_sources.py | 27 ++ tests/tools/test_user_sync_store.py | 338 ++++++++++++++++++++++++ tests/tools/test_user_sync_sync.py | 72 ++++- zensical.toml | 4 +- 17 files changed, 878 insertions(+), 94 deletions(-) create mode 100644 src/osw/tools/user_sync/prune.py create mode 100644 tests/tools/test_user_sync_store.py diff --git a/docs/tools/index.md b/docs/tools/index.md index 45dd54c3..0c303199 100644 --- a/docs/tools/index.md +++ b/docs/tools/index.md @@ -2,8 +2,10 @@ Besides the Python API, osw ships two adapters that talk to a live instance: the `osw` command line client, and an MCP server for agent clients such as -Claude Code. Both run the same operations from one shared, SDK-free core -(`osw.service`), so a command and its matching tool behave identically. They +Claude Code. This section also covers standalone scripts built on the Python +API. The current one is the [user sync](user-sync.md). The two adapters run +the same operations from one shared, SDK-free core (`osw.service`), so a +command and its matching tool behave identically. They differ in exactly one way: only the CLI accepts filesystem paths. ## Setup @@ -49,4 +51,5 @@ extras are listed in the [Get Started guide](../get-started.md#optional-extras). | ---- | -------- | | [CLI](cli.md) | The `.env` quick start, the full command reference, and the global flags | | [MCP server](mcp.md) | The tool surface, the no-filesystem-access and one-server-per-instance rules, and how to register the server with a client such as Claude Code | +| [User Sync (script)](user-sync.md) | A Python script that creates or updates `User` items from the MediaWiki accounts of an instance, with the options, the data it writes and how it reconciles existing items | | [Configuration](configuration.md) | What both adapters share: where credentials and settings come from, and the full environment-variable reference | diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 4499bd4c..6ab49168 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -4,6 +4,10 @@ Create or update OSW `User` items from the MediaWiki accounts of an OSL instance, enriching ORCID users from the public ORCID API. The run is idempotent and previews every change before writing. +This is a Python script built on the osw library. It is not a command of the +`osw` CLI and not an MCP tool, see [Tools](index.md). It runs from a source +checkout, because `examples/` is not part of the installed package. + ## When to use it OSL instances gain accounts from manual creation and from whitelisted ORCID @@ -16,12 +20,20 @@ semantic `User` item until this tool reconciles them. uv run python examples/user_sync.py --domain your-instance.example.org --dry-run ``` -Credentials are read from `accounts.pwd.yaml` (or `--cred-filepath`), the same -mechanism as `OswExpress`. Start with `--dry-run` to preview, then run without -it to write after confirming at the prompt. - -To also remove data no longer wanted (disabled optional fields and -`employment_contract_status`) from existing items, add `--prune`: +The script reads the credential file given by `--cred-filepath`, otherwise +`accounts.pwd.yaml` in the working directory, otherwise the file named by the +`OSW_CRED_FILEPATH` environment variable. It does not read a `.env` file. +`--domain` falls back to the `OSW_DOMAIN` environment variable. See +[Credentials](configuration.md#credentials) for the file format. Start with +`--dry-run` to preview, then run without it to write after confirming at the +prompt. + +To also clean up existing items, add `--prune`. Without it, the sync never +writes `employment_contract_status` and never removes anything. With it, +`employment_contract_status` on existing items is overwritten with an empty +value. So are `website` and `organization`, unless `--with-websites` or +`--with-organizations` enables them. This happens in a second edit of the +page's jsondata after the store: ```bash uv run python examples/user_sync.py --domain your-instance.example.org --prune --dry-run @@ -49,11 +61,14 @@ only core identity plus email. | `--with-websites` | Also store ORCID researcher URLs (opt-in). | | `--with-organizations` | Also create and link Organization items from ORCID affiliations (opt-in). | | `--with-extras` | Enable both websites and organizations. | -| `--prune` | Remove disabled optional fields and `employment_contract_status` from existing items (default off; a normal run only adds). | +| `--include-blocked` | Do not skip blocked MediaWiki accounts. | +| `--prune` | Overwrite `employment_contract_status`, and `website` or `organization` when their opt-in flag is not given, on existing items with an empty value, in a second edit of the page's jsondata after the store (default off; a normal run never writes `employment_contract_status` and never removes anything). | ## What it does -1. Enumerate MediaWiki accounts, skipping bots and privileged groups +1. Enumerate MediaWiki accounts, skipping blocked accounts (unless + `--include-blocked` is set; the User item of a skipped blocked account is + left unchanged), bots and privileged groups (`bot`, `sysop`, `bureaucrat`, `interface-admin`) and MediaWiki reserved system usernames. Accounts whose username is an exact ORCID iD are exempt from the group filter (so a person's own admin account is synced); other @@ -62,10 +77,14 @@ only core identity plus email. always; researcher URLs and affiliations only when their flag is enabled. 3. Map each account to a proposed `User` item with a deterministic id (uuid5 on the ORCID iD, else the username), so re-runs are idempotent. -4. Reconcile against existing items by `username` into NEW, GAP_FILL, CONFLICT, - REMOVE or UNCHANGED, then preview; gap-fills apply automatically and conflicts - are resolved interactively. Removals only appear with `--prune`; a normal run - never removes anything. +4. Reconcile against existing items. Existing items are found by `username` + across all `User` items. An item whose deterministic page already exists is + updated and not created again. Each user gets one of four categories: NEW, + GAP_FILL, CONFLICT or UNCHANGED. Each changed field has one of three + statuses: GAP_FILL, CONFLICT or REMOVE. The preview shows a removed field as + `-field`. A user with only gap-fills or removals is GAP_FILL. Gap-fills apply + automatically and conflicts are resolved interactively. Removals appear only + with `--prune`; a normal run never removes anything. 5. Store items, create `User:` redirects to each item, ensure any linked Organization items, verify by reloading, and warn about users with no email. @@ -76,14 +95,16 @@ only core identity plus email. | --- | --- | | `username`, `first_name`, `surname`, `label`, `orcid` | Always (core identity). | | `email` | Standard: always attempted; missing email is warned, not fatal; never removed. | -| `website` | Opt-in (`--with-websites`); removed from existing only with `--prune`. | -| `organization` | Opt-in (`--with-organizations`); removed from existing only with `--prune`. | -| `employment_contract_status` | Never written; removed from existing only with `--prune`. | +| `website` | Opt-in (`--with-websites`); without that flag, on existing items overwritten with an empty value only with `--prune`, in a second jsondata edit after the store. | +| `organization` | Opt-in (`--with-organizations`); without that flag, on existing items overwritten with an empty value only with `--prune`, in a second jsondata edit after the store. | +| `employment_contract_status` | Never written without `--prune`; with `--prune`, overwritten with an empty value on existing items in a second jsondata edit after the store. | ## Notes - MediaWiki does not expose other users' email or real name, so ORCID is the only source of rich data. Non-ORCID accounts get username-based placeholder - names, flagged in the preview. -- On a first accepted run the library autofetches schemas and may regenerate - `src/osw/model/entity.py`; restore it if you do not want that change. + names, flagged in the preview. Placeholder names are written only for new + items and are never proposed for an existing item. +- In a source checkout (for developers), the first accepted run autofetches + schemas and may regenerate `src/osw/model/entity.py`. Restore it if you do not + want that change. diff --git a/src/osw/tools/user_sync/build.py b/src/osw/tools/user_sync/build.py index 7dfd9b69..5ba89430 100644 --- a/src/osw/tools/user_sync/build.py +++ b/src/osw/tools/user_sync/build.py @@ -1,9 +1,13 @@ """Build pydantic User/Organization entities from proposals. Must run within an open OSW connection (the models resolve linked entities on -construction). Organization references are passed as a list of page-title IRIs, -which oold stores in the entity's ``__iris__`` map; the reference attribute is -never read back here, as that would trigger a network resolve. +construction). Organization references are passed as a list of page-title IRIs; +the reference attribute is never read back here, as that would trigger a +network resolve. + +An empty field is set to None, so its key is absent from the serialized item. +The store merges with the stored page, so an absent key keeps the stored value. +Clearing a stored value is done by ``prune.clear_jsondata_keys`` instead. """ from __future__ import annotations @@ -24,17 +28,13 @@ def _labels(text: str) -> List[Any]: def _strip_protected(entity: Any) -> None: - """Clear protected relations (e.g. employment_contract_status). + """Unset protected relations (e.g. employment_contract_status). - Set to an empty list rather than removed: the model re-applies its default - when the field is absent, and the store only overwrites a relation that is - present as an explicit empty value (with ``remove_empty=False``). + The attribute is set to None, so the key is absent from the serialized item + and the model default is not written. A stored value is left unchanged. """ - iris = getattr(entity, "__iris__", None) - if isinstance(iris, dict): - for name in PROTECTED_FIELDS: - if name in iris: - iris[name] = [] + for name in PROTECTED_FIELDS: + setattr(entity, name, None) def build_organization(proposed: ProposedOrganization) -> Organization: @@ -66,9 +66,11 @@ def build_user(proposed: ProposedUser) -> User: def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> Any: """Apply the accepted fields onto a loaded existing entity, in place. - Fields whose proposal is empty (REMOVE) are cleared; protected fields in - ``apply_fields`` are stripped from the entity's ``__iris__`` map. + An empty organization or website value is set to None, which leaves the + stored value unchanged. Protected fields are always unset. """ + if "username" in apply_fields: + entity.username = proposed.username if "label" in apply_fields: entity.label = _labels(proposed.label) if "first_name" in apply_fields: @@ -80,10 +82,10 @@ def apply_update(entity: Any, proposed: ProposedUser, apply_fields: Set[str]) -> if "emails" in apply_fields: entity.email = set(proposed.emails) if "websites" in apply_fields: - entity.website = set(proposed.websites) + entity.website = set(proposed.websites) if proposed.websites else None if "organizations" in apply_fields: - # Empty list both links (non-empty) and clears (empty, on removal). - entity.__iris__["organization"] = list(proposed.organizations) - if any(name in apply_fields for name in PROTECTED_FIELDS): - _strip_protected(entity) + entity.organization = ( + list(proposed.organizations) if proposed.organizations else None + ) + _strip_protected(entity) return entity diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py index 57d25b83..4973e715 100644 --- a/src/osw/tools/user_sync/config.py +++ b/src/osw/tools/user_sync/config.py @@ -43,6 +43,8 @@ class SyncConfig: link_organizations: bool = False # Remove disabled optional fields and protected fields from existing items. prune: bool = False + # Blocked MediaWiki accounts are skipped unless this is set. + include_blocked: bool = False orcid_api_base: str = ORCID_API_BASE_DEFAULT excluded_groups: List[str] = field(default_factory=lambda: list(SYSTEM_GROUPS)) @@ -111,8 +113,9 @@ def build_arg_parser() -> argparse.ArgumentParser: "--prune", dest="prune", action="store_true", - help="Remove disabled optional fields and employment_contract_status from " - "existing items (data cleanup). Off by default; a normal run only adds.", + help="Remove disabled optional fields from existing items and overwrite " + "employment_contract_status with an empty value (data cleanup). Off by " + "default; a normal run only adds.", ) account_scope = parser.add_mutually_exclusive_group() account_scope.add_argument( @@ -133,6 +136,12 @@ def build_arg_parser() -> argparse.ArgumentParser: action="store_false", help="Do not skip MediaWiki reserved system accounts (Maintenance script etc.).", ) + parser.add_argument( + "--include-blocked", + dest="include_blocked", + action="store_true", + help="Do not skip blocked MediaWiki accounts.", + ) return parser @@ -152,4 +161,5 @@ def config_from_args(argv: Optional[List[str]] = None) -> SyncConfig: include_websites=args.include_websites or args.with_extras, link_organizations=args.link_organizations or args.with_extras, prune=args.prune, + include_blocked=args.include_blocked, ) diff --git a/src/osw/tools/user_sync/existing.py b/src/osw/tools/user_sync/existing.py index e9e9ac59..6bbaa3f7 100644 --- a/src/osw/tools/user_sync/existing.py +++ b/src/osw/tools/user_sync/existing.py @@ -9,8 +9,13 @@ from dataclasses import dataclass, field from typing import Any, Dict, List, Optional +from osw.wiki_tools import SearchParam + from .config import USER_CATEGORY +# Results per semantic search request. +PAGE_SIZE = 1000 + @dataclass class ExistingUsers: @@ -34,6 +39,36 @@ def _as_entity_list(result: Any) -> List[Any]: return [result] +def _query_user_titles( + osw: Any, category: str, limit: Optional[int] = None +) -> List[str]: + """Return the titles of all items of a category, paging through the results. + + The next offset is the one the wiki reports (``query-continue-offset``), not + the offset plus the page size, because the wiki can cut a page shorter. + """ + titles: List[str] = [] + offset = 0 + while True: + query = SearchParam( + query=f"[[HasType::{category}]]|offset={offset}", + limit=PAGE_SIZE, + return_meta=True, + ) + result = osw.site.semantic_search(query)[0] + titles.extend(result.titles) + if limit is not None and len(titles) >= limit: + return titles[:limit] + if not result.truncated: + return titles + if result.next_offset is None or result.next_offset <= offset: + raise RuntimeError( + f"The wiki reported the next offset {result.next_offset} after " + f"offset {offset} for '{category}'. Paging cannot continue." + ) + offset = result.next_offset + + def load_existing_users( osw: Any, category: str = USER_CATEGORY, @@ -42,14 +77,15 @@ def load_existing_users( """Query and load all existing User items, indexed by username. Args: - osw: An authenticated OSW connection exposing ``query_instances`` and - ``load_entity``. + osw: An authenticated OSW connection exposing ``site.semantic_search`` + and ``load_entity``. category: The User category page title. limit: Load at most this many items (for testing). + + Raises: + RuntimeError: If the wiki reports a next offset that does not advance. """ - titles = osw.query_instances(category=category) - if limit is not None: - titles = titles[:limit] + titles = _query_user_titles(osw, category, limit) if not titles: return ExistingUsers() entities = _as_entity_list(osw.load_entity(titles)) @@ -59,3 +95,35 @@ def load_existing_users( if username: by_username[username] = entity return ExistingUsers(by_username=by_username) + + +def adopt_existing_pages( + osw: Any, existing: ExistingUsers, proposed_users: List[Any] +) -> List[str]: + """Add items that exist at a proposed user's deterministic page title. + + A user without a match by ``username`` would be created at its deterministic + page title. If that page already exists (for example, it has no username or + is not typed as a User), it is loaded and indexed under the proposed + username, so the sync reconciles against it instead of creating it again. + + Returns: + The page titles that were adopted. + """ + candidates = [p for p in proposed_users if existing.get(p.username) is None] + if not candidates: + return [] + titles = [p.full_page_title for p in candidates] + missing = set(osw._get_missing_page_titles(titles, confirm_delay_s=0)) + present = [p for p in candidates if p.full_page_title not in missing] + if not present: + return [] + entities = _as_entity_list(osw.load_entity([p.full_page_title for p in present])) + by_uuid = {str(getattr(e, "uuid", None)): e for e in entities if e is not None} + adopted: List[str] = [] + for proposed in present: + entity = by_uuid.get(str(proposed.uuid)) + if entity is not None: + existing.by_username[proposed.username] = entity + adopted.append(proposed.full_page_title) + return adopted diff --git a/src/osw/tools/user_sync/prune.py b/src/osw/tools/user_sync/prune.py new file mode 100644 index 00000000..6e1e62a2 --- /dev/null +++ b/src/osw/tools/user_sync/prune.py @@ -0,0 +1,38 @@ +"""Clear stored relations of existing user items (``--prune``). + +The store merges with the stored page, so an absent key keeps the stored value. +The keys are therefore overwritten with an empty list in a second edit of the +page's jsondata slot. +""" + +from __future__ import annotations + +import json +from typing import Any, Iterable + +from osw.wtsite import WtPage + + +def clear_jsondata_keys(osw: Any, title: str, keys: Iterable[str]) -> bool: + """Set each present, non-empty key in the page's jsondata slot to ``[]``. + + The page is read with a new WtPage, which downloads the current revision and + bypasses the WtSite page cache. Other keys are written back unchanged. + Returns True if the page was written, False if nothing changed. + """ + page = WtPage(wtSite=osw.site, title=title) + content = page.get_slot_content("jsondata") + if isinstance(content, str): + content = json.loads(content) + if not isinstance(content, dict): + return False + changed = False + for key in keys: + if key in content and content[key] not in (None, [], "", {}): + content[key] = [] + changed = True + if not changed: + return False + page.set_slot_content("jsondata", content) + page.edit(comment="user-sync: prune") + return True diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py index 5e1c2ae9..2bbab370 100644 --- a/src/osw/tools/user_sync/reconcile.py +++ b/src/osw/tools/user_sync/reconcile.py @@ -1,9 +1,11 @@ """Reconcile proposed User items against the ones already stored. Pure logic: given proposed users and an index of existing items, classify each -into NEW, GAP_FILL, CONFLICT or UNCHANGED and record the per-field differences -(including REMOVE diffs for disabled optional fields and protected fields). -The interactive layer consumes this plan; nothing here does IO. +user into one of four categories: NEW, GAP_FILL, CONFLICT or UNCHANGED. Each +changed field has one of three statuses: GAP_FILL, CONFLICT or REMOVE (REMOVE is +used for disabled optional fields and protected fields, only when pruning). A user +with only gap-fills or removals is GAP_FILL. Placeholder names are never compared +for an existing item. The interactive layer consumes this plan; nothing here does IO. """ from __future__ import annotations @@ -27,6 +29,8 @@ SET_FIELDS = ("emails", "websites", "organizations") URL_FIELDS = ("orcid", "websites") RECONCILED_FIELDS = SCALAR_FIELDS + SET_FIELDS +# Fields derived from a username placeholder when no name is known. +PLACEHOLDER_FIELDS = ("label", "first_name", "surname") # Fields that are only written when opted in; removed from existing when disabled. OPTIONAL_FIELDS = ("websites", "organizations") @@ -155,6 +159,9 @@ def reconcile_user( diffs: List[FieldDiff] = [] has_conflict = has_change = False for name in RECONCILED_FIELDS: + if proposed.placeholder_name and name in PLACEHOLDER_FIELDS: + # A placeholder is only written for a new item, never for an existing one. + continue if name in OPTIONAL_FIELDS and name not in enabled_fields: # Disabled optional field: remove any existing value only when pruning. if prune and not _is_empty(ef[name]): @@ -166,6 +173,11 @@ def reconcile_user( diffs.append(FieldDiff(name, ef[name], pf[name], status)) has_conflict = has_conflict or status == CONFLICT has_change = has_change or status == GAP_FILL + # An item found by its page title instead of its username has no username + # yet. Fill it, so the next run matches the item by username. + if not getattr(existing, "username", None): + diffs.append(FieldDiff("username", None, proposed.username, GAP_FILL)) + has_change = True # Protected fields are stripped from existing items only when pruning. if prune: for name in PROTECTED_FIELDS: diff --git a/src/osw/tools/user_sync/sources.py b/src/osw/tools/user_sync/sources.py index 63b60eed..24f0072c 100644 --- a/src/osw/tools/user_sync/sources.py +++ b/src/osw/tools/user_sync/sources.py @@ -1,7 +1,7 @@ """External data sources for the user-item sync tool. -This module wraps the MediaWiki ``allusers`` API. The ORCID public-API client is -added in a later phase. +This module wraps the MediaWiki ``allusers`` API to list accounts and contains a +client for the public ORCID API that fetches and parses ORCID records. """ from __future__ import annotations @@ -100,6 +100,8 @@ def enumerate_mw_users( include_non_orcid: bool = True, limit: Optional[int] = None, batch: Any = "max", + exclude_blocked: bool = True, + skipped_blocked: Optional[List[str]] = None, ) -> List[MwUser]: """List MediaWiki accounts to sync. @@ -114,6 +116,9 @@ def enumerate_mw_users( include_non_orcid: If False, keep only ORCID-username accounts. limit: Keep at most this many accounts after filtering. batch: ``aulimit`` value passed to the API. + exclude_blocked: If True, skip blocked accounts, ORCID or not. + skipped_blocked: If given, receives the names of the skipped blocked + accounts. """ excluded = set(excluded_groups) excluded_names = set(excluded_usernames) @@ -122,6 +127,10 @@ def enumerate_mw_users( user = MwUser.from_api(raw) if user.name in excluded_names: continue + if exclude_blocked and user.blocked: + if skipped_blocked is not None: + skipped_blocked.append(user.name) + continue if not user.is_orcid and excluded.intersection(user.groups): continue if not include_non_orcid and not user.is_orcid: diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index c8fb7c7f..88902189 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -15,10 +15,11 @@ from .build import apply_update, build_organization, build_user from .config import SyncConfig -from .existing import load_existing_users +from .existing import adopt_existing_pages, load_existing_users from .interactive import Prompter, Resolution, resolve_plan from .mapping import ProposedOrganization, map_user -from .reconcile import reconcile +from .prune import clear_jsondata_keys +from .reconcile import REMOVE, reconcile from .redirects import ensure_redirect from .sources import ( RESERVED_USERNAMES, @@ -36,6 +37,7 @@ class SyncReport: updated: List[str] = field(default_factory=list) skipped: List[str] = field(default_factory=list) failed: Dict[str, str] = field(default_factory=dict) + pruned: List[str] = field(default_factory=list) redirects_created: List[str] = field(default_factory=list) organizations: List[str] = field(default_factory=list) missing_email: List[str] = field(default_factory=list) @@ -45,16 +47,22 @@ def summary(self) -> str: state = "applied" if self.accepted else "no changes written" return ( f"{state}: created={len(self.created)} updated={len(self.updated)} " - f"skipped={len(self.skipped)} failed={len(self.failed)} " + f"skipped={len(self.skipped)} pruned={len(self.pruned)} " + f"failed={len(self.failed)} " f"redirects={len(self.redirects_created)} orgs={len(self.organizations)} " f"missing_email={len(self.missing_email)}" ) def _build_proposals(config: SyncConfig, osw: Any, session: Any): - """Enumerate MediaWiki users, enrich ORCID users and map to proposals.""" + """Enumerate MediaWiki users, enrich ORCID users and map to proposals. + + Returns the proposed users, the organizations and the names of the blocked + accounts that were skipped. + """ excluded = tuple(config.excluded_groups) if config.exclude_bot_group else () reserved = RESERVED_USERNAMES if config.exclude_system_usernames else () + skipped_blocked: List[str] = [] mw_users = enumerate_mw_users( osw.mw_site, excluded_groups=excluded, @@ -62,6 +70,8 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any): include_orcid=config.include_orcid, include_non_orcid=config.include_non_orcid, limit=config.limit, + exclude_blocked=not config.include_blocked, + skipped_blocked=skipped_blocked, ) cache: Dict[str, Any] = {} proposed_users = [] @@ -87,7 +97,7 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any): proposed_users.append(proposed) for org in orgs: org_map.setdefault(org.uuid, org) - return proposed_users, org_map + return proposed_users, org_map, skipped_blocked def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: @@ -103,9 +113,7 @@ def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: report.failed["organizations"] = str(exc) -def _store_users( - osw: Any, resolution: Resolution, report: SyncReport, remove_empty: bool = True -) -> None: +def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: entities = [] for resolved in resolution.resolved: proposed = resolved.change.proposed @@ -130,7 +138,6 @@ def _store_users( OSW.StoreEntityParam( entities=entities, overwrite=True, - remove_empty=remove_empty, edit_comment="user-sync", ) ) @@ -138,6 +145,36 @@ def _store_users( report.failed["store"] = str(exc) +# Names of reconciled fields mapped to their keys in the item's jsondata. +_JSONDATA_KEYS = { + "websites": "website", + "organizations": "organization", + "employment_contract_status": "employment_contract_status", +} + + +def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: + """Overwrite removed fields of existing items with an empty value.""" + for resolved in resolution.resolved: + if resolved.action != "update": + continue + keys = [ + _JSONDATA_KEYS[d.name] + for d in resolved.change.diffs + if d.status == REMOVE + and d.name in resolved.apply_fields + and d.name in _JSONDATA_KEYS + ] + if not keys: + continue + title = resolved.change.proposed.full_page_title + try: + if clear_jsondata_keys(osw, title, keys): + report.pruned.append(title) + except Exception as exc: # pragma: no cover - network failure path + report.failed[f"prune:{title}"] = str(exc) + + def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None: # Every in-scope user has an item (created or pre-existing), so ensure the # redirect for all of them; a re-run repairs any missing redirect. @@ -189,9 +226,10 @@ def run_user_sync( raise ValueError("run_user_sync requires an authenticated OSW connection") prompter = prompter or Prompter() - proposed_users, org_map = _build_proposals(config, osw, session) + proposed_users, org_map, skipped_blocked = _build_proposals(config, osw, session) missing_email = [p.username for p in proposed_users if not p.emails] existing = load_existing_users(osw) + adopt_existing_pages(osw, existing, proposed_users) plan = reconcile( proposed_users, existing, @@ -200,6 +238,11 @@ def run_user_sync( ) summary_lines = [] + if skipped_blocked: + summary_lines.append( + f"Skipped blocked accounts ({len(skipped_blocked)}): " + + ", ".join(skipped_blocked) + ) if config.link_organizations and org_map: summary_lines.append(f"Organizations to ensure: {len(org_map)}") if config.create_redirects: @@ -220,8 +263,9 @@ def run_user_sync( if config.link_organizations: _store_organizations(osw, org_map, report) - # When pruning, keep emptied fields in the payload so they overwrite (delete). - _store_users(osw, resolution, report, remove_empty=not config.prune) + _store_users(osw, resolution, report) + if config.prune and "store" not in report.failed: + _prune_users(osw, resolution, report) if config.create_redirects: _create_redirects(osw, resolution, report) _verify(osw, report) diff --git a/tests/tools/test_user_sync_build.py b/tests/tools/test_user_sync_build.py index 2aba32a8..3040ad43 100644 --- a/tests/tools/test_user_sync_build.py +++ b/tests/tools/test_user_sync_build.py @@ -30,48 +30,58 @@ def _entity(): surname="", orcid=None, email=set(), - website=set(), - __iris__={ - "organization": ["Item:OSWa"], - "employment_contract_status": "Item:OSWx", - }, + website={"https://x"}, + organization=["Item:OSWa"], + employment_contract_status="Item:OSWx", ) -def test_strip_protected_empties_only_protected(): +def test_strip_protected_unsets_only_protected(): entity = _entity() _strip_protected(entity) - assert entity.__iris__["employment_contract_status"] == [] - assert entity.__iris__["organization"] == ["Item:OSWa"] + assert entity.employment_contract_status is None + assert entity.organization == ["Item:OSWa"] def test_apply_update_strips_protected_when_in_fields(): entity = _entity() apply_update(entity, _proposed(), {"employment_contract_status"}) - assert entity.__iris__["employment_contract_status"] == [] + assert entity.employment_contract_status is None -def test_apply_update_keeps_protected_when_not_in_fields(): +def test_apply_update_strips_protected_when_not_in_fields(): entity = _entity() apply_update(entity, _proposed(surname="New"), {"surname"}) - assert "employment_contract_status" in entity.__iris__ + assert entity.employment_contract_status is None assert entity.surname == "New" -def test_apply_update_clears_organization_when_empty(): +def test_apply_update_unsets_organization_when_empty(): entity = _entity() apply_update(entity, _proposed(organizations=[]), {"organizations"}) - assert entity.__iris__["organization"] == [] + assert entity.organization is None def test_apply_update_sets_organization_when_present(): entity = _entity() apply_update(entity, _proposed(organizations=["Item:OSWb"]), {"organizations"}) - assert entity.__iris__["organization"] == ["Item:OSWb"] + assert entity.organization == ["Item:OSWb"] -def test_apply_update_clears_website(): +def test_apply_update_unsets_website_when_empty(): entity = _entity() - entity.website = {"https://x"} apply_update(entity, _proposed(websites=[]), {"websites"}) - assert entity.website == set() + assert entity.website is None + + +def test_apply_update_sets_website_when_present(): + entity = _entity() + apply_update(entity, _proposed(websites=["https://y"]), {"websites"}) + assert entity.website == {"https://y"} + + +def test_apply_update_sets_username_when_accepted(): + entity = _entity() + entity.username = None + apply_update(entity, _proposed(username="u"), {"username"}) + assert entity.username == "u" diff --git a/tests/tools/test_user_sync_config.py b/tests/tools/test_user_sync_config.py index fcf3cc05..b987c907 100644 --- a/tests/tools/test_user_sync_config.py +++ b/tests/tools/test_user_sync_config.py @@ -84,3 +84,9 @@ def test_run_user_sync_requires_connection(): with pytest.raises(ValueError, match="connection"): run_user_sync(SyncConfig()) + + +def test_include_blocked_flag(): + assert SyncConfig().include_blocked is False + assert config_from_args([]).include_blocked is False + assert config_from_args(["--include-blocked"]).include_blocked is True diff --git a/tests/tools/test_user_sync_existing.py b/tests/tools/test_user_sync_existing.py index d584866b..b08e3546 100644 --- a/tests/tools/test_user_sync_existing.py +++ b/tests/tools/test_user_sync_existing.py @@ -2,18 +2,57 @@ from types import SimpleNamespace -from osw.tools.user_sync.existing import load_existing_users +import pytest + +from osw import wiki_tools +from osw.tools.user_sync.existing import ( + ExistingUsers, + adopt_existing_pages, + load_existing_users, +) + + +def _ask_result(titles, continue_offset=None): + """An SMW ``ask`` response as wiki_tools.semantic_search reads it.""" + result = { + "query": { + "results": { + title: {"fulltext": title, "exists": "1", "printouts": {}} + for title in titles + } + } + } + if continue_offset is not None: + result["query-continue-offset"] = continue_offset + return result + + +class FakeSite: + """Answers ``ask`` with the given responses, one per request, in order.""" + + def __init__(self, responses): + self._responses = list(responses) + self.queries = [] + + def api(self, action, **params): + assert action == "ask" + self.queries.append(params["query"]) + return self._responses[len(self.queries) - 1] + + def semantic_search(self, query): + return wiki_tools.semantic_search(self, query) class FakeOsw: - def __init__(self, titles, entities): - self._titles = titles + def __init__(self, titles, entities, pages=None, missing=()): self._entities = entities + self.site = FakeSite(pages or [_ask_result(titles)]) + self.missing = set(missing) self.loaded = None - def query_instances(self, category): - assert category.startswith("Category:OSW") - return list(self._titles) + def _get_missing_page_titles(self, titles, confirm_delay_s=5): + assert confirm_delay_s == 0 + return [t for t in titles if t in self.missing] def load_entity(self, titles): self.loaded = list(titles) @@ -57,3 +96,64 @@ def test_load_respects_limit(): existing = load_existing_users(osw, limit=1) assert osw.loaded == ["Item:OSW1"] assert set(existing.by_username) == {"A"} + + +def test_load_pages_through_all_results_with_the_reported_offset(): + entities = { + "Item:OSW1": SimpleNamespace(username="A"), + "Item:OSW2": SimpleNamespace(username="B"), + "Item:OSW3": SimpleNamespace(username="C"), + } + # the wiki cuts the first page at 2 results and reports offset 2 + pages = [ + _ask_result(["Item:OSW1", "Item:OSW2"], continue_offset=2), + _ask_result(["Item:OSW3"]), + ] + osw = FakeOsw([], entities, pages=pages) + existing = load_existing_users(osw) + assert set(existing.by_username) == {"A", "B", "C"} + assert len(osw.site.queries) == 2 + assert "|offset=0" in osw.site.queries[0] + assert "|offset=2" in osw.site.queries[1] + assert osw.loaded == ["Item:OSW1", "Item:OSW2", "Item:OSW3"] + + +def test_load_raises_when_the_reported_offset_does_not_advance(): + pages = [ + _ask_result(["Item:OSW1"], continue_offset=0), + _ask_result(["Item:OSW1"], continue_offset=0), + ] + osw = FakeOsw([], {}, pages=pages) + with pytest.raises(RuntimeError, match="offset"): + load_existing_users(osw) + assert len(osw.site.queries) == 1 + + +def _proposed(username, uid): + return SimpleNamespace( + username=username, + uuid=uid, + full_page_title=f"Item:OSW{uid}", + ) + + +def test_adopt_loads_a_page_that_exists_at_the_deterministic_title(): + found = SimpleNamespace(username=None, uuid="1") + osw = FakeOsw( + [], {"Item:OSW1": found}, pages=[_ask_result([])], missing={"Item:OSW2"} + ) + existing = ExistingUsers() + adopted = adopt_existing_pages( + osw, existing, [_proposed("Alice", "1"), _proposed("Bob", "2")] + ) + assert adopted == ["Item:OSW1"] + assert osw.loaded == ["Item:OSW1"] + assert existing.get("Alice") is found + assert existing.get("Bob") is None + + +def test_adopt_skips_users_already_matched_by_username(): + osw = FakeOsw([], {}, pages=[_ask_result([])]) + existing = ExistingUsers(by_username={"Alice": SimpleNamespace(username="Alice")}) + assert adopt_existing_pages(osw, existing, [_proposed("Alice", "1")]) == [] + assert osw.loaded is None diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py index 4a592afd..73c3e6a0 100644 --- a/tests/tools/test_user_sync_reconcile.py +++ b/tests/tools/test_user_sync_reconcile.py @@ -36,6 +36,7 @@ def _proposed(**kw): def _entity(**kw): base = dict( + username=ORCID, label="Lukas Koschmieder", first_name="Lukas", surname="Koschmieder", @@ -46,6 +47,7 @@ def _entity(**kw): ) base.update(kw) return SimpleNamespace( + username=base["username"], label=[SimpleNamespace(text=base["label"])] if base["label"] else [], first_name=base["first_name"], surname=base["surname"], @@ -200,3 +202,37 @@ def test_reconcile_plan_counts(): counts = plan.counts() assert counts == {NEW: 1, GAP_FILL: 0, CONFLICT: 1, UNCHANGED: 1} assert [c.proposed.username for c in plan.conflicts] == ["conflict-user"] + + +def test_placeholder_name_is_not_compared_for_an_existing_item(): + proposed = _proposed( + first_name="bob", surname="bob", label="bob", placeholder_name=True + ) + for existing in ( + _entity(label="Robert Smith", first_name="Robert", surname="Smith"), + _entity(label=None, first_name=None, surname=None), + ): + change = reconcile_user(proposed, existing) + names = {d.name for d in change.diffs} + assert not names & {"label", "first_name", "surname"} + assert change.category == UNCHANGED + + +def test_placeholder_name_is_kept_for_a_new_item(): + change = reconcile_user(_proposed(placeholder_name=True), None) + assert change.category == NEW + + +def test_missing_username_is_filled_on_an_existing_item(): + # An item found by its page title instead of its username has none yet. + change = reconcile_user(_proposed(), _entity(username=None)) + assert change.category == GAP_FILL + assert [(d.name, d.proposed) for d in change.diffs] == [("username", ORCID)] + + +def test_placeholder_user_without_username_still_gets_it(): + change = reconcile_user( + _proposed(placeholder_name=True, label=ORCID), _entity(username=None) + ) + assert change.category == GAP_FILL + assert [d.name for d in change.diffs] == ["username"] diff --git a/tests/tools/test_user_sync_sources.py b/tests/tools/test_user_sync_sources.py index aa90ff6e..758603d0 100644 --- a/tests/tools/test_user_sync_sources.py +++ b/tests/tools/test_user_sync_sources.py @@ -142,3 +142,30 @@ def test_partition_by_orcid(): parts = partition_by_orcid(users) assert [u.name for u in parts["orcid"]] == ["0000-0002-6374-9831"] assert [u.name for u in parts["other"]] == ["Alice"] + + +def test_blocked_users_are_skipped_by_default_and_listed(): + site = FakeSite([ + _page([ + {"name": "Alice", "groups": ["user"]}, + {"name": "Mallory", "groups": ["user"], "blockid": 5}, + {"name": "0000-0002-6374-9831", "groups": ["user"], "blockid": 6}, + ]) + ]) + skipped = [] + users = enumerate_mw_users(site, skipped_blocked=skipped) + assert [u.name for u in users] == ["Alice"] + assert skipped == ["Mallory", "0000-0002-6374-9831"] + + +def test_blocked_users_are_kept_with_exclude_blocked_false(): + site = FakeSite([ + _page([ + {"name": "Alice", "groups": ["user"]}, + {"name": "Mallory", "groups": ["user"], "blockid": 5}, + ]) + ]) + skipped = [] + users = enumerate_mw_users(site, exclude_blocked=False, skipped_blocked=skipped) + assert [u.name for u in users] == ["Alice", "Mallory"] + assert skipped == [] diff --git a/tests/tools/test_user_sync_store.py b/tests/tools/test_user_sync_store.py new file mode 100644 index 00000000..16bd2c09 --- /dev/null +++ b/tests/tools/test_user_sync_store.py @@ -0,0 +1,338 @@ +"""Tests for the store paths of the sync and the merge with stored page content.""" + +import json +import uuid +from types import SimpleNamespace +from typing import Any, Dict, Union + +from opensemantic.base.v1 import User + +from osw.core import OSW, OverwriteOptions +from osw.data.import_utility import uuid_to_full_page_title +from osw.tools.user_sync import sync as sync_module +from osw.tools.user_sync.build import apply_update, build_user +from osw.tools.user_sync.config import SyncConfig +from osw.tools.user_sync.interactive import Prompter +from osw.tools.user_sync.mapping import ProposedUser, user_uuid +from osw.tools.user_sync.prune import clear_jsondata_keys +from osw.tools.user_sync.sources import MwUser +from osw.tools.user_sync.sync import run_user_sync +from osw.wtsite import WtPage + +ITEM_TITLE = "Item:OSWb0b1a0a0a0a04a0a8a0a0a0a0a0a0a01" + + +class FakeMwSite: + def __init__(self, users): + self._users = users + + def api(self, action, **params): + return {"query": {"allusers": self._users}} + + +class FakeStoreOsw: + """Fake connection whose store_entity returns the stage StoreEntityResult.""" + + def __init__(self, users, existing=None, drop_username=False): + self.mw_site = FakeMwSite(users) + self.site = SimpleNamespace(semantic_search=self._semantic_search) + self.existing = existing or {} + self.drop_username = drop_username + self.store_params = [] + + def _semantic_search(self, param): + titles = list(self.existing) + return [ + SimpleNamespace( + titles=titles, truncated=False, next_offset=None, count=len(titles) + ) + ] + + def _get_missing_page_titles(self, titles, confirm_delay_s=5): + return [t for t in titles if t not in self.existing] + + def load_entity(self, titles): + if isinstance(titles, str): + titles = [titles] + loaded = [] + for title in titles: + entity = self.existing.get(title) + if entity is None: + name = None if self.drop_username else "loaded" + entity = SimpleNamespace(username=name) + loaded.append(entity) + return loaded + + def store_entity(self, param): + self.store_params.append(param) + return OSW.StoreEntityResult(change_id="c1", pages={}) + + +def _run(osw, **config): + config.setdefault("auto_apply", True) + return run_user_sync( + SyncConfig(domain="d", **config), + osw=osw, + prompter=Prompter(lambda prompt="": "y", lambda text="": None), + session=None, + ) + + +def _patch_redirects(monkeypatch): + written = [] + + def fake_ensure(osw, username, item_title): + written.append((username, item_title)) + return f"User:{username}" + + monkeypatch.setattr(sync_module, "ensure_redirect", fake_ensure) + return written + + +def _title(username): + return uuid_to_full_page_title(user_uuid(MwUser(name=username))) + + +def test_accepted_run_stores_and_writes_redirects(monkeypatch): + written = _patch_redirects(monkeypatch) + osw = FakeStoreOsw([ + {"name": "Alice", "groups": ["user"]}, + {"name": "Bob", "groups": ["user"]}, + ]) + report = _run(osw) + assert report.accepted is True + param = osw.store_params[0] + assert param.overwrite == OverwriteOptions.true + assert param.remove_empty is True + assert report.created == [_title("Alice"), _title("Bob")] + assert report.updated == [] + assert written == [("Alice", _title("Alice")), ("Bob", _title("Bob"))] + assert report.redirects_created == ["User:Alice", "User:Bob"] + assert report.failed == {} + + +def test_verify_reports_entity_without_username(monkeypatch): + _patch_redirects(monkeypatch) + osw = FakeStoreOsw([{"name": "Alice", "groups": ["user"]}], drop_username=True) + report = _run(osw) + assert "verify" in report.failed + + +def _existing_user(): + return SimpleNamespace( + username="Alice", + uuid=user_uuid(MwUser(name="Alice")), + label=[SimpleNamespace(text="Alice")], + first_name="Alice", + surname="Alice", + orcid=None, + email=set(), + website={"https://example.org"}, + __iris__={ + "organization": ["Item:OSWa"], + "employment_contract_status": "Item:OSWb", + }, + ) + + +def test_prune_run_stores_with_default_and_clears_keys_afterwards(monkeypatch): + _patch_redirects(monkeypatch) + # the fake entity is not a model, so skip the pydantic validation of the param + monkeypatch.setattr( + sync_module, + "OSW", + SimpleNamespace(StoreEntityParam=lambda **kw: SimpleNamespace(**kw)), + ) + cleared = [] + + def fake_clear(osw, title, keys): + cleared.append((title, sorted(keys))) + return True + + monkeypatch.setattr(sync_module, "clear_jsondata_keys", fake_clear) + existing = _existing_user() + osw = FakeStoreOsw( + [{"name": "Alice", "groups": ["user"]}], existing={ITEM_TITLE: existing} + ) + report = _run(osw, prune=True) + assert report.updated == [_title("Alice")] + param = osw.store_params[0] + assert not hasattr(param, "remove_empty") # the store_entity default applies + assert param.entities[0] is existing + assert cleared == [ + (_title("Alice"), ["employment_contract_status", "organization", "website"]) + ] + assert report.pruned == [_title("Alice")] + assert "pruned=1" in report.summary() + + +def test_normal_run_never_clears_keys(monkeypatch): + _patch_redirects(monkeypatch) + monkeypatch.setattr( + sync_module, + "OSW", + SimpleNamespace(StoreEntityParam=lambda **kw: SimpleNamespace(**kw)), + ) + calls = [] + monkeypatch.setattr( + sync_module, "clear_jsondata_keys", lambda *a, **k: calls.append(a) + ) + osw = FakeStoreOsw( + [{"name": "Alice", "groups": ["user"]}], existing={ITEM_TITLE: _existing_user()} + ) + report = _run(osw) + assert calls == [] + assert report.pruned == [] + + +# -------------------------------------------------------------------------- +# merge with the stored page on stage +# -------------------------------------------------------------------------- + + +class OfflineWtPage(WtPage): + """A WtPage that pretends to exist without touching a wiki.""" + + 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"} + + +REMOTE_STATUS = "Item:OSWb" + + +def _proposed(user_id, **kw): + args = dict( + uuid=user_id, + username="Alice", + first_name="Alice", + surname="Alice", + label="Alice", + orcid=None, + emails=[], + websites=[], + organizations=[], + ) + args.update(kw) + return ProposedUser(**args) + + +def _json(entity): + return json.loads(entity.json(exclude_none=True)) + + +def test_built_user_has_no_employment_contract_status(): + user = build_user(_proposed(uuid.uuid4(), websites=["https://example.org"])) + assert "employment_contract_status" not in _json(user) + + +def _loaded_user(user_id): + return User( + uuid=user_id, + username="Alice", + first_name="Alice", + surname="Alice", + label=[{"text": "Alice"}], + website={"https://example.org"}, + organization=["Item:OSWa"], + ) + + +def test_normal_update_keeps_remote_employment_contract_status(): + user_id = uuid.uuid4() + remote = { + "type": ["Category:OSWd9aa0bca9b0040d8af6f5c091bf9eec7"], + "uuid": str(user_id), + "username": "Alice", + "first_name": "Alice", + "surname": "Alice", + "label": [{"text": "Alice", "lang": "en"}], + "organization": ["Item:OSWa"], + "employment_contract_status": REMOTE_STATUS, + } + page = OfflineWtPage(title=uuid_to_full_page_title(user_id)) + page.set_slot_content("jsondata", dict(remote)) + entity = _loaded_user(user_id) + apply_update( + entity, + _proposed(user_id, first_name="Alicia", websites=["https://example.org"]), + {"first_name", "websites"}, + ) + assert "employment_contract_status" not in _json(entity) + merged = OSW._apply_overwrite_policy( + OSW._ApplyOverwriteParam( + page=page, entity=entity, policy=True, inplace=False, remove_empty=True + ) + ).get_slot_content("jsondata") + assert merged["employment_contract_status"] == REMOTE_STATUS + assert merged["first_name"] == "Alicia" + + +# -------------------------------------------------------------------------- +# clear_jsondata_keys +# -------------------------------------------------------------------------- + + +class FakePrunePage: + """Stands in for WtPage: holds a jsondata slot and records the writes.""" + + def __init__(self, content): + self.content = content + self.comments = [] + + def get_slot_content(self, slot): + assert slot == "jsondata" + return json.loads(json.dumps(self.content)) + + def set_slot_content(self, slot, content): + assert slot == "jsondata" + self.content = content + + def edit(self, comment=None): + self.comments.append(comment) + + +def _patch_page(monkeypatch, page): + from osw.tools.user_sync import prune + + monkeypatch.setattr(prune, "WtPage", lambda wtSite=None, title=None: page) + + +def test_clear_jsondata_keys_empties_present_keys_only(monkeypatch): + content = { + "website": ["https://example.org"], + "organization": ["Item:OSWa"], + "employment_contract_status": "Item:OSWb", + "short_name": [{"text": "x", "lang": "en"}], + } + page = FakePrunePage(content) + _patch_page(monkeypatch, page) + written = clear_jsondata_keys( + SimpleNamespace(site=None), + ITEM_TITLE, + ["website", "organization", "employment_contract_status"], + ) + assert written is True + assert page.content["website"] == [] + assert page.content["organization"] == [] + assert page.content["employment_contract_status"] == [] + assert page.content["short_name"] == content["short_name"] + assert page.comments == ["user-sync: prune"] + + +def test_clear_jsondata_keys_does_not_add_absent_keys_or_write(monkeypatch): + page = FakePrunePage({"website": [], "username": "Alice"}) + _patch_page(monkeypatch, page) + written = clear_jsondata_keys( + SimpleNamespace(site=None), ITEM_TITLE, ["website", "organization"] + ) + assert written is False + assert page.content == {"website": [], "username": "Alice"} + assert page.comments == [] diff --git a/tests/tools/test_user_sync_sync.py b/tests/tools/test_user_sync_sync.py index 017c6c18..d9b012ee 100644 --- a/tests/tools/test_user_sync_sync.py +++ b/tests/tools/test_user_sync_sync.py @@ -1,7 +1,12 @@ """Unit tests for the orchestrator (dry-run path, no live models).""" +from types import SimpleNamespace + +from osw.data.import_utility import uuid_to_full_page_title from osw.tools.user_sync.config import SyncConfig from osw.tools.user_sync.interactive import Prompter +from osw.tools.user_sync.mapping import user_uuid +from osw.tools.user_sync.sources import MwUser from osw.tools.user_sync.sync import SyncReport, run_user_sync ORCID = "0000-0002-6374-9831" @@ -17,6 +22,9 @@ class FakeMwSite: + def __init__(self, extra_users=()): + self.extra_users = list(extra_users) + def api(self, action, **params): assert action == "query" and params["list"] == "allusers" return { @@ -24,21 +32,29 @@ def api(self, action, **params): "allusers": [ {"name": ORCID, "groups": ["user"]}, {"name": "SyncBot", "groups": ["user", "bot"]}, + *self.extra_users, ] } } class FakeOsw: - def __init__(self): - self.mw_site = FakeMwSite() + def __init__(self, extra_users=(), existing_pages=None): + self.mw_site = FakeMwSite(extra_users) + self.site = SimpleNamespace(semantic_search=self._semantic_search) + self.existing_pages = existing_pages or {} + self.loaded = [] self.store_calls = 0 - def query_instances(self, category): - return [] + def _semantic_search(self, query): + return [SimpleNamespace(titles=[], truncated=False, next_offset=None, count=0)] + + def _get_missing_page_titles(self, titles, confirm_delay_s=5): + return [t for t in titles if t not in self.existing_pages] def load_entity(self, titles): - return [] + self.loaded.extend(titles) + return [self.existing_pages[t] for t in titles] def store_entity(self, param): self.store_calls += 1 @@ -107,3 +123,49 @@ def test_requires_connection(): assert "connection" in str(exc) else: # pragma: no cover raise AssertionError("expected ValueError") + + +def _dry_run_output(osw, **config): + io = CapturingIO() + report = run_user_sync( + SyncConfig(domain="d", dry_run=True, **config), + osw=osw, + prompter=Prompter(io.input, io.output), + session=FakeSession(), + ) + return report, "\n".join(io.outputs) + + +def test_preview_lists_skipped_blocked_accounts(): + osw = FakeOsw(extra_users=[{"name": "Mallory", "groups": ["user"], "blockid": 5}]) + _, joined = _dry_run_output(osw) + assert "Skipped blocked accounts (1): Mallory" in joined + assert "new=1" in joined # only the ORCID user + + +def test_include_blocked_keeps_blocked_accounts(): + osw = FakeOsw(extra_users=[{"name": "Mallory", "groups": ["user"], "blockid": 5}]) + _, joined = _dry_run_output(osw, include_blocked=True) + assert "Skipped blocked accounts" not in joined + assert "new=2" in joined + + +def test_existing_page_at_the_deterministic_title_is_not_created_again(): + uid = user_uuid(MwUser(name=ORCID)) + title = uuid_to_full_page_title(uid) + stored = SimpleNamespace( + uuid=uid, + username=None, + label=[], + first_name=None, + surname=None, + orcid=None, + email=set(), + website=set(), + organization=set(), + ) + osw = FakeOsw(existing_pages={title: stored}) + _, joined = _dry_run_output(osw) + assert osw.loaded == [title] + assert "new=0" in joined + assert "gap_fill=1" in joined diff --git a/zensical.toml b/zensical.toml index ab63c179..8a2dfe2b 100644 --- a/zensical.toml +++ b/zensical.toml @@ -18,6 +18,7 @@ nav = [ { "Overview" = "tools/index.md" }, { "CLI" = "tools/cli.md" }, { "MCP" = "tools/mcp.md" }, + { "User Sync (script)" = "tools/user-sync.md" }, { "Configuration" = "tools/configuration.md" }, ]}, { "API Reference" = [ @@ -28,9 +29,6 @@ nav = [ { "Controllers" = "api/controller.md" }, { "Model" = "api/model.md" }, ]}, - { "Tools" = [ - { "User Sync" = "tools/user-sync.md" }, - ]}, { "Development" = "dev.md" }, ] From 0b8dc974e972e8adaa7a2bb03bd3ae4396589f40 Mon Sep 17 00:00:00 2001 From: LukasGold <41064839+LukasGold@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:20:11 +0100 Subject: [PATCH 18/20] fix(user-sync): count and redirect only stored items (#224) - count a title as created or updated only after store_entity stored it - record each failed title of StoreEntityPartialError as store: - write redirects only for stored or pre-existing items - drop new links to organizations that failed to store - prune only updates that were stored --- src/osw/tools/user_sync/sync.py | 89 ++++-- tests/tools/test_user_sync_store_failures.py | 271 +++++++++++++++++++ 2 files changed, 341 insertions(+), 19 deletions(-) create mode 100644 tests/tools/test_user_sync_store_failures.py diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index 88902189..bd2e992f 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -8,8 +8,8 @@ from __future__ import annotations -from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from dataclasses import dataclass, field, replace +from typing import Any, Dict, FrozenSet, List, Optional, Tuple from osw.core import OSW @@ -19,7 +19,7 @@ from .interactive import Prompter, Resolution, resolve_plan from .mapping import ProposedOrganization, map_user from .prune import clear_jsondata_keys -from .reconcile import REMOVE, reconcile +from .reconcile import REMOVE, existing_fields, reconcile from .redirects import ensure_redirect from .sources import ( RESERVED_USERNAMES, @@ -100,6 +100,13 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any): return proposed_users, org_map, skipped_blocked +def _record_partial(report: SyncReport, exc: Exception) -> List[str]: + """Record each failed title of a partial store; return the stored titles.""" + for title, error in exc.failed.items(): + report.failed[f"store:{title}"] = str(error) + return list(exc.stored) + + def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: if not org_map: return @@ -109,40 +116,70 @@ def _store_organizations(osw: Any, org_map, report: SyncReport) -> None: OSW.StoreEntityParam(entities=entities, edit_comment="user-sync: orgs") ) report.organizations = [org.full_page_title for org in org_map.values()] + except OSW.StoreEntityPartialError as exc: + report.organizations = _record_partial(report, exc) except Exception as exc: # pragma: no cover - network failure path report.failed["organizations"] = str(exc) -def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: +def _store_users( + osw: Any, + resolution: Resolution, + report: SyncReport, + failed_orgs: FrozenSet[str] = frozenset(), +) -> None: + """Store created and updated items; count a title only once it is stored. + + New links to an organization in ``failed_orgs`` are dropped, so no item + points to an organization that was not stored. A link the existing item + already has is kept. A re-run proposes the dropped links again. + """ entities = [] + pending: List[Tuple[str, str]] = [] # (action, title) per entity for resolved in resolution.resolved: proposed = resolved.change.proposed + drop = set(proposed.organizations) & failed_orgs + if drop and resolved.change.existing is not None: + drop -= existing_fields(resolved.change.existing)["organizations"] + if drop: + proposed = replace( + proposed, + organizations=[o for o in proposed.organizations if o not in drop], + ) try: if resolved.action == "create": entities.append(build_user(proposed)) - report.created.append(proposed.full_page_title) elif resolved.action == "update": entities.append( apply_update( resolved.change.existing, proposed, resolved.apply_fields ) ) - report.updated.append(proposed.full_page_title) else: report.skipped.append(proposed.full_page_title) + continue + pending.append((resolved.action, proposed.full_page_title)) except Exception as exc: # pragma: no cover - build failure path report.failed[f"build:{proposed.username}"] = str(exc) - if entities: - try: - osw.store_entity( - OSW.StoreEntityParam( - entities=entities, - overwrite=True, - edit_comment="user-sync", - ) + if not entities: + return + try: + osw.store_entity( + OSW.StoreEntityParam( + entities=entities, + overwrite=True, + edit_comment="user-sync", ) - except Exception as exc: # pragma: no cover - network failure path - report.failed["store"] = str(exc) + ) + stored = {title for _, title in pending} + except OSW.StoreEntityPartialError as exc: + stored = set(_record_partial(report, exc)) + except Exception as exc: # pragma: no cover - network failure path + report.failed["store"] = str(exc) + stored = set() + for action, title in pending: + if title in stored: + (report.created if action == "create" else report.updated).append(title) # Names of reconciled fields mapped to their keys in the item's jsondata. @@ -155,9 +192,12 @@ def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: """Overwrite removed fields of existing items with an empty value.""" + updated = set(report.updated) for resolved in resolution.resolved: if resolved.action != "update": continue + if resolved.change.proposed.full_page_title not in updated: + continue # the update was not stored keys = [ _JSONDATA_KEYS[d.name] for d in resolved.change.diffs @@ -176,10 +216,14 @@ def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None: - # Every in-scope user has an item (created or pre-existing), so ensure the - # redirect for all of them; a re-run repairs any missing redirect. + # A redirect needs an existing target: an item stored in this run, or one + # that existed before it (update or skip). A failed create gets none; a + # re-run writes it once the item is stored. + created = set(report.created) for resolved in resolution.resolved: proposed = resolved.change.proposed + if resolved.action == "create" and proposed.full_page_title not in created: + continue try: written = ensure_redirect(osw, proposed.username, proposed.full_page_title) if written: @@ -261,9 +305,16 @@ def run_user_sync( _warn_missing_email(prompter, missing_email) return report + failed_orgs: FrozenSet[str] = frozenset() if config.link_organizations: _store_organizations(osw, org_map, report) - _store_users(osw, resolution, report) + stored_orgs = set(report.organizations) + failed_orgs = frozenset( + o.full_page_title + for o in org_map.values() + if o.full_page_title not in stored_orgs + ) + _store_users(osw, resolution, report, failed_orgs=failed_orgs) if config.prune and "store" not in report.failed: _prune_users(osw, resolution, report) if config.create_redirects: diff --git a/tests/tools/test_user_sync_store_failures.py b/tests/tools/test_user_sync_store_failures.py new file mode 100644 index 00000000..ad7769e3 --- /dev/null +++ b/tests/tools/test_user_sync_store_failures.py @@ -0,0 +1,271 @@ +"""Failed and partial stores in an accepted sync run (issue 224). + +The model builders, the redirect writer and the plan are replaced with fakes, +so these tests run the orchestrator without a wiki connection. +""" + +import uuid +from types import SimpleNamespace + +import pytest + +from osw.core import OSW +from osw.tools.user_sync import sync +from osw.tools.user_sync.config import SyncConfig +from osw.tools.user_sync.interactive import Prompter, Resolution, ResolvedChange +from osw.tools.user_sync.mapping import ProposedOrganization, ProposedUser +from osw.tools.user_sync.reconcile import REMOVE, FieldDiff, UserChange + + +def _user(name, organizations=()): + return ProposedUser( + uuid=uuid.uuid5(uuid.NAMESPACE_DNS, name), + username=name, + first_name=name, + surname=name, + label=name, + emails=[f"{name}@example.org"], + organizations=list(organizations), + ) + + +def _org(name): + return ProposedOrganization(uuid=uuid.uuid5(uuid.NAMESPACE_URL, name), name=name) + + +def _resolved(user, action): + existing = None if action == "create" else SimpleNamespace(username=user.username) + category = "new" if action == "create" else "gap_fill" + return ResolvedChange(UserChange(user, existing, category), action, set()) + + +def _partial_error(stored, failed): + result = OSW.StoreEntityResult.construct( + change_id="c", + pages={t: object() for t in stored}, + failed={t: RuntimeError(f"boom {t}") for t in failed}, + ) + return OSW.StoreEntityPartialError(result) + + +class FakeOsw: + """Records every store call; ``outcomes`` maps an edit comment to an outcome.""" + + def __init__(self, outcomes=None): + self.outcomes = outcomes or {} + self.stored_payloads = {} + + def store_entity(self, param): + titles = [e.title for e in param.entities] + self.stored_payloads[param.edit_comment] = list(param.entities) + outcome = self.outcomes.get(param.edit_comment) + if callable(outcome): + raise outcome(titles) + if isinstance(outcome, Exception): + raise outcome + + def load_entity(self, titles): + return [SimpleNamespace(username="u") for _ in titles] + + +@pytest.fixture +def run(monkeypatch): + """Run an accepted sync over a fixed plan and return (report, redirects).""" + + def _run(osw, resolved, org_map=None, **config): + redirects = [] + users = [r.change.proposed for r in resolved] + monkeypatch.setattr( + sync, "_build_proposals", lambda c, o, s: (users, org_map or {}, []) + ) + monkeypatch.setattr(sync, "adopt_existing_pages", lambda *a: None) + # The real StoreEntityParam validates entities; the fakes are plain objects. + monkeypatch.setattr( + sync, + "OSW", + SimpleNamespace( + StoreEntityParam=SimpleNamespace, + StoreEntityPartialError=OSW.StoreEntityPartialError, + ), + ) + monkeypatch.setattr(sync, "load_existing_users", lambda o: None) + monkeypatch.setattr(sync, "reconcile", lambda *a, **k: None) + monkeypatch.setattr( + sync, + "resolve_plan", + lambda *a, **k: Resolution(resolved=resolved, accepted=True), + ) + monkeypatch.setattr( + sync, + "build_user", + lambda p: SimpleNamespace( + title=p.full_page_title, organization=list(p.organizations) + ), + ) + monkeypatch.setattr( + sync, + "apply_update", + lambda e, p, f: SimpleNamespace( + title=p.full_page_title, organization=list(p.organizations) + ), + ) + monkeypatch.setattr( + sync, + "build_organization", + lambda o: SimpleNamespace(title=o.full_page_title), + ) + + def fake_redirect(osw_, username, title): + redirects.append(title) + return f"User:{username}" + + monkeypatch.setattr(sync, "ensure_redirect", fake_redirect) + report = sync.run_user_sync( + SyncConfig(domain="d", auto_apply=True, **config), + osw=osw, + prompter=Prompter(lambda p="": "y", lambda t="": None), + ) + return report, redirects + + return _run + + +def test_partial_store_counts_only_stored_titles(run): + ok, bad = _user("ok"), _user("bad") + osw = FakeOsw({ + "user-sync": lambda titles: _partial_error( + stored=[ok.full_page_title], failed=[bad.full_page_title] + ) + }) + report, _ = run(osw, [_resolved(ok, "create"), _resolved(bad, "create")]) + assert report.created == [ok.full_page_title] + assert f"store:{bad.full_page_title}" in report.failed + + +def test_failed_store_counts_nothing_as_created_or_updated(run): + new, old = _user("new"), _user("old") + osw = FakeOsw({"user-sync": RuntimeError("down")}) + report, _ = run(osw, [_resolved(new, "create"), _resolved(old, "update")]) + assert report.created == [] + assert report.updated == [] + assert "store" in report.failed + + +def test_no_redirect_for_a_created_item_that_was_not_stored(run): + ok, bad, old = _user("ok"), _user("bad"), _user("old") + osw = FakeOsw({ + "user-sync": lambda titles: _partial_error( + stored=[ok.full_page_title], + failed=[bad.full_page_title, old.full_page_title], + ) + }) + _, redirects = run( + osw, + [_resolved(ok, "create"), _resolved(bad, "create"), _resolved(old, "update")], + ) + # ``old`` already existed, so its redirect target exists even though the + # update failed. ``bad`` was never stored. + assert sorted(redirects) == sorted([ok.full_page_title, old.full_page_title]) + + +def test_user_is_not_stored_with_a_link_to_a_failed_organization(run): + org = _org("Acme") + linked = _user("linked", organizations=[org.full_page_title]) + osw = FakeOsw({"user-sync: orgs": RuntimeError("down")}) + report, _ = run( + osw, + [_resolved(linked, "create")], + org_map={org.uuid: org}, + link_organizations=True, + ) + assert report.organizations == [] + stored = osw.stored_payloads.get("user-sync", []) + assert "user-sync: orgs" in osw.stored_payloads + assert all(org.full_page_title not in e.organization for e in stored) + # The user itself is still stored, counted and redirected. + assert report.created == [linked.full_page_title] + assert report.redirects_created == ["User:linked"] + + +def test_partial_organization_store_drops_only_the_failed_link(run): + good, gone = _org("Good"), _org("Gone") + linked = _user("linked", organizations=[good.full_page_title, gone.full_page_title]) + osw = FakeOsw({ + "user-sync: orgs": lambda titles: _partial_error( + stored=[good.full_page_title], failed=[gone.full_page_title] + ) + }) + report, _ = run( + osw, + [_resolved(linked, "create")], + org_map={good.uuid: good, gone.uuid: gone}, + link_organizations=True, + ) + assert report.organizations == [good.full_page_title] + assert f"store:{gone.full_page_title}" in report.failed + (stored,) = osw.stored_payloads["user-sync"] + assert stored.organization == [good.full_page_title] + + +def test_accepted_sync_counts_and_redirects_every_user(run): + new, old, same = _user("new"), _user("old"), _user("same") + osw = FakeOsw() + report, redirects = run( + osw, + [_resolved(new, "create"), _resolved(old, "update"), _resolved(same, "skip")], + ) + assert report.accepted is True + assert report.created == [new.full_page_title] + assert report.updated == [old.full_page_title] + assert report.skipped == [same.full_page_title] + assert report.failed == {} + assert sorted(redirects) == sorted([ + new.full_page_title, + old.full_page_title, + same.full_page_title, + ]) + param_entities = osw.stored_payloads["user-sync"] + assert [e.title for e in param_entities] == [ + new.full_page_title, + old.full_page_title, + ] + + +def test_prune_skips_an_update_that_was_not_stored(run, monkeypatch): + ok, bad = _user("ok"), _user("bad") + cleared = [] + monkeypatch.setattr( + sync, "clear_jsondata_keys", lambda o, t, k: cleared.append(t) or True + ) + resolved = [] + for user in (ok, bad): + r = _resolved(user, "update") + r.change.diffs = [FieldDiff("websites", ["x"], [], REMOVE)] + r.apply_fields = {"websites"} + resolved.append(r) + osw = FakeOsw({ + "user-sync": lambda titles: _partial_error( + stored=[ok.full_page_title], failed=[bad.full_page_title] + ) + }) + report, _ = run(osw, resolved, prune=True) + assert cleared == [ok.full_page_title] + assert report.pruned == [ok.full_page_title] + + +def test_failed_organization_keeps_a_link_the_existing_item_has(run): + kept, new = _org("Kept"), _org("New") + user = _user("old", organizations=[kept.full_page_title, new.full_page_title]) + resolved = _resolved(user, "update") + resolved.change.existing = SimpleNamespace( + username="old", __iris__={"organization": [kept.full_page_title]} + ) + osw = FakeOsw({"user-sync: orgs": RuntimeError("down")}) + run( + osw, + [resolved], + org_map={kept.uuid: kept, new.uuid: new}, + link_organizations=True, + ) + (stored,) = osw.stored_payloads["user-sync"] + assert stored.organization == [kept.full_page_title] From 9b36e4ef8417b8847ba304535a95c0eedaf628b9 Mon Sep 17 00:00:00 2001 From: LukasGold <41064839+LukasGold@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:04:23 +0100 Subject: [PATCH 19/20] fix(user-sync): prune by field type and decide from stored jsondata - set list fields to [] and delete single-valued keys on --prune - read stored jsondata via offline_pages, without an extra request - decide protected-field removal from the stored jsondata - a second --prune run no longer proposes the same removal --- docs/tools/user-sync.md | 18 +++++---- src/osw/tools/user_sync/config.py | 4 +- src/osw/tools/user_sync/existing.py | 50 ++++++++++++++++++++++--- src/osw/tools/user_sync/prune.py | 27 ++++++++++--- src/osw/tools/user_sync/reconcile.py | 26 +++++++++++-- src/osw/tools/user_sync/sync.py | 2 +- tests/tools/test_user_sync_existing.py | 46 ++++++++++++++++++++++- tests/tools/test_user_sync_reconcile.py | 34 +++++++++++++++++ tests/tools/test_user_sync_store.py | 35 +++++++++++++++-- tests/tools/test_user_sync_sync.py | 26 +++++++++++-- 10 files changed, 234 insertions(+), 34 deletions(-) diff --git a/docs/tools/user-sync.md b/docs/tools/user-sync.md index 6ab49168..6d016f66 100644 --- a/docs/tools/user-sync.md +++ b/docs/tools/user-sync.md @@ -30,10 +30,12 @@ prompt. To also clean up existing items, add `--prune`. Without it, the sync never writes `employment_contract_status` and never removes anything. With it, -`employment_contract_status` on existing items is overwritten with an empty -value. So are `website` and `organization`, unless `--with-websites` or -`--with-organizations` enables them. This happens in a second edit of the -page's jsondata after the store: +`employment_contract_status` is removed from the stored jsondata of existing +items. `website` and `organization` are overwritten with an empty list, unless +`--with-websites` or `--with-organizations` enables them. This happens in a +second edit of the page's jsondata after the store. The sync decides from the +stored jsondata, not from the loaded item, which shows the model default for +`employment_contract_status`. A second `--prune` run therefore changes nothing: ```bash uv run python examples/user_sync.py --domain your-instance.example.org --prune --dry-run @@ -62,7 +64,7 @@ only core identity plus email. | `--with-organizations` | Also create and link Organization items from ORCID affiliations (opt-in). | | `--with-extras` | Enable both websites and organizations. | | `--include-blocked` | Do not skip blocked MediaWiki accounts. | -| `--prune` | Overwrite `employment_contract_status`, and `website` or `organization` when their opt-in flag is not given, on existing items with an empty value, in a second edit of the page's jsondata after the store (default off; a normal run never writes `employment_contract_status` and never removes anything). | +| `--prune` | Remove `employment_contract_status` from the stored jsondata of existing items, and overwrite `website` or `organization` with an empty list when their opt-in flag is not given, in a second edit of the page's jsondata after the store (default off; a normal run never writes `employment_contract_status` and never removes anything). | ## What it does @@ -95,9 +97,9 @@ only core identity plus email. | --- | --- | | `username`, `first_name`, `surname`, `label`, `orcid` | Always (core identity). | | `email` | Standard: always attempted; missing email is warned, not fatal; never removed. | -| `website` | Opt-in (`--with-websites`); without that flag, on existing items overwritten with an empty value only with `--prune`, in a second jsondata edit after the store. | -| `organization` | Opt-in (`--with-organizations`); without that flag, on existing items overwritten with an empty value only with `--prune`, in a second jsondata edit after the store. | -| `employment_contract_status` | Never written without `--prune`; with `--prune`, overwritten with an empty value on existing items in a second jsondata edit after the store. | +| `website` | Opt-in (`--with-websites`); without that flag, on existing items overwritten with an empty list only with `--prune`, in a second jsondata edit after the store. | +| `organization` | Opt-in (`--with-organizations`); without that flag, on existing items overwritten with an empty list only with `--prune`, in a second jsondata edit after the store. | +| `employment_contract_status` | Never written without `--prune`; with `--prune`, removed from the stored jsondata of existing items in a second jsondata edit after the store. | ## Notes diff --git a/src/osw/tools/user_sync/config.py b/src/osw/tools/user_sync/config.py index 4973e715..8d0bac7e 100644 --- a/src/osw/tools/user_sync/config.py +++ b/src/osw/tools/user_sync/config.py @@ -113,8 +113,8 @@ def build_arg_parser() -> argparse.ArgumentParser: "--prune", dest="prune", action="store_true", - help="Remove disabled optional fields from existing items and overwrite " - "employment_contract_status with an empty value (data cleanup). Off by " + help="Overwrite disabled optional fields of existing items with an empty " + "list and remove employment_contract_status (data cleanup). Off by " "default; a normal run only adds.", ) account_scope = parser.add_mutually_exclusive_group() diff --git a/src/osw/tools/user_sync/existing.py b/src/osw/tools/user_sync/existing.py index 6bbaa3f7..5ba79801 100644 --- a/src/osw/tools/user_sync/existing.py +++ b/src/osw/tools/user_sync/existing.py @@ -6,10 +6,13 @@ from __future__ import annotations +import copy from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple +from osw.core import OSW from osw.wiki_tools import SearchParam +from osw.wtsite import WtSite from .config import USER_CATEGORY @@ -22,10 +25,16 @@ class ExistingUsers: """Existing User items indexed by their MediaWiki username.""" by_username: Dict[str, Any] = field(default_factory=dict) + # The jsondata as stored on the page, per username. The loaded entity cannot + # tell a stored value from a model default. + stored_jsondata: Dict[str, dict] = field(default_factory=dict) def get(self, username: str) -> Optional[Any]: return self.by_username.get(username) + def get_stored(self, username: str) -> Optional[dict]: + return self.stored_jsondata.get(username) + def _as_entity_list(result: Any) -> List[Any]: """Normalize load_entity output (single, list, or LoadEntityResult).""" @@ -39,6 +48,29 @@ def _as_entity_list(result: Any) -> List[Any]: return [result] +def _load_with_stored(osw: Any, titles: List[str]) -> Tuple[List[Any], Dict[str, dict]]: + """Load the entities and read the stored jsondata of the same pages. + + The pages are fetched once and handed to load_entity as offline pages, so the + jsondata is read without a second request. A copy is taken first, because + load_entity removes empty keys from the slot content in place. + + Returns: + The entities and the stored jsondata keyed by the uuid in it. + """ + pages = osw.site.get_page(WtSite.GetPageParam(titles=titles)).pages + offline_pages = {page.title: page for page in pages} + stored: Dict[str, dict] = {} + for page in pages: + jsondata = page.get_slot_content("jsondata") + if isinstance(jsondata, dict): + stored[str(jsondata.get("uuid"))] = copy.deepcopy(jsondata) + result = osw.load_entity( + OSW.LoadEntityParam(titles=titles, offline_pages=offline_pages) + ) + return _as_entity_list(result), stored + + def _query_user_titles( osw: Any, category: str, limit: Optional[int] = None ) -> List[str]: @@ -88,13 +120,16 @@ def load_existing_users( titles = _query_user_titles(osw, category, limit) if not titles: return ExistingUsers() - entities = _as_entity_list(osw.load_entity(titles)) - by_username: Dict[str, Any] = {} + entities, stored = _load_with_stored(osw, titles) + existing = ExistingUsers() for entity in entities: username = getattr(entity, "username", None) if username: - by_username[username] = entity - return ExistingUsers(by_username=by_username) + existing.by_username[username] = entity + jsondata = stored.get(str(getattr(entity, "uuid", None))) + if jsondata is not None: + existing.stored_jsondata[username] = jsondata + return existing def adopt_existing_pages( @@ -118,12 +153,15 @@ def adopt_existing_pages( present = [p for p in candidates if p.full_page_title not in missing] if not present: return [] - entities = _as_entity_list(osw.load_entity([p.full_page_title for p in present])) + entities, stored = _load_with_stored(osw, [p.full_page_title for p in present]) by_uuid = {str(getattr(e, "uuid", None)): e for e in entities if e is not None} adopted: List[str] = [] for proposed in present: entity = by_uuid.get(str(proposed.uuid)) if entity is not None: existing.by_username[proposed.username] = entity + jsondata = stored.get(str(proposed.uuid)) + if jsondata is not None: + existing.stored_jsondata[proposed.username] = jsondata adopted.append(proposed.full_page_title) return adopted diff --git a/src/osw/tools/user_sync/prune.py b/src/osw/tools/user_sync/prune.py index 6e1e62a2..b2e1b417 100644 --- a/src/osw/tools/user_sync/prune.py +++ b/src/osw/tools/user_sync/prune.py @@ -1,8 +1,10 @@ """Clear stored relations of existing user items (``--prune``). The store merges with the stored page, so an absent key keeps the stored value. -The keys are therefore overwritten with an empty list in a second edit of the -page's jsondata slot. +The keys are therefore changed in a second edit of the page's jsondata slot. +A key of a list or set field is overwritten with an empty list. A key of a +single-valued field is deleted, because an empty list is not a valid value for +it. A key that is not a field of User is deleted as well. """ from __future__ import annotations @@ -10,14 +12,24 @@ import json from typing import Any, Iterable +from opensemantic.base.v1 import User +from pydantic.v1.fields import SHAPE_SINGLETON + from osw.wtsite import WtPage +def _is_multi_valued(key: str) -> bool: + """True if the User field is a list or a set, False for any other key.""" + model_field = User.__fields__.get(key) + return model_field is not None and model_field.shape != SHAPE_SINGLETON + + def clear_jsondata_keys(osw: Any, title: str, keys: Iterable[str]) -> bool: - """Set each present, non-empty key in the page's jsondata slot to ``[]``. + """Clear each present, non-empty key in the page's jsondata slot. - The page is read with a new WtPage, which downloads the current revision and - bypasses the WtSite page cache. Other keys are written back unchanged. + A list or set field becomes ``[]``. Any other key is deleted. The page is + read with a new WtPage, which downloads the current revision and bypasses the + WtSite page cache. Other keys are written back unchanged. Returns True if the page was written, False if nothing changed. """ page = WtPage(wtSite=osw.site, title=title) @@ -29,7 +41,10 @@ def clear_jsondata_keys(osw: Any, title: str, keys: Iterable[str]) -> bool: changed = False for key in keys: if key in content and content[key] not in (None, [], "", {}): - content[key] = [] + if _is_multi_valued(key): + content[key] = [] + else: + del content[key] changed = True if not changed: return False diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py index 2bbab370..4cbbcc58 100644 --- a/src/osw/tools/user_sync/reconcile.py +++ b/src/osw/tools/user_sync/reconcile.py @@ -91,6 +91,18 @@ def _existing_has_iri(entity: Any, name: str) -> bool: return isinstance(iris, dict) and bool(iris.get(name)) +def _has_protected_value( + entity: Any, name: str, stored_jsondata: Optional[Dict[str, Any]] +) -> bool: + """True if the protected field holds a value that pruning would remove. + + Without stored jsondata the loaded entity is the only source. + """ + if stored_jsondata is None: + return _existing_has_iri(entity, name) + return not _is_empty(stored_jsondata.get(name)) + + def existing_fields(entity: Any) -> Dict[str, Any]: """Normalized comparable field view of a loaded User entity.""" return { @@ -151,6 +163,7 @@ def reconcile_user( existing: Optional[Any], enabled_fields: FrozenSet[str] = frozenset(), prune: bool = False, + stored_jsondata: Optional[Dict[str, Any]] = None, ) -> UserChange: if existing is None: return UserChange(proposed=proposed, existing=None, category=NEW) @@ -178,10 +191,11 @@ def reconcile_user( if not getattr(existing, "username", None): diffs.append(FieldDiff("username", None, proposed.username, GAP_FILL)) has_change = True - # Protected fields are stripped from existing items only when pruning. + # Protected fields are stripped from existing items only when pruning. The + # stored jsondata decides, because a loaded entity also shows model defaults. if prune: for name in PROTECTED_FIELDS: - if _existing_has_iri(existing, name): + if _has_protected_value(existing, name, stored_jsondata): diffs.append(FieldDiff(name, "set", None, REMOVE)) has_change = True category = CONFLICT if has_conflict else GAP_FILL if has_change else UNCHANGED @@ -217,7 +231,13 @@ def reconcile( prune: bool = False, ) -> ReconcilePlan: changes = [ - reconcile_user(p, existing.get(p.username), enabled_fields, prune) + reconcile_user( + p, + existing.get(p.username), + enabled_fields, + prune, + existing.get_stored(p.username), + ) for p in proposed_users ] return ReconcilePlan(changes=changes) diff --git a/src/osw/tools/user_sync/sync.py b/src/osw/tools/user_sync/sync.py index bd2e992f..2cfeb0a2 100644 --- a/src/osw/tools/user_sync/sync.py +++ b/src/osw/tools/user_sync/sync.py @@ -191,7 +191,7 @@ def _store_users( def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None: - """Overwrite removed fields of existing items with an empty value.""" + """Clear removed fields of stored existing items (see clear_jsondata_keys).""" updated = set(report.updated) for resolved in resolution.resolved: if resolved.action != "update": diff --git a/tests/tools/test_user_sync_existing.py b/tests/tools/test_user_sync_existing.py index b08e3546..7775da39 100644 --- a/tests/tools/test_user_sync_existing.py +++ b/tests/tools/test_user_sync_existing.py @@ -10,6 +10,18 @@ adopt_existing_pages, load_existing_users, ) +from osw.wtsite import WtPage + + +class FakeWtPage(WtPage): + """A WtPage that serves a fixed jsondata slot without a wiki.""" + + def __init__(self, title, jsondata=None): + self.title = title + self._jsondata = jsondata or {} + + def get_slot_content(self, slot, *args, **kwargs): + return self._jsondata def _ask_result(titles, continue_offset=None): @@ -33,6 +45,11 @@ class FakeSite: def __init__(self, responses): self._responses = list(responses) self.queries = [] + self.stored = {} + + def get_page(self, param): + pages = [FakeWtPage(t, self.stored.get(t, {"uuid": t})) for t in param.titles] + return SimpleNamespace(pages=pages, errors=[]) def api(self, action, **params): assert action == "ask" @@ -54,8 +71,10 @@ def _get_missing_page_titles(self, titles, confirm_delay_s=5): assert confirm_delay_s == 0 return [t for t in titles if t in self.missing] - def load_entity(self, titles): + def load_entity(self, param): + titles = param.titles self.loaded = list(titles) + assert set(param.offline_pages) == set(titles) return [self._entities[t] for t in titles] @@ -157,3 +176,28 @@ def test_adopt_skips_users_already_matched_by_username(): existing = ExistingUsers(by_username={"Alice": SimpleNamespace(username="Alice")}) assert adopt_existing_pages(osw, existing, [_proposed("Alice", "1")]) == [] assert osw.loaded is None + + +def test_load_records_the_stored_jsondata_per_username(): + entities = { + "Item:OSW1": SimpleNamespace(username="Alice", uuid="u1"), + "Item:OSW2": SimpleNamespace(username="Bob", uuid="u2"), + } + osw = FakeOsw(["Item:OSW1", "Item:OSW2"], entities) + osw.site.stored = { + "Item:OSW1": {"uuid": "u1", "username": "Alice", "website": []}, + "Item:OSW2": {"uuid": "u2", "username": "Bob"}, + } + existing = load_existing_users(osw) + assert existing.get_stored("Alice") == osw.site.stored["Item:OSW1"] + assert "employment_contract_status" not in existing.get_stored("Bob") + assert existing.get_stored("missing") is None + + +def test_adopt_records_the_stored_jsondata(): + found = SimpleNamespace(username=None, uuid="1") + osw = FakeOsw([], {"Item:OSW1": found}, pages=[_ask_result([])]) + osw.site.stored = {"Item:OSW1": {"uuid": "1", "label": [{"text": "A"}]}} + existing = ExistingUsers() + adopt_existing_pages(osw, existing, [_proposed("Alice", "1")]) + assert existing.get_stored("Alice") == osw.site.stored["Item:OSW1"] diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py index 73c3e6a0..1117b26a 100644 --- a/tests/tools/test_user_sync_reconcile.py +++ b/tests/tools/test_user_sync_reconcile.py @@ -157,6 +157,40 @@ def test_protected_field_removed_when_pruning(): assert change.category == GAP_FILL +def test_protected_default_not_stored_is_not_removed(): + # The loaded entity shows the model default IRI, the stored page has none. + entity = _entity() + entity.__iris__ = {"employment_contract_status": "Item:OSWdefault"} + change = reconcile_user( + _proposed(), entity, prune=True, stored_jsondata={"username": ORCID} + ) + assert [d for d in change.diffs if d.status == REMOVE] == [] + assert change.category == UNCHANGED + + +def test_protected_field_stored_is_removed_when_pruning(): + entity = _entity() + entity.__iris__ = {"employment_contract_status": "Item:OSWx"} + change = reconcile_user( + _proposed(), + entity, + prune=True, + stored_jsondata={"employment_contract_status": "Item:OSWx"}, + ) + removes = [d.name for d in change.diffs if d.status == REMOVE] + assert removes == ["employment_contract_status"] + + +def test_reconcile_uses_the_stored_jsondata_of_the_index(): + entity = _entity() + entity.__iris__ = {"employment_contract_status": "Item:OSWdefault"} + existing = ExistingUsers( + by_username={ORCID: entity}, stored_jsondata={ORCID: {"username": ORCID}} + ) + plan = reconcile([_proposed()], existing, prune=True) + assert plan.changes[0].category == UNCHANGED + + def test_missing_label_is_gap_fill(): change = reconcile_user(_proposed(label="Lukas Koschmieder"), _entity(label=None)) assert change.category == GAP_FILL diff --git a/tests/tools/test_user_sync_store.py b/tests/tools/test_user_sync_store.py index 16bd2c09..56ea1639 100644 --- a/tests/tools/test_user_sync_store.py +++ b/tests/tools/test_user_sync_store.py @@ -30,12 +30,25 @@ def api(self, action, **params): return {"query": {"allusers": self._users}} +class FakeWtPage(WtPage): + """A WtPage that serves a fixed jsondata slot without a wiki.""" + + def __init__(self, title, jsondata=None): + self.title = title + self._jsondata = jsondata or {} + + def get_slot_content(self, slot, *args, **kwargs): + return self._jsondata + + class FakeStoreOsw: """Fake connection whose store_entity returns the stage StoreEntityResult.""" def __init__(self, users, existing=None, drop_username=False): self.mw_site = FakeMwSite(users) - self.site = SimpleNamespace(semantic_search=self._semantic_search) + self.site = SimpleNamespace( + semantic_search=self._semantic_search, get_page=self._get_page + ) self.existing = existing or {} self.drop_username = drop_username self.store_params = [] @@ -51,7 +64,11 @@ def _semantic_search(self, param): def _get_missing_page_titles(self, titles, confirm_delay_s=5): return [t for t in titles if t not in self.existing] + def _get_page(self, param): + return SimpleNamespace(pages=[FakeWtPage(t) for t in param.titles], errors=[]) + def load_entity(self, titles): + titles = getattr(titles, "titles", titles) if isinstance(titles, str): titles = [titles] loaded = [] @@ -311,22 +328,34 @@ def test_clear_jsondata_keys_empties_present_keys_only(monkeypatch): "organization": ["Item:OSWa"], "employment_contract_status": "Item:OSWb", "short_name": [{"text": "x", "lang": "en"}], + "unknown_key": "x", } page = FakePrunePage(content) _patch_page(monkeypatch, page) written = clear_jsondata_keys( SimpleNamespace(site=None), ITEM_TITLE, - ["website", "organization", "employment_contract_status"], + ["website", "organization", "employment_contract_status", "unknown_key"], ) assert written is True assert page.content["website"] == [] assert page.content["organization"] == [] - assert page.content["employment_contract_status"] == [] + assert "employment_contract_status" not in page.content + assert "unknown_key" not in page.content assert page.content["short_name"] == content["short_name"] assert page.comments == ["user-sync: prune"] +def test_clear_jsondata_keys_leaves_empty_arrays_alone(monkeypatch): + page = FakePrunePage({"website": [], "organization": [], "username": "Alice"}) + _patch_page(monkeypatch, page) + written = clear_jsondata_keys( + SimpleNamespace(site=None), ITEM_TITLE, ["website", "organization"] + ) + assert written is False + assert page.comments == [] + + def test_clear_jsondata_keys_does_not_add_absent_keys_or_write(monkeypatch): page = FakePrunePage({"website": [], "username": "Alice"}) _patch_page(monkeypatch, page) diff --git a/tests/tools/test_user_sync_sync.py b/tests/tools/test_user_sync_sync.py index d9b012ee..fa1701c9 100644 --- a/tests/tools/test_user_sync_sync.py +++ b/tests/tools/test_user_sync_sync.py @@ -8,6 +8,7 @@ from osw.tools.user_sync.mapping import user_uuid from osw.tools.user_sync.sources import MwUser from osw.tools.user_sync.sync import SyncReport, run_user_sync +from osw.wtsite import WtPage ORCID = "0000-0002-6374-9831" @@ -21,6 +22,17 @@ } +class FakeWtPage(WtPage): + """A WtPage that serves a fixed jsondata slot without a wiki.""" + + def __init__(self, title, jsondata=None): + self.title = title + self._jsondata = jsondata or {} + + def get_slot_content(self, slot, *args, **kwargs): + return self._jsondata + + class FakeMwSite: def __init__(self, extra_users=()): self.extra_users = list(extra_users) @@ -41,7 +53,9 @@ def api(self, action, **params): class FakeOsw: def __init__(self, extra_users=(), existing_pages=None): self.mw_site = FakeMwSite(extra_users) - self.site = SimpleNamespace(semantic_search=self._semantic_search) + self.site = SimpleNamespace( + semantic_search=self._semantic_search, get_page=self._get_page + ) self.existing_pages = existing_pages or {} self.loaded = [] self.store_calls = 0 @@ -52,9 +66,13 @@ def _semantic_search(self, query): def _get_missing_page_titles(self, titles, confirm_delay_s=5): return [t for t in titles if t not in self.existing_pages] - def load_entity(self, titles): - self.loaded.extend(titles) - return [self.existing_pages[t] for t in titles] + def _get_page(self, param): + pages = [FakeWtPage(t) for t in param.titles] + return SimpleNamespace(pages=pages, errors=[]) + + def load_entity(self, param): + self.loaded.extend(param.titles) + return [self.existing_pages[t] for t in param.titles] def store_entity(self, param): self.store_calls += 1 From ebda514c731718ab20a084b93288491b3b944627 Mon Sep 17 00:00:00 2001 From: LukasGold <41064839+LukasGold@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:11:24 +0100 Subject: [PATCH 20/20] fix(user-sync): prune a single-valued key stored with an empty value - a stored [] on employment_contract_status is now removed by --prune --- src/osw/tools/user_sync/prune.py | 22 ++++++++++++++++++---- src/osw/tools/user_sync/reconcile.py | 5 ++++- tests/tools/test_user_sync_reconcile.py | 15 +++++++++++++++ tests/tools/test_user_sync_store.py | 16 ++++++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/osw/tools/user_sync/prune.py b/src/osw/tools/user_sync/prune.py index b2e1b417..f0dd85c7 100644 --- a/src/osw/tools/user_sync/prune.py +++ b/src/osw/tools/user_sync/prune.py @@ -17,15 +17,29 @@ from osw.wtsite import WtPage +_EMPTY = (None, [], "", {}) -def _is_multi_valued(key: str) -> bool: + +def is_multi_valued(key: str) -> bool: """True if the User field is a list or a set, False for any other key.""" model_field = User.__fields__.get(key) return model_field is not None and model_field.shape != SHAPE_SINGLETON +def needs_clearing(key: str, value: Any) -> bool: + """True if a present stored value of ``key`` must be cleared by pruning. + + A list or set field needs it only when it is not empty. Any other key needs + it whenever it is present, also with an empty value such as ``[]``, which is + not a valid value for a single-valued field. + """ + if is_multi_valued(key): + return value not in _EMPTY + return True + + def clear_jsondata_keys(osw: Any, title: str, keys: Iterable[str]) -> bool: - """Clear each present, non-empty key in the page's jsondata slot. + """Clear each key in the page's jsondata slot that needs_clearing selects. A list or set field becomes ``[]``. Any other key is deleted. The page is read with a new WtPage, which downloads the current revision and bypasses the @@ -40,8 +54,8 @@ def clear_jsondata_keys(osw: Any, title: str, keys: Iterable[str]) -> bool: return False changed = False for key in keys: - if key in content and content[key] not in (None, [], "", {}): - if _is_multi_valued(key): + if key in content and needs_clearing(key, content[key]): + if is_multi_valued(key): content[key] = [] else: del content[key] diff --git a/src/osw/tools/user_sync/reconcile.py b/src/osw/tools/user_sync/reconcile.py index 4cbbcc58..36f50ebf 100644 --- a/src/osw/tools/user_sync/reconcile.py +++ b/src/osw/tools/user_sync/reconcile.py @@ -17,6 +17,7 @@ from .config import PROTECTED_FIELDS from .existing import ExistingUsers from .mapping import ProposedUser +from .prune import needs_clearing NEW = "new" GAP_FILL = "gap_fill" @@ -100,7 +101,9 @@ def _has_protected_value( """ if stored_jsondata is None: return _existing_has_iri(entity, name) - return not _is_empty(stored_jsondata.get(name)) + if name not in stored_jsondata: + return False + return needs_clearing(name, stored_jsondata[name]) def existing_fields(entity: Any) -> Dict[str, Any]: diff --git a/tests/tools/test_user_sync_reconcile.py b/tests/tools/test_user_sync_reconcile.py index 1117b26a..d49ef8a7 100644 --- a/tests/tools/test_user_sync_reconcile.py +++ b/tests/tools/test_user_sync_reconcile.py @@ -270,3 +270,18 @@ def test_placeholder_user_without_username_still_gets_it(): ) assert change.category == GAP_FILL assert [d.name for d in change.diffs] == ["username"] + + +def test_protected_field_stored_as_empty_list_is_removed_when_pruning(): + # [] is not a valid value for the single-valued field, so it is removed. + entity = _entity() + entity.__iris__ = {"employment_contract_status": "Item:OSWdefault"} + change = reconcile_user( + _proposed(), + entity, + prune=True, + stored_jsondata={"username": ORCID, "employment_contract_status": []}, + ) + assert [(d.name, d.status) for d in change.diffs] == [ + ("employment_contract_status", REMOVE) + ] diff --git a/tests/tools/test_user_sync_store.py b/tests/tools/test_user_sync_store.py index 56ea1639..955b7a6c 100644 --- a/tests/tools/test_user_sync_store.py +++ b/tests/tools/test_user_sync_store.py @@ -365,3 +365,19 @@ def test_clear_jsondata_keys_does_not_add_absent_keys_or_write(monkeypatch): assert written is False assert page.content == {"website": [], "username": "Alice"} assert page.comments == [] + + +def test_clear_jsondata_keys_deletes_an_empty_single_valued_key(monkeypatch): + page = FakePrunePage({ + "website": [], + "employment_contract_status": [], + "username": "Alice", + }) + _patch_page(monkeypatch, page) + written = clear_jsondata_keys( + SimpleNamespace(site=None), + ITEM_TITLE, + ["website", "employment_contract_status"], + ) + assert written is True + assert page.content == {"website": [], "username": "Alice"}