Skip to content

Commit

Permalink
Make sure we always pass required parameters. (#328)
Browse files Browse the repository at this point in the history
* Make sure we always pass required parameters.

* Minor fixes

* typo

* Send second parameter to Browers

* Make sure to give all clients a response factory

* Do not use deprecated code

* cs
  • Loading branch information
Nyholm committed Aug 1, 2018
1 parent a68ddeb commit f091eff
Show file tree
Hide file tree
Showing 18 changed files with 78 additions and 57 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@

The change log shows what have been Added, Changed, Deprecated and Removed between versions.

## 0.18.0

### Changed

- It is now mandatory to pass a client to the `Browser`'s constructor.

### Deprecated

- Not passing a RequestFactory to `Browser`.
- Not passing a ResponseFactory to the client's constructor.

## 0.17.2

### Changed
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ composer require kriswallsmith/buzz
This page will just show you the basics, please [read the full documentation](doc/index.md).

```php
$browser = new Buzz\Browser();
$client = new Buzz\Client\FileGetContents([], new Psr17ResponseFactory());
$browser = new Buzz\Browser($client, new Psr17RequestFactory());
$response = $browser->get('http://www.google.com');

echo $browser->getLastRequest()."\n";
Expand All @@ -41,7 +42,7 @@ You can also use the low-level HTTP classes directly.
```php
$request = new PSR7Request('GET', 'https://google.com/foo');

$client = new Buzz\Client\FileGetContents();
$client = new Buzz\Client\FileGetContents([], new Psr17ResponseFactory());
$response = $client->send($request, ['timeout' => 4]);

echo $response->getStatusCode();
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
"psr/http-message": "^1.0",
"psr/http-client": "^0.1",
"php-http/httplug": "^1.1",
"nyholm/psr7": "^0.3",
"nyholm/psr7": "^1.0@dev",
"symfony/options-resolver": "^3.4 || ^4.0",
"http-interop/http-factory": "^0.3"
"psr/http-factory": "^1.0"
},
"require-dev": {
"friendsofphp/php-cs-fixer": "^2.11",
Expand Down
2 changes: 1 addition & 1 deletion doc/client.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ There are 3 clients: `FileGetContents`, `Curl` and `MultiCurl`.
```php
$request = new PSR7Request('GET', 'https://example.com');

$client = new Buzz\Client\FileGetContents(['allow_redirects'=>true]);
$client = new Buzz\Client\FileGetContents(['allow_redirects' => true], new Psr17ResponseFactory());
$response = $client->send($request, ['timeout' => 4]);
```

Expand Down
4 changes: 3 additions & 1 deletion doc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ When a `Browser` in constructed you have to select a [Client](/doc/client.md) to
to use the Bowser:

```php
$browser = new Buzz\Browser();
$client = new Buzz\Client\FileGetContents([], new Psr17ResponseFactory());
$browser = new Buzz\Browser($client, new Psr17RequestFactory());

$response = $browser->get('https://example.com');
$response = $browser->get('https://example.com', ['User-Agent'=>'Buzz']);
$response = $browser->post('https://example.com', ['User-Agent'=>'Buzz'], 'http-post-body');
Expand Down
26 changes: 10 additions & 16 deletions lib/Browser.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
namespace Buzz;

use Buzz\Client\BuzzClientInterface;
use Buzz\Client\FileGetContents;
use Buzz\Exception\ClientException;
use Buzz\Exception\InvalidArgumentException;
use Buzz\Exception\LogicException;
use Buzz\Middleware\MiddlewareInterface;
use Http\Message\RequestFactory;
use Http\Message\ResponseFactory;
use Interop\Http\Factory\RequestFactoryInterface;
use Interop\Http\Factory\ResponseFactoryInterface;
use Nyholm\Psr7\Factory\MessageFactory;
use Nyholm\Psr7\Factory\Psr17Factory;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

Expand All @@ -38,22 +35,19 @@ class Browser implements BuzzClientInterface
private $lastResponse;

/**
* @param BuzzClientInterface|null $client
* @param RequestFactoryInterface|RequestFactory|null $requestFactory
* @param ResponseFactoryInterface|ResponseFactory|null $responseFactory To change the default response factory for FileGetContents
* @param BuzzClientInterface $client
* @param RequestFactoryInterface|RequestFactory|null $requestFactory
*/
public function __construct(
BuzzClientInterface $client = null,
$requestFactory = null,
$responseFactory = null
) {
$this->client = $client ?: new FileGetContents([], $responseFactory ?: new MessageFactory());

public function __construct(BuzzClientInterface $client, $requestFactory = null)
{
if (null === $requestFactory) {
$requestFactory = new MessageFactory();
@trigger_error('Not passing a RequestFactory to Browser constructor is deprecated.', E_USER_DEPRECATED);
$requestFactory = new Psr17Factory();
} elseif (!$requestFactory instanceof RequestFactoryInterface && !$requestFactory instanceof RequestFactory) {
throw new InvalidArgumentException('$requestFactory not a valid RequestFactory');
}

$this->client = $client;
$this->requestFactory = $requestFactory;
}

Expand Down
10 changes: 5 additions & 5 deletions lib/Client/AbstractClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use Buzz\Configuration\ParameterBag;
use Buzz\Exception\InvalidArgumentException;
use Http\Message\ResponseFactory;
use Interop\Http\Factory\ResponseFactoryInterface;
use Nyholm\Psr7\Factory\MessageFactory;
use Nyholm\Psr7\Factory\Psr17Factory;
use Psr\Http\Message\ResponseFactoryInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

abstract class AbstractClient
Expand All @@ -33,9 +33,9 @@ public function __construct(array $options = [], $responseFactory = null)
$this->options = new ParameterBag();
$this->options = $this->doValidateOptions($options);
if (null === $responseFactory) {
$responseFactory = new MessageFactory();
}
if (!$responseFactory instanceof ResponseFactoryInterface && !$responseFactory instanceof ResponseFactory) {
@trigger_error('Not passing a ResponseFactory to Buzz client constructor is deprecated.', E_USER_DEPRECATED);
$responseFactory = new Psr17Factory();
} elseif (!$responseFactory instanceof ResponseFactoryInterface && !$responseFactory instanceof ResponseFactory) {
throw new InvalidArgumentException('$responseFactory not a valid ResponseFactory');
}
$this->responseFactory = $responseFactory;
Expand Down
6 changes: 3 additions & 3 deletions lib/Message/ResponseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use Buzz\Exception\InvalidArgumentException;
use Http\Message\ResponseFactory as HTTPlugResponseFactory;
use Interop\Http\Factory\ResponseFactoryInterface as InteropResponseFactory;
use Psr\Http\Message\ResponseFactoryInterface as PsrResponseFactory;
use Psr\Http\Message\ResponseInterface;

/**
Expand All @@ -20,11 +20,11 @@ class ResponseBuilder
private $response;

/**
* @param HTTPlugResponseFactory|InteropResponseFactory $responseFactory
* @param HTTPlugResponseFactory|PsrResponseFactory $responseFactory
*/
public function __construct($responseFactory)
{
if (!$responseFactory instanceof HTTPlugResponseFactory && !$responseFactory instanceof InteropResponseFactory) {
if (!$responseFactory instanceof HTTPlugResponseFactory && !$responseFactory instanceof PsrResponseFactory) {
throw new InvalidArgumentException('First parameter to ResponseBuilder must be a response factory');
}

Expand Down
12 changes: 7 additions & 5 deletions tests/Functional/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Buzz\Client\FileGetContents;
use Buzz\Client\MultiCurl;
use Buzz\Exception\InvalidArgumentException;
use Nyholm\Psr7\Factory\Psr17Factory;
use Nyholm\Psr7\Request;
use Nyholm\Psr7\Response;
use PHPUnit\Framework\TestCase;
Expand All @@ -23,6 +24,7 @@ public function testBrowserPassingOption()
$options = ['foobar' => true, 'timeout' => 4];

$client = $this->getMockBuilder(BuzzClientInterface::class)
->disableOriginalConstructor()
->setMethods(['sendRequest'])
->getMock();

Expand All @@ -31,7 +33,7 @@ public function testBrowserPassingOption()
->with($this->anything(), $this->equalTo($options))
->willReturn(new Response());

$browser = new Browser($client);
$browser = new Browser($client, new Psr17Factory());
$browser->sendRequest($request, $options);
}

Expand All @@ -40,7 +42,7 @@ public function testBrowserPassingOption()
*/
public function testOptionInConstructor($class)
{
$client = new $class(['timeout' => 4]);
$client = new $class(['timeout' => 4], new Psr17Factory());
$this->assertInstanceOf($class, $client);
}

Expand All @@ -53,7 +55,7 @@ public function testOptionInSendRequest($class)
$this->markTestSkipped('The test server is not configured.');
}

$client = new $class();
$client = new $class([], new Psr17Factory());

$response = $client->sendRequest(new Request('GET', $_SERVER['BUZZ_TEST_SERVER']), ['timeout' => 4]);
$this->assertInstanceOf(ResponseInterface::class, $response);
Expand All @@ -65,7 +67,7 @@ public function testOptionInSendRequest($class)
public function testWrongOptionInConstructor($class)
{
$this->expectException(InvalidArgumentException::class);
new $class(['foobar' => true]);
new $class(['foobar' => true], new Psr17Factory());
}

/**
Expand All @@ -74,7 +76,7 @@ public function testWrongOptionInConstructor($class)
public function testWrongOptionInSendRequest($class)
{
$this->expectException(InvalidArgumentException::class);
$client = new $class();
$client = new $class([], new Psr17Factory());

$client->sendRequest(new Request('GET', '/'), ['foobar' => true]);
}
Expand Down
9 changes: 5 additions & 4 deletions tests/Functional/MiddlewareChainTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Buzz\Client\AbstractClient;
use Buzz\Middleware\MiddlewareInterface;
use Http\Client\Tests\PHPUnitUtility;
use Nyholm\Psr7\Factory\Psr17Factory;
use Nyholm\Psr7\Request;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
Expand All @@ -26,7 +27,7 @@ public function testChainOrder(AbstractClient $client)
MyMiddleware::$hasBeenHandled = false;
MyMiddleware::$handleCount = 0;

$browser = new Browser($client);
$browser = new Browser($client, new Psr17Factory());
$browser->addMiddleware(new MyMiddleware(
function () {
++MyMiddleware::$handleCount;
Expand Down Expand Up @@ -69,9 +70,9 @@ function () {
public function getHttpClients()
{
return [
[new \Buzz\Client\MultiCurl()],
[new \Buzz\Client\FileGetContents()],
[new \Buzz\Client\Curl()],
[new \Buzz\Client\MultiCurl([], new Psr17Factory())],
[new \Buzz\Client\FileGetContents([], new Psr17Factory())],
[new \Buzz\Client\Curl([], new Psr17Factory())],
];
}
}
Expand Down
5 changes: 3 additions & 2 deletions tests/Integration/BrowserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

use Buzz\Browser;
use Buzz\Client\FileGetContents;
use Nyholm\Psr7\Factory\Psr17Factory;

class BrowserIntegrationTest extends BaseIntegrationTest
{
protected function createHttpAdapter()
{
$client = new FileGetContents();
$browser = new Browser($client);
$client = new FileGetContents([], new Psr17Factory());
$browser = new Browser($client, new Psr17Factory());

return $browser;
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Integration/CurlIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
namespace Buzz\Test\Integration;

use Buzz\Client\Curl;
use Nyholm\Psr7\Factory\Psr17Factory;

class CurlIntegrationTest extends BaseIntegrationTest
{
protected function createHttpAdapter()
{
$client = new Curl();
$client = new Curl([], new Psr17Factory());

return $client;
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Integration/FileGetContentsIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
namespace Buzz\Test\Integration;

use Buzz\Client\FileGetContents;
use Nyholm\Psr7\Factory\Psr17Factory;

class FileGetContentsIntegrationTest extends BaseIntegrationTest
{
protected function createHttpAdapter()
{
$client = new FileGetContents();
$client = new FileGetContents([], new Psr17Factory());

return $client;
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Integration/MultiCurlIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
namespace Buzz\Test\Integration;

use Buzz\Client\MultiCurl;
use Nyholm\Psr7\Factory\Psr17Factory;

class MultiCurlIntegrationTest extends BaseIntegrationTest
{
protected function createHttpAdapter()
{
$client = new MultiCurl();
$client = new MultiCurl([], new Psr17Factory());

return $client;
}
Expand Down
11 changes: 7 additions & 4 deletions tests/Unit/BrowserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Buzz\Browser;
use Buzz\Client\Curl;
use Nyholm\Psr7\Factory\Psr17Factory;
use Nyholm\Psr7\Request;
use Nyholm\Psr7\Response;
use PHPUnit\Framework\TestCase;
Expand All @@ -21,9 +22,11 @@ class BrowserTest extends TestCase

protected function setUp()
{
$this->client = $this->getMockBuilder('Buzz\Client\Curl')->getMock();
$this->client = $this->getMockBuilder('Buzz\Client\Curl')
->disableOriginalConstructor()
->getMock();

$this->browser = new Browser($this->client);
$this->browser = new Browser($this->client, new Psr17Factory());
}

/**
Expand Down Expand Up @@ -73,8 +76,8 @@ public function testLastMessages()

public function testGetClient()
{
$client = new Curl();
$browser = new Browser($client);
$client = new Curl([], new Psr17Factory());
$browser = new Browser($client, new Psr17Factory());
$this->assertSame($client, $browser->getClient());
}

Expand Down
3 changes: 2 additions & 1 deletion tests/Unit/Client/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Buzz\Client\FileGetContents;
use Buzz\Client\MultiCurl;
use Buzz\Exception\ClientException;
use Nyholm\Psr7\Factory\Psr17Factory;
use Nyholm\Psr7\Request;
use PHPUnit\Framework\TestCase;

Expand All @@ -24,7 +25,7 @@ public function testSendToInvalidUrl($host, $client)
$request = new Request('GET', 'http://'.$host.':12345');

/** @var BuzzClientInterface $client */
$client = new $client();
$client = new $client([], new Psr17Factory());
$client->sendRequest($request, ['timeout' => 0.1]);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/Unit/Client/FileGetContentsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Buzz\Client\FileGetContents;
use Buzz\Configuration\ParameterBag;
use Nyholm\Psr7\Factory\Psr17Factory;
use Nyholm\Psr7\Request;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
Expand All @@ -27,7 +28,7 @@ public function testConvertsARequestToAContextArray()
'Content-Length' => '15',
], 'foo=bar&bar=baz');

$client = new StreamClient();
$client = new StreamClient([], new Psr17Factory());
$expected = [
'http' => [
'method' => 'POST',
Expand Down
Loading

0 comments on commit f091eff

Please sign in to comment.