-
Notifications
You must be signed in to change notification settings - Fork 53
Add Content-type plugin to set content-type header automatically #85
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 1 commit
889f2df
5d43a86
9ef69cf
2d1a885
bc7b746
77d63c4
6162941
e6ba6f1
93c6bf2
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,59 @@ | ||
<?php | ||
|
||
namespace spec\Http\Client\Common\Plugin; | ||
|
||
use PhpSpec\Exception\Example\SkippingException; | ||
use Psr\Http\Message\RequestInterface; | ||
use Psr\Http\Message\StreamInterface; | ||
use PhpSpec\ObjectBehavior; | ||
use Prophecy\Argument; | ||
|
||
class ContentTypePluginSpec extends ObjectBehavior | ||
{ | ||
function it_is_initializable() | ||
{ | ||
$this->shouldHaveType('Http\Client\Common\Plugin\ContentTypePlugin'); | ||
} | ||
|
||
function it_is_a_plugin() | ||
{ | ||
$this->shouldImplement('Http\Client\Common\Plugin'); | ||
} | ||
|
||
function it_adds_json_content_type_header(RequestInterface $request) | ||
{ | ||
$request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); | ||
$request->getBody()->shouldBeCalled()->willReturn(json_encode(['foo' => 'bar'])); | ||
$request->withHeader('Content-Type', 'application/json')->shouldBeCalled()->willReturn($request); | ||
|
||
$this->handleRequest($request, function () {}, function () {}); | ||
} | ||
|
||
function it_adds_xml_content_type_header(RequestInterface $request) | ||
{ | ||
$request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); | ||
$request->getBody()->shouldBeCalled()->willReturn('<foo>bar</foo>'); | ||
$request->withHeader('Content-Type', 'application/xml')->shouldBeCalled()->willReturn($request); | ||
|
||
$this->handleRequest($request, function () {}, function () {}); | ||
} | ||
|
||
function it_does_not_set_content_type_header(RequestInterface $request) | ||
{ | ||
$request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); | ||
$request->getBody()->shouldBeCalled()->willReturn('foo'); | ||
$request->withHeader('Content-Type', null)->shouldNotBeCalled(); | ||
|
||
$this->handleRequest($request, function () {}, function () {}); | ||
} | ||
|
||
function it_does_not_set_content_type_header_if_already_one(RequestInterface $request) | ||
{ | ||
$request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(true); | ||
$request->getBody()->shouldNotBeCalled()->willReturn('foo'); | ||
$request->withHeader('Content-Type', null)->shouldNotBeCalled(); | ||
|
||
$this->handleRequest($request, function () {}, function () {}); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
<?php | ||
|
||
namespace Http\Client\Common\Plugin; | ||
|
||
use Http\Client\Common\Plugin; | ||
use Psr\Http\Message\RequestInterface; | ||
|
||
/** | ||
* Allow to set the correct content type header on the request automatically only if it is not set . | ||
* | ||
* @author Karim Pinchon <[email protected]> | ||
*/ | ||
final class ContentTypePlugin implements Plugin | ||
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. May be nice to add some options to this plugin to avoid detection on too large stream, or stream with a null size |
||
{ | ||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function handleRequest(RequestInterface $request, callable $next, callable $first) | ||
{ | ||
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. We should avoid content type detection if stream is not rewindable |
||
if (!$request->hasHeader('Content-Type')) { | ||
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. We should early return here if the body size of the stream is equal to 0 |
||
$stream = $request->getBody(); | ||
|
||
if ($this->isJson($stream)) { | ||
$request = $request->withHeader('Content-Type', 'application/json'); | ||
|
||
return $next($request); | ||
} | ||
|
||
if ($this->isXml($stream)) { | ||
$request = $request->withHeader('Content-Type', 'application/xml'); | ||
|
||
return $next($request); | ||
} | ||
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. Some content type that can be added to check (but not necessarly in this PR) :
|
||
} | ||
|
||
return $next($request); | ||
} | ||
|
||
/** | ||
* @param $stream | ||
* | ||
* @return bool | ||
*/ | ||
private function isJson($stream) | ||
{ | ||
json_decode($stream); | ||
|
||
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. We should rewind the stream here |
||
return json_last_error() == JSON_ERROR_NONE; | ||
} | ||
|
||
/** | ||
* @param $stream | ||
* | ||
* @return \SimpleXMLElement|false | ||
*/ | ||
private function isXml($stream) | ||
{ | ||
libxml_use_internal_errors(true); | ||
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 we reset to the original value? |
||
|
||
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. We should rewind the stream here |
||
return simplexml_load_string($stream); | ||
} | ||
} |
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.
Can you use a real stream implementation instead of a string ?
I think this test case will fail with a stream implementation as there is no rewind