test: CLARIN consumer-contract coverage + main/dtq merge gate (fixes 8 defects)

[jr-rk]

What

The CLARIN/UFAL surface was merged onto dtq (main → dtq, 2026-08-17) with
zero tests, while three main-lineage consumers depend on it
(dspace-rest-test, dspace-import-clarin, dspace-item-importer). This adds
the CLARIN counterpart to test_repo_usage_contract.py — replaying those
consumers' real call chains with only the HTTP transport mocked — the fixes
those tests pin, and CI that makes a future main+dtq merge validatable.

68 → 137 tests. No network in any test.

Fixes

Each crashed or mis-answered a real consumer call. All characterization-tested
(test written to the intended behaviour first, then the method fixed).

Method	Was	Now
get_bundle_by_name / get_resource_policy / get_items_from_collection	NoneType subscript crash on any non-200	clean None / []
get_resource_policy	IndexError on an empty policy list	None
get_user_by_email	truthy, uuid-less User on 404 — slips past the consumer's if user: guard	None
create_submit_group	Group(None) crash on a 201 with an empty body	None
Group(None) / User(None)	TypeError	empty object
get_owningCollection	empty truthy Collection on non-200, so ISstag's src/repo/_audit.py owning_col is None and last_err.status_code == 401 reauth retry was dead code	None, last_err recorded

New tests

File	Covers
test_clarin_read.py (21)	get_resource_policy (raw-dict contract), get_bundle_by_name, get_items_from_collection, get_item_by_handle, get_user_by_email, get_clarinlruallowances*, get_owningCollection
test_clarin_write.py (17)	create_resource_policy (params/body/truthiness/401-reauth), update_resource_policy_group, create_submit_group, add_member, remove_metadata
test_models_clarin.py (12)	License, Label, Group, User construction + to_dict/as_dict
test_clarin_usage_contract.py (9)	the three consumers' multi-call chains, end-to-end
test_transport_hardening.py (7)	timeout/proxies on every verb, verify_response, last_err reset

Plus _helpers.py CLARIN builders, the no-arg get_items() form, a
get_owningCollection reauth contract in test_repo_usage_contract.py, and
pytest.ini markers.

CI — the merge gate

tests.yml now builds on dtq / main / feat/** / fix/**. A new
differential-contract job runs the CLARIN suite against both the dtq
implementation and the main implementation (swapped in from origin/main): a
test green on both proves the merge preserves that behaviour. Tests that encode
a dtq fix or behaviour change are marked @pytest.mark.dtq_only and skipped
on the main leg. Adds a 70% coverage floor and secret-gated consumer-smoke
jobs (skipped until CONSUMER_READ_TOKEN exists).

Verified locally: leg A (dtq) 137 pass; leg B (main impl) 45 shared
contracts pass, 14 dtq_only deselected — and those 14 fail on the main
implementation, confirming the deltas are real.

---

Review resolutions

Follow-up commits after review:

• 6faffa8 — add_member posted a malformed eperson uri-list body
  (/epersons/{uuid}, missing /eperson); DSpace rejects it with 422. Fixed to
  the canonical /eperson/epersons/{uuid}; the test that had pinned the wrong
  value is corrected and marked dtq_only.
• dc2e654 (Copilot 👍, both threads resolved) — get_resource_policy and
  get_user_by_email now record self._last_err on the non-200 path, so a
  caller can distinguish an HTTP error from a genuine empty/not-found result;
  get_user_by_email also no longer logs a plain 404 at error level or emits
  the raw email. Tests assert last_err carries the status code.

Verified after each: leg A (dtq) 137 pass; leg B (main impl) 44 pass / 15
dtq_only deselected.

[Copilot]

Pull request overview

Adds comprehensive CLARIN/UFAL consumer-contract coverage and CI gating to safely validate future main↔dtq merges. It expands the test suite to characterize real consumer call chains (transport mocked only), pins multiple dtq fixes, and introduces a “differential contract” workflow that runs the CLARIN suite against both the dtq and origin/main implementations.

Changes:

• Add new CLARIN read/write/model/usage-contract test suites plus shared helpers and pytest markers.
• Harden several client methods (null/empty/failed-response handling) and make Group(None) / User(None) safe.
• Update CI to enforce a coverage floor and add a dtq-vs-main differential contract job (plus optional consumer smoke jobs).

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_transport_hardening.py	New dtq-only tests for timeout/proxy plumbing, verify_response, and last_err reset behavior
tests/test_repo_usage_contract.py	Adds a contract test for get_owningCollection + 401-driven reauth predicate
tests/test_models_clarin.py	New CLARIN model tests for License, Label, Group, User (including None construction)
tests/test_client_read.py	Adds dtq-only coverage for the no-arg get_items() pagination defaults
tests/test_clarin_write.py	New CLARIN write-surface characterization tests (policies, submit groups, membership, metadata removal)
tests/test_clarin_usage_contract.py	New end-to-end consumer call-chain contract tests (CLARIN side)
tests/test_clarin_read.py	New CLARIN read-surface characterization tests (policies, bundles, allowances, handle, email, owningCollection)
tests/_helpers.py	Adds CLARIN-oriented JSON builders and IDs used across the new suites
requirements-test.txt	Adds pytest-cov for CI-enforced coverage floor
pytest.ini	Registers clarin, dtq_only, dq markers for suite slicing
dspace_rest_client/models.py	Makes Group(None) / User(None) safe by normalizing api_resource
dspace_rest_client/client.py	Hardens CLARIN-related client methods (avoid crashes, return clean None/[]) and improves get_owningCollection behavior
.gitignore	Ignores coverage artifacts
.github/workflows/tests.yml	Adds coverage enforcement, differential-contract job, and secret-gated consumer smoke jobs

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (6)

dspace_rest_client/client.py:453

• api_patch() assumes the 403 response body is JSON; if it isn't, parse_json() returns None and if 'message' in r_json raises TypeError, so CSRF retry handling can crash.

            _logger.debug(r.text)
            r_json = parse_json(r)
            if 'message' in r_json and 'CSRF token' in r_json['message']:
                if retry:
                    _logger.warning(f'Too many retries updating token: {r.status_code}: {r.text}')
                else:
                    _logger.debug("Retrying request with updated CSRF token")
                    return self.api_patch(url, operation, path, value, params, True)

dspace_rest_client/client.py:1085

• get_items() can crash with TypeError: argument of type 'NoneType' is not iterable when the response body is empty/non-JSON (because parse_json() returns None and the code does if '_embedded' in r_json). Guard for a falsy r_json before membership tests, as done in get_items_from_collection().

        r = self.api_get(url, params=params)
        r_json = parse_json(response=r)
        if '_embedded' in r_json:
            if 'items' in r_json['_embedded']:

dspace_rest_client/client.py:880

• create_bitstream() assumes parse_json() always returns a dict on 401/403. If the body is empty or non-JSON, parse_json() returns None and if 'message' in r_json raises TypeError, preventing the intended retry/authenticate path.

This issue also appears on line 1082 of the same file.

        if not retry and r.status_code in (401, 403):
            r_json = parse_json(r)
            if 'message' in r_json and 'CSRF token' in r_json['message']:
                _logger.debug("Retrying request with updated CSRF token")
            else:
                self.authenticate()
            return self.create_bitstream(bundle, name, path, mime, metadata, True)

dspace_rest_client/client.py:365

• api_put_uri() assumes the 403 response body is JSON; if it isn't, parse_json() returns None and if 'message' in r_json raises TypeError, so CSRF retry handling can crash instead of retrying/returning the response.

This issue also appears on line 446 of the same file.

            r_json = parse_json(r)
            if 'message' in r_json and 'CSRF token' in r_json['message']:
                if retry:
                    _logger.warning(f'Too many retries updating token: {r.status_code}: {r.text}')
                else:
                    _logger.debug("Retrying request with updated CSRF token")
                    return self.api_put_uri(url, params=params, uri_list=uri_list, retry=True)

dspace_rest_client/client.py:1041

• get_item() constructs and returns an Item even when the HTTP request fails or the body isn't valid JSON. That can leak a uuid-less/truthy object (or an object missing links) and also drops the failing response details. Prefer returning None on non-200 / unparseable bodies and recording _last_err like other hardened methods.

            r = self.api_get(url, None, None)
            r_json = parse_json(response=r)
            return Item(r_json)

dspace_rest_client/client.py:100

• __init__(..., proxies=PROXY_DICT, ...) uses a mutable dict as a default argument, so if any instance mutates self.proxies in-place it can unintentionally affect future instances. Prefer proxies=None and copy the class default when initializing self.proxies.

    def __init__(self, api_endpoint=API_ENDPOINT, username=USERNAME, password=PASSWORD, solr_endpoint=SOLR_ENDPOINT,
                 solr_auth=SOLR_AUTH, fake_user_agent=False, proxies=PROXY_DICT, timeout=None):

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (6)

dspace_rest_client/client.py:878

• create_bitstream() assumes parse_json(r) returns a dict; if the 401/403 body is non-JSON, parse_json() returns None and the 'message' in r_json check raises TypeError, preventing the intended retry/authenticate path.

        if not retry and r.status_code in (401, 403):
            r_json = parse_json(r)
            if 'message' in r_json and 'CSRF token' in r_json['message']:
                _logger.debug("Retrying request with updated CSRF token")
            else:

dspace_rest_client/client.py:438

• In api_patch(), parse_json(r) can return None for non-JSON 403 responses; the subsequent if 'message' in r_json ... will then raise TypeError and prevent the CSRF retry path from running.

        r = self.session.patch(url, json=[data], params=params, headers=self.request_headers,
                               proxies=self.proxies, timeout=self.timeout)

dspace_rest_client/client.py:380

• In api_delete(), parse_json(r) can return None (non-JSON 403 bodies happen), and the later if 'message' in r_json ... will raise TypeError, skipping the CSRF retry. Since this method was just updated to always pass proxies/timeout, it's a good spot to also make the CSRF branch resilient to non-JSON responses.

        r = self.session.delete(url, params=params, headers=self.request_headers,
                                proxies=self.proxies, timeout=self.timeout)

dspace_rest_client/client.py:1084

• get_items() will crash with TypeError: argument of type 'NoneType' is not iterable when the response body is non-JSON (e.g., a 5xx HTML/plain-text error), because parse_json() returns None and the code does if '_embedded' in r_json:. It also drops the failing response instead of recording last_err.

        r = self.api_get(url, params=params)
        r_json = parse_json(response=r)
        if '_embedded' in r_json:

dspace_rest_client/client.py:1041

• get_item() constructs and returns an Item even on non-200 responses (and even when JSON parsing fails), which can yield a truthy, uuid-less Item and hides HTTP errors from callers. Given this PR introduces last_err, returning None on non-200 and recording the failing response is safer and consistent with other methods.

            r = self.api_get(url, None, None)
            r_json = parse_json(response=r)
            return Item(r_json)

dspace_rest_client/client.py:360

• api_put_uri() can raise TypeError on a 403 with a non-JSON body: parse_json() returns None and the code does 'message' in r_json. Guarding for a None parse keeps the CSRF-retry logic robust.

This issue also appears in the following locations of the same file:

• line 379
• line 437
• line 874

            if 'message' in r_json and 'CSRF token' in r_json['message']: