diff --git a/src/Session/Session.php b/src/Session/Session.php index ba8aaab..e03111c 100644 --- a/src/Session/Session.php +++ b/src/Session/Session.php @@ -12,7 +12,10 @@ class Session implements SessionInterface private array $data; - private array $original = []; + /** + * The JSON encoding of the data as last loaded or saved. + */ + private ?string $original = null; private bool $loaded = false; @@ -29,6 +32,11 @@ public function __construct(string $id = 'default', array $data = [], SessionSto $this->storage = $storage; } + public function getId(): string + { + return $this->id; + } + public function setStorage(SessionStorageInterface $storage): void { $this->storage = $storage; @@ -62,11 +70,19 @@ public function save(): void return; } $this->lazyLoad(); - if ($this->data === $this->original) { + $encoded = $this->encode(); + if ($encoded !== null && $encoded === $this->original) { return; } $this->storage->save($this->id, $this->data); + $this->original = $encoded; + } + + public function reload(): void + { + $this->loaded = false; + $this->lazyLoad(); } /** @@ -76,8 +92,18 @@ private function lazyLoad(): void { if (! $this->loaded && isset($this->storage)) { $this->data = $this->storage->load($this->id); - $this->original = $this->data; + $this->original = $this->encode(); $this->loaded = true; } } + + /** + * Encode the data as JSON, so that changes within objects are detected. + */ + private function encode(): ?string + { + $encoded = json_encode($this->data); + + return $encoded === false ? null : $encoded; + } } diff --git a/src/Session/SessionInterface.php b/src/Session/SessionInterface.php index c666756..6e88de2 100644 --- a/src/Session/SessionInterface.php +++ b/src/Session/SessionInterface.php @@ -8,6 +8,11 @@ interface SessionInterface { + /** + * Get the session ID. + */ + public function getId(): string; + /** * Set the storage for this session. */ @@ -30,6 +35,13 @@ public function get(string $key): mixed; */ public function save(); + /** + * Reload the session data from storage, if storage is defined. + * + * Unsaved changes are discarded. + */ + public function reload(); + /** * Clear the session data. */ diff --git a/src/Session/Storage/File.php b/src/Session/Storage/File.php index 05e3e10..4df60c4 100644 --- a/src/Session/Storage/File.php +++ b/src/Session/Storage/File.php @@ -44,14 +44,17 @@ public function save(string $sessionId, array $data): void public function load(string $sessionId): array { - $data = []; $filename = $this->getFilename($sessionId); - if (is_readable($filename)) { - $raw = file_get_contents($filename); - if ($raw !== false) { - $data = json_decode($raw, true); - } + if (! is_readable($filename) || ! ($handle = fopen($filename, 'r'))) { + return []; } + // A shared lock avoids reading a file that save() has truncated but not yet written. + // If locking is unsupported, read anyway. + flock($handle, LOCK_SH); + $raw = stream_get_contents($handle); + flock($handle, LOCK_UN); + fclose($handle); + $data = $raw !== false ? json_decode($raw, true) : []; return is_array($data) ? $data : []; } diff --git a/tests/Session/SessionTest.php b/tests/Session/SessionTest.php new file mode 100644 index 0000000..a6f732a --- /dev/null +++ b/tests/Session/SessionTest.php @@ -0,0 +1,121 @@ +storage = new class() implements SessionStorageInterface { + public array $sessions = []; + + public int $saveCount = 0; + + public function load(string $sessionId): array + { + return $this->sessions[$sessionId] ?? []; + } + + public function save(string $sessionId, array $data): void + { + $this->saveCount++; + $this->sessions[$sessionId] = $data; + } + }; + } + + public function testGetId(): void + { + $this->assertSame('default', (new Session())->getId()); + $this->assertSame('foo', (new Session('foo'))->getId()); + } + + public function testSaveOnlyWritesChanges(): void + { + $session = new Session('test', [], $this->storage); + $session->save(); + $this->assertSame(0, $this->storage->saveCount); + + $session->set('token', 'foo'); + $session->save(); + $this->assertSame(1, $this->storage->saveCount); + $this->assertSame([ + 'token' => 'foo', + ], $this->storage->sessions['test']); + + $session->save(); + $session->set('token', 'foo'); + $session->save(); + $this->assertSame(1, $this->storage->saveCount); + + $session->set('token', 'bar'); + $session->save(); + $this->assertSame(2, $this->storage->saveCount); + } + + public function testSaveWritesChangedObject(): void + { + $value = new class() implements \JsonSerializable { + public string $token = 'foo'; + + public function jsonSerialize(): mixed + { + return $this->token; + } + }; + $session = new Session('test', [], $this->storage); + $session->set('token', $value); + $session->save(); + $this->assertSame(1, $this->storage->saveCount); + + $value->token = 'bar'; + $session->save(); + $this->assertSame(2, $this->storage->saveCount); + } + + public function testReload(): void + { + $session = new Session('test', [], $this->storage); + $this->storage->sessions['test'] = [ + 'token' => 'old', + ]; + $this->assertSame('old', $session->get('token')); + + // Another process changes the stored data. + $this->storage->sessions['test'] = [ + 'token' => 'new', + ]; + $this->assertSame('old', $session->get('token')); + $session->reload(); + $this->assertSame('new', $session->get('token')); + + // Reloaded data is the new baseline for save(). + $session->save(); + $this->assertSame(0, $this->storage->saveCount); + } + + public function testReloadDiscardsUnsavedChanges(): void + { + $session = new Session('test', [], $this->storage); + $session->set('token', 'unsaved'); + $session->reload(); + $this->assertNull($session->get('token')); + } + + public function testReloadWithoutStorage(): void + { + $session = new Session('test', [ + 'token' => 'foo', + ]); + $session->reload(); + $this->assertSame('foo', $session->get('token')); + } +} diff --git a/tests/Session/Storage/FileTest.php b/tests/Session/Storage/FileTest.php new file mode 100644 index 0000000..8fc39fc --- /dev/null +++ b/tests/Session/Storage/FileTest.php @@ -0,0 +1,81 @@ +dir = sys_get_temp_dir() . '/platformsh-client-test-' . bin2hex(random_bytes(8)); + } + + protected function tearDown(): void + { + $filename = $this->dir . '/sess-test/sess-test.json'; + if (file_exists($filename)) { + unlink($filename); + } + foreach ([$this->dir . '/sess-test', $this->dir] as $dir) { + if (is_dir($dir)) { + rmdir($dir); + } + } + } + + public function testSaveAndLoad(): void + { + $storage = new File($this->dir); + $this->assertSame([], $storage->load('test')); + $storage->save('test', [ + 'foo' => 'bar', + ]); + $this->assertSame([ + 'foo' => 'bar', + ], $storage->load('test')); + $storage->save('test', []); + $this->assertSame([], $storage->load('test')); + } + + public function testLoadWaitsForExclusiveLock(): void + { + $storage = new File($this->dir); + $storage->save('test', [ + 'token' => 'old', + ]); + $filename = $this->dir . '/sess-test/sess-test.json'; + + // A child process truncates the file under an exclusive lock, as + // file_put_contents() does, and writes the new data after a delay. + $script = <<<'PHP' + $h = fopen($argv[1], 'c'); + flock($h, LOCK_EX); + ftruncate($h, 0); + echo "locked\n"; + usleep(500000); + fwrite($h, '{"token":"new"}'); + fflush($h); + flock($h, LOCK_UN); + fclose($h); + PHP; + $process = proc_open([PHP_BINARY, '-r', $script, $filename], [ + 1 => ['pipe', 'w'], + ], $pipes); + $this->assertIsResource($process); + $this->assertSame("locked\n", fgets($pipes[1])); + + $data = $storage->load('test'); + + fclose($pipes[1]); + $this->assertSame(0, proc_close($process)); + $this->assertSame([ + 'token' => 'new', + ], $data); + } +}