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.

Sign up for GitHub

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

Jump to bottom

Truncating response body #55

Open
wants to merge 3 commits into
base: 1.x
Choose a base branch
from
Open

Truncating response body #55

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd956e4
Testing for empty body on cache hit
JimTools Mar 7, 2024
674fd7d
Replace response body on cache hit
JimTools Mar 7, 2024
5693234
Removing unused import
JimTools Aug 8, 2024
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
22 changes: 18 additions & 4 deletions src/Cache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@

use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\StreamFactoryInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;

use Slim\Psr7\Factory\StreamFactory;
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency is not needed.

use function in_array;
use function is_array;
use function is_numeric;
Expand Down Expand Up @@ -45,18 +47,28 @@ class Cache implements MiddlewareInterface
*/
protected $mustRevalidate;

/**
* @var StreamFactoryInterface
*/
protected $streamFactory;

/**
* Create new HTTP cache
*
* @param string $type The cache type: "public" or "private"
* @param int $maxAge The maximum age of client-side cache
* @param bool $mustRevalidate must-revalidate
*/
public function __construct(string $type = 'private', int $maxAge = 86400, bool $mustRevalidate = false)
{
public function __construct(
StreamFactoryInterface $streamFactory,
string $type = 'private',
int $maxAge = 86400,
bool $mustRevalidate = false
) {
$this->type = $type;
$this->maxAge = $maxAge;
$this->mustRevalidate = $mustRevalidate;
$this->streamFactory = $streamFactory;
Copy link
Author

Choose a reason for hiding this comment

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

Maybe for now it's better to add slim/psr7 and call StreamFactory this would make it easier for anyone already using this package?

Copy link
Contributor

@flavioheleno flavioheleno Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd rather require psr/http-factory (and likely psr/http-factory-implementation) than slim/psr7 as it keeps interoperable with other psr7 implementation packages

Copy link
Contributor

Choose a reason for hiding this comment

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

Only the user should decide which StreamFactoryInterface implementation should be used in their application.

Copy link
Author

Choose a reason for hiding this comment

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

@flavioheleno @odan thank you for taken the time to review the PR I’ll update when I get some time.

}

/**
Expand Down Expand Up @@ -101,7 +113,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
if ($ifNoneMatch) {
$etagList = preg_split('@\s*,\s*@', $ifNoneMatch);
if (is_array($etagList) && (in_array($etag, $etagList) || in_array('*', $etagList))) {
return $response->withStatus(304);
return $response->withStatus(304)
->withBody($this->streamFactory->createStream(''));
}
}
}
Expand All @@ -118,7 +131,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
$ifModifiedSince = $request->getHeaderLine('If-Modified-Since');

if ($ifModifiedSince && $lastModified <= strtotime($ifModifiedSince)) {
return $response->withStatus(304);
return $response->withStatus(304)
->withBody($this->streamFactory->createStream(''));
}
}

Expand Down
58 changes: 48 additions & 10 deletions tests/CacheTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@
use Slim\Psr7\Factory\ResponseFactory;
use Slim\Psr7\Factory\ServerRequestFactory;

use Slim\Psr7\Factory\StreamFactory;
use function gmdate;
use function time;

class CacheTest extends TestCase
{
private function createCache(string $type = 'privte', int $maxAge = 86400, bool $mustRevalidate = false): Cache
{
return new Cache(new StreamFactory(), $type, $maxAge, $mustRevalidate);
}


public function requestFactory(): ServerRequestInterface
{
$serverRequestFactory = new ServerRequestFactory();
Expand Down Expand Up @@ -64,7 +71,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface

public function testCacheControlHeader()
{
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory();

$res = $cache->process($req, $this->createRequestHandler());
Expand All @@ -76,7 +83,7 @@ public function testCacheControlHeader()

public function testCacheControlHeaderWithMustRevalidate()
{
$cache = new Cache('private', 86400, true);
$cache = $this->createCache('private', 86400, true);
$req = $this->requestFactory();

$res = $cache->process($req, $this->createRequestHandler());
Expand All @@ -88,7 +95,7 @@ public function testCacheControlHeaderWithMustRevalidate()

public function testCacheControlHeaderWithZeroMaxAge()
{
$cache = new Cache('private', 0, false);
$cache = $this->createCache('private', 0, false);
$req = $this->requestFactory();

$res = $cache->process($req, $this->createRequestHandler());
Expand All @@ -100,7 +107,7 @@ public function testCacheControlHeaderWithZeroMaxAge()

public function testCacheControlHeaderDoesNotOverrideExistingHeader()
{
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory();

$res = $this->createResponse()->withHeader('Cache-Control', 'no-cache,no-store');
Expand All @@ -116,7 +123,7 @@ public function testLastModifiedWithCacheHit()
$now = time();
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 86400);
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);

$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);

Expand All @@ -131,7 +138,7 @@ public function testLastModifiedWithCacheHitAndNewerDate()
$now = time();
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 172800); // <-- Newer date
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);

$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
Expand All @@ -145,7 +152,7 @@ public function testLastModifiedWithCacheHitAndOlderDate()
$now = time();
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); // <-- Older date
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);

$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
Expand All @@ -159,7 +166,7 @@ public function testLastModifiedWithCacheMiss()
$now = time();
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now - 86400);
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);

$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
Expand All @@ -172,7 +179,7 @@ public function testETagWithCacheHit()
{
$etag = 'abc';
$ifNoneMatch = 'abc';
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch);

$res = $this->createResponse()->withHeader('Etag', $etag);
Expand All @@ -185,12 +192,43 @@ public function testETagWithCacheMiss()
{
$etag = 'abc';
$ifNoneMatch = 'xyz';
$cache = new Cache('public', 86400);
$cache = $this->createCache('public', 86400);
$req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch);

$res = $this->createResponse()->withHeader('Etag', $etag);
$res = $cache->process($req, $this->createRequestHandler($res));

$this->assertEquals(200, $res->getStatusCode());
}

public function testETagReturnsNoBodyOnCacheHit(): void
{
$etag = 'abc';
$cache = $this->createCache();
$req = $this->requestFactory()->withHeader('If-None-Match', $etag);

$res = $this->createResponse()->withHeader('Etag', $etag);
$res->getBody()->write('payload data');
$res = $cache->process($req, $this->createRequestHandler($res));

self::assertSame(304, $res->getStatusCode());
self::assertSame('', (string) $res->getBody());
}

public function testLastModifiedReturnsNoBodyOnCacheHit(): void
{
$now = time() + 86400;
$lastModified = gmdate('D, d M Y H:i:s T', $now);
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now);
$cache = $this->createCache();

$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
$res->getBody()->write('payload data');

$res = $cache->process($req, $this->createRequestHandler($res));

self::assertEquals(304, $res->getStatusCode());
self::assertSame('', (string) $res->getBody());
}
}