Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feature #26509 [BrowserKit] Avoid nullable values in some Client's me…
…thods (ossinkine)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[BrowserKit] Avoid nullable values in some Client's methods

| Q             | A
| ------------- | ---
| Branch?       | master | Bug fix?      | yes/no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes <!-- please add some, will be required by reviewers -->
| License | MIT

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->
I suggest the some methods in `Client` should not return `null` for simplify usage.
If you are trying to get response from client when `request` method was not called, it seems an exception should be throwed.

Commits
-------

c2c2853 Avoid nullable values in some Client's methods
  • Loading branch information
fabpot committed Mar 15, 2018
2 parents bf120d0 + c2c2853 commit 5511ddc
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 7 deletions.
36 changes: 31 additions & 5 deletions src/Symfony/Component/BrowserKit/Client.php
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\BrowserKit;

use Symfony\Component\BrowserKit\Exception\BadMethodCallException;
use Symfony\Component\DomCrawler\Crawler;
use Symfony\Component\DomCrawler\Link;
use Symfony\Component\DomCrawler\Form;
Expand Down Expand Up @@ -183,20 +184,30 @@ public function getCookieJar()
/**
* Returns the current Crawler instance.
*
* @return Crawler|null A Crawler instance
* @return Crawler A Crawler instance
*/
public function getCrawler()
{
if (null === $this->crawler) {
@trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
// throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
}

return $this->crawler;
}

/**
* Returns the current BrowserKit Response instance.
*
* @return Response|null A BrowserKit Response instance
* @return Response A BrowserKit Response instance
*/
public function getInternalResponse()
{
if (null === $this->internalResponse) {
@trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
// throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
}

return $this->internalResponse;
}

Expand All @@ -206,22 +217,32 @@ public function getInternalResponse()
* The origin response is the response instance that is returned
* by the code that handles requests.
*
* @return object|null A response instance
* @return object A response instance
*
* @see doRequest()
*/
public function getResponse()
{
if (null === $this->response) {
@trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
// throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
}

return $this->response;
}

/**
* Returns the current BrowserKit Request instance.
*
* @return Request|null A BrowserKit Request instance
* @return Request A BrowserKit Request instance
*/
public function getInternalRequest()
{
if (null === $this->internalRequest) {
@trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
// throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
}

return $this->internalRequest;
}

Expand All @@ -231,12 +252,17 @@ public function getInternalRequest()
* The origin request is the request instance that is sent
* to the code that handles requests.
*
* @return object|null A Request instance
* @return object A Request instance
*
* @see doRequest()
*/
public function getRequest()
{
if (null === $this->request) {
@trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
// throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
}

return $this->request;
}

Expand Down
@@ -0,0 +1,16 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\BrowserKit\Exception;

class BadMethodCallException extends \BadMethodCallException
{
}
44 changes: 44 additions & 0 deletions src/Symfony/Component/BrowserKit/Tests/ClientTest.php
Expand Up @@ -94,6 +94,16 @@ public function testGetRequest()
$this->assertEquals('http://example.com/', $client->getRequest()->getUri(), '->getCrawler() returns the Request of the last request');
}

/**
* @group legacy
* @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
*/
public function testGetRequestNull()
{
$client = new TestClient();
$this->assertNull($client->getRequest());
}

public function testGetRequestWithXHR()
{
$client = new TestClient();
Expand Down Expand Up @@ -124,6 +134,16 @@ public function testGetResponse()
$this->assertInstanceOf('Symfony\Component\BrowserKit\Response', $client->getResponse(), '->getCrawler() returns the Response of the last request');
}

/**
* @group legacy
* @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getResponse()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
*/
public function testGetResponseNull()
{
$client = new TestClient();
$this->assertNull($client->getResponse());
}

public function testGetInternalResponse()
{
$client = new TestClient();
Expand All @@ -135,6 +155,16 @@ public function testGetInternalResponse()
$this->assertInstanceOf('Symfony\Component\BrowserKit\Tests\SpecialResponse', $client->getResponse());
}

/**
* @group legacy
* @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getInternalResponse()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
*/
public function testGetInternalResponseNull()
{
$client = new TestClient();
$this->assertNull($client->getInternalResponse());
}

public function testGetContent()
{
$json = '{"jsonrpc":"2.0","method":"echo","id":7,"params":["Hello World"]}';
Expand All @@ -153,6 +183,16 @@ public function testGetCrawler()
$this->assertSame($crawler, $client->getCrawler(), '->getCrawler() returns the Crawler of the last request');
}

/**
* @group legacy
* @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getCrawler()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
*/
public function testGetCrawlerNull()
{
$client = new TestClient();
$this->assertNull($client->getCrawler());
}

public function testRequestHttpHeaders()
{
$client = new TestClient();
Expand Down Expand Up @@ -720,6 +760,10 @@ public function testInternalRequest()
$this->assertInstanceOf('Symfony\Component\BrowserKit\Request', $client->getInternalRequest());
}

/**
* @group legacy
* @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getInternalRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
*/
public function testInternalRequestNull()
{
$client = new TestClient();
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpKernel/Client.php
Expand Up @@ -25,8 +25,8 @@
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @method Request|null getRequest() A Request instance
* @method Response|null getResponse() A Response instance
* @method Request getRequest() A Request instance
* @method Response getResponse() A Response instance
*/
class Client extends BaseClient
{
Expand Down

0 comments on commit 5511ddc

Please sign in to comment.