core icon indicating copy to clipboard operation
core copied to clipboard

Add Souin as available provider to the existing system

Open darkweak opened this issue 4 years ago • 11 comments

Q A
Branch? main
Tickets #4512
License MIT
Doc PR api-platform/docs#...

Add Souin as an available purger provider instead of the Varnish one to be able to use natively Souin as a cache system inside the Caddy reverse-proxy and refactor the AddTagsListener to be able to customise the HTTP header tag and the separator instead of the forced Cache-Tag from Cloudflare.

darkweak avatar Nov 09 '21 14:11 darkweak

note that the ci is broken we're trying to fix underlying issues

soyuka avatar Nov 09 '21 15:11 soyuka

@soyuka @dunglas this PR is ready for review

darkweak avatar Nov 12 '21 09:11 darkweak

The tests on the main branch break the PHPUnit ones on PRs. Due to the bb2180f0acc091ce62b414bbc80d76971b300e0c commit.

darkweak avatar Nov 12 '21 11:11 darkweak

ping @dunglas to review as we discussed with @soyuka

darkweak avatar Nov 19 '21 15:11 darkweak

@soyuka I reverted the Cache-Tag into Cache-Tags. Can you re-review for the last time? Ty

darkweak avatar Dec 15 '21 23:12 darkweak

Hi there, what about this?

darkweak avatar Mar 03 '22 11:03 darkweak

Hey, is there a Chance this feature will be in 2.7 ?

BeyerJC avatar May 10 '22 05:05 BeyerJC

I don't think because they never answered 🙃

darkweak avatar Jun 14 '22 06:06 darkweak

@soyuka @dunglas Can I have a review please ? Everything is working as expected with tests. It would be a great add-on on the v3 to deal with the caddy native HTTP cache system.

darkweak avatar Jul 04 '22 20:07 darkweak

We're focus on releasing API Platform 3 (which is in beta). We'll review this PR as well as other new features when API Platform 3 will be tagged.

dunglas avatar Jul 05 '22 05:07 dunglas

Ready to review/merge.

darkweak avatar Sep 22 '22 10:09 darkweak

ping @dunglas @soyuka

darkweak avatar Sep 22 '22 17:09 darkweak

Can I have a last review please?

darkweak avatar Oct 15 '22 11:10 darkweak

Nice! A few comments lefts but we're almost good to merge imo! Thanks @darkweak sorry for the delays!

soyuka avatar Nov 05 '22 09:11 soyuka

@soyuka note this discussion: https://github.com/darkweak/souin/issues/277#issuecomment-1327339148

It seems Souin will start passing any request as soon as the Authorization header (or Cookie) is present in the request, it doesn't provide (nor plans to provide) any sort of "user context"-like feature FOS HTTP Cache bundle, which IIRC APIP relied on before.

dkarlovi avatar Nov 25 '22 11:11 dkarlovi

Ready for the merge @soyuka

darkweak avatar Nov 25 '22 19:11 darkweak

@soyuka note this discussion: darkweak/souin#277 (comment)

It seems Souin will start passing any request as soon as the Authorization header (or Cookie) is present in the request, it doesn't provide (nor plans to provide) any sort of "user context"-like feature FOS HTTP Cache bundle, which IIRC APIP relied on before.

Well, then there is no reason that souin should be integrated into APP.

offtopic:

@darkweak would like to consider changing the souin logic?

Thanks!

divine avatar Dec 16 '22 02:12 divine

@divine note this PR https://github.com/darkweak/souin/pull/283

Anyway, TBH the api platform team won't merge this PR I guess.

darkweak avatar Dec 16 '22 07:12 darkweak

merci @darkweak

soyuka avatar Dec 17 '22 11:12 soyuka

I cherry-picked it in my PR just to be a bit less provider specific. souin will have native support from API Platform 3.1 (coming out mid-January) you can try it using dev-main for now.

soyuka avatar Dec 17 '22 11:12 soyuka