From 0c7411b9602d1469ab6a9785076f362bba9e451b Mon Sep 17 00:00:00 2001 From: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:33:28 +0200 Subject: [PATCH] test: CLARIN consumer-contract suite + main-tailored differential CI Add characterization tests that replay how the three main-lineage consumers (dspace-import-clarin, dspace-rest-test, dspace-item-importer) actually call this library, plus a branch-differential CI job that runs the shared CLARIN contract against BOTH the dtq and main implementations. Only the HTTP transport is mocked; the real client builds URLs and parses responses. This PR is test + CI infrastructure only - no dspace_rest_client/ changes. The shared-surface fixes and dtq behaviour deltas that the tests would otherwise pin live in a separate, stacked PR, so this one stays scoped to validating main's usage surface and proposes nothing for main. Markers (pytest.ini): clarin - shared contract, must hold on both main and dtq dtq_only - existing dtq-only surface/behaviour, deselected on the main leg Co-Authored-By: Claude Opus 4.8 --- .github/workflows/tests.yml | 128 ++++++++++++++++- .gitignore | 2 + pytest.ini | 9 ++ requirements-test.txt | 2 + tests/_helpers.py | 66 +++++++++ tests/test_clarin_read.py | 202 ++++++++++++++++++++++++++ tests/test_clarin_usage_contract.py | 192 +++++++++++++++++++++++++ tests/test_clarin_write.py | 214 ++++++++++++++++++++++++++++ tests/test_client_read.py | 17 +++ tests/test_models_clarin.py | 94 ++++++++++++ tests/test_transport_hardening.py | 126 ++++++++++++++++ 11 files changed, 1050 insertions(+), 2 deletions(-) create mode 100644 pytest.ini create mode 100644 tests/test_clarin_read.py create mode 100644 tests/test_clarin_usage_contract.py create mode 100644 tests/test_clarin_write.py create mode 100644 tests/test_models_clarin.py create mode 100644 tests/test_transport_hardening.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index affd373..87d8625 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,8 +2,11 @@ name: Tests on: push: - branches: [ dtq ] + # dtq is the mainline; main is the CLARIN branch we are validating a merge + # into. feat/** and fix/** get CI before they open a PR. + branches: [ dtq, main, 'feat/**', 'fix/**' ] pull_request: + workflow_dispatch: # manual validation of a merge candidate permissions: contents: read @@ -37,4 +40,125 @@ jobs: pip install -r requirements-test.txt - name: Run tests - run: python -m pytest tests/ -v + # Coverage floor guards against the CLARIN surface silently sliding back + # toward the zero it had before test/clarin-usage-coverage landed. + run: > + python -m pytest tests/ -v + --cov=dspace_rest_client --cov-report=term-missing + --cov-fail-under=70 + + differential-contract: + # THE MERGE GATE. Run the CLARIN consumer-contract suite against BOTH the + # dtq implementation (this checkout) and the main implementation (swapped in + # from origin/main). A test green on both proves the merge preserves that + # behaviour. Tests that deliberately encode a dtq fix or behaviour change + # are marked @pytest.mark.dtq_only and are skipped on the main leg. + # + # Leg selection targets the four CLARIN test files explicitly: the DQ test + # modules import dtq-only symbols (ResourcePolicy, ...) at module scope, so + # collecting them against the main implementation would be an import error. + name: CLARIN contract vs ${{ matrix.impl }} impl + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + impl: [dtq, main] + + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 # need origin/main to swap the implementation in + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.10" + cache: pip + cache-dependency-path: requirements-test.txt + + - name: Install test deps + run: | + python -m pip install --upgrade pip + pip install -r requirements-test.txt + + - name: Swap in the ${{ matrix.impl }} implementation + if: matrix.impl != 'dtq' + run: git checkout "origin/${{ matrix.impl }}" -- dspace_rest_client/ + + - name: Run CLARIN contract suite + run: | + FILES="tests/test_clarin_read.py tests/test_clarin_write.py \ + tests/test_models_clarin.py tests/test_clarin_usage_contract.py" + if [ "${{ matrix.impl }}" = "dtq" ]; then + python -m pytest $FILES -v # full CLARIN surface, incl. dtq_only + else + python -m pytest $FILES -v -m "not dtq_only" # shared contract only + fi + + # ---- Consumer smoke jobs ------------------------------------------------- + # Turn "the API surface is a superset" into "the consumers still import/run". + # Gated on CONSUMER_READ_TOKEN: until that read-scoped token for the private + # consumer repos exists, the gate job reports enabled=false and consumer-smoke + # is skipped (a clean green), per plan §6.3. + check-consumer-token: + runs-on: ubuntu-latest + outputs: + enabled: ${{ steps.probe.outputs.enabled }} + steps: + - id: probe + env: + TOKEN: ${{ secrets.CONSUMER_READ_TOKEN }} + run: echo "enabled=${{ env.TOKEN != '' }}" >> "$GITHUB_OUTPUT" + + consumer-smoke: + needs: check-consumer-token + if: needs.check-consumer-token.outputs.enabled == 'true' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - repo: DSpace-ISstag-integration + ref: main + smoke: python -m pytest tests/ mcp/tests/ -q + - repo: dspace-rest-test + ref: master + smoke: python -c "import dspace_rest_client.client" + - repo: dspace-import-clarin + ref: main + smoke: python -c "import dspace_rest_client.client" + # dspace-item-importer is intentionally omitted until its .gitmodules + # is repointed off the deleted `dtq-dev` branch (plan §6.3 / brief §5). + steps: + - uses: actions/checkout@v6 + with: + path: candidate + + - uses: actions/checkout@v6 + with: + repository: dataquest-dev/${{ matrix.repo }} + ref: ${{ matrix.ref }} + token: ${{ secrets.CONSUMER_READ_TOKEN }} + submodules: recursive + path: consumer + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.10" + + - name: Point the consumer submodule at this candidate commit + run: | + rm -rf consumer/libs/dspace-rest-python + cp -r candidate consumer/libs/dspace-rest-python + + - name: Install and smoke + working-directory: consumer + run: | + python -m pip install --upgrade pip + pip install ./libs/dspace-rest-python + if [ -f requirements.lock ]; then pip install -r requirements.lock; fi + if [ -f libs/dspace-rest-python/requirements-test.txt ]; then + pip install -r libs/dspace-rest-python/requirements-test.txt + fi + ${{ matrix.smoke }} diff --git a/.gitignore b/.gitignore index deba30b..eb1f1d5 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,5 @@ __pypackages__/ env/ venv/ .idea/ +.coverage +coverage.xml diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..1f08f44 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,9 @@ +[pytest] +# Markers used to slice the suite for the branch-differential CI run +# (see .github/workflows/tests.yml :: differential-contract). A pytest.ini +# takes precedence over any [tool.pytest.ini_options] a future pyproject.toml +# might add, so the marker registry stays in one place regardless of packaging. +markers = + dtq_only: behaviour introduced on dtq; not expected to hold on the main implementation + clarin: exercises the CLARIN/UFAL surface (main-lineage consumers) + dq: exercises the DQ integration surface (DSpace-ISstag-integration) diff --git a/requirements-test.txt b/requirements-test.txt index 575ad6c..f8ec9b4 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -5,3 +5,5 @@ pytest>=7.0 requests-mock>=1.11 requests +# coverage floor is enforced in CI (tests.yml :: test job, --cov-fail-under) +pytest-cov>=4.0 diff --git a/tests/_helpers.py b/tests/_helpers.py index d4183cc..ab8a11d 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -32,6 +32,10 @@ COLLECTION_UUID = "22222222-2222-2222-2222-222222222222" BITSTREAM_UUID = "9f54ef33-c454-4d8e-a5fe-79d8291045ba" ANON_GROUP_UUID = "6ecfd145-3b7d-429e-ab31-ef6905a05763" +# Used by the CLARIN-side suites (eperson/group lookups, submit groups). +EPERSON_UUID = "33333333-3333-3333-3333-333333333333" +GROUP_UUID = "44444444-4444-4444-4444-444444444444" +BUNDLE_UUID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" def make_client(api_endpoint: str = API) -> DSpaceClient: @@ -112,3 +116,65 @@ def policy_json(pid: int = 1, action: str = "READ", group_name: str = "Anonymous if start_date is not None: d["startDate"] = start_date return d + + +def raw_policy_json(pid: int = 1, action: str = "READ", **extra) -> dict: + """A resource policy in the *raw* shape the CLARIN ``get_resource_policy`` + returns (a plain dict the caller subscripts as ``["id"]``), not a model.""" + d = {"id": pid, "action": action, "type": "resourcepolicy"} + d.update(extra) + return d + + +def group_json(uuid: str = GROUP_UUID, name: str = "Anonymous", + permanent: bool = False, **extra) -> dict: + d = {"uuid": uuid, "name": name, "type": "group", "permanent": permanent} + d.update(extra) + return d + + +def user_json(uuid: str = EPERSON_UUID, email: str = "tester@dspace.test", + name: str = "Tester", netid: str = None, can_login: bool = True, + **extra) -> dict: + d = {"uuid": uuid, "type": "eperson", "name": name, "email": email, + "canLogIn": can_login} + if netid is not None: + d["netid"] = netid + d.update(extra) + return d + + +def label_json(lid: int = 10, label: str = "PUB", title: str = "Publicly available", + icon: str = "pub.png", extended: bool = False) -> dict: + return {"id": lid, "label": label, "title": title, "icon": icon, + "extended": extended} + + +def license_json(lid: int = 1, name: str = "CC-BY", + definition: str = "https://creativecommons.org/licenses/by/4.0/", + confirmation: int = 1, required_info: str = "SEND_TOKEN", + label: dict = None, extended: list = None) -> dict: + d = {"id": lid, "name": name, "definition": definition, + "confirmation": confirmation, "requiredInfo": required_info} + if label is not None: + d["clarinLicenseLabel"] = label + if extended is not None: + d["extendedClarinLicenseLabels"] = extended + return d + + +def clarin_allowance_json(aid: int = 1, **extra) -> dict: + d = {"id": aid, "type": "clarinlruallowance"} + d.update(extra) + return d + + +def search_envelope(items: list) -> dict: + """The ``discover/search/objects`` HAL envelope, wrapping each item as an + ``indexableObject``. Used by ``get_items_from_collection`` and + ``search_objects``. + """ + return {"_embedded": {"searchResult": { + "page": {"totalElements": len(items)}, + "_embedded": {"objects": [ + {"_embedded": {"indexableObject": it}} for it in items]}}}} diff --git a/tests/test_clarin_read.py b/tests/test_clarin_read.py new file mode 100644 index 0000000..78663c4 --- /dev/null +++ b/tests/test_clarin_read.py @@ -0,0 +1,202 @@ +""" +CLARIN/UFAL read surface - the methods the three main-lineage consumers +(dspace-rest-test, dspace-import-clarin, dspace-item-importer) call but which +had zero coverage after `main` was merged into `dtq`. + +Same discipline as the DQ suite: mock only the HTTP transport, let the real +client build URLs / parse responses. Each test names the consumer it mirrors. + +Marks: + clarin - shared surface, must hold on both `main` and `dtq` + dtq_only - asserts a fix or a method that exists only on `dtq`; deselected + when the differential-contract CI job runs against `main`. +""" +import unittest + +import pytest +import requests_mock + +import _helpers # noqa: F401 +from _helpers import ( + make_client, sent_params, embedded, item_json, bundle_json, raw_policy_json, + user_json, clarin_allowance_json, search_envelope, + API, ITEM_UUID, COLLECTION_UUID, BUNDLE_UUID, EPERSON_UUID) +from dspace_rest_client.models import Bundle, Item, Collection, User + +pytestmark = pytest.mark.clarin + + +class TestGetResourcePolicyDict(unittest.TestCase): + """Mirrors dspace-import-clarin - get_resource_policy(uuid)["id"]. + The dict-subscript contract that blocks the resource-policy API unification; + it must keep returning a raw dict, not a model.""" + + URL = f"{API}/authz/resourcepolicies/search/resource" + + def test_returns_first_raw_dict_with_id_subscript(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("resourcepolicies", + [raw_policy_json(pid=7), + raw_policy_json(pid=8)])) + rp = c.get_resource_policy(BUNDLE_UUID) + self.assertIsInstance(rp, dict) + self.assertEqual(rp["id"], 7) # dict subscript, first policy + + def test_sends_uuid_and_both_embeds(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("resourcepolicies", [raw_policy_json()])) + c.get_resource_policy(BUNDLE_UUID) + qs = sent_params(m.last_request) + self.assertEqual(qs["uuid"], [BUNDLE_UUID]) + self.assertEqual(sorted(qs["embed"]), ["eperson", "group"]) + + +class TestGetBundleByName(unittest.TestCase): + """Mirrors dspace-import-clarin - get_bundle_by_name('ORIGINAL', item).""" + + URL = f"{API}/core/items/{ITEM_UUID}/bundles" + + def test_matches_named_bundle(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("bundles", [ + bundle_json("b1", "LICENSE"), + bundle_json("b2", "ORIGINAL")])) + b = c.get_bundle_by_name("ORIGINAL", ITEM_UUID) + self.assertIsInstance(b, Bundle) + self.assertEqual((b.uuid, b.name), ("b2", "ORIGINAL")) + + def test_no_match_returns_none(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("bundles", [bundle_json("b1", "LICENSE")])) + self.assertIsNone(c.get_bundle_by_name("ORIGINAL", ITEM_UUID)) + + +class TestGetItemsFromCollection(unittest.TestCase): + """Mirrors dspace-import-clarin - get_items_from_collection(collection).""" + + URL = f"{API}/discover/search/objects" + + def test_parses_search_envelope(self): + c = make_client() + other = "22222222-2222-2222-2222-222222222222" + with requests_mock.Mocker() as m: + m.get(self.URL, json=search_envelope([ + item_json(ITEM_UUID, "A"), item_json(other, "B")])) + items = c.get_items_from_collection(COLLECTION_UUID) + self.assertEqual(len(items), 2) + self.assertTrue(all(isinstance(i, Item) for i in items)) + self.assertEqual([i.uuid for i in items], [ITEM_UUID, other]) + + def test_sends_scope_dsotype_sort_embed(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=search_envelope([])) + c.get_items_from_collection(COLLECTION_UUID) + qs = sent_params(m.last_request) + self.assertEqual(qs["scope"], [COLLECTION_UUID]) + self.assertEqual(qs["dsoType"], ["ITEM"]) + self.assertEqual(qs["sort"], ["dc.date.accessioned,DESC"]) + self.assertEqual(qs["embed"], ["thumbnail"]) + + +class TestGetItemByHandle(unittest.TestCase): + """Mirrors dspace-rest-test - get_item_by_handle(handle).""" + + URL = f"{API}/core/items/search/byHandle" + + def test_returns_first_item(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("items", [item_json(ITEM_UUID, "T")])) + item = c.get_item_by_handle("123456789/42") + self.assertIsInstance(item, Item) + self.assertEqual(item.uuid, ITEM_UUID) + self.assertEqual(sent_params(m.last_request)["handle"], ["123456789/42"]) + + def test_none_handle_short_circuits(self): + c = make_client() + with requests_mock.Mocker() as m: + self.assertIsNone(c.get_item_by_handle(None)) + self.assertFalse(m.called) + + def test_no_match_returns_none(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=embedded("items", [])) + self.assertIsNone(c.get_item_by_handle("123456789/0")) + + def test_non_json_body_returns_none(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, status_code=500, text="error") + self.assertIsNone(c.get_item_by_handle("123456789/42")) + + +class TestGetUserByEmail(unittest.TestCase): + """Mirrors dspace-rest-test - get_user_by_email(email).""" + + URL = f"{API}/eperson/epersons/search/byEmail" + + def test_returns_user_with_uuid_and_email(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json=user_json(email="a@b.c", netid="n1")) + u = c.get_user_by_email("a@b.c") + self.assertIsInstance(u, User) + self.assertEqual((u.uuid, u.email), (EPERSON_UUID, "a@b.c")) + self.assertEqual(sent_params(m.last_request)["email"], ["a@b.c"]) + + +class TestGetClarinAllowances(unittest.TestCase): + """Mirrors dspace-rest-test - get_clarinlruallowances[_by_bitstream_and_user].""" + + def test_returns_embedded_list(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/clarinlruallowances", + json=embedded("clarinlruallowances", [clarin_allowance_json(1)])) + allowances = c.get_clarinlruallowances() + self.assertEqual(len(allowances), 1) + self.assertEqual(allowances[0]["id"], 1) + + def test_error_returns_none(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/clarinlruallowances", status_code=500, text="boom") + self.assertIsNone(c.get_clarinlruallowances()) + + def test_by_bitstream_and_user_sends_both_params(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/clarinlruallowances/search/byBitstreamAndUser", + json=embedded("clarinlruallowances", [clarin_allowance_json(9)])) + out = c.get_clarinlruallowances_by_bitstream_and_user("bs-1", "usr-1") + self.assertEqual(out[0]["id"], 9) + qs = sent_params(m.last_request) + self.assertEqual(qs["bitstreamUUID"], ["bs-1"]) + self.assertEqual(qs["userUUID"], ["usr-1"]) + + +@pytest.mark.dtq_only +class TestGetOwningCollection(unittest.TestCase): + """dtq-only method. Mirrors src/repo/_audit.py:105-111, which relies on a + None return + last_err.status_code to drive its 401 reauth retry.""" + + URL = f"{API}/core/items/{ITEM_UUID}/owningCollection" + + def test_returns_typed_collection(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(self.URL, json={"uuid": COLLECTION_UUID, "name": "Coll", + "type": "collection"}) + col = c.get_owningCollection(ITEM_UUID) + self.assertIsInstance(col, Collection) + self.assertEqual(col.uuid, COLLECTION_UUID) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_clarin_usage_contract.py b/tests/test_clarin_usage_contract.py new file mode 100644 index 0000000..7dbd04e --- /dev/null +++ b/tests/test_clarin_usage_contract.py @@ -0,0 +1,192 @@ +""" +CLARIN-side integration contracts - the whole multi-call sequences the three +main-lineage consumers run, the counterpart to test_repo_usage_contract.py. + +Where the per-method tests pin one call, these replay a flow end-to-end (only +the HTTP transport mocked) so a library change that individually looks harmless +but breaks a *chain* a consumer depends on still fails here. + +Each class names the consumer repo it mirrors. clarin = must hold on both +main and dtq; dtq_only = relies on a dtq fix/behaviour, deselected on the main +leg of the differential-contract CI job. +""" +import unittest + +import pytest +import requests_mock + +import _helpers # noqa: F401 +from _helpers import ( + make_client, sent_params, embedded, item_json, bundle_json, raw_policy_json, + user_json, group_json, license_json, label_json, clarin_allowance_json, + search_envelope, API, ITEM_UUID, COLLECTION_UUID, BUNDLE_UUID, BITSTREAM_UUID, + EPERSON_UUID, GROUP_UUID) +from dspace_rest_client.models import Item, Bundle, Group, User, License + +pytestmark = pytest.mark.clarin + + +class TestImportClarinPolicyChain(unittest.TestCase): + """Mirrors dspace-import-clarin - locate the ORIGINAL bundle, read its + resource policy as a raw dict, move the policy to a new group.""" + + def test_bundle_then_policy_dict_then_group_update(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/items/{ITEM_UUID}/bundles", + json=embedded("bundles", [ + bundle_json("lic", "LICENSE"), + bundle_json(BUNDLE_UUID, "ORIGINAL")])) + m.get(f"{API}/authz/resourcepolicies/search/resource", + json=embedded("resourcepolicies", [raw_policy_json(pid=55)])) + m.put(f"{API}/authz/resourcepolicies/55/group", + status_code=200, json={}) + + bundle = c.get_bundle_by_name("ORIGINAL", ITEM_UUID) + self.assertEqual(bundle.uuid, BUNDLE_UUID) + + policy = c.get_resource_policy(bundle.uuid) + pid = policy["id"] # dict subscript, not a model + self.assertEqual(pid, 55) + + r = c.update_resource_policy_group(pid, GROUP_UUID) + self.assertEqual(r.status_code, 200) + self.assertEqual(m.last_request.method, "PUT") + + +class TestImportClarinLicenseIngest(unittest.TestCase): + """Mirrors dspace-import-clarin - read a collection's items, then build the + License dicts the importer writes out.""" + + def test_collection_items_then_license_to_dict(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/discover/search/objects", + json=search_envelope([item_json(ITEM_UUID, "A"), + item_json(COLLECTION_UUID, "B")])) + items = c.get_items_from_collection(COLLECTION_UUID) + self.assertEqual(len(items), 2) + self.assertTrue(all(isinstance(i, Item) for i in items)) + + lic = License(license_json(lid=1, name="CC-BY", + label=label_json(lid=9, label="PUB"))) + out = lic.to_dict() + self.assertEqual(out["license_id"], 1) + self.assertEqual(out["label_id"], 9) + + +class TestImportClarinMetadataRemoval(unittest.TestCase): + """Mirrors dspace-import-clarin - read items, remove an indexed metadata + value from one (the 3-arg remove_metadata(item, field, place) form).""" + + def test_items_then_indexed_remove(self): + c = make_client() + self_href = f"{API}/core/items/{ITEM_UUID}" + with requests_mock.Mocker() as m: + m.get(f"{API}/discover/search/objects", + json=search_envelope([item_json( + ITEM_UUID, "A", _links={"self": {"href": self_href}})])) + m.patch(self_href, status_code=200, + json=item_json(ITEM_UUID, "A", id=ITEM_UUID)) + + items = c.get_items_from_collection(COLLECTION_UUID) + c.remove_metadata(items[0], "dc.title", 0) + + body = m.last_request.json() + self.assertEqual(body[0]["op"], "remove") + self.assertEqual(body[0]["path"], "/metadata/dc.title/0") + + +class TestRestTestSubmitterSetup(unittest.TestCase): + """Mirrors dspace-rest-test - resolve a user by email, create a collection + submitter group, add the user to it.""" + + def test_email_group_member(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/eperson/epersons/search/byEmail", + json=user_json(uuid=EPERSON_UUID, email="sub@dq.sk")) + m.post(f"{API}/core/collections/{COLLECTION_UUID}/submittersGroup", + status_code=201, json=group_json(GROUP_UUID, "submitters")) + m.post(f"{API}/eperson/groups/{GROUP_UUID}/epersons", status_code=204) + + user = c.get_user_by_email("sub@dq.sk") + self.assertIsInstance(user, User) + + group = c.create_submit_group( + type("C", (), {"uuid": COLLECTION_UUID})()) + self.assertIsInstance(group, Group) + + self.assertTrue(c.add_member(group, user)) + + +class TestRestTestBitstreamPolicyFlow(unittest.TestCase): + """Mirrors dspace-rest-test - resolve an item by handle, find its ORIGINAL + bundle, grant a resource policy, then read user allowances.""" + + def test_handle_bundle_policy_allowance(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/items/search/byHandle", + json=embedded("items", [item_json(ITEM_UUID, "T")])) + m.get(f"{API}/core/items/{ITEM_UUID}/bundles", + json=embedded("bundles", [bundle_json(BUNDLE_UUID, "ORIGINAL")])) + m.post(f"{API}/authz/resourcepolicies", status_code=201, json={"id": 1}) + m.get(f"{API}/core/clarinlruallowances", + json=embedded("clarinlruallowances", [clarin_allowance_json(1)])) + + item = c.get_item_by_handle("123456789/42") + self.assertEqual(item.uuid, ITEM_UUID) + + bundle = c.get_bundle_by_name("ORIGINAL", item.uuid) + self.assertEqual(bundle.name, "ORIGINAL") + + ok = c.create_resource_policy( + BITSTREAM_UUID, data={"action": "READ"}, group_uuid=GROUP_UUID) + self.assertTrue(ok) + + allowances = c.get_clarinlruallowances() + self.assertEqual(len(allowances), 1) + + +class TestItemImporterPolicyRead(unittest.TestCase): + """Mirrors dspace-item-importer - create a policy, then read it back using + the `.get('id')` access form (not the `["id"]` subscript import-clarin uses).""" + + def test_create_then_get_with_dict_get(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(f"{API}/authz/resourcepolicies", status_code=201, json={"id": 88}) + m.get(f"{API}/authz/resourcepolicies/search/resource", + json=embedded("resourcepolicies", [raw_policy_json(pid=88)])) + + self.assertTrue( + c.create_resource_policy(BITSTREAM_UUID, data={"action": "READ"})) + + policy = c.get_resource_policy(BUNDLE_UUID) + self.assertEqual(policy.get("id"), 88) # .get(), not [...] + + +@pytest.mark.dtq_only +class TestRestTestNoArgGetItems(unittest.TestCase): + """Mirrors dspace-rest-test/tests/integration/create_bitstreams.py:145 - + the no-arg get_items() call. B1: on main the shadowed second def returned [] + (its `if len(all_items) < 3:` branch flipped); on dtq it paginates and + returns real items with page=0&size=20.""" + + def test_no_arg_get_items_paginates_and_returns_items(self): + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/items", json=embedded("items", [ + item_json("11111111-1111-1111-1111-111111111111", "A"), + item_json("22222222-2222-2222-2222-222222222222", "B"), + item_json("33333333-3333-3333-3333-333333333333", "C")])) + items = c.get_items() + self.assertEqual(len(items), 3) + qs = sent_params(m.last_request) + self.assertEqual(qs["page"], ["0"]) + self.assertEqual(qs["size"], ["20"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_clarin_write.py b/tests/test_clarin_write.py new file mode 100644 index 0000000..a4d4853 --- /dev/null +++ b/tests/test_clarin_write.py @@ -0,0 +1,214 @@ +""" +CLARIN/UFAL write surface - resource-policy creation, submitter-group setup, +group membership and metadata removal. Called by dspace-rest-test and +dspace-import-clarin; zero coverage before this suite. + +Mock only the HTTP transport. Each test names the consumer it mirrors. +See test_clarin_read.py for the clarin / dtq_only marker meaning. +""" +import unittest + +import pytest +import requests_mock + +import _helpers # noqa: F401 +from _helpers import ( + make_client, sent_params, item_json, embedded, + API, ITEM_UUID, BITSTREAM_UUID, COLLECTION_UUID, EPERSON_UUID, GROUP_UUID) +from dspace_rest_client.models import Group, User, Collection, Item + +pytestmark = pytest.mark.clarin + +LOGIN_URL = f"{API}/authn/login" +STATUS_URL = f"{API}/authn/status" + + +class TestCreateResourcePolicy(unittest.TestCase): + """Mirrors dspace-rest-test / dspace-item-importer - + create_resource_policy(resource, data, group_uuid=...) truthiness.""" + + URL = f"{API}/authz/resourcepolicies" + + def test_sends_resource_and_group_params_and_body(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, status_code=201, json={"id": 5}) + ok = c.create_resource_policy( + BITSTREAM_UUID, data={"action": "READ"}, group_uuid=GROUP_UUID) + self.assertTrue(ok) + qs = sent_params(m.last_request) + self.assertEqual(qs["resource"], [BITSTREAM_UUID]) + self.assertEqual(qs["group"], [GROUP_UUID]) + self.assertNotIn("eperson", qs) + self.assertEqual(m.last_request.json(), {"action": "READ"}) + + def test_sends_eperson_param_when_given(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, status_code=201, json={"id": 5}) + c.create_resource_policy( + BITSTREAM_UUID, data={"action": "READ"}, eperson_uuid=EPERSON_UUID) + qs = sent_params(m.last_request) + self.assertEqual(qs["eperson"], [EPERSON_UUID]) + self.assertNotIn("group", qs) + + def test_omits_absent_optional_params(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, status_code=201, json={"id": 5}) + c.create_resource_policy(BITSTREAM_UUID, data={"action": "READ"}) + qs = sent_params(m.last_request) + self.assertEqual(qs["resource"], [BITSTREAM_UUID]) + self.assertNotIn("group", qs) + self.assertNotIn("eperson", qs) + + def test_returns_true_only_on_201(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, status_code=201, json={"id": 5}) + self.assertIs(c.create_resource_policy(BITSTREAM_UUID, data={}), True) + + def test_non_201_returns_false(self): + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, status_code=422, json={"message": "bad"}) + self.assertIs(c.create_resource_policy(BITSTREAM_UUID, data={}), False) + + def test_401_reauthenticates_and_retries(self): + """A 401 triggers authenticate()+retry, so a policy create that hit an + expired session still succeeds. Shared: both main and dtq's api_post + carry this reauth block (verified against the main impl), so it is a + contract both must keep - not a dtq-only delta.""" + c = make_client() + with requests_mock.Mocker() as m: + m.post(self.URL, [ + {"status_code": 401, + "json": {"message": "Authentication is required"}}, + {"status_code": 201, "json": {"id": 5}}]) + m.post(LOGIN_URL, status_code=200) + m.get(STATUS_URL, status_code=200, json={"authenticated": True}) + ok = c.create_resource_policy(BITSTREAM_UUID, data={"action": "READ"}) + self.assertTrue(ok) + posts = [h for h in m.request_history + if h.method == "POST" and h.url.startswith(self.URL)] + self.assertEqual(len(posts), 2) # first 401, retry 201 + + +class TestUpdateResourcePolicyGroup(unittest.TestCase): + """Mirrors dspace-import-clarin - update_resource_policy_group(id, group).""" + + def test_puts_uri_list_and_returns_response(self): + c = make_client() + url = f"{API}/authz/resourcepolicies/77/group" + with requests_mock.Mocker() as m: + m.put(url, status_code=200, json={}) + r = c.update_resource_policy_group(77, GROUP_UUID) + self.assertEqual(r.status_code, 200) # returns raw Response + self.assertEqual(m.last_request.text, + f"{API}/eperson/groups/{GROUP_UUID}") + self.assertEqual( + m.last_request.headers["Content-type"], "text/uri-list") + + def test_403_csrf_retries_once(self): + c = make_client() + url = f"{API}/authz/resourcepolicies/77/group" + with requests_mock.Mocker() as m: + m.put(url, [ + {"status_code": 403, "json": {"message": "Invalid CSRF token"}}, + {"status_code": 200, "json": {}}]) + r = c.update_resource_policy_group(77, GROUP_UUID) + self.assertEqual(r.status_code, 200) + self.assertEqual( + len([h for h in m.request_history if h.method == "PUT"]), 2) + + +class TestCreateSubmitGroup(unittest.TestCase): + """Mirrors dspace-rest-test - create_submit_group(collection).""" + + def _collection(self): + return Collection({"uuid": COLLECTION_UUID, "type": "collection"}) + + def test_posts_to_submitters_group_url_and_returns_group(self): + c = make_client() + url = f"{API}/core/collections/{COLLECTION_UUID}/submittersGroup" + with requests_mock.Mocker() as m: + m.post(url, status_code=201, + json={"uuid": GROUP_UUID, "name": "submitters", "type": "group"}) + g = c.create_submit_group(self._collection()) + self.assertIsInstance(g, Group) + self.assertEqual(g.uuid, GROUP_UUID) + + def test_non_201_returns_none(self): + c = make_client() + url = f"{API}/core/collections/{COLLECTION_UUID}/submittersGroup" + with requests_mock.Mocker() as m: + m.post(url, status_code=500, text="boom") + self.assertIsNone(c.create_submit_group(self._collection())) + + +class TestAddMember(unittest.TestCase): + """Mirrors dspace-rest-test - add_member(group, eperson).""" + + def _group(self): + return Group({"uuid": GROUP_UUID, "name": "submitters"}) + + def _user(self): + return User({"uuid": EPERSON_UUID, "email": "a@b.c"}) + + def test_non_204_returns_false(self): + c = make_client() + url = f"{API}/eperson/groups/{GROUP_UUID}/epersons" + with requests_mock.Mocker() as m: + m.post(url, status_code=422, json={"message": "nope"}) + self.assertFalse(c.add_member(self._group(), self._user())) + + def test_rejects_non_group_and_non_user_without_request(self): + c = make_client() + with requests_mock.Mocker() as m: + self.assertFalse(c.add_member("not-a-group", self._user())) + self.assertFalse(c.add_member(self._group(), "not-a-user")) + self.assertFalse(m.called) + + +class TestRemoveMetadata(unittest.TestCase): + """Mirrors dspace-import-clarin - remove_metadata(item, field, place).""" + + def _item(self): + return Item(item_json( + ITEM_UUID, "T", + _links={"self": {"href": f"{API}/core/items/{ITEM_UUID}"}})) + + def test_with_place_patches_indexed_path(self): + c = make_client() + url = f"{API}/core/items/{ITEM_UUID}" + with requests_mock.Mocker() as m: + m.patch(url, status_code=200, + json=item_json(ITEM_UUID, "T", id=ITEM_UUID)) + c.remove_metadata(self._item(), "dc.title", 0) + body = m.last_request.json() + self.assertEqual(body[0]["op"], "remove") + self.assertEqual(body[0]["path"], "/metadata/dc.title/0") + + @pytest.mark.dtq_only + def test_place_none_removes_whole_field(self): + """B2: BEHAVIOUR CHANGE vs main. On main `place` was mandatory and a + None place was a no-op; on dtq place=None removes EVERY value of the + field (path has no index).""" + c = make_client() + url = f"{API}/core/items/{ITEM_UUID}" + with requests_mock.Mocker() as m: + m.patch(url, status_code=200, + json=item_json(ITEM_UUID, "T", id=ITEM_UUID)) + c.remove_metadata(self._item(), "dc.title") # place defaults None + body = m.last_request.json() + self.assertEqual(body[0]["path"], "/metadata/dc.title") + + def test_invalid_dso_returns_self_without_request(self): + c = make_client() + with requests_mock.Mocker() as m: + self.assertIs(c.remove_metadata(None, "dc.title", 0), c) + self.assertFalse(m.called) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_client_read.py b/tests/test_client_read.py index f48b04a..9b67890 100644 --- a/tests/test_client_read.py +++ b/tests/test_client_read.py @@ -7,6 +7,7 @@ """ import unittest +import pytest import requests_mock import _helpers # noqa: F401 @@ -78,6 +79,22 @@ def test_parses_embedded_items_with_paging_params(self): self.assertEqual(p["page"], ["2"]) self.assertEqual(p["size"], ["50"]) + @pytest.mark.dtq_only + def test_no_arg_defaults_to_first_page(self): + """B1: the no-arg get_items() form (dspace-rest-test .../create_bitstreams.py + :145) sends page=0&size=20 and returns items. On main the *second*, + shadowing `def get_items(self)` gated on 'collections' and always + returned [] - this is the behaviour that changes on merge.""" + c = make_client() + with requests_mock.Mocker() as m: + m.get(f"{API}/core/items", + json=embedded("items", [item_json("i1", "one")])) + items = c.get_items() + self.assertEqual(len(items), 1) + p = sent_params(m.last_request) + self.assertEqual(p["page"], ["0"]) + self.assertEqual(p["size"], ["20"]) + class TestGetItem(unittest.TestCase): diff --git a/tests/test_models_clarin.py b/tests/test_models_clarin.py new file mode 100644 index 0000000..89f3d4c --- /dev/null +++ b/tests/test_models_clarin.py @@ -0,0 +1,94 @@ +""" +CLARIN/UFAL model classes - License, Label (dspace-import-clarin) and the +Group / User objects the submitter-setup flow builds. No coverage before this. + +See test_clarin_read.py for the clarin / dtq_only marker meaning. +""" +import unittest + +import pytest + +import _helpers # noqa: F401 +from _helpers import ( + group_json, user_json, license_json, label_json, EPERSON_UUID, GROUP_UUID) +from dspace_rest_client.models import License, Label, Group, User + +pytestmark = pytest.mark.clarin + + +class TestLicense(unittest.TestCase): + """Mirrors dspace-import-clarin - License(...) / .to_dict().""" + + def test_core_fields(self): + lic = License(license_json(lid=3, name="CC-BY", confirmation=1, + required_info="SEND_TOKEN")) + self.assertEqual(lic.id, 3) + self.assertEqual(lic.name, "CC-BY") + self.assertEqual(lic.confirmation, 1) + self.assertEqual(lic.requiredInfo, "SEND_TOKEN") + self.assertTrue(lic.definition) + + def test_nested_clarin_license_label_becomes_label(self): + lic = License(license_json(label=label_json(lid=10, label="PUB"))) + self.assertIsInstance(lic.licenseLabel, Label) + self.assertEqual(lic.licenseLabel.label, "PUB") + + def test_extended_labels_list(self): + lic = License(license_json(extended=[label_json(11, "A"), + label_json(12, "B")])) + self.assertEqual(len(lic.extendedLicenseLabel), 2) + self.assertTrue(all(isinstance(x, Label) for x in lic.extendedLicenseLabel)) + + def test_to_dict_keys(self): + lic = License(license_json(lid=3, label=label_json(lid=10))) + self.assertEqual( + set(lic.to_dict()), + {"name", "license_id", "definition", "confirmation", + "required_info", "label_id"}) + self.assertEqual(lic.to_dict()["license_id"], 3) + self.assertEqual(lic.to_dict()["label_id"], 10) + + def test_to_dict_label_id_none_when_no_label(self): + lic = License(license_json()) + self.assertIsNone(lic.to_dict()["label_id"]) + + def test_from_empty_resource_does_not_crash(self): + self.assertIsNone(License({}).name) + + +class TestLabel(unittest.TestCase): + """Mirrors dspace-import-clarin - Label(...) / .to_dict().""" + + def test_core_fields_and_to_dict(self): + lab = Label(label_json(lid=7, label="PUB", title="Public", icon="p.png", + extended=True)) + self.assertEqual((lab.label, lab.title, lab.icon), ("PUB", "Public", "p.png")) + d = lab.to_dict() + self.assertEqual(d["label_id"], 7) + self.assertTrue(d["is_extended"]) + + def test_extended_defaults_false(self): + self.assertFalse(Label({"label": "x"}).extended) + + +class TestGroup(unittest.TestCase): + """Group is built by create_submit_group / consumed by add_member.""" + + def test_fields_and_as_dict(self): + g = Group(group_json(uuid=GROUP_UUID, name="submitters", permanent=True)) + self.assertEqual((g.uuid, g.name, g.permanent), (GROUP_UUID, "submitters", True)) + self.assertEqual(g.as_dict()["name"], "submitters") + + +class TestUser(unittest.TestCase): + """User is built by get_user_by_email / consumed by add_member.""" + + def test_fields_and_as_dict(self): + u = User(user_json(uuid=EPERSON_UUID, email="a@b.c", netid="n1")) + self.assertEqual((u.uuid, u.email, u.netid), (EPERSON_UUID, "a@b.c", "n1")) + self.assertTrue(u.canLogIn) + self.assertEqual(u.as_dict()["email"], "a@b.c") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_transport_hardening.py b/tests/test_transport_hardening.py new file mode 100644 index 0000000..643a7f7 --- /dev/null +++ b/tests/test_transport_hardening.py @@ -0,0 +1,126 @@ +""" +Transport hardening introduced on dtq - timeout / proxies on every verb, the +verify_response helper, and the last_err bookkeeping. These are the changes +that make the newly merged CLARIN methods actually surface transient failures +(instead of hanging) - so they are proven here against the low-level api_*. + +All dtq_only: main's client takes no timeout/proxies and has no +verify_response / last_err. +""" +import unittest + +import pytest + +import _helpers # noqa: F401 +from _helpers import make_client, API + +pytestmark = [pytest.mark.dtq_only] + + +class FakeResp: + """Minimal stand-in for requests.Response - enough for update_token / + parse_json / verify_response to run without a real transport.""" + + def __init__(self, status_code=200, body=None, bad_json=False): + self.status_code = status_code + self.headers = {} + self.url = "http://dspace.test" + self.text = "" if body is None else str(body) + self._body = {} if body is None else body + self._bad_json = bad_json + + def json(self): + if self._bad_json: + raise ValueError("not json") + return self._body + + +class RecordingSession: + """Captures the kwargs each verb is called with.""" + + def __init__(self, resp): + self._resp = resp + self.calls = {} + + def _verb(self, name): + def f(url, **kw): + self.calls[name] = kw + return self._resp + return f + + def __getattr__(self, name): + if name in ("get", "post", "put", "delete", "patch"): + return self._verb(name) + raise AttributeError(name) + + +def _client_with_session(timeout=42, proxies=None): + c = make_client() + c.timeout = timeout + c.proxies = proxies if proxies is not None else {"http": "http://proxy:3128"} + sess = RecordingSession(FakeResp(200, {})) + c.session = sess + return c, sess + + +class TestTimeoutAndProxiesOnEveryVerb(unittest.TestCase): + + def test_timeout_is_passed_on_every_verb(self): + c, sess = _client_with_session(timeout=7) + c.api_get(f"{API}/x") + c.api_post(f"{API}/x", params=None, json={}) + c.api_put(f"{API}/x", params=None, json={}) + c.api_delete(f"{API}/x", params=None) + for verb in ("get", "post", "put", "delete"): + self.assertEqual(sess.calls[verb].get("timeout"), 7, + f"{verb} did not pass timeout to the transport") + + def test_proxies_are_passed_on_every_verb(self): + proxies = {"http": "http://proxy:3128", "https": "http://proxy:3128"} + c, sess = _client_with_session(proxies=proxies) + c.api_get(f"{API}/x") + c.api_post(f"{API}/x", params=None, json={}) + c.api_put(f"{API}/x", params=None, json={}) + c.api_delete(f"{API}/x", params=None) + for verb in ("get", "post", "put", "delete"): + self.assertEqual(sess.calls[verb].get("proxies"), proxies, + f"{verb} did not pass proxies to the transport") + + def test_clarin_get_honours_custom_timeout(self): + """A CLARIN read (get_clarinlruallowances) must ride the same timeout.""" + c, sess = _client_with_session(timeout=3) + c.get_clarinlruallowances() + self.assertEqual(sess.calls["get"].get("timeout"), 3) + + +class TestVerifyResponse(unittest.TestCase): + + def test_non_200_records_last_err_and_returns_false(self): + c = make_client() + r = FakeResp(503, "unavailable") + self.assertFalse(c.verify_response(r, "id:1")) + self.assertIs(c.last_err, r) + + def test_200_ok_returns_true(self): + c = make_client() + self.assertTrue(c.verify_response(FakeResp(200, {"ok": True}), "id:1")) + + def test_as_json_on_invalid_body_returns_false(self): + c = make_client() + self.assertFalse( + c.verify_response(FakeResp(200, bad_json=True), "id:1", as_json=True)) + + +class TestLastErrReset(unittest.TestCase): + + def test_last_err_is_reset_at_the_start_of_each_request(self): + """A stale error from a previous call must not be read by the next one: + api_* clears _last_err on entry.""" + c, _sess = _client_with_session() + c._last_err = FakeResp(500, "stale") + c.api_get(f"{API}/x") + self.assertIsNone(c.last_err) + + +if __name__ == "__main__": + unittest.main()