-
-
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 13 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,52 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Factory; | ||
|
||
use Symfony\Bridge\PsrHttpMessage\PsrHttpMessageFactoryInterface; | ||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Zend\Diactoros\ServerRequest as DiactorosRequest; | ||
use Zend\Diactoros\Response as DiactorosResponse; | ||
use Zend\Diactoros\UploadedFile as DiactorosUploadedFile; | ||
|
||
/** | ||
* 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 DiactorosRequest( | ||
|
||
); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createResponse(Response $symfonyResponse) | ||
{ | ||
return new DiactorosResponse( | ||
|
||
); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createUploadedFile(UploadedFile $symfonyUploadedFile) | ||
{ | ||
return new DiactorosUploadedFile( | ||
null, | ||
null, | ||
null | ||
); | ||
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 conversion is incomplete here:
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.
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. Ok cookies are not added as headers in Symfony 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. 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 commentThe 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 commentThe 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). |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Factory; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Psr\Http\Message\UploadedFileInterface; | ||
use Symfony\Bridge\PsrHttpMessage\HttpFoundationFactoryInterface; | ||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
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(), | ||
$this->getFiles($psrHttpMessageRequest->getUploadedFiles()), | ||
$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. |
||
} | ||
|
||
/** | ||
* Converts to the input array to $_FILES structure. | ||
* | ||
* @param array $uploadedFiles | ||
* | ||
* @return array | ||
*/ | ||
private function getFiles(array $uploadedFiles) | ||
{ | ||
$files = array(); | ||
|
||
foreach ($uploadedFiles as $key => $value) { | ||
if ($value instanceof UploadedFileInterface) { | ||
$files[$key] = $this->createUploadedFile($value); | ||
} else { | ||
$files[$key] = $this->getFiles($value); | ||
} | ||
} | ||
|
||
return $files; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createUploadedFile(UploadedFileInterface $psrUploadedFile) | ||
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. should be private too IMO (same reasons than for the other way) |
||
{ | ||
$tempnam = $this->getTemporaryPath(); | ||
$psrUploadedFile->moveTo($tempnam); | ||
|
||
$clientFileName = $psrUploadedFile->getClientFilename(); | ||
|
||
return new UploadedFile( | ||
$tempnam, | ||
null === $clientFileName ? '' : $clientFileName, | ||
$psrUploadedFile->getClientMediaType(), | ||
$psrUploadedFile->getSize(), | ||
$psrUploadedFile->getError(), | ||
true | ||
); | ||
} | ||
|
||
/** | ||
* Gets a temporary file path. | ||
* | ||
* @return string | ||
*/ | ||
protected function getTemporaryPath() | ||
{ | ||
return tempnam(sys_get_temp_dir(), uniqid('symfony', true)); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createResponse(ResponseInterface $psrHttpMessageResponse) | ||
{ | ||
return new Response( | ||
|
||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Psr\Http\Message\UploadedFileInterface; | ||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
/** | ||
* Creates Symfony Request and Response instances 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 instance from a PSR-7 one. | ||
* | ||
* @param ServerRequestInterface $psrHttpMessageRequest | ||
* | ||
* @return Request | ||
*/ | ||
public function createRequest(ServerRequestInterface $psrHttpMessageRequest); | ||
|
||
/** | ||
* Creates Symfony UploadedFile instance from PSR-7 ones. | ||
* | ||
* @param UploadedFileInterface $psrUploadedFile | ||
* | ||
* @return UploadedFile | ||
*/ | ||
public function createUploadedFile(UploadedFileInterface $psrUploadedFile); | ||
|
||
/** | ||
* Creates a Symfony Response instance 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,45 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage; | ||
|
||
use Psr\Http\Message\ServerRequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Psr\Http\Message\UploadedFileInterface; | ||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
/** | ||
* Creates PSR HTTP Request and Response instances from Symfony ones. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
interface PsrHttpMessageFactoryInterface | ||
{ | ||
/** | ||
* Creates a PSR-7 Request instance from a Symfony one. | ||
* | ||
* @param Request $symfonyRequest | ||
* | ||
* @return ServerRequestInterface | ||
*/ | ||
public function createRequest(Request $symfonyRequest); | ||
|
||
/** | ||
* Creates a PSR-7 UploadedFile instance from a Symfony one. | ||
* | ||
* @param UploadedFile $symfonyUploadedFile | ||
* | ||
* @return UploadedFileInterface | ||
*/ | ||
public function createUploadedFile(UploadedFile $symfonyUploadedFile); | ||
|
||
/** | ||
* Creates a PSR-7 Response instance 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() | ||
{ | ||
|
||
} | ||
} |
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.
I don't think this should be public
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.
Don't you think it can be useful in many cases to convert from Symfony to PSR UploadedFile and vice versa?
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.
Well, we could make it public later if we get requests about it. But for now, I would keep it private until we get more feedback (increasing the visibility of a method is easy. Decreasing it is forbidden outside major versions, and it is still a pain for major versions if we want to provide an easy upgrade)