[Form] Reset cache when AbstractTypeExtension::getExtendedTypes change
#62804
Conversation
|
I don't think we should create new resource types. The existing |
| { | ||
| $hash = hash_init('xxh128'); | ||
|
|
||
| $ret = ($this->staticMethod)(); |
There was a problem hiding this comment.
This implementation is broken. When calling it in isFresh on an unserialized data, you cannot assume that this method still exists (and is still static). The check needs to handle gracefully the case where the code was changed (as that's precisely the use case for having such resource)
I guess you are talking about It won't work though, will it? It only checks the public/protected API. Here, we must execute the method and retrieve the data. |
|
ah indeed. I guess the reason why |
| private string $hash; | ||
|
|
||
| /** | ||
| * @param array $staticMethod A callable represented as [className, methodName] |
There was a problem hiding this comment.
| * @param array $staticMethod A callable represented as [className, methodName] | |
| * @param array{string, string} $staticMethod A callable represented as [className, methodName] |
There was a problem hiding this comment.
and maybe we should even have 2 parameters instead of this array.
| * @param array $staticMethod A callable represented as [className, methodName] | ||
| */ | ||
| public function __construct( | ||
| private $staticMethod, |
|
|
||
| public function __toString(): string | ||
| { | ||
| return 'static_method_resource'.$this->staticMethod[0].'::'.$this->staticMethod[1]; |
There was a problem hiding this comment.
| return 'static_method_resource'.$this->staticMethod[0].'::'.$this->staticMethod[1]; | |
| return 'static_method_resource:'.$this->staticMethod[0].'::'.$this->staticMethod[1]; |
This will be more readable in case this is investigated, by having a delimiter before the class name.
|
|
||
| public function __unserialize(array $data): void | ||
| { | ||
| $this->hash = array_shift($data); |
There was a problem hiding this comment.
Instead of using array_shift, it should access the keys in the array as __serialize returns an associative array.
|
|
||
| $ret = ($this->staticMethod)(); | ||
|
|
||
| if (is_iterable($ret)) { |
There was a problem hiding this comment.
why this special case for iterable ?
And why ignoring the keys in that iterable (which will make this unusable for RegisterListenersPass) ?
OskarStark
changed the title
AbstractTypeExtension::getExtendedTypes change
3 participants
Not finished, obviously. But is this the way to go?