Skip to content

[Plugin] Add a plugin to record and replay responses #172

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

Closed

Conversation

GaryPEGEOT
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
Documentation TODO
License MIT

What's in this PR?

Add a VCR like plugin to allow storing and replaying response

Why?

Allow user to do functional tests why production-like data and provide same functionality as CsaGuzzleBundle

Example Usage

<?php
//...
/** @var \Http\Client\Common\Plugin\NamingStrategyInterface $strategy */
$strategy = new MyNamingStrategy()
$plugin = new RecordAndReplayPlugin($strategy, 'some/directory/in/vcs');

// Will perform an HTTP request
$res = $plugin->handleRequest($request, $next, $first)->wait();

// Will be fetch from the FS
$mock = $plugin->handleRequest($request, $next, $first)->wait();

assert($res === $mock);

Checklist

  • Updated CHANGELOG.md to describe new feature
  • Documentation pull request created

To Do

  • provide a default naming strategy
  • make PR to the bundle to provide configuration

I sadly have no idea how to write PHPSpec tests, so I writed it with PHPUnit, sorry about that...

@dbu
Copy link
Contributor

dbu commented Mar 18, 2019

cool, this is interesting. and nice to see how simple it is. when comparing to https://github.com/php-vcr/php-vcr/, i guess "cassettes" would be the realm of the naming strategy (and could be added later, having a simple naming strategy would be enough to get started imho)

the one thing that i would want to have is to have control over whether recording is ok or not. maybe we could split the plugins into RecordPlugin and ReplayPlugin? or rather have a flag to the plugin to tell it how to behave? separate plugins can be used in own code, and in bundle config the DI could decide which plugin to register.

the problem when you don't have that is that you don't notice when you have missing fixtures and the CI will keep doing actual requests.


namespace Http\Client\Common\Plugin;

use GuzzleHttp\Psr7;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we read and write requests without a dependency on guzzle? and does that thing handle any response or only guzzle responses?

afaik there is no PSR for (de)serializing responses. if we need to depend on guzzle for it, we should create a separate repository for this plugin that depends on guzzle psr7.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly it only "deserialize" Guzzle Response, but can serialize any PSR 7 compatible Message. Adding a "mode" (record, replay or both) is a good idea, I will add it. Adding a new repo would justify the dependency on Guzzzle PSR7 and Symfony's filesystem as well (And PHPUnit in dev), but how can we proceed to create it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can create an empty repository, then you can move the code to a pull request to that repository. like e.g. https://github.com/php-http/logger-plugin

i suggest the name php-http/vcr-plugin. record-and-replay-plugin is too long, and recorder-plugin is incomplete. when we agree on the name, i can create the repo.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vcr-plugin is fine to me :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turns out the https://github.com/php-http/vcr-plugin repository already exists and jerome started something but abandoned it. @sagikazarmark says he talked with jerome and he does not intend to continue.

please do a pull request on it where you completely replace whats in there. jerome started to implement the whole recording and all, but i find your approach much better as there is less code to maintain.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! I'll start working on this then

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryPEGEOT
Copy link
Author

Closing as it is duplicated.

@GaryPEGEOT GaryPEGEOT closed this Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants