Skip to content
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

[HTTP Cache] Doesn't work with stateOptions #6754

Open
simondaigre opened this issue Oct 25, 2024 · 13 comments
Open

[HTTP Cache] Doesn't work with stateOptions #6754

simondaigre opened this issue Oct 25, 2024 · 13 comments

Comments

@simondaigre
Copy link
Contributor

API Platform version(s) affected: 3.x, 4.x

Description
HTTP Cache doesn't work with DTO using stateOptions, the current implementation in PurgeHttpCacheListener only searches for Doctrine entities.

@mrossard
Copy link
Contributor

mrossard commented Oct 26, 2024

I tried looking at this a few weeks ago, but that's actually trickier than it looks : it's implemented with a Doctrine EventListener, which is a pretty "safe" way to get the correct impacted objects; if those are the ApiResources it's rather straightforward.

With StateOptions you can't easily do the same thing : from the doctrine you'd have to cycle through all ApiResources types to check which ones are actually targeting the entity class you're updating. I ended up triggering the cache invalidation from my State Processors, which doesn't seem ideal...but I'm not sure there's an easier way to do this.

@mrossard
Copy link
Contributor

mrossard commented Oct 30, 2024

I see 2 missing pieces to this puzzle:

  • a way to implement some kind of getApiResourceClassesForEntityClass() method
  • a way to transform an entity into the corresponding resources

The first one could be done at the same time the metadata for ApiResources is gathered (AttributesResourceMetadataCollectionFactory or something?) I suppose.

The second one is more of a problem since there's no "standard" way to do this right now. @soyuka is your symfony PR for a Mapper close to getting merged? It could be a solution to rely on this, couldn't it?

@g-ra
Copy link

g-ra commented Nov 5, 2024

You have the option to override PurgeHttpCacheListener, and I believe it would be beneficial to create a custom gatherResourceAndItemTags method to manage cache invalidation for DTOs that rely on state processors.

Additionally, I would recommend adding an attribute to each DTO that links it to its corresponding entity class. In the gatherResourceAndItemTags method, you could then check for this attribute, and if it's present, include it in the cache invalidation process.

It would, of course, be ideal to have such functionality available out-of-the-box for handling custom resources and DTOs more seamlessly.

here is your service.yaml:

    App\HttpCache\PurgeHttpCacheListener:
        tags:
            - { name: doctrine.event_listener, event: preUpdate }
            - { name: doctrine.event_listener, event: onFlush }
            - { name: doctrine.event_listener, event: postFlush }    
    api_platform.doctrine.listener.http_cache.purge:
        class: App\HttpCache\PurgeHttpCacheListener

here is you custom Listener:

final class PurgeHttpCacheListener
{
    use ClassInfoTrait;
    private readonly PropertyAccessorInterface $propertyAccessor;

    private array $tags = [];

    public function __construct(
        private readonly UrlGeneratorInterface $urlGenerator,
        #[Autowire(service: 'api_platform.http_cache.purger.souin')]
        private readonly PurgerInterface $purger, private readonly IriConverterInterface | LegacyIriConverterInterface $iriConverter, private readonly ResourceClassResolverInterface | LegacyResourceClassResolverInterface $resourceClassResolver, ?PropertyAccessorInterface $propertyAccessor = null)
    {
        $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
    }

@simondaigre
Copy link
Contributor Author

@g-ra Thanks you for your solution, I will look into it.

@mrossard
Copy link
Contributor

mrossard commented Nov 5, 2024

You have the option to override PurgeHttpCacheListener, and I believe it would be beneficial to create a custom gatherResourceAndItemTags method to manage cache invalidation for DTOs that rely on state processors.

Additionally, I would recommend adding an attribute to each DTO that links it to its corresponding entity class. In the gatherResourceAndItemTags method, you could then check for this attribute, and if it's present, include it in the cache invalidation process.

I think you're missing one part of the problem here:

  • you already have the link to the entity from the resource, in the stateOptions.
  • you don't have the inverse link from the entity to the resources, and (possibly more) importantly this plural is NOT e typo!
  • Another use case I stumbled upon recently is that you can have updates on an entity that affect a calculated field on a resource that doesn't use it directly...tracking that automatically is pretty much undoable.

I think there needs to be a way to plug some user code into the invalidation process, possibly using a dedicated attribute on the entities...you could use it to specify which resources need to be purged (and how to convert the entity into them), or even have some custom code like it's done for links handling.

@simondaigre
Copy link
Contributor Author

Not totally related to this issue, but I'm assuming there is the same problem with the Mercure bridge.

@soyuka
Copy link
Member

soyuka commented Dec 13, 2024

IMO there's a non-doctrine solution that can be implemented. Let me know if you want to contribute it I'll give you the implementation details I've in mind.

@simondaigre
Copy link
Contributor Author

simondaigre commented Dec 13, 2024

@soyuka I'd like to have your ideas for this.

@mrossard
Copy link
Contributor

I'd love to find time for this too, if @simondaigre can't.

@soyuka
Copy link
Member

soyuka commented Dec 17, 2024

I see 2 solutions:

1. Improve the doctrine listener

For this to work, when an update occurs we shall look at every resources to find if it has that entity as stateOptions::entityClass to know what IRI to generate. This is way too heavy though, and we would need to preload this within the metadata, keep some cache of the tuple DTO <=> Entity that could then be used. This gets quite complicated with relations but it should work.

I'm not a huge fan of this solution.

2. Use a processor

Having a Processor* that gets called on write operations, would be able to send a purge on the IRI that just got modified. It could also loop on the Property Metadata to gather links and send purge requests on them as well.

This is the easiest solution, it has some drawbacks as if you change an entity from a command or through another resource you may end up with stale cache (developers need to be aware and send manual purges for these ?). I like this though as we stay in the "resource" domain and only work around IRIs without caring about the data source.

* this makes me think that we should have a way to execute some processor on kernel.terminate, I think that the ProcessorInterface is appropriate although that we should definitely execute this kind of code after the response has been sent.

@mrossard
Copy link
Contributor

mrossard commented Dec 17, 2024

One option I was wondering about was the possibility to have a manual handling of this with a mechanism similar to the recently added handleLinks option.

It could be something along the lines of "gatherInvalidatedUris()", which would be called by the processor you're mentioning in option 2...this would let devs manage "by hand" their most unusual cases without manually purging themselves (for example i have resourceA with a field computed from resourceB - an aggregate of some sort for example - but resourceA doesn't actually contain resourceB, that would let the devs invalidate the impacted resourceA with custom logic).

@soyuka
Copy link
Member

soyuka commented Dec 18, 2024

Indeed this is a good idea I need to find some time maybe during holidays! We've 4.1 coming out in January this could be a great feature.

@mrossard
Copy link
Contributor

I'd love to find the time for this as it would be much better than what i'm doing right now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants