Dispatch before playback event for each http interaction in cassette

[tylercollier]

Fix for #101.

As I mention in this comment, I followed Ruby VCR's lead (and aaa2000's suggestion) and dispatch the "before playback" event before each HTTP interaction in the cassette.

I did not do change the 'after playback' event, since there should be one anyway.

I did end up moving the dispatch into the configuration, a la Ruby's VCR. This means the dispatch method needs to be public.

[aaa2000]

I think that the request of the BeforePlaybackEvent should be the storedRequest.

[tylercollier]

Yes, thanks!

[adri]

Thanks. I think its all good except that dispatching events happens in the configuration. Is this only for convenience? Why not create a event dispatcher class which is injected into the classes which need it?
I don't think that dispatching events and configuration belong together.

[adri]

Well it's ok for now. Can you please rebase?

[tylercollier]

@adri, I dispatched from configuration for convenience, like you said. I believe this is how I saw it in Ruby.

When you ask me to rebase, do you mean squash my two commits into one? Or do you mean rebase the branch onto master?

[adri]

Hey Tyler,

Ah yeah I meant rebasing the branch on master. At the moment I can't merge automatically :-)
I meanwhile fixed the test setup, the PHP 7 tests failures don't let the build fail now. So we can see after the rebase if everything builds nicely.

[tylercollier]

You already merged aaa2000's pull request, right? My pull request is incompatible. We wouldn't want both implementations. I gave you push access to my fork in case I'm misunderstanding and you want to take care of it quickly yourself.

[adri]

Ok I understand. Should the VCR_BEFORE_PLAYBACK event really be issued for each stored request in the cassette? I would expect it is called only once with just a request like in @aaa2000 implementation.

[adri]

In Ruby it is https://www.relishapp.com/vcr/vcr/v/2-9-1/docs/hooks/before-playback-hook

Your block should accept up to 2 arguments. The first argument will be
the HTTP interaction that is about to be used for play back. The second
argument will be the current cassette.

I read it as it is issuing only one event, am I wrong?

[tylercollier]

It's up to us I suppose, but that's how it's done in Ruby's VCR. I was just trusting that they did that for a good reason, and not an arbitrary one. But I admit I didn't do research to verify, and nothing jumps out at me as to do it one way or the other.

I read it as it is issuing only one event, am I wrong?

I think this sentence: The first argument will be the HTTP interaction that is about to be used for play back. means it will be for each request in the cassette. See the code.

[aaa2000]

I prefer @tylercollier implementation because my PR dispatches the VCR_BEFORE_PLAYBACK and VCR_AFTER_PLAYBACK event when the cassette has no response. I think that it breaks the VCRBundle https://github.com/php-vcr/VCRBundle/blob/master/EventListener/PlaybackListener.php#L26

[tylercollier]

@adri Did you have a chance to look at this?

We just implemented a deployment strategy which requires all tests to pass before it will deploy, so my co-workers and I are depending on this change. We will use it to ignore HTTP headers with PHP/Curl version differences.

[adri]

Not yet unfortunately. I hope I will soon.

We will use it to ignore HTTP headers with PHP/Curl version differences.

This should be possible with using custom request matchers. You can disable the headers request matcher and add your own if you would like to test for some specific headers. See http://php-vcr.github.io/documentation/configuration/.

[tylercollier]

Here's what I did in the meantime:

// Each developer will have different versions of PHP, Curl, and
// perhaps other libraries. The library versions are often included in API
// requests, meaning that VCR would create a cassette for one developer
// differently than another, at least for the User-Agent header. To resolve
// this, we'll remove the User-Agent header.
\VCR\VCR::configure()->addRequestMatcher(
    'modified_headers_matcher',
    function (\VCR\Request $first, \VCR\Request $second) {
        // In the following, we are blacklisting the 'User-Agent' key.
        $firstHeaders = array_diff_key($first->getHeaders(), array_flip(['User-Agent']));
        $secondHeaders = array_diff_key($second->getHeaders(), array_flip(['User-Agent']));
        return array_filter($firstHeaders) === array_filter($secondHeaders);
    }
);
\VCR\VCR::configure()->enableRequestMatchers(['method', 'url', 'host', 'body', 'post_fields', 'query_string', 'modified_headers_matcher']);

[adri]

Closing in favour of #127.