Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/self_reference_validation #71

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tomreitz
Copy link
Collaborator

@tomreitz tomreitz commented Mar 10, 2025

This PR fixes two bugs that prevented self-references (such as objectiveAssessment.parentObjectiveAssessmentReference) from validating correctly:

  1. reference fields were resolved to an endpoint by simply removing the Reference suffix, which obviously doesn't work for parentObjectiveReferences
  2. cache keys (which are used to efficiently look up whether a referenced payload has been found) were sensitive to dict key ordering

This PR fixes both bugs and ensures that self-references validate correctly.

Additionally, the PR fixes several stray references to line_counter which was changed in this PR to line_number (I ran into errors about line_counter being undefined while testing this PR).

tomreitz and others added 2 commits March 10, 2025 10:08
This PR fixes two bugs that prevented self-references (such as `objectiveAssessment.parentObjectiveAssessmentReference` from validating correctly:
1. reference fields were resolved to an endpoint by simply removing the `Reference` suffix, which obviously doesn't work for `parentObjectiveReference`s
2. cache keys (which are used to efficiently look up whether a referenced payload has been found) were sensitive to dict key ordering

This PR fixes both bugs and ensures that self-references validate correctly.
@tomreitz tomreitz requested a review from jalvord1 March 10, 2025 15:22
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.

2 participants