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

QA fixes/improvements #10

Merged
merged 20 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
6 changes: 6 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ matrix:
- php: 7.3
env:
- DEPS=latest
- php: 7.4
env:
- DEPS=lowest
- php: 7.4
env:
- DEPS=latest

before_install:
- if [[ $TEST_COVERAGE != 'true' ]]; then phpenv config-rm xdebug.ini || return 0 ; fi
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"require-dev": {
"laminas/laminas-coding-standard": "~1.0.0",
"laminas/laminas-diactoros": "^1.6",
"phpunit/phpunit": "^7.0.2"
"phpunit/phpunit": "^7.5.20 || ^8.5.2 || ^9.0.1"
},
"conflict": {
"phpspec/prophecy": "<1.7.2"
Expand Down
2 changes: 2 additions & 0 deletions src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* @license https://github.com/mezzio/mezzio-session-ext/blob/master/LICENSE.md New BSD License
*/

declare(strict_types=1);

namespace Mezzio\Session\Ext;

use Mezzio\Session\SessionPersistenceInterface;
Expand Down
61 changes: 36 additions & 25 deletions src/PhpSessionPersistence.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
* @license https://github.com/mezzio/mezzio-session-ext/blob/master/LICENSE.md New BSD License
*/

declare(strict_types=1);

namespace Mezzio\Session\Ext;

use Dflydev\FigCookies\FigRequestCookies;
use Dflydev\FigCookies\FigResponseCookies;
use Dflydev\FigCookies\SetCookie;
use Mezzio\Session\GenerateIdPersistenceInterface;
use Mezzio\Session\InitializePersistenceIdInterface;
use Mezzio\Session\Session;
use Mezzio\Session\SessionCookiePersistenceInterface;
Expand All @@ -22,6 +23,7 @@

use function bin2hex;
use function filemtime;
use function filter_var;
use function getlastmod;
use function gmdate;
use function ini_get;
Expand All @@ -30,6 +32,7 @@
use function session_id;
use function session_name;
use function session_start;
use function session_status;
use function session_write_close;
use function sprintf;
use function time;
Expand All @@ -53,6 +56,14 @@
*/
class PhpSessionPersistence implements InitializePersistenceIdInterface, SessionPersistenceInterface
{
/**
* This unusual past date value is taken from the php-engine source code and
* used "as is" for consistency.
*/
public const CACHE_PAST_DATE = 'Thu, 19 Nov 1981 08:52:00 GMT';

public const HTTP_DATE_FORMAT = 'D, d M Y H:i:s T';

/**
* Use non locking mode during session initialization?
*
Expand All @@ -78,21 +89,13 @@ class PhpSessionPersistence implements InitializePersistenceIdInterface, Session
private $cacheLimiter;

/** @var array */
private static $supported_cache_limiters = [
private static $supportedCacheLimiters = [
'nocache' => true,
'public' => true,
'private' => true,
'private_no_expire' => true,
];

/**
* This unusual past date value is taken from the php-engine source code and
* used "as is" for consistency.
*/
public const CACHE_PAST_DATE = 'Thu, 19 Nov 1981 08:52:00 GMT';

public const HTTP_DATE_FORMAT = 'D, d M Y H:i:s T';

/**
* Memoize session ini settings before starting the request.
*
Expand All @@ -110,6 +113,15 @@ public function __construct(bool $nonLocking = false)
$this->cacheExpire = (int) ini_get('session.cache_expire');
}

/**
* @internal
* @return bool the non-locking mode used during initialization
*/
public function isNonLocking() : bool
{
return $this->nonLocking;
}

public function initializeSessionFromRequest(ServerRequestInterface $request) : SessionInterface
{
$sessionId = FigRequestCookies::get($request, session_name())->getValue() ?? '';
Expand All @@ -129,23 +141,23 @@ public function persistSession(SessionInterface $session, ResponseInterface $res
// - the session is marked as regenerated
// - the id is empty, but the data has changed (new session)
if ($session->isRegenerated()
|| ('' === $id && $session->hasChanged())
|| ($id === '' && $session->hasChanged())
) {
$id = $this->regenerateSession();
} elseif ($this->nonLocking && $session->hasChanged()) {
// we reopen the initial session only if there are changes to write
$this->startSession($id);
}

if (PHP_SESSION_ACTIVE === session_status()) {
if (session_status() === PHP_SESSION_ACTIVE) {
$_SESSION = $session->toArray();
session_write_close();
}

// If we do not have an identifier at this point, it means a new
// session was created, but never written to. In that case, there's
// no reason to provide a cookie back to the user.
if ('' === $id) {
if ($id === '') {
return $response;
}

Expand All @@ -160,10 +172,10 @@ public function persistSession(SessionInterface $session, ResponseInterface $res
return $response;
}

public function initializeId(SessionInterface $session): SessionInterface
public function initializeId(SessionInterface $session) : SessionInterface
{
$id = $session->getId();
if ('' === $id || $session->isRegenerated()) {
if ($id === '' || $session->isRegenerated()) {
$session = new Session($session->toArray(), $this->generateSessionId());
}

Expand All @@ -190,7 +202,7 @@ private function startSession(string $id, array $options = []) : void
*/
private function regenerateSession() : string
{
if (PHP_SESSION_ACTIVE === session_status()) {
if (session_status() === PHP_SESSION_ACTIVE) {
session_destroy();
}

Expand Down Expand Up @@ -268,7 +280,7 @@ private function addCacheHeaders(ResponseInterface $response) : ResponseInterfac

$cacheHeaders = $this->generateCacheHeaders();
foreach ($cacheHeaders as $name => $value) {
if (false !== $value) {
if ($value !== false) {
$response = $response->withHeader($name, $value);
}
}
Expand All @@ -283,12 +295,12 @@ private function addCacheHeaders(ResponseInterface $response) : ResponseInterfac
private function generateCacheHeaders() : array
{
// Unsupported cache_limiter
if (! isset(self::$supported_cache_limiters[$this->cacheLimiter])) {
if (! isset(self::$supportedCacheLimiters[$this->cacheLimiter])) {
return [];
}

// cache_limiter: 'nocache'
if ('nocache' === $this->cacheLimiter) {
if ($this->cacheLimiter === 'nocache') {
return [
'Expires' => self::CACHE_PAST_DATE,
'Cache-Control' => 'no-store, no-cache, must-revalidate',
Expand All @@ -300,7 +312,7 @@ private function generateCacheHeaders() : array
$lastModified = $this->getLastModified();

// cache_limiter: 'public'
if ('public' === $this->cacheLimiter) {
if ($this->cacheLimiter === 'public') {
return [
'Expires' => gmdate(self::HTTP_DATE_FORMAT, time() + $maxAge),
'Cache-Control' => sprintf('public, max-age=%d', $maxAge),
Expand All @@ -309,7 +321,7 @@ private function generateCacheHeaders() : array
}

// cache_limiter: 'private'
if ('private' === $this->cacheLimiter) {
if ($this->cacheLimiter === 'private') {
return [
'Expires' => self::CACHE_PAST_DATE,
'Cache-Control' => sprintf('private, max-age=%d', $maxAge),
Expand All @@ -328,6 +340,7 @@ private function generateCacheHeaders() : array
* Return the Last-Modified header line based on main script of execution
* modified time. If unable to get a valid timestamp we use this class file
* modification time as fallback.
*
* @return string|false
*/
private function getLastModified()
Expand All @@ -341,12 +354,10 @@ private function getLastModified()
*/
private function responseAlreadyHasCacheHeaders(ResponseInterface $response) : bool
{
return (
$response->hasHeader('Expires')
return $response->hasHeader('Expires')
|| $response->hasHeader('Last-Modified')
|| $response->hasHeader('Cache-Control')
|| $response->hasHeader('Pragma')
);
|| $response->hasHeader('Pragma');
}

private function getCookieLifetime(SessionInterface $session) : int
Expand Down
16 changes: 11 additions & 5 deletions test/ConfigProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,37 @@
* @license https://github.com/mezzio/mezzio-session-ext/blob/master/LICENSE.md New BSD License
*/

declare(strict_types=1);

namespace MezzioTest\Session\Ext;

use Mezzio\Session\Ext\ConfigProvider;
use PHPUnit\Framework\TestCase;

class ConfigProviderTest extends TestCase
{
public function setUp()
/** @var ConfigProvider */
private $provider;

protected function setUp() : void
{
$this->provider = new ConfigProvider();
}

public function testInvocationReturnsArray()
public function testInvocationReturnsArray() : array
{
$config = ($this->provider)();
$this->assertInternalType('array', $config);
$this->assertIsArray($config);

return $config;
}

/**
* @depends testInvocationReturnsArray
*/
public function testReturnedArrayContainsDependencies(array $config)
public function testReturnedArrayContainsDependencies(array $config) : void
{
$this->assertArrayHasKey('dependencies', $config);
$this->assertInternalType('array', $config['dependencies']);
$this->assertIsArray($config['dependencies']);
}
}
6 changes: 3 additions & 3 deletions test/PhpSessionPersistenceFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

class PhpSessionPersistenceFactoryTest extends TestCase
{
public function testFactoryConfigProducesPhpSessionPersistenceInterfaceService()
public function testFactoryConfigProducesPhpSessionPersistenceInterfaceService() : void
{
$container = $this->prophesize(ContainerInterface::class);
$factory = new PhpSessionPersistenceFactory();
Expand All @@ -26,7 +26,7 @@ public function testFactoryConfigProducesPhpSessionPersistenceInterfaceService()
$container->has('config')->willReturn(false);
$persistence = $factory($container->reveal());
$this->assertInstanceOf(PhpSessionPersistence::class, $persistence);
$this->assertAttributeSame(false, 'nonLocking', $persistence);
$this->assertFalse($persistence->isNonLocking());

// test php-session-persistence with non-locking config set to false and true
foreach ([false, true] as $nonLocking) {
Expand All @@ -41,7 +41,7 @@ public function testFactoryConfigProducesPhpSessionPersistenceInterfaceService()
],
]);
$persistence = $factory($container->reveal());
$this->assertAttributeSame($nonLocking, 'nonLocking', $persistence);
$this->assertSame($nonLocking, $persistence->isNonLocking());
}
}
}
Loading