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

Add support for caching the underlying stream #124

Merged
merged 4 commits into from
Aug 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ dist: trusty
matrix:
include:
- php: 7.1
env: ANALYSIS='true'
- php: 7.2
- php: 7.3
env: ANALYSIS='true'
- php: nightly
allow_failures:
- php: nightly
Expand Down
7 changes: 6 additions & 1 deletion src/Factory/ServerRequestFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Slim\Psr7\Cookies;
use Slim\Psr7\Headers;
use Slim\Psr7\Request;
use Slim\Psr7\Stream;
use Slim\Psr7\UploadedFile;

class ServerRequestFactory implements ServerRequestFactoryInterface
Expand Down Expand Up @@ -90,7 +91,11 @@ public static function createFromGlobals(): Request
$headers = Headers::createFromGlobals();
$cookies = Cookies::parseHeader($headers->getHeader('Cookie', []));

$body = (new StreamFactory())->createStreamFromFile('php://input');
// Cache the php://input stream as it cannot be re-read
$cacheResource = fopen('php://temp', 'wb+');
$cache = $cacheResource ? new Stream($cacheResource) : null;

$body = (new StreamFactory())->createStreamFromFile('php://input', 'r', $cache);
$uploadedFiles = UploadedFile::createFromGlobals($_SERVER);

$request = new Request($method, $uri, $headers, $cookies, $_SERVER, $body, $uploadedFiles);
Expand Down
13 changes: 8 additions & 5 deletions src/Factory/StreamFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ public function createStream(string $content = ''): StreamInterface
/**
* {@inheritdoc}
*/
public function createStreamFromFile(string $filename, string $mode = 'r'): StreamInterface
{
public function createStreamFromFile(
string $filename,
string $mode = 'r',
StreamInterface $cache = null
): StreamInterface {
$resource = fopen($filename, $mode);

if (!is_resource($resource)) {
Expand All @@ -49,20 +52,20 @@ public function createStreamFromFile(string $filename, string $mode = 'r'): Stre
);
}

return new Stream($resource);
return new Stream($resource, $cache);
}

/**
* {@inheritdoc}
*/
public function createStreamFromResource($resource): StreamInterface
public function createStreamFromResource($resource, StreamInterface $cache = null): StreamInterface
{
if (!is_resource($resource)) {
throw new InvalidArgumentException(
'Parameter 1 of StreamFactory::createStreamFromResource() must be a resource.'
);
}

return new Stream($resource);
return new Stream($resource, $cache);
}
}
42 changes: 40 additions & 2 deletions src/Stream.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,29 @@ class Stream implements StreamInterface
protected $isPipe;

/**
* @param resource $stream A PHP resource handle.
* @var bool
*/
protected $finished;

/**
* @var StreamInterface | null
*/
protected $cache;

/**
* @param resource $stream A PHP resource handle.
* @param StreamInterface $cache A stream to cache $stream (useful for non-seekable streams)
*
* @throws InvalidArgumentException If argument is not a resource.
*/
public function __construct($stream)
public function __construct($stream, StreamInterface $cache = null)
{
$this->attach($stream);

if ($cache && (!$cache->isSeekable() || !$cache->isWritable())) {
throw new RuntimeException('Cache stream must be seekable and writable');
}
$this->cache = $cache;
}

/**
Expand Down Expand Up @@ -137,6 +153,11 @@ public function __toString(): string
return '';
}

if ($this->cache && $this->finished) {
$this->cache->rewind();
return $this->cache->getContents();
}

try {
$this->rewind();
return $this->getContents();
Expand Down Expand Up @@ -292,6 +313,12 @@ public function read($length): string
}

if (is_string($data)) {
if ($this->cache) {
$this->cache->write($data);
}
if ($this->eof()) {
$this->finished = true;
}
return $data;
}

Expand Down Expand Up @@ -322,13 +349,24 @@ public function write($string)
*/
public function getContents(): string
{
if ($this->cache && $this->finished) {
$this->cache->rewind();
return $this->cache->getContents();
}

$contents = false;

if ($this->stream) {
$contents = stream_get_contents($this->stream);
}

if (is_string($contents)) {
if ($this->cache) {
$this->cache->write($contents);
}
if ($this->eof()) {
$this->finished = true;
}
return $contents;
}

Expand Down
14 changes: 14 additions & 0 deletions tests/Factory/ServerRequestFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Interop\Http\Factory\ServerRequestFactoryTestCase;
use InvalidArgumentException;
use Psr\Http\Message\UriInterface;
use ReflectionClass;
use Slim\Psr7\Environment;
use Slim\Psr7\Factory\ServerRequestFactory;
use Slim\Psr7\Factory\UriFactory;
Expand Down Expand Up @@ -117,6 +118,19 @@ public function testCreateFromGlobalsBodyPointsToPhpInput()
$this->assertEquals('php://input', $request->getBody()->getMetadata('uri'));
}

public function testCreateFromGlobalsSetsACache()
{
$request = ServerRequestFactory::createFromGlobals();

// ensure that the Stream's $cache property has been set for this php://input stream
$stream = $request->getBody();
$class = new ReflectionClass($stream);
$property = $class->getProperty('cache');
$property->setAccessible(true);
$cacheStreamValue = $property->getValue($stream);
$this->assertNotNull($cacheStreamValue);
}

public function testCreateFromGlobalsWithUploadedFiles()
{
$_SERVER = Environment::mock([
Expand Down
39 changes: 39 additions & 0 deletions tests/StreamTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,43 @@ private function openPipeStream()
$this->pipeFh = popen('echo 12', 'r');
$this->pipeStream = new Stream($this->pipeFh);
}

public function testReadOnlyCachedStreamsAreDisallowed()
{
$resource = fopen('php://temp', 'w+');
$cache = new Stream(fopen('php://temp', 'r'));

$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Cache stream must be seekable and writable');
new Stream($resource, $cache);
}

public function testNonSeekableCachedStreamsAreDisallowed()
{
$resource = fopen('php://temp', 'w+');
$cache = new Stream(fopen('php://output', 'w'));

$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Cache stream must be seekable and writable');

new Stream($resource, $cache);
}

public function testCachedStreamsGetsContentFromTheCache()
{
$resource = popen('echo HelloWorld', 'r');
$stream = new Stream($resource, new Stream(fopen('php://temp', 'w+')));

$this->assertEquals("HelloWorld\n", $stream->getContents());
$this->assertEquals("HelloWorld\n", $stream->getContents());
}

public function testCachedStreamsFillsCacheOnRead()
{
$resource = fopen('data://,0', 'r');
$stream = new Stream($resource, new Stream(fopen('php://temp', 'w+')));

$this->assertEquals("0", $stream->read(100));
$this->assertEquals("0", $stream->__toString());
}
}