From 217b1017b46295340bc79cee050a91c45c0d379d Mon Sep 17 00:00:00 2001 From: Andreas Raeder Date: Tue, 15 Sep 2026 16:36:56 +0200 Subject: [PATCH 01/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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():