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
Fix cache invalidation for modified tags #13
Conversation
The current implementation only invalidates tags using the new updated attribute values used by the tag. Older values are retained in the caches for the old values. The fix basically tracks if the primary key attributes, or the composite tag attributes change, and uses the old value for invalidating the cache. In the case of composite keys it also invalidates caches using the new attribute values. With the primary key the assumption is that there is no need to flush the cache for the new value, since this should be unique.
@fps01 please take a look at this feature |
if (count($this->primaryKey()) == 1) | ||
{ | ||
$key = $this->primaryKey()[0]; | ||
$primary_key = isset($oldfields[$key]) ? $oldfields[$key] : $this->$key; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$oldfields is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my bad, should be an optional arg to objectTag, I made the commit using the github website, so just an oversight.
Fixed issues(maybe) Passes Travis tests at least... waiting on scrutinizer... Still need a unit test, but not sure how to build what I need... |
Thanks! Everything is ok. Just fix a code style. I will mark lines for fixing. |
* @return string tag name | ||
*/ | ||
public function objectTag() | ||
public function objectTag($oldfields = []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename $oldfields
to $oldFields
All that seems to be missing is unit tests for the new behaviours. |
Thanks! |
The current implementation only invalidates tags using the current (new) attribute values used by the tag. Caches that use the old value retain the entities.
The fix basically tracks if the primary key attributes, or the composite tag attributes change, and uses the old value for invalidating the cache. In the case of composite keys it also invalidates caches using the new attribute values.
With the primary key the assumption is that there is no need to flush the cache for the new value, since this should be unique.
I didn't have time to create unit tests using your framework, but basically you create an entity, get it using the cache, modify the primary key, or the one of the composite tagged attributes and save. You can still get the outdated models from the caches using the old values...
There is still a problem related to deleting. If you modify the record's attributes and then delete, it can't detect the old values, and they're no longer available since yii nulls $_oldattributes. This is a pretty niche case though.