-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore(cache): improve invalidation logic #45
base: master
Are you sure you want to change the base?
Conversation
sidux
commented
Jul 8, 2024
•
edited
Loading
edited
- add more tests
- since tags now have the name of the property or the method added we need to check how to invalidate them all (drop redis database ?)
- check possible edge cases with imbricated requests
- add test to check key invalidation when manual tags change
1df0ed1
to
3dfa67f
Compare
3dfa67f
to
4d8df97
Compare
1f0a28c
to
b8729a3
Compare
Issued by Coverage Checker: |
private function normalizePrefixName(string $name): string | ||
{ | ||
return str_replace( | ||
['\\', 'SharedResponse', 'Embedded', '_Shared'], |
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.
needs tests
if (!$member->isPublic() || \count($member->getParameters()) > 0) { | ||
throw new \LogicException( | ||
sprintf( | ||
'Method %s::%s must be public and have no parameters to be used as a tag.', |
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.
needs test
if (!$member->isInitialized($object)) { | ||
throw new \LogicException( | ||
sprintf( | ||
'Property %s::%s must be initialized to be used as a tag.', |
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.
needs test
if ($response !== null && \is_object($response)) { | ||
$prefix = $this->normalizePrefixName(\get_class($response)); | ||
} else { | ||
$prefix = $this->normalizePrefixName( |
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.
needs tests
foreach ($ref->getProperties() as $propRef) { | ||
$subObject = $this->getPropertyValue($ref, $object, $propRef->getName()); | ||
|
||
$registeredTags = $this->guessObjectsTags($subObject, $prefix, $excludedClasses, $registeredTags); |
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.
is there a possible embedded requests edge case ?
b8729a3
to
2d77187
Compare