-
-
Notifications
You must be signed in to change notification settings - Fork 57
Initial support #1
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
Changes from 12 commits
999be72
47c6564
6a4ab9b
1ed3bff
1b81ee0
1e3307c
7e7fbf6
43d4c48
bb43a07
656318b
40c2973
a246407
77360d6
6f5815b
5240539
8b4a078
14dac4e
8c3c90e
65261ee
7e44bc2
1e480a9
9127c84
b360068
4d11665
e129154
91a13dc
ef384ae
bcfa698
8a2bd7b
f41f3e4
9c6f180
7d8bdc8
eacce8f
7012db5
b887c36
ca2ea3b
577e633
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
vendor/ | ||
composer.lock | ||
phpunit.xml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
language: php | ||
|
||
sudo: false | ||
|
||
matrix: | ||
include: | ||
- php: 5.3 | ||
- php: 5.4 | ||
- php: 5.5 | ||
- php: 5.6 | ||
- php: 5.3 | ||
env: deps=low | ||
- php: 5.6 | ||
env: deps=high | ||
- php: nightly | ||
- php: hhvm | ||
allow_failures: | ||
- php: nightly | ||
- php: hhvm | ||
fast_finish: true | ||
|
||
env: | ||
global: | ||
- deps=no | ||
- SYMFONY_DEPRECATIONS_HELPER=weak | ||
|
||
before_install: | ||
- composer self-update | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then phpenv config-rm xdebug.ini; fi; | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]] && [ $(php -r "echo PHP_MINOR_VERSION;") -le 4 ]; then echo "extension = apc.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi; | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then (cd src/Symfony/Component/Debug/Resources/ext && phpize && ./configure && make && echo "extension = $(pwd)/modules/symfony_debug.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini); fi; | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then php -i; fi; | ||
# Set the COMPOSER_ROOT_VERSION to the right version according to the branch being built | ||
- if [ "$TRAVIS_BRANCH" = "master" ]; then export COMPOSER_ROOT_VERSION=dev-master; else export COMPOSER_ROOT_VERSION="$TRAVIS_BRANCH".x-dev; fi; | ||
|
||
install: | ||
- if [ "$deps" = "no" ]; then export SYMFONY_DEPRECATIONS_HELPER=strict; fi; | ||
- if [ "$deps" = "no" ]; then composer --prefer-source install; fi; | ||
|
||
script: | ||
- phpunit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Factory; | ||
|
||
use Symfony\Bridge\PsrHttpMessage\PsrHttpMessageFactoryInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Zend\Diactoros\ServerRequest; | ||
use Zend\Diactoros\Response as DiactorosResponse; | ||
|
||
/** | ||
* Builds Psr\HttpMessage instances using the Zend Diactoros implementation. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class DiactorosFactory implements PsrHttpMessageFactoryInterface | ||
{ | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createRequest(Request $symfonyRequest) | ||
{ | ||
return new ServerRequest( | ||
|
||
); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createResponse(Response $symfonyResponse) | ||
{ | ||
return new DiactorosResponse( | ||
|
||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Factory; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Symfony\Bridge\PsrHttpMessage\HttpFoundationFactoryInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
/** | ||
* {@inheritdoc} | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class HttpFoundationFactory implements HttpFoundationFactoryInterface | ||
{ | ||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createRequest(ServerRequestInterface $psrHttpMessageRequest) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rename it to |
||
{ | ||
$parsedBody = $psrHttpMessageRequest->getParsedBody(); | ||
$request = is_array($parsedBody) ? $parsedBody : array(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather name this variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that we have 2 request objects already in this method, so it might be confusing to call the variable The alternative is indeed to reuse the |
||
|
||
return new Request( | ||
$psrHttpMessageRequest->getQueryParams(), | ||
$request, | ||
$psrHttpMessageRequest->getAttributes(), | ||
$psrHttpMessageRequest->getCookieParams(), | ||
array(), // TODO | ||
$psrHttpMessageRequest->getServerParams(), | ||
$psrHttpMessageRequest->getBody()->__toString() | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about headers, which might have been modified in the request compared to server params ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
} | ||
|
||
private function getFiles(array $uploadedFiles) | ||
{ | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createResponse(ResponseInterface $psrHttpMessageResponse) | ||
{ | ||
return new Response( | ||
|
||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to handle the conversion of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I was just looking at that. The issue is that we will need a cookie parser. Should I look for an existing library, create a parser in the bridge or add it to HttpFoundation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guzzle 3 has a cookie parser (https://github.com/Guzzle3/parser/blob/master/Cookie/CookieParser.php) but I couldn't find it in the last version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed (added a custom factory method). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dunglas in case you would like to look @simensen do have something https://github.com/dflydev/dflydev-fig-cookies |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
/** | ||
* Creates Symfony Request and Response from PSR-7 ones. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
interface HttpFoundationFactoryInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we need interfaces here (there is no reason to reimplement the conversion in a different way IMO). And I would rather name them converters than factories There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stof I agree but it was to be consistent with the other side, and the other side needs an interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it actually need one though ? If we want to make it usable with other PSR-7 implementation, we could just accept a constructor callable to instantiate the Request/Response rather than forcing to reimplement the whole conversion (which is not only about the constructor) |
||
{ | ||
/** | ||
* Creates a Symfony Request from a PSR-7 one. | ||
* | ||
* @param ServerRequestInterface $psrHttpMessageRequest | ||
* | ||
* @return Request | ||
*/ | ||
public function createRequest(ServerRequestInterface $psrHttpMessageRequest); | ||
|
||
/** | ||
* Creates a Symfony Response from a PSR-7 one. | ||
* | ||
* @param ResponseInterface $psrHttpMessageResponse | ||
* | ||
* @return Response | ||
*/ | ||
public function createResponse(ResponseInterface $psrHttpMessageResponse); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
/** | ||
* Creates PSR HTTP Request and Response from Symfony ones. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
interface PsrHttpMessageFactoryInterface | ||
{ | ||
/** | ||
* Creates a PSR-7 Request from a Symfony one. | ||
* | ||
* @param Request $symfonyRequest | ||
* | ||
* @return ServerRequestInterface | ||
*/ | ||
public function createRequest(Request $symfonyRequest); | ||
|
||
/** | ||
* Creates a PSR-7 Response from a Symfony one. | ||
* | ||
* @param Response $symfonyResponse | ||
* | ||
* @return ResponseInterface | ||
*/ | ||
public function createResponse(Response $symfonyResponse); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Tests\Factory; | ||
|
||
/** | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class DiactorosFactoryTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function testCreateRequest() | ||
{ | ||
|
||
} | ||
|
||
public function testCreateResponse() | ||
{ | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Tests\Factory; | ||
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; | ||
use Symfony\Bridge\PsrHttpMessage\Tests\Fixtures\ServerRequest; | ||
use Symfony\Bridge\PsrHttpMessage\Tests\Fixtures\Stream; | ||
|
||
/** | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class HttpFoundationFactoryTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
private $factory; | ||
|
||
public function setup() | ||
{ | ||
$this->factory = new HttpFoundationFactory(); | ||
} | ||
|
||
public function testCreateRequest() | ||
{ | ||
$stdClass = new \stdClass(); | ||
$serverRequest = new ServerRequest( | ||
'1.1', | ||
array(), | ||
new Stream('The body'), | ||
'/about/kevin', | ||
'GET', | ||
'http://les-tilleuls.coop/about/kevin', | ||
array('country' => 'France'), | ||
array('city' => 'Lille'), | ||
array('url' => 'http://les-tilleuls.coop'), | ||
array(), | ||
array('url' => 'http://dunglas.fr'), | ||
array('custom' => $stdClass) | ||
); | ||
|
||
$symfonyRequest = $this->factory->createRequest($serverRequest); | ||
|
||
$this->assertEquals('http://les-tilleuls.coop', $symfonyRequest->query->get('url')); | ||
$this->assertEquals('http://dunglas.fr', $symfonyRequest->request->get('url')); | ||
$this->assertEquals($stdClass, $symfonyRequest->attributes->get('custom')); | ||
$this->assertEquals('Lille', $symfonyRequest->cookies->get('city')); | ||
$this->assertEquals('France', $symfonyRequest->server->get('country')); | ||
$this->assertEquals('The body', $symfonyRequest->getContent()); | ||
} | ||
|
||
public function testCreateRequestWithNullParsedBody() | ||
{ | ||
$serverRequest = new ServerRequest( | ||
'1.1', | ||
array(), | ||
new Stream(), | ||
'/', | ||
'GET', | ||
null, | ||
array(), | ||
array(), | ||
array(), | ||
array(), | ||
null, | ||
array() | ||
); | ||
|
||
$this->assertCount(0, $this->factory->createRequest($serverRequest)->request); | ||
} | ||
|
||
public function testCreateRequestWithObjectParsedBody() | ||
{ | ||
$serverRequest = new ServerRequest( | ||
'1.1', | ||
array(), | ||
new Stream(), | ||
'/', | ||
'GET', | ||
null, | ||
array(), | ||
array(), | ||
array(), | ||
array(), | ||
new \stdClass(), | ||
array() | ||
); | ||
|
||
$this->assertCount(0, $this->factory->createRequest($serverRequest)->request); | ||
} | ||
|
||
public function testCreateResponse() | ||
{ | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Tests\Fixtures; | ||
|
||
use Psr\Http\Message\MessageInterface; | ||
use Psr\Http\Message\StreamInterface; | ||
|
||
/** | ||
* Message. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class Message implements MessageInterface | ||
{ | ||
private $version = '1.1'; | ||
private $headers = array(); | ||
private $body; | ||
|
||
public function __construct($version = '1.1', array $headers = array(), StreamInterface $body = null) | ||
{ | ||
$this->version = $version; | ||
$this->headers = $headers; | ||
$this->body = null === $body ? new Stream() : $body; | ||
} | ||
|
||
public function getProtocolVersion() | ||
{ | ||
return $this->version; | ||
} | ||
|
||
public function withProtocolVersion($version) | ||
{ | ||
throw \BadMethodCallException('Not implemented.'); | ||
} | ||
|
||
public function getHeaders() | ||
{ | ||
return $this->headers; | ||
} | ||
|
||
public function hasHeader($name) | ||
{ | ||
return isset($this->headers[$name]); | ||
} | ||
|
||
public function getHeader($name) | ||
{ | ||
return $this->hasHeader($name) ? $this->headers[$name] : array(); | ||
} | ||
|
||
public function getHeaderLine($name) | ||
{ | ||
return $this->hasHeader($name) ? join(',', $this->headers[$name]) : ''; | ||
} | ||
|
||
public function withHeader($name, $value) | ||
{ | ||
throw \BadMethodCallException('Not implemented.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. throw new \BadMethodCallException('Not implemented.'); |
||
} | ||
|
||
public function withAddedHeader($name, $value) | ||
{ | ||
throw \BadMethodCallException('Not implemented.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. throw new \BadMethodCallException('Not implemented.'); |
||
} | ||
|
||
public function withoutHeader($name) | ||
{ | ||
throw \BadMethodCallException('Not implemented.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. throw new \BadMethodCallException('Not implemented.'); |
||
} | ||
|
||
public function getBody() | ||
{ | ||
return $this->body; | ||
} | ||
|
||
public function withBody(StreamInterface $body) | ||
{ | ||
throw \BadMethodCallException('Not implemented.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. throw new \BadMethodCallException('Not implemented.'); |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conversion is incomplete here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cookies are not added as headers in Symfony Response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not always hardcoded: https://github.com/zendframework/zend-diactoros/blob/be067c409377671b8268236443d993374e1987fa/src/Response.php#L160 (which is very similar to what we do in HttpFoundation btw)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed it but still not usable because there is not getter for the status text in the Symfony response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, if we don't convert the status text, it is not a big issue. I don't think it is the most used feature in HttpFoundation (I'm not even sure it is documented, and setting the status code in the constructor does not allow to set the status text).
But we should convert cookies to
Set-Cookie
headers