search

huacnlee / rails-settings-cached

Global settings for your Rails application.
Other
1.06k stars 202 forks source link

Update the cached value for the key when value set #86

Closed justin808 closed 8 years ago

justin808 commented 8 years ago

If setting a cache value is called within a TX and then the value is accessed, then the old value is retrieved from the cache.

The fix is to clear the cached value for the var name immediately after calling save!. Note, it is still appropriate to clear cache values upon commit due to race conditions.

Review on Reviewable

justin808 commented 8 years ago

@huacnlee Can you let me know if this looks good? Thanks.

huacnlee commented 8 years ago

I don't know what your means, please give a example codes for this case.

justin808 commented 8 years ago

See added specs, @huacnlee. Thanks.

justin808 commented 8 years ago

@huacnlee Have you had a chance to look at my PR?

mameier commented 8 years ago

An easier way to ensure consistency of cached settings (even during transactions, and in test) is to write the specific cache entry when an attribute is changed. See my pull request #88

justin808 commented 8 years ago

@huacnlee Any word?

@mameier Your suggestion would require modifying the code with a new API.

mameier commented 8 years ago

The API is unchanged. All existing tests pass. Only the behavior is what I (and some others) expect: as soon as you set a setting, the new value is available.

Or do you mean that I should write a failing test that is fixed by the code change?

justin808 commented 8 years ago

@huacnlee I rebased on top of master, and I included the suggestion in #88 from @mameier (good call!)

Can we get this merged and new release of the gem?

tovodeverett commented 8 years ago

This resolves some of the issue in my test suite, but not all of them. In particular, it appears that calls to Setting.destroy do not clear the cache for the key in question.

justin808 commented 8 years ago

@huacnlee Any word on this PR?

justin808 commented 8 years ago

@huacnlee Any ETA on when you might push a new gem version? I've got a large production system that needs this change. Thanks!

huacnlee commented 8 years ago

@justin808 version 0.5.4

justin808 commented 8 years ago

@huacnlee Big thanks. Sorry I missed that! Have you considered tagging your versions. I do that with my gem: https://github.com/shakacode/react_on_rails/.

See https://github.com/shakacode/react_on_rails/blob/master/docs%2Freleasing.md

I use the gem "gem-release".