feat(httpcache): introduce PurgeTagProviderInterface extension point#7970
feat(httpcache): introduce PurgeTagProviderInterface extension point#7970guillaumedelre wants to merge 1 commit into
Conversation
|
The associated documentation PR is available at api-platform/docs#2282. |
851f135 to
daa7ad5
Compare
|
Sorry for the ping, I've made it because it's in the
|
|
Sorry for the noise on this PR. A push to |
PurgeHttpCacheListener cannot invalidate sub-resource collection IRIs
such as /parents/{id}/children because it lacks the parent uri_variables
when processing the child entity. The new PurgeTagProviderInterface
lets users plug in custom tag collection strategies for these cases.
Symfony: implementing PurgeTagProviderInterface is sufficient; the
registerForAutoconfiguration hook tags the service automatically with
api_platform.http_cache.purge_tag_provider. Laravel: bind implementations
and tag them with PurgeTagProviderInterface::class via $app->tag().
Signed-off-by: Guillaume Delré <delre.guillaume@gmail.com>
|
The "PHPUnit (PHP 8.5) (Symfony 8.1)" and "PHPUnit (PHP 8.5) (Symfony dev)" failures are pre-existing and unrelated to this PR. The failing test is |
daa7ad5 to
88bd233
Compare
| /** | ||
| * @return iterable<string> | ||
| */ | ||
| public function getTagsForResource(object $entity): iterable; |
There was a problem hiding this comment.
Shouldn't this method better receive both the old entity (before an update) and the new entity (after an update)? In the example provided in the initial issue, it will otherwise be difficult to detect when a child changes it's parent and hence 2 separate tags need to be purged?
Also providing old entity and new entity would provide an easy method for the provider to detect if the method was called from an insertion or deletion.
There was a problem hiding this comment.
Thank you for the feedback, you're right that the current signature is insufficient for the parent-change use case.
When a Child changes its parent, two tags need to be purged: /parents/{old_id}/children and /parents/{new_id}/children. With the current getTagsForResource(object $entity), the implementor only receives the entity in its new state and has no way to compute the old parent's tag.
The fix I have in mind is to change the signature to:
public function getTagsForResource(object $entity, ?object $previousEntity = null): iterable;Where previousEntity is null on insertion and populated on update/deletion. This is BC-safe since the second parameter is nullable with a default value.
The key architectural point is where this is called. The current implementation calls providers from inside PurgeHttpCacheListener (a Doctrine ORM event listener). At that layer there is no previous_data — Doctrine's changeset only exposes scalar/identifier diffs, not a fully hydrated snapshot of the previous entity.
API Platform already solves this at a higher level: ReadProvider clones the entity before deserialization and stores it in $context['previous_data']. This snapshot is exactly what we need. The right place to call the providers is therefore the processor layer (e.g. PersistProcessor or a dedicated purge processor), where $context['previous_data'] is naturally available for PUT/PATCH operations and null for POST.
I can rework the PR to move the provider calls to that layer and update the interface accordingly. @soyuka what do you think?
There was a problem hiding this comment.
Yeah, good point. We currently have an own implementation of the listener, where we use the Doctrine changeset to manually calculate the state of the previous entity. It works well for us, but cannot guarantee it is bulletproof:
https://github.com/ecamp/ecamp3/blob/6bebc1320d7f92bea8ecda743517632556b807a1/api/src/HttpCache/PurgeHttpCacheListener.php#L140C22-L140C39
Regarding the signature, shouldn't $entity also be nullable in case of deletions? In that case the provider could distinguish all 3 cases (updates, deletions, inserts) based on which parameter is null or populated.
Summary
Closes #7965
Documentation
api-platform/docs#2282
Test plan