Add Fastly support

[andrerom]

Closes #403

Changes:

• Use batch tag purging to reduce risk of hitting API limits
• Support PurgeCapable
• Drop multi services, does not seems to be a good fit for Purge, & unsure what the use case is
• Added unit testing

Follow Up topics:

• Add support in Bundle
• Abstract Reverse proxy TTL to use Surrogate Control on Fastly and optionally others Use Surrogate-Control across all backends instead of custom X-Reverse-Proxy-TTL #455

Context of this:

• eZ Platform (v3) is being worked on by @ViniTou to remove several of our HttpCache abstractions and move to FosHttpCache 2.x, which means we among other things we'd like Fastly support on FosHttpCache level instead ;)

[dbu]

thanks for picking this one up again. great if we can get this in!

while having some demo or something to validate that it works with the actual api, i agree that the automated tests should not talk with actual fastly. could you use the MockClient of httplug to expect requests and return canned responses so we can do a functional test within our application boundary?

[dbu]

I suggest that we do some functional tests with the httplug mock client and/or the vcr-plugin. That can at least ensure we know what we send out, and "documents" what we expect as interaction with fastly. With php-vcr it would be doable to switch tests to talk to an actual fastly account with credentials and check if it really works.

[andrerom]

I suggest that we do some functional tests with the httplug mock client
and/or the vcr-plugin.

Any existing FOSHttpCache examples of that?

[dbu]

no such examples, so far we have only unit tests and full integration tests (we install nginx and varnish in travis-ci).
you can set the vcr plugin to record mode and run it with an actual fastly, to have it generate the fixtures. maybe we can use a constant that can be set to activate passthrough mode and configuration for a fastly account so that phpunit.xml can be used to locally run tests against an actual fastly instance, while using the fixtures in the ci.

[andrerom]

@dbu / @Toflar Unit tests added, so I'm considering this to be more or less ready.

Only thing I'm considering from my POV is adding refresh() using two requests, one to soft purge the url and then second one to load it. WDYT?

[Toflar]

Only thing I'm considering from my POV is adding refresh() using two requests, one to soft purge the url and then second one to load it. WDYT?

I did that in my LiteSpeed PR as well so if it works, why not? 😄

[dbu]

sure, if we can't do it in one then that sounds fine. with varnish / symfony you can do a request forcing a backend lookup, but when that is not available, doing 2 requests seems fine to me. needs to be sure there is no race condition though (doing the request before the cache got purged)

[andrerom]

Added.

@vvuksan Would you be able to verify if what's added here* successfully will trigger a refresh of a url? Or if you would recommend another way? e.g undocumented API or something ;)

* TL;DR;

1. soft PURGE of url
2. HEAD request for url

[andrerom]

@dbu / @Toflar Ready for review afaik.

[dbu]

build errors should go away once symfony/symfony#32802 is tagged

[dbu]

nice! this seems reasonable uncomplicated

[dbu]

oh, and link to fastly docs is of course a good idea!

can you please also add an entry in the changelog and add info in the documentation? https://github.com/FriendsOfSymfony/FOSHttpCache/tree/master/doc

i'd squash all commits, even though that will mash together the git attribution to just one of you. hope you don't mind? otherwise i squash to 2 commits to keep your things separate.

@Toflar do you see anything else or should i merge when we have the documentation? i plan a new minor version once this is released.

[Toflar]

@Toflar do you see anything else or should i merge when we have the documentation? i plan a new minor version once this is released.

Just a minor CS nitpick, the rest looks very simple and straightforward to me. Great addition because it requires zero proxy setup. I wish we had the same for KeyCDN :)
Should definitely be merged once the docs are added. Well done, @andrerom!

[andrerom]

Fixed a few things, notable:

• Please use GH suggest feature so we can speed up here ;)
• Realized that I was missing API endpoint for invalidation() and clear() => So unless unclear: There has been no functional tests here yet, that is a must before release. Contribution on that would be great, otherwise I'll need to check internally who might be able to do that.

[dbu]

very cool! this is exactly what such documentation should help people with, remove wtf moments 👍

[dbu]

Please use GH suggest feature so we can speed up here ;)

oh, will try that next time, seems nice.

Realized that I was missing API endpoint for invalidation() and clear() => So unless unclear: There has been no functional tests here yet, that is a must before release. Contribution on that would be great, otherwise I'll need to check internally who might be able to do that.

i never used fastly. maybe @hpatoio wants to try to do that?

[hpatoio]

i never used fastly. maybe @hpatoio wants to try to do that?

Ciao. Cool you finalized my PR. We are not using this bundle anymore (went for a different solution) but I'm happy to help. What do you exactly need ? Sorry but the conversation in the PR is a bit long and a recap could help

[dbu]

hey hpatoio, good to see you (even if just virtually) - andré said that there are some tests missing. @andrerom can you explain in more detail what tests you would want? end-to-end tests with an actual fastly seems difficult to achieve to me...

[andrerom]

end-to-end tests with an actual fastly seems difficult to achieve to me...

this is what I had in mind

[hpatoio]

end-to-end tests with an actual fastly seems difficult to achieve to me...

Got it. For NGINX/Varnish we create services usign the .sh and the we run the e2e tests. For Fastly you wanna do the same but using a real Fastly service. Going to figure out something.

[dbu]

can we wrap this up somehow? i'd be ok to merge without functional test and add a warning in the doc that fastly support is experimental.

[Toflar]

To be fair I think there's nothing else we can do other than checking that the API requests are correct. We don't need to functional test the API of Fastly, that's their responsiblity. So imho this doesn't need to be experimental :)

[andrerom]

Ok with me, rebased and pushed.

[dbu]

@Toflar well, without an end-to-end test we can't be sure that our assumptions about the fastly api are correct. but yeah, utlimately that is up to everybody building an application with these components, as otherwise we still can't be sure if it works. and really would not want to run something that talks to fastly in the FOSHttpCache travis-ci setup.

@andrerom can you please apply the cs fixer thingies? the travis failure is "only" the python sphinx installation to test the doc. that is a known problem and not related to your changes.

[andrerom]

@dbu CS fixes and change to use request body has been pushed.

[Toflar]

Found a minor docs formatting issue but I would love to see this merged as is 👍

[dbu]

hooray!

[dbu]

thanks a lot for your persistence!

[Toflar]

Thanks André - great stuff!