Add check for cache tags in JSON-LD alter hook.

[alxp]

GitHub Issue: (link) Islandora/documentation#2039

What does this Pull Request do?

Fixes a white screen when getting a JSON-LD representation of a node when the controller includes an entity access check.

What's new?

Adds an 'isset' to fix a null array access in the jsonld normalizer alter function.

• Does this change add any new dependencies?
  no

• Does this change require any other modifications to be made to the repository
  no

• Could this change impact execution of existing code?
  No

How should this be tested?

Problem occurs when dealing with unpublished content.

If you have a controller that correctly checks for entity access, e.g., in routing.yml:

islandora_premis.premis:
  path: '/node/{node}/premis'
  defaults:
    _controller: '\Drupal\islandora_premis\Controller\IslandoraPremisPremisController::main'
  options:
    parameters:
      node:
        type: 'entity:node'
  requirements:
    _entity_access: 'node.view'

The entity access check makes it such that the output is not cacheable as it would be without it. Thus there is no 'cache ability' array element.

All entities accessible to the logged-in user should now be visible as JSON-LD, e.g.

/node/{nid}?_premis=jsonld

Documentation Status

• Does this change existing behaviour that's currently documented? no
• Does this change require new pages or sections of documentation? no

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/8-x-committers

[alxp]

Looks like no problem requiring filehash:'^1.1 || ^2'

[nigelgbanks]

Was a bit concerned about changes to filehash so I had a look into where its used by islandora, and I could only find this instance:

islandora/islandora.module

Lines 178 to 203 in 2923a1a

	/**
	* Implements hook_file_update().
	*/
	function islandora_file_update(FileInterface $file) {
	// Exit early if unchanged.
	if ($file->filehash != NULL && $file->original->filehash != NULL && $file->filehash['sha1'] == $file->original->filehash['sha1']) {
	return;
	}
	$utils = \Drupal::service('islandora.utils');
	// Execute index reactions.
	$utils->executeFileReactions('\Drupal\islandora\Plugin\ContextReaction\IndexReaction', $file);
	// Execute derivative reactions.
	foreach ($utils->getReferencingMedia($file->id()) as $media) {
	$node = $utils->getParentNode($media);
	if ($node) {
	$utils->executeDerivativeReactions(
	'\Drupal\islandora\Plugin\ContextReaction\DerivativeReaction',
	$node,
	$media
	);
	}
	}
	}

.

That being said it doesn't look like the hook_file_update actually exists in either Drupal 8.x / 9.x.

https://api.drupal.org/api/drupal/8.0.x/search/hook_file_update
https://api.drupal.org/api/drupal/9.4.x/search/hook_file_update

So I don't think this code is ever called? Perhaps the dependency along with this function could be removed completely?

[nigelgbanks]

Oh there is also a view included https://github.com/Islandora/islandora/blob/2.x/modules/islandora_core_feature/config/install/views.view.file_checksum.yml

[seth-shaw-unlv]

@nigelgbanks, it won't show up in a search because it is covered by hook_entity_update. This allows us to trigger new derivatives when the file gets replaced.

[nigelgbanks]

@seth-shaw-unlv ah that makes sense. Could a possible issue arise if the user chooses any other algorithm than sha1 for filehash (I've got a client currently pinned to MD5 only)? In that cause the function would always exit early? That being said I think the default behaviour is for Drupal to create a new File entity when changing the file of a Media Entity? In which case this wouldn't get called? Though I'm just speculating.

[seth-shaw-unlv]

@nigelgbanks, the Drupal web UI doesn't let you replace a file, but there are various other ways to do it (e.g. the Migrate API import update flag, custom code, and the endpoints our Crayfish microservices hit).

As for pinning MD5, 🤷‍♂️ I haven't tested it.

[jordandukart]

Think if there's additional discussion required about Islandora's usage of filehash another issue should be made as this ticket is separate and bumping filehash for sake of the tests passing.

[jordandukart]

JSON-LD really should be updated in the future to follow the new suggested way of handling cacheability.

Unrelated to this PR in general.

[jordandukart]

Looks fine to me after discussion/testing. Tests are failing due to PHP8.1 deprecations now only.