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

Warn about empty typed relation entity references (issue-1489) #50

Merged
merged 2 commits into from
Sep 3, 2020

Conversation

seth-shaw-unlv
Copy link
Contributor

GitHub Issue: Islandora/documentation#1489

What does this Pull Request do?

Catches an issue where the JSON-LD will WSOD if you accidentally give it a typed relation with an empty entity reference and warns you about it.

What's new?

  • Added sanity check on empty entity references in typed relations during JSON-LD alter which logs a warning.
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? You may want to re-index your repository in Fedora if you've been unwittingly loosing data to the error this fixes.

How should this be tested?

A description of what steps someone could take to:

  • Somehow create a typed relation without an entity reference. (I did it by accident with a bad migration.)
  • Attempt to view the resulting node's JSON-LD and see a WSOD.
  • Apply the PR
  • Attempt to view the node's JSON-LD again and see valid JSON-LD return
  • Check your logs. There should be a warning about a missing entity reference in the typed relation field for that node.

Interested parties

@Islandora/8-x-committers

@seth-shaw-unlv
Copy link
Contributor Author

Actually, maybe we should make the sanity check after we try to load the referenced entity, to make sure it is valid....

@seth-shaw-unlv
Copy link
Contributor Author

@whikloj, yeah, I was wondering if that would happen. I threw that PR together on a box that didn't have phpcs installed. I'll clean this up this morning.

@jordandukart jordandukart self-assigned this Sep 2, 2020
@manez manez requested a review from jordandukart September 2, 2020 17:20
@jordandukart
Copy link
Member

To refine the test case:

  • Create a Person taxonomy term such that a Linked Agent can be made from an Islandora entity.
  • Create an entity with that Linked Agent
  • Delete the taxonomy term
  • Clear the cache and attempt to retrieve the jsonld
  • WSOD

@jordandukart
Copy link
Member

This does what it says on the tin, 👍 from me.

However, noticed if you retrieve the JSONLD for a child and then delete the parent, the cache isn't invalidated and you have stale JSONLD being served. Likely needs its own issue but since I've never looked at this code before wonder if this is a known issue or if I am missing something @seth-shaw-unlv?

@seth-shaw-unlv
Copy link
Contributor Author

@jordandukart, I agree that the JSON-LD cache invalidation is a separate issue.

I'll also note that Islandora 8 doesn't have the 24-hour wait between approval and merge. Thanks!

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.

3 participants