Skip to content

Need alternative link index to make use of embeds#652

Closed
LotteHofstede wants to merge 10 commits intoDSpace:mainfrom
atmire:alternative-links
Closed

Need alternative link index to make use of embeds#652
LotteHofstede wants to merge 10 commits intoDSpace:mainfrom
atmire:alternative-links

Conversation

@LotteHofstede
Copy link
Copy Markdown
Contributor

@LotteHofstede LotteHofstede commented Apr 16, 2020

References

This PR closes #626

Description

This PR adds an extra index for alternative links to objects stored in the object cache.

Instructions for Reviewers

Changes

Some objects can be requested by different links than just their self link. For example, in the _links section of an item, you will find a link to its owning collection formulated as the self link of the item, appended by /owningCollection.

"_links": {
  "owningCollection": {
    "href": "https://example-server.org/api/core/items/0136fe00-43ca-439e-ae6c-b2b08c2b8f5e/owningCollection"
  },
  "self": {
    "href": "https://example-server.org/api/core/items/0136fe00-43ca-439e-ae6c-b2b08c2b8f5e"
  }
}  

When retrieving this collection, you'll notice the self link is not the same as the link mentioned in item's link section:

"_links": {
    "self": {
      "href": "https://example-server.org/api/core/collections/282164f5-d325-4740-8dd1-fa4d6d3e7200"
    }
  }

The owningCollection link mentioned in _links section of the item is an example of an alternative link.

Previously, when for example, the owningCollection was embedded inside the item REST response, the embedded collection was stored in the object cache using its self link. Subsequently, the frontend would request this collection using its alternative link because the alternative link was not known to the cache.

This PR adds an index to map the alternative links on the matching self links. For the example mentioned above, the entry of this index would look something like this:

https://example-server.org/api/core/items/0136fe00-43ca-439e-ae6c-b2b08c2b8f5e/owningCollection https://example-server.org/api/core/collections/282164f5-d325-4740-8dd1-fa4d6d3e7200

To keep this index up to date, the following changes were made to the ObjectCacheService:

  • When something is added to the object cache, its alternative link is compared to its self link. If they differ:

    • A new entry is added to the index.
    • The alternative link is also added to a list in the ObjectCacheEntry's state.
  • When something is removed from the object cache:

    • All links in the _links section of the object are removed from the new index, if they existed
    • The array of alternative links in the ObjectCacheEntry are also removed from the index

How to test

A simple way of testing this PR is by visiting an item page, where the owningCollection is always embedded and referenced by an alternativeLink.
If you go to a previous version of the code base, you'll notice a request is sent out to the alternative link ([itemLink]/owningCollection). This request has become redundant in this PR, and will not show up in the network tab of your browser anymore. If you look at the store, you will also be able to see the mapping in core.index.object/alt-link-to-self-link.

To test if the index is actually kept up to date, you could remove a relationship from the item on its edit page. This causes the item to be decached, which should also remove the alternative link to its owningCollection from the index in the store.

Lists

Unfortunately, this PR does not solve the same issue for alternative links to object lists. Lists are not directly stored in the object cache, and can therefore not be mapped in the index.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests). Exceptions may be made if previously agreed upon.

  • My PR passes TSLint validation using yarn run lint

  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.

  • My PR passes all specs/tests and includes new/updated specs for any bug fixes, improvements or new features. A few reminders about what constitutes good tests:

Copy link
Copy Markdown
Contributor

@atarix83 atarix83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @LotteHofstede @artlowel

I've tried to merge this PR into #645 to see if it fixes the #644
The result is very odd, because the resource policies page crashes after a while and with it the browser too.
What I could figure out is that a subscriptions hangs forever, you can see the warning in browser's console
Schermata da 2020-05-07 18-55-22

I've created a new branch where both PRs are merged here to help to reproduce the issue

@tdonohue
Copy link
Copy Markdown
Member

tdonohue commented May 8, 2020

@LotteHofstede : I gave this a quick test today (using yarn start). I can reproduce the odd error that @atarix83 noted with this PR alone and the Submission UI. If I start a new submission, everything initially seems to work. But after filling out a few fields it tries to auto-save (at least I think that's what's going on). At that point, the browser locks up and the Google DevTools console shows a never ending stream of warnings....it almost seems like an infinite loop is being encountered. I then have to kill the browser tab entirely (as the browser tab's CPU spikes to 100%).

@heathergreerklein heathergreerklein added this to the 7.0beta3 milestone Jun 24, 2020
@tdonohue tdonohue changed the title Alternative links Need alternative link index to make use of embeds Jun 24, 2020
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Jun 24, 2020

This pull request introduces 2 alerts when merging c71db3a into 5cef15e - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Jul 13, 2020

This pull request introduces 2 alerts when merging c71db3a into e5742c4 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@heathergreerklein heathergreerklein added the e/16 Estimate in hours label Jul 15, 2020
@tdonohue tdonohue modified the milestones: 7.0beta4, 7.0beta5 Sep 24, 2020
@artlowel artlowel mentioned this pull request Dec 3, 2020
6 tasks
@artlowel
Copy link
Copy Markdown
Member

artlowel commented Dec 3, 2020

Closing, replaced by #961

@artlowel artlowel closed this Dec 3, 2020
@tdonohue tdonohue removed this from the 7.0beta5 milestone Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e/16 Estimate in hours merge conflict

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need alternative link index to make use of embeds

5 participants