Skip to content

Commit 0b8dc97

Browse files
committed
fix(user-sync): count and redirect only stored items (#224)
- count a title as created or updated only after store_entity stored it - record each failed title of StoreEntityPartialError as store:<title> - write redirects only for stored or pre-existing items - drop new links to organizations that failed to store - prune only updates that were stored
1 parent e97daa4 commit 0b8dc97

2 files changed

Lines changed: 341 additions & 19 deletions

File tree

‎src/osw/tools/user_sync/sync.py‎

Lines changed: 70 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
from __future__ import annotations
1010

11-
from dataclasses import dataclass, field
12-
from typing import Any, Dict, List, Optional
11+
from dataclasses import dataclass, field, replace
12+
from typing import Any, Dict, FrozenSet, List, Optional, Tuple
1313

1414
from osw.core import OSW
1515

@@ -19,7 +19,7 @@
1919
from .interactive import Prompter, Resolution, resolve_plan
2020
from .mapping import ProposedOrganization, map_user
2121
from .prune import clear_jsondata_keys
22-
from .reconcile import REMOVE, reconcile
22+
from .reconcile import REMOVE, existing_fields, reconcile
2323
from .redirects import ensure_redirect
2424
from .sources import (
2525
RESERVED_USERNAMES,
@@ -100,6 +100,13 @@ def _build_proposals(config: SyncConfig, osw: Any, session: Any):
100100
return proposed_users, org_map, skipped_blocked
101101

102102

103+
def _record_partial(report: SyncReport, exc: Exception) -> List[str]:
104+
"""Record each failed title of a partial store; return the stored titles."""
105+
for title, error in exc.failed.items():
106+
report.failed[f"store:{title}"] = str(error)
107+
return list(exc.stored)
108+
109+
103110
def _store_organizations(osw: Any, org_map, report: SyncReport) -> None:
104111
if not org_map:
105112
return
@@ -109,40 +116,70 @@ def _store_organizations(osw: Any, org_map, report: SyncReport) -> None:
109116
OSW.StoreEntityParam(entities=entities, edit_comment="user-sync: orgs")
110117
)
111118
report.organizations = [org.full_page_title for org in org_map.values()]
119+
except OSW.StoreEntityPartialError as exc:
120+
report.organizations = _record_partial(report, exc)
112121
except Exception as exc: # pragma: no cover - network failure path
113122
report.failed["organizations"] = str(exc)
114123

115124

116-
def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None:
125+
def _store_users(
126+
osw: Any,
127+
resolution: Resolution,
128+
report: SyncReport,
129+
failed_orgs: FrozenSet[str] = frozenset(),
130+
) -> None:
131+
"""Store created and updated items; count a title only once it is stored.
132+
133+
New links to an organization in ``failed_orgs`` are dropped, so no item
134+
points to an organization that was not stored. A link the existing item
135+
already has is kept. A re-run proposes the dropped links again.
136+
"""
117137
entities = []
138+
pending: List[Tuple[str, str]] = [] # (action, title) per entity
118139
for resolved in resolution.resolved:
119140
proposed = resolved.change.proposed
141+
drop = set(proposed.organizations) & failed_orgs
142+
if drop and resolved.change.existing is not None:
143+
drop -= existing_fields(resolved.change.existing)["organizations"]
144+
if drop:
145+
proposed = replace(
146+
proposed,
147+
organizations=[o for o in proposed.organizations if o not in drop],
148+
)
120149
try:
121150
if resolved.action == "create":
122151
entities.append(build_user(proposed))
123-
report.created.append(proposed.full_page_title)
124152
elif resolved.action == "update":
125153
entities.append(
126154
apply_update(
127155
resolved.change.existing, proposed, resolved.apply_fields
128156
)
129157
)
130-
report.updated.append(proposed.full_page_title)
131158
else:
132159
report.skipped.append(proposed.full_page_title)
160+
continue
161+
pending.append((resolved.action, proposed.full_page_title))
133162
except Exception as exc: # pragma: no cover - build failure path
134163
report.failed[f"build:{proposed.username}"] = str(exc)
135-
if entities:
136-
try:
137-
osw.store_entity(
138-
OSW.StoreEntityParam(
139-
entities=entities,
140-
overwrite=True,
141-
edit_comment="user-sync",
142-
)
164+
if not entities:
165+
return
166+
try:
167+
osw.store_entity(
168+
OSW.StoreEntityParam(
169+
entities=entities,
170+
overwrite=True,
171+
edit_comment="user-sync",
143172
)
144-
except Exception as exc: # pragma: no cover - network failure path
145-
report.failed["store"] = str(exc)
173+
)
174+
stored = {title for _, title in pending}
175+
except OSW.StoreEntityPartialError as exc:
176+
stored = set(_record_partial(report, exc))
177+
except Exception as exc: # pragma: no cover - network failure path
178+
report.failed["store"] = str(exc)
179+
stored = set()
180+
for action, title in pending:
181+
if title in stored:
182+
(report.created if action == "create" else report.updated).append(title)
146183

147184

148185
# Names of reconciled fields mapped to their keys in the item's jsondata.
@@ -155,9 +192,12 @@ def _store_users(osw: Any, resolution: Resolution, report: SyncReport) -> None:
155192

156193
def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None:
157194
"""Overwrite removed fields of existing items with an empty value."""
195+
updated = set(report.updated)
158196
for resolved in resolution.resolved:
159197
if resolved.action != "update":
160198
continue
199+
if resolved.change.proposed.full_page_title not in updated:
200+
continue # the update was not stored
161201
keys = [
162202
_JSONDATA_KEYS[d.name]
163203
for d in resolved.change.diffs
@@ -176,10 +216,14 @@ def _prune_users(osw: Any, resolution: Resolution, report: SyncReport) -> None:
176216

177217

178218
def _create_redirects(osw: Any, resolution: Resolution, report: SyncReport) -> None:
179-
# Every in-scope user has an item (created or pre-existing), so ensure the
180-
# redirect for all of them; a re-run repairs any missing redirect.
219+
# A redirect needs an existing target: an item stored in this run, or one
220+
# that existed before it (update or skip). A failed create gets none; a
221+
# re-run writes it once the item is stored.
222+
created = set(report.created)
181223
for resolved in resolution.resolved:
182224
proposed = resolved.change.proposed
225+
if resolved.action == "create" and proposed.full_page_title not in created:
226+
continue
183227
try:
184228
written = ensure_redirect(osw, proposed.username, proposed.full_page_title)
185229
if written:
@@ -261,9 +305,16 @@ def run_user_sync(
261305
_warn_missing_email(prompter, missing_email)
262306
return report
263307

308+
failed_orgs: FrozenSet[str] = frozenset()
264309
if config.link_organizations:
265310
_store_organizations(osw, org_map, report)
266-
_store_users(osw, resolution, report)
311+
stored_orgs = set(report.organizations)
312+
failed_orgs = frozenset(
313+
o.full_page_title
314+
for o in org_map.values()
315+
if o.full_page_title not in stored_orgs
316+
)
317+
_store_users(osw, resolution, report, failed_orgs=failed_orgs)
267318
if config.prune and "store" not in report.failed:
268319
_prune_users(osw, resolution, report)
269320
if config.create_redirects:

0 commit comments

Comments
 (0)