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

Update dstore/Cache::put #154

Merged
merged 1 commit into from
May 24, 2017
Merged

Update dstore/Cache::put #154

merged 1 commit into from
May 24, 2017

Conversation

kitsonk
Copy link
Member

@kitsonk kitsonk commented Sep 1, 2015

Previous behaviour was to remove the item while waiting for return from the master store. Now Cache will presume the update will complete and only remove the item if the master store rejects the put.

Fixes #117.

src/Cache.js Outdated
cachingStore.remove((directives && directives.id) || this.getIdentity(object));
return when(this.inherited(arguments), function (result) {
cachingStore.put(object, directives);
return when(this.inherited(arguments)).then(function (result) {
// now put result in cache
var cachedPutResult =
cachingStore.put(object && typeof result === 'object' ? result : object, directives);
Copy link
Member

Choose a reason for hiding this comment

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

This can be reduced to an if, since if result is not an object, we've already done this prior to calling inherited.

@kriszyp
Copy link
Member

kriszyp commented Feb 3, 2016

This looks good to me.

@edhager edhager force-pushed the master branch 2 times, most recently from 1dbb25d to a5655fe Compare August 18, 2016 15:59
@dylans
Copy link
Member

dylans commented May 24, 2017

I think this one needs a rebase. We have enough things to do a 1.2 release soon so we should rebase and land this if time permits @kitsonk.

@dylans dylans added this to the 1.2 milestone May 24, 2017
@kitsonk
Copy link
Member Author

kitsonk commented May 24, 2017

Yeah, the rebase totally screwed up... 😢 This is simply too stale.

Previous behaviour was to remove the item while waiting for return
from the master store.  Now Cache will presume the update will
complete and only remove the item if the master store rejects the
`put`.
@kitsonk
Copy link
Member Author

kitsonk commented May 24, 2017

A cherry-pick to a new branch worked.

@kitsonk kitsonk merged commit a0e5169 into master May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants