Repository navigation
fix(models): parse resource policies granted to an eperson - #27
Merged
Merged
Conversation
added 2 commits
October 5, 2026 21:56
A resource policy is granted either to a group or to a single eperson;
DSpace embeds the other one as null. For an eperson policy the response
carries "_embedded": {"group": null, "eperson": {...}}, and
ResourcePolicy.__init__ called .get() on the null group, raising
AttributeError. The error is deterministic, so get_resourcepolicy()
failed for every resource holding such a policy and the valid group
policies in the same response were lost with it. This hit production
on the TUL DSpace, where some bitstreams carry a READ policy for a
person.
A null or missing group no longer crashes: groupName/groupUUID stay
None. The eperson's uuid is captured as epersonUUID, so callers can
tell "policy for a person" from "policy with no group". It is included
in as_dict() and read back by the cached-format branch, so a round trip
preserves it. No personal data (name, email) is stored.
The regression test replays the recorded TUL response, anonymised,
from tests/fixtures.
Review follow-up. epersonUUID is not needed to fix the crash, no consumer reads it, and it would put a pseudonymous person id into downstream caches, so it is removed again; the fix stays minimal: a null group leaves groupName/groupUUID None. A live-format policy whose _embedded has neither a group nor an eperson is not a recognised shape, so it now logs a WARNING naming the policy id instead of passing silently. A pure eperson policy logs nothing.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new tests use assertNoLogs, which is unavailable on the declared supported Python 3.8 and 3.9 versions.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Fixes parsing of resource policies granted to an eperson while preserving group-policy handling and warning on invalid policy shapes.
Changes:
- Safely handles null or missing embedded groups.
- Adds warning logging for policies without a group or eperson.
- Adds model and client regression tests with an anonymized fixture.
| File | Description |
|---|---|
dspace_rest_client/models.py |
Handles eperson policies and malformed policy warnings. |
tests/test_models.py |
Tests model parsing and warnings. |
tests/test_client_read.py |
Tests end-to-end policy response parsing. |
tests/_helpers.py |
Adds JSON fixture loading. |
tests/fixtures/resourcepolicies_eperson_and_group.json |
Provides the regression response fixture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
TestCase.assertNoLogs only exists since Python 3.10, but setup.py declares python_requires >= 3.8, so the eperson-policy tests raised AttributeError on 3.8/3.9 before testing anything. A small no_warnings_logged() helper in tests/_helpers.py captures WARNING records on the logger and asserts there are none, on every supported version.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A DSpace resource policy is granted either to a group or to a single eperson; the other side is embedded as
null. For an eperson policy the API returns:ResourcePolicy.__init__called.get('name')on the null group and raisedAttributeError: 'NoneType' object has no attribute 'get'. Because it is deterministic,get_resourcepolicy()failed for every resource that holds such a policy, and the valid group policies returned in the same response were lost with it.This happened in production on the TUL DSpace (2026-10-05): a
license.txtbitstream carried one READ policy for a person and one inherited READ policy for Anonymous, and the policy lookup for it failed on every run.Solution
_embedded.groupno longer crashes;groupName/groupUUIDstayNonefor an eperson policy. Nothing about the eperson is stored.dspace.models, with a NullHandler likedspace.client). A plain eperson policy logs nothing.Tests
test_client_read.py::TestGetResourcePolicy::test_eperson_policy_next_to_group_policy_parses_bothreplays the recorded TUL response (anonymised,tests/fixtures/resourcepolicies_eperson_and_group.json) throughget_resourcepolicy. Both policies parse and no warning is logged.test_models.py::TestResourcePolicy::test_eperson_policy_has_no_group_and_no_warningcovers the eperson policy and itsas_dict()round trip.test_models.py::TestResourcePolicy::test_policy_without_group_and_eperson_warns_with_idcovers the case with neither a group nor an eperson.