Skip to content

fix: harden CLARIN shared surface + dtq owningCollection reauth - #24

Open
jr-rk wants to merge 1 commit into
test/clarin-usage-coveragefrom
fix/clarin-shared-surface-hardening
Open

fix: harden CLARIN shared surface + dtq owningCollection reauth#24
jr-rk wants to merge 1 commit into
test/clarin-usage-coveragefrom
fix/clarin-shared-surface-hardening

Conversation

@jr-rk

@jr-rk jr-rk commented Aug 20, 2026

Copy link
Copy Markdown

Purpose

The dspace_rest_client/ production fixes that were previously bundled with the
test/CI PR, split out per review so the test PR stays scoped. Stacked on
test/clarin-usage-coverage (base of this PR) — it supplies the _helpers
builders, pytest.ini markers and the differential-contract job these tests
run under. Merge that PR first.

Targets dtq. The failure-mode assertions are dtq_only and deselected on the
main leg; main carries the same bugs, but propagating these fixes to main is
deliberately out of scope here.

Fixes (each characterization-tested: test written first, then the method)

Method Was Now
get_bundle_by_name / get_items_from_collection / get_resource_policy NoneType subscript crash on non-200 clean None / []
get_resource_policy IndexError on empty policy list; no error/empty distinction None, records last_err
get_user_by_email truthy uuid-less User on 404 (slips past if user:) None, records last_err, no noisy 404 log
create_submit_group Group(None) crash on a 201 with empty body None
add_member posts a bare /epersons/{uuid} href → DSpace 422/404 canonical /eperson/epersons/{uuid}
Group(None) / User(None) TypeError empty object
get_owningCollection (dtq-only) empty truthy Collection on non-200 → ISstag _audit.py 401-reauth was dead code None + last_err

Verified locally

Leg A (full suite, dtq impl with these fixes): 137 pass.

Characterization-tested fixes (test written to the intended behaviour first,
then the method corrected) to methods the main-lineage consumers call:

  get_bundle_by_name / get_items_from_collection / get_resource_policy
      non-200 or empty body -> clean None/[] instead of a NoneType subscript
      crash; get_resource_policy also records last_err so a caller can tell an
      HTTP error from a genuine empty result.
  get_user_by_email        404/any non-200 -> None (+ last_err), not a truthy
      uuid-less User that slips past a consumer `if user:` guard.
  create_submit_group      201 with an empty body -> None, not Group(None).
  add_member               post the canonical /eperson/epersons/{uuid} href;
      the bare /epersons/ path 404s and DSpace rejects the uri-list.
  Group(None) / User(None) empty object, not a TypeError.
  get_owningCollection     (dtq-only) non-200 -> None + last_err, so ISstag's
      src/repo/_audit.py `owning_col is None and last_err.status_code == 401`
      reauth retry stops being dead code.

Stacked on the test/CI PR, which supplies the _helpers builders, pytest.ini
markers and the differential-contract job these tests run under. Merge that
PR first. The failure-mode assertions are marked dtq_only (deselected on the
main leg): main carries the same bugs, but propagating these fixes to main is
deliberately out of scope here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant