Add Content-type plugin to set content-type header automatically #85
Conversation
… now just JSON and XML)
| function it_adds_xml_content_type_header(RequestInterface $request) | ||
| { | ||
| $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); | ||
| $request->getBody()->shouldBeCalled()->willReturn('<foo>bar</foo>'); |
There was a problem hiding this comment.
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
src/Plugin/ContentTypePlugin.php
Outdated
| private function isJson($stream) | ||
| { | ||
| json_decode($stream); | ||
|
|
There was a problem hiding this comment.
We should rewind the stream here
src/Plugin/ContentTypePlugin.php
Outdated
| private function isXml($stream) | ||
| { | ||
| libxml_use_internal_errors(true); | ||
|
|
There was a problem hiding this comment.
We should rewind the stream here
| * {@inheritdoc} | ||
| */ | ||
| public function handleRequest(RequestInterface $request, callable $next, callable $first) | ||
| { |
There was a problem hiding this comment.
We should avoid content type detection if stream is not rewindable
src/Plugin/ContentTypePlugin.php
Outdated
| * | ||
| * @author Karim Pinchon <[email protected]> | ||
| */ | ||
| final class ContentTypePlugin implements Plugin |
There was a problem hiding this comment.
May be nice to add some options to this plugin to avoid detection on too large stream, or stream with a null size
| $request = $request->withHeader('Content-Type', 'application/xml'); | ||
|
|
||
| return $next($request); | ||
| } |
There was a problem hiding this comment.
Some content type that can be added to check (but not necessarly in this PR) :
- Image types by using a library image (should be optional IMO)
- Binary type by checking if there is a null character (\0) in the stream (that's how git does to detect binary vs text)
- Text type (inverse of the binary type, imo the default content type we want to add to the stream)
| */ | ||
| public function handleRequest(RequestInterface $request, callable $next, callable $first) | ||
| { | ||
| if (!$request->hasHeader('Content-Type')) { |
There was a problem hiding this comment.
We should early return here if the body size of the stream is equal to 0
src/Plugin/ContentTypePlugin.php
Outdated
| */ | ||
| private function isXml($stream) | ||
| { | ||
| libxml_use_internal_errors(true); |
There was a problem hiding this comment.
Should we reset to the original value?
src/Plugin/ContentTypePlugin.php
Outdated
| return $next($request); | ||
| } | ||
|
|
||
| if ($this->skipDetection && $streamSize >= $this->sizeLimit) { |
|
It should go to "if (0 == $streamSize)" condition.
|
src/Plugin/ContentTypePlugin.php
Outdated
| return $next($request); | ||
| } | ||
|
|
||
| if (0 == $streamSize) { |
There was a problem hiding this comment.
We should use strict comparison and then do an extra one for checking if null.
There was a problem hiding this comment.
Not here, as we skip every stream that has a size equal to null, don't we want to skip only thoses with skipDetection and null size ?
I mean if stream is rewindable, but we cannot determine it's size but still want to auto detect the content type why skipping this ?
There was a problem hiding this comment.
I agree, I made it too quickly.
What's in this PR?
Add a new plugin which allow to detect JSON and XML content in body request and automatically set the right content-type header.
Why?
Set the content-type header is often unpleasing and detect whether the content is JSON or XML is relatively simple so it could save time.
Example Usage
Checklist