Navigation Menu

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

4139: Minor changes to opensearch cache #1364

Merged
merged 1 commit into from Mar 25, 2019

Conversation

cableman
Copy link
Member

@cableman cableman commented Feb 12, 2019

Link to issue

https://platform.dandigbib.org/issues/4139

Description

In working with ereolen.dk we had issues with ting object that contained content from other objects and cache misses after the first view (which should not happen as the first view populated the cache).

Screenshot of the result

No screenshot

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

Linked to ding2/ting-client#29

Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
@JrgenGNielsen
Copy link
Contributor

reviewed and approved

@@ -554,13 +565,18 @@ function _opensearch_predict_cache(array $collections, TingClientRequest $reques
$object_request->setProfile($profile);
}

// Build both requests for local id and PID.
$object_id_request = clone $object_request;
$object_id_request->setObjectId($object->id);
$local_id_request = clone $object_request;
$local_id_request->setLocalId($object->localId);
Copy link
Member

Choose a reason for hiding this comment

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

@cableman I know this is not from this PR, but isn't this a bit dangerous? The localId is not guaranteed to unique over all owners, so we are really not sure this object is the correct cache response for this localID

@kasperg kasperg merged commit 18dfe13 into ding2:master Mar 25, 2019
@cableman cableman deleted the feature/4139-cache-issues branch March 25, 2019 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants