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

3905: Cache objects by local id as well as id #1284

Merged
merged 1 commit into from
Dec 6, 2018

Conversation

kasperg
Copy link
Member

@kasperg kasperg commented Dec 3, 2018

Link to issue

https://platform.dandigbib.org/issues/3905
https://platform.dandigbib.org/issues/3828

Description

Issue 3828 (#1261) introduced the possibility for requesting OpenSearch objects
by local id (Faust) as well as object id (PID). In parallel the cache
handling was updated with #1271
. The two changes did not take each other into
account.

This resulted in situations for local id requests without object ids
where types were mismanaged causing fatal type errors.

To handle the situation better this change does two things:

  1. Cache objects in responses by local id as well as object id.
  2. Try to retrieve objects for the cache by local id (if defined) as
    well as object id (if defined)

To ensure that the change integrity I have tested that doing a search only results in a single request to OpenSearch and that a repeated search does not result in any request. Thus the gains achieved in #1271 should be intact.

Checklist

  • My complies with our coding guidelines.
  • My code passes our static analysis suite. If not then I have added a comment explaining why this change should be exempt from the code standards and process.
  • My code passes our continuous integration process. If not then I have added a comment explaining why this change should be exempt from the code standards and process.

Additional comments or questions

This only increases the complexity of our cache. I also considered creating a new object request class in Ting Client based on Local Id instead of Object Id. That might have made the implementation here a bit cleaner. In the end I opted for this approach.

Issue 3828 introduced the possibility for requesting OpenSearch objects 
by local id (Faust) as well as object id (PID). In parallel the cache 
handling was updated. The two changes did not take each other into
account.

This resulted in situations for local id requests without object ids 
where types were mismanaged causing fatal type errors.

To handle the situation better this change does two things:

1. Cache objects in responses by local id as well as object id.
2. Try to retrieve objects for the cache by local id (if defined) as
well as object id (if defined)
@kasperg kasperg merged commit c5714e2 into ding2:master Dec 6, 2018
@xendk xendk deleted the 3905-opensearch-localid-cache branch October 15, 2020 11:27
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