-
Notifications
You must be signed in to change notification settings - Fork 343
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
Improved cache-invalidation for media/contact/account #3778
Improved cache-invalidation for media/contact/account #3778
Conversation
337c76c
to
71d6953
Compare
78e0794
to
9778e92
Compare
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.
Have you clarified with @chirimoya if we want that feature?
return []; | ||
} | ||
|
||
$ids = $this->converter->convertIdsToGroupedIds($value, ['a' => [], 'c' => []]); |
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.
Should we maybe introduce constants in this class for a
and c
?
if ($alias) { | ||
$entity->getId()->willReturn(1); | ||
$this->invalidationHandler->invalidateReference($alias, 1)->shouldBeCalled(); | ||
} |
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.
} else {
$this->invalidationhandler->invalidateReference(Argument::any)->shouldNotBeCalled();
}
?
|
||
foreach ($result as $item) { | ||
if ($item->getMediaId()) { | ||
$mediaReferenceStore->add($item->getMediaId()); |
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.
Shouldn't also be other stores considered here? AFAIK there could also e.g. pages returned here, right?
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 already done in preResolve
method
/** | ||
* Invalidate references when account/contact are persisted. | ||
*/ | ||
class CacheInvalidationListener |
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.
Can't we have one of these listeners, instead of adding one in each bundle?
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.
in which bundle would you add that?
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.
HttpCacheBundle?
9778e92
to
09fb6a3
Compare
09fb6a3
to
f588be9
Compare
@danrot this should go green now |
f588be9
to
f27debe
Compare
f6355c4
to
4b8f184
Compare
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.
Creating e.g. a new contact or deleting an existing contact with a category does clear the cache as expected
4b8f184
to
f18b17f
Compare
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.
Removing e.g. a contact is still not working for me.
f18b17f
to
320702b
Compare
320702b
to
32879b6
Compare
What's in this PR?
This PR add the reference-store to media/contact/account content-types to improve cache-invalidation.
To Do