Skip to content
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

Streaming body parsers foundation #69

Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0e88cc5
Streaming body parsers foundation
WyriHaximus Sep 27, 2016
758ad27
Added documentation to the parser interface and added cancel method t…
WyriHaximus Sep 29, 2016
304611f
Updated the raw body parser to comply wth the new parser interface
WyriHaximus Sep 29, 2016
dee32e3
Updated raw body parser test
WyriHaximus Sep 29, 2016
1ae613f
No body parser
WyriHaximus Sep 29, 2016
2dcee0f
Updated the factory, without the no body parser this PR didn't really…
WyriHaximus Sep 29, 2016
68340bb
No body parser tests
WyriHaximus Sep 29, 2016
d089c56
Changing then to done: https://github.com/reactphp/http/pull/69#discu…
WyriHaximus Oct 25, 2016
75aac07
Mark raw body parser as internal
WyriHaximus Oct 25, 2016
7a1414a
Mark no body parser as internal
WyriHaximus Oct 25, 2016
2c40605
Throw exception when content-length is missing
WyriHaximus Oct 25, 2016
a2083d2
NoBodyParser is gone as it will serve no purpose now that RawBodyPars…
WyriHaximus Oct 27, 2016
0afcef1
Clarified that the body is emitted in chunks
WyriHaximus Oct 27, 2016
8810190
Removed NoBodyParser from factory
WyriHaximus Oct 27, 2016
fe58c44
Transformed RawBodyParser to a parser that emits data chunks as they …
WyriHaximus Oct 27, 2016
5ae429e
Followed @jsor's suggestion to drop the factory method in favour of a…
WyriHaximus Nov 15, 2016
b3436bb
Factory create documentation
WyriHaximus Dec 16, 2016
f18d114
RawBodyParser::cancel docblock
WyriHaximus Dec 16, 2016
e6d5a16
Marked RawBodyParser final, it shouldn't be extended but wrapped/deco…
WyriHaximus Dec 16, 2016
2dacc3d
Added a expected arguments to streaming parser interface
WyriHaximus Dec 16, 2016
76fd4cb
Updated readme with request body parser as example
WyriHaximus Dec 16, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/StreamingBodyParser/Factory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace React\Http\StreamingBodyParser;

use React\Http\Request;

class Factory
{
/**
* @param Request $request
* @return ParserInterface
*/
public static function create(Request $request)
{
$headers = $request->getHeaders();
$headers = array_change_key_case($headers, CASE_LOWER);

if (
!isset($headers['content-length'])
) {
return NoBodyParser::create($request);
}

return RawBodyParser::create($request);
}
}
48 changes: 48 additions & 0 deletions src/StreamingBodyParser/NoBodyParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

namespace React\Http\StreamingBodyParser;

use Evenement\EventEmitterTrait;
use React\Http\Request;

class NoBodyParser implements ParserInterface
{
use EventEmitterTrait;

/**
* @var Request
*/
private $request;

/**
* @param Request $request
* @return ParserInterface
*/
public static function create(Request $request)
{
return new static($request);
}

/**
* @param Request $request
*/
private function __construct(Request $request)
{
$this->request = $request;
$this->request->on('end', [$this, 'end']);
}

public function cancel()
{
$this->request->removeListener('end', [$this, 'end']);
$this->emit('end');
}

/**
* @internal
*/
public function end()
{
$this->emit('end');
}
}
32 changes: 32 additions & 0 deletions src/StreamingBodyParser/ParserInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace React\Http\StreamingBodyParser;

use Evenement\EventEmitterInterface;
use React\Http\Request;

/**
* Parser can emit the following events:
* end - When done or canceled (required)
* body - Raw request body (optional)
* post - Post field (optional)
* file - Uploaded file (optional)
*/
interface ParserInterface extends EventEmitterInterface
{
/**
* Factory method creating the parser.
*
* @param Request $request
* @return ParserInterface
*/
public static function create(Request $request);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the parser implementations are now @internal, i think this method can be removed from the interface and the implementations and simple constructor based instantiantion can be used in Factory.


/**
* Cancel parsing the request body stream.
* Parser will still emit end event to any listeners.
*
* @return void
*/
public function cancel();
}
56 changes: 56 additions & 0 deletions src/StreamingBodyParser/RawBodyParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

namespace React\Http\StreamingBodyParser;

use Evenement\EventEmitterTrait;
use React\Http\Request;
use React\Promise\ExtendedPromiseInterface;

class RawBodyParser implements ParserInterface
{
use EventEmitterTrait;

/**
* @var ExtendedPromiseInterface
*/
private $promise;

/**
* @param Request $request
* @return ParserInterface
*/
public static function create(Request $request)
{
return new static($request);
}

/**
* @param Request $request
*/
private function __construct(Request $request)
{
$headers = $request->getHeaders();
$headers = array_change_key_case($headers, CASE_LOWER);

$this->promise = ContentLengthBufferedSink::createPromise(
$request,
$headers['content-length']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if there is no (or an invalid)content-length header? I know there is a check in the Factory, but since this class isn't marked as internal, i think there should be an additional check here or in create.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marked the class internal (just as NoBodyParser) and throwing an exception in case the header is missing. (In case someone ignores the internal tag.) The value of the header is cast to an integer in the sink forcing it to 0 length in case of an invalid value.

)->then([$this, 'finish']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, this should be done(). Since you're supporting react/promise < ^2.2, i'm not sure if you there should be a method_exists check here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing it to be ^2.2

}

/**
* @param string $buffer
*
* @internal
*/
public function finish($buffer)
{
$this->emit('body', [$buffer]);
$this->emit('end');
}

public function cancel()
{
$this->promise->cancel();
}
}
26 changes: 26 additions & 0 deletions tests/StreamingBodyParser/FactoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace React\Tests\Http\StreamingBodyParser;

use React\Http\StreamingBodyParser\Factory;
use React\Http\Request;
use React\Tests\Http\TestCase;

class FactoryTest extends TestCase
{
public function testContentLength()
{
$request = new Request('POST', 'http://example.com/', [], 1.1, [
'content-length' => 123,
]);
$parser = Factory::create($request);
$this->assertInstanceOf('React\Http\StreamingBodyParser\RawBodyParser', $parser);
}

public function testNoHeaders()
{
$request = new Request('POST', 'http://example.com/');
$parser = Factory::create($request);
$this->assertInstanceOf('React\Http\StreamingBodyParser\NoBodyParser', $parser);
}
}
26 changes: 26 additions & 0 deletions tests/StreamingBodyParser/NoBodyParserTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace React\Tests\Http\StreamingBodyParser;

use React\Http\Request;
use React\Http\StreamingBodyParser\NoBodyParser;
use React\Tests\Http\TestCase;

class NoBodyParserTest extends TestCase
{
public function testRequestEnd()
{
$request = new Request('POST', 'http://example.com/');
$parser = NoBodyParser::create($request);
$parser->on('end', $this->expectCallableOnce());
$request->close();
}

public function testCancelParser()
{
$request = new Request('POST', 'http://example.com/');
$parser = NoBodyParser::create($request);
$parser->on('end', $this->expectCallableOnce());
$parser->cancel();
}
}
24 changes: 24 additions & 0 deletions tests/StreamingBodyParser/RawBodyParserTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace React\Tests\Http\StreamingBodyParser;

use React\Http\StreamingBodyParser\RawBodyParser;
use React\Http\Request;
use React\Tests\Http\TestCase;

class RawBodyParserTest extends TestCase
{
public function testNoContentLength()
{
$body = '';
$request = new Request('POST', 'http://example.com/', [], 1.1, [
'content-length' => 3,
]);
$parser = RawBodyParser::create($request);
$parser->on('body', function ($rawBody) use (&$body) {
$body = $rawBody;
});
$request->emit('data', ['abc']);
$this->assertSame('abc', $body);
}
}