Islandora / documentation

Contains islandora's documentation and main issue queue.
MIT License
104 stars 72 forks source link

Move NormalizerAlterReaction and JsonldTypeAlterReaction to jsonld module #1297

Open seth-shaw-unlv opened 4 years ago

seth-shaw-unlv commented 4 years ago

In a hypothetical world where someone uses the jsonld module without islandora they may want to leverage the JsonldTypeAlterReaction.

E.g. controlled access terms' corporate body type should use this reaction but currently doesn't because it creates a dependency on the islandora module.

seth-shaw-unlv commented 4 years ago

Clarification: we could add the necessary context config to make corporate_bodies work by placing it in install/optional/ as a stop-gap; but then it would only work as expected when used with islandora. Installing jsonld and controlled_access_terms without islandora would cause it to silently fail.

seth-shaw-unlv commented 4 years ago

I looked through what would be necessary to do this again today. Most of it could move over cleanly.

The blocker is NormalizerAlterReaction's use of IslandoraUtils' getRestUrl which we use to get a consistent URL. We could move this helper function to the JSONLD module; but the helper function is used more broadly (i.e. in the EventGenerator and LinkHeaderSubscriber) for more formats than just jsonld, which feels disingenuous.

The upshot: either;

  1. We add the necessary config to controlled_access_term with things as they currently stand; which would enforce a dependency on both jsonld and islandora to get this feature;
  2. We move over the helper-function to the JSON-LD module which expands it's functional scope a little beyond JSON-LD and enforces an Islandora-ism on it;
  3. We don't try to reuse the context and simply rely on controlled_access_terms_jsonld_alter_normalized_array as we have for other bits (yes, it is a re-implementation, but it avoids the other issues); or
  4. (unlikely) Figure out some other way to get consistent URLs that doesn't force us to use a helper.

I think using controlled_access_terms_jsonld_alter_normalized_array is probably the simplest solution that limits dependencies.

dannylamb commented 4 years ago

I find option 2 the least objectionable, FWIW.

seth-shaw-unlv commented 4 years ago

Ok, @dannylamb. I'll go with that if nobody objects; but I'll give the other @Islandora/8-x-committers a chance to weigh in before I move forward.

whikloj commented 4 years ago

Because the getRestUrl() method relies on a rest endpoint for entities, I think that means we also need to add a dependency on rest to jsonld.

I'd like present another option of having controlled_access_terms implement its own abstract class and a context setup for the json-ld hook, which could be completely separate from Islandora's?

Then things written for controlled_access_terms would be isolated from those written for islandora (and vice-versa) and both of these don't affect people using the json-ld module to just serialize their data (and not muck around in it).

As I did the original work in Islandora and I have the most virulent desire to keep jsonld pure, I'm happy to do the work in controlled_access_terms.