search

csarrazi / guzzle-cache-middleware

A Guzzle Cache middleware
54 stars 10 forks source link

Adapters ignore cache headers (expires / cache-control) #4

Open firegore opened 6 years ago

firegore commented 6 years ago

The DoctrineAdapter does not check the Cache-Control / Expire Header if the content should be cached at all.

csarrazi commented 6 years ago

Hi @firegore! Currently, no adapter take into account the cache headers. The adapters can only be configured with a default cache duration.

In any case, feel free to make a PR, if you wish this feature implemented! :)

andreicio commented 4 years ago

There's a guzzle middleware that does that, but it has to be manually integrated with the CsaGuzzleBundle yourself, or switch to the bundle it is designed for.

csarrazi commented 4 years ago

Hum, I think it would be interesting to provide a means to have plugins in the bundle itself.

andreicio commented 4 years ago

I'll give it a shot. I'd like to discuss the implementation with you before I start though. I'll send you an email with a proposal.

csarrazi commented 4 years ago

Sure, no problem! Or this can be discussed here if you prefer. I don't really mind ;)

andreicio commented 4 years ago

The package I linked to in my earlier comment is not the middleware itself, but rather a plugin package that integrates the kevinrob/guzzle-cache-middleware in the EightPointsGuzzleBundle. That bundle is another package that integrates guzzle in symfony, offering plugin configuration per client, instead of globally, but sacrifices ease of integration. It is much harder to integrate existing middleware with the 8p solution.

In the case of the CsaGuzzleBundle, using kevinrob's middleware can be done simply by registering the desired services and tagging the middleware appropriately. I could write a simple bundle that would do that and offer a simple configuration, to avoid including it in the main guzzle bundle as another built-in plugin. On the other hand I believe that it would add unneeded code, since, as I said, all that is required is some service declarations. I see now that you would like to simplify support for 3rd party middleware. You already have a readme page on the subject, and to be honest I don't see how you could simplify it further. Let's use this example and see what could be improved, if anything.

csarrazi commented 4 years ago

True. What I'd like to be able to do at some point is to let people configure middleware independently or override middleware configuration for a given client.

One thing to note though is that some things might be a little bit tricky to configure unless these middleware have a factory to initialize them (things like injecting services inside clients).