From c9fd5fe58fcc8a5741864f6ebc08de1551b76c8d Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:19:50 +0200 Subject: [PATCH 1/9] Ignore test files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 78921a9b77..300b99e6ba 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ # Top level directories /log/ +/bench/ /package-lock.json # Top-level+Sub directories From 9e5b13cba585870bec94a6e58448c27bd019046d Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:38:18 +0200 Subject: [PATCH 2/9] perf(http): throttle random session cleanup and delete expired database sessions at once The `RANDOM_REQUESTS` cleanup strategy went over all sessions on roughly one in fifty requests. As the amount of sessions grows with traffic, this made throughput collapse under load (#2309). Random cleanups now run at most once per minute. Expired database sessions are now removed with a single query instead of one query per session. Co-Authored-By: Claude Opus 5.5 --- docs/1-essentials/01-routing.md | 2 +- packages/http/src/Session/CleanupStrategy.php | 2 +- .../src/Session/ManageSessionMiddleware.php | 35 +++++++++++++++- .../Managers/DatabaseSessionManager.php | 27 ++++++++---- .../Http/SessionCleanupStrategyTest.php | 41 +++++++++++++++++++ 5 files changed, 94 insertions(+), 13 deletions(-) diff --git a/docs/1-essentials/01-routing.md b/docs/1-essentials/01-routing.md index d6788bf7dc..bea480bdcb 100644 --- a/docs/1-essentials/01-routing.md +++ b/docs/1-essentials/01-routing.md @@ -1062,7 +1062,7 @@ return new DatabaseSessionConfig( Sessions expire based on the last activity time. This means that as long as a user is actively using the application, their session remains valid. -By default, Tempest removes expired session randomly at the end of a request, in a deferred task. This can be configured by specifying a {b`Tempest\Http\Session\CleanupStrategy`} in the [session configuration](#session-configuration). +By default, Tempest removes expired sessions at the end of a random request, in a deferred task. To keep the cost of cleaning up from growing with traffic, this happens at most once per minute. This can be configured by specifying a {b`Tempest\Http\Session\CleanupStrategy`} in the [session configuration](#session-configuration). ```php app/Sessions/session.config.php use Tempest\Http\Session\Config\DatabaseSessionConfig; diff --git a/packages/http/src/Session/CleanupStrategy.php b/packages/http/src/Session/CleanupStrategy.php index e08b709e33..5b89eba7e6 100644 --- a/packages/http/src/Session/CleanupStrategy.php +++ b/packages/http/src/Session/CleanupStrategy.php @@ -10,7 +10,7 @@ enum CleanupStrategy case EVERY_REQUEST; /** - * Expired sessions will be cleaned up after a random request. This is the default strategy, and provides a good balance between performance and cleanup frequency. + * Expired sessions will be cleaned up after a random request, at most once per minute. This is the default strategy, and provides a good balance between performance and cleanup frequency. */ case RANDOM_REQUESTS; diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index 5b7e31aea3..70bb65deaa 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -2,12 +2,17 @@ namespace Tempest\Http\Session; +use Tempest\Clock\Clock; use Tempest\Core\DeferredTasks; use Tempest\Http\Request; use Tempest\Http\Response; use Tempest\Router\HttpMiddleware; use Tempest\Router\HttpMiddlewareCallable; +use Tempest\Support\Filesystem; use Tempest\Support\Priority; +use Throwable; + +use function Tempest\internal_storage_path; /** * This middleware is responsible for creating the session and saving it on response. @@ -15,11 +20,17 @@ #[Priority(Priority::FRAMEWORK - 20)] final readonly class ManageSessionMiddleware implements HttpMiddleware { + /** + * Minimum amount of seconds between two cleanups when using the `RANDOM_REQUESTS` strategy. + */ + private const int RANDOM_REQUESTS_CLEANUP_INTERVAL = 60; + public function __construct( private SessionManager $sessionManager, private Session $session, private SessionConfig $config, private DeferredTasks $deferredTasks, + private Clock $clock, ) {} public function __invoke(Request $request, HttpMiddlewareCallable $next): Response @@ -52,8 +63,28 @@ private function scheduleSessionCleanup(): void private function maybeScheduleSessionCleanup(): void { - if (random_int(min: 1, max: 50) === 1) { - $this->scheduleSessionCleanup(); + if (random_int(min: 1, max: 50) !== 1) { + return; + } + + // Cleaning up has to go over all sessions, so it is throttled to keep + // its cost from growing along with the amount of sessions and traffic. + $path = internal_storage_path('session-cleanup'); + $now = $this->clock->now()->getTimestamp()->getSeconds(); + + try { + $lastCleanupAt = Filesystem\is_file($path) ? (int) Filesystem\read_file($path) : 0; + } catch (Throwable) { + $lastCleanupAt = 0; + } + + if (($now - $lastCleanupAt) < self::RANDOM_REQUESTS_CLEANUP_INTERVAL) { + return; } + + // The cleanup is claimed before it runs, so that concurrent requests don't start one as well. + Filesystem\write_file($path, (string) $now, flags: LOCK_EX); + + $this->scheduleSessionCleanup(); } } diff --git a/packages/http/src/Session/Managers/DatabaseSessionManager.php b/packages/http/src/Session/Managers/DatabaseSessionManager.php index ad6f1cf964..4fdd8b67d2 100644 --- a/packages/http/src/Session/Managers/DatabaseSessionManager.php +++ b/packages/http/src/Session/Managers/DatabaseSessionManager.php @@ -95,18 +95,27 @@ public function deleteExpiredSessions(): void ->now() ->minus($this->config->expiration); - $expiredSessions = query(DatabaseSession::class) - ->select() - ->where('last_active_at < ?', $expired->format(FormatPattern::SQL_DATE_TIME)) - ->all(); + $expiredBefore = $expired->format(FormatPattern::SQL_DATE_TIME); - foreach ($expiredSessions as $expiredSession) { + $expiredIds = array_map( + static fn (DatabaseSession $session) => (string) $session->id, query(DatabaseSession::class) - ->delete() - ->where('id', $expiredSession->id) - ->execute(); + ->select('id') + ->where('last_active_at < ?', $expiredBefore) + ->all(), + ); + + if ($expiredIds === []) { + return; + } + + query(DatabaseSession::class) + ->delete() + ->where('last_active_at < ?', $expiredBefore) + ->execute(); - event(new SessionDeleted(new SessionId((string) $expiredSession->id))); + foreach ($expiredIds as $expiredId) { + event(new SessionDeleted(new SessionId($expiredId))); } } diff --git a/tests/Integration/Http/SessionCleanupStrategyTest.php b/tests/Integration/Http/SessionCleanupStrategyTest.php index 444ac468f2..fd32b936c9 100644 --- a/tests/Integration/Http/SessionCleanupStrategyTest.php +++ b/tests/Integration/Http/SessionCleanupStrategyTest.php @@ -20,8 +20,11 @@ use Tempest\Http\Session\SessionManager; use Tempest\Http\Status; use Tempest\Router\HttpMiddlewareCallable; +use Tempest\Support\Filesystem; use Tests\Tempest\Integration\FrameworkIntegrationTestCase; +use function Tempest\internal_storage_path; + /** * @internal */ @@ -31,6 +34,8 @@ final class SessionCleanupStrategyTest extends FrameworkIntegrationTestCase protected function cleanup(): void { $this->clearDeferredTasks(); + + Filesystem\delete(internal_storage_path('session-cleanup')); } #[Test] @@ -63,6 +68,41 @@ public function disabled_does_not_schedule_cleanup_task(): void ); } + #[Test] + public function random_requests_schedules_cleanup_at_most_once_per_interval(): void + { + $clock = $this->clock('2025-01-01 00:00:00'); + + $config = new FileSessionConfig( + expiration: Duration::minutes(30), + cleanupStrategy: CleanupStrategy::RANDOM_REQUESTS, + path: 'tests/sessions', + ); + + // The odds of a cleanup are 1 in 50 per request, so 1000 requests are virtually guaranteed to trigger one. + $this->assertTrue($this->cleanupScheduledWithin(1000, $config)); + $this->assertFalse($this->cleanupScheduledWithin(1000, $config)); + + $clock->plus(Duration::seconds(30)); + $this->assertFalse($this->cleanupScheduledWithin(1000, $config)); + + $clock->plus(Duration::seconds(31)); + $this->assertTrue($this->cleanupScheduledWithin(1000, $config)); + } + + private function cleanupScheduledWithin(int $requests, FileSessionConfig $config): bool + { + for ($i = 0; $i < $requests; $i++) { + $this->invokeMiddlewareWith($config); + + if (array_key_exists('tempest:session-cleanup', $this->container->get(DeferredTasks::class)->getTasks())) { + return true; + } + } + + return false; + } + private function invokeMiddlewareWith(FileSessionConfig $config): void { $this->clearDeferredTasks(); @@ -83,6 +123,7 @@ private function invokeMiddlewareWith(FileSessionConfig $config): void session: $session, config: $config, deferredTasks: $this->container->get(DeferredTasks::class), + clock: $clock, ); $response = $middleware( From 9af62abadbc5535964bec01c4a8d704f2da417c6 Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:41:33 +0200 Subject: [PATCH 3/9] perf(http): only persist new sessions once they contain data Clients that don't send a session cookie, like API consumers, crawlers and load testing tools, got a new session persisted on every request. Besides the write itself, this made the amount of sessions grow along with traffic, which in turn made cleaning them up increasingly expensive (#2309). New sessions are now only persisted once something is stored in them. The previous URL alone doesn't count, since it is tracked on every request; `Back` falls back to the `Referer` header when it's missing. Co-Authored-By: Claude Opus 5.5 --- docs/1-essentials/01-routing.md | 2 + .../src/Session/ManageSessionMiddleware.php | 23 ++- .../Managers/DatabaseSessionManager.php | 1 + .../Session/Managers/FileSessionManager.php | 1 + .../Session/Managers/RedisSessionManager.php | 1 + packages/http/src/Session/PreviousUrl.php | 2 +- packages/http/src/Session/Session.php | 15 ++ .../Http/ManageSessionMiddlewareTest.php | 156 ++++++++++++++++++ 8 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 tests/Integration/Http/ManageSessionMiddlewareTest.php diff --git a/docs/1-essentials/01-routing.md b/docs/1-essentials/01-routing.md index bea480bdcb..65ba54888e 100644 --- a/docs/1-essentials/01-routing.md +++ b/docs/1-essentials/01-routing.md @@ -979,6 +979,8 @@ return new CookieConfig( Sessions in Tempest are managed by the {b`Tempest\Http\Session\Session`} class. It can be injected anywhere needed. As soon as the {b`Tempest\Http\Session\Session`} is injected, it is started behind the scenes. +New sessions are only persisted once something is stored in them. This way, clients that don't keep cookies, such as API consumers or crawlers, don't fill up the session storage. + ```php use Tempest\Http\Session\Session; diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index 70bb65deaa..f4d3910cde 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -43,7 +43,10 @@ public function __invoke(Request $request, HttpMiddlewareCallable $next): Respon try { return $next($request); } finally { - $this->sessionManager->save($this->session); + if ($this->shouldPersist()) { + $this->sessionManager->save($this->session); + $this->session->markAsPersisted(); + } match ($this->config->cleanupStrategy) { CleanupStrategy::EVERY_REQUEST => $this->scheduleSessionCleanup(), @@ -53,6 +56,24 @@ public function __invoke(Request $request, HttpMiddlewareCallable $next): Respon } } + /** + * Clients that don't send a session cookie, like API consumers, crawlers or load testing tools, get a new session on every request. + * Those sessions are only persisted once they contain something worth keeping, so that they don't pile up in storage. + */ + private function shouldPersist(): bool + { + if (! $this->session->isNew) { + return true; + } + + $data = $this->session->all(); + + // The previous URL is tracked on every request, so it alone doesn't warrant persisting a new session. + unset($data[PreviousUrl::PREVIOUS_URL_SESSION_KEY]); + + return $data !== []; + } + private function scheduleSessionCleanup(): void { $this->deferredTasks->add( diff --git a/packages/http/src/Session/Managers/DatabaseSessionManager.php b/packages/http/src/Session/Managers/DatabaseSessionManager.php index 4fdd8b67d2..75ebcb910e 100644 --- a/packages/http/src/Session/Managers/DatabaseSessionManager.php +++ b/packages/http/src/Session/Managers/DatabaseSessionManager.php @@ -33,6 +33,7 @@ public function getOrCreate(SessionId $id): Session id: $id, createdAt: $now, lastActiveAt: $now, + isNew: true, ); event(new SessionCreated($session)); diff --git a/packages/http/src/Session/Managers/FileSessionManager.php b/packages/http/src/Session/Managers/FileSessionManager.php index 070acf0d39..6f1e2bfcc4 100644 --- a/packages/http/src/Session/Managers/FileSessionManager.php +++ b/packages/http/src/Session/Managers/FileSessionManager.php @@ -34,6 +34,7 @@ public function getOrCreate(SessionId $id): Session id: $id, createdAt: $now, lastActiveAt: $now, + isNew: true, ); event(new SessionCreated($session)); diff --git a/packages/http/src/Session/Managers/RedisSessionManager.php b/packages/http/src/Session/Managers/RedisSessionManager.php index 6e19e9f4ab..2252bcb737 100644 --- a/packages/http/src/Session/Managers/RedisSessionManager.php +++ b/packages/http/src/Session/Managers/RedisSessionManager.php @@ -35,6 +35,7 @@ public function getOrCreate(SessionId $id): Session id: $id, createdAt: $now, lastActiveAt: $now, + isNew: true, ); event(new SessionCreated($session)); diff --git a/packages/http/src/Session/PreviousUrl.php b/packages/http/src/Session/PreviousUrl.php index 9fe22a66eb..a515ecff97 100644 --- a/packages/http/src/Session/PreviousUrl.php +++ b/packages/http/src/Session/PreviousUrl.php @@ -12,7 +12,7 @@ */ final readonly class PreviousUrl { - private const string PREVIOUS_URL_SESSION_KEY = '#previous_url'; + public const string PREVIOUS_URL_SESSION_KEY = '#previous_url'; private const string INTENDED_URL_SESSION_KEY = '#intended_url'; diff --git a/packages/http/src/Session/Session.php b/packages/http/src/Session/Session.php index 78846c4a92..1ed46ad81e 100644 --- a/packages/http/src/Session/Session.php +++ b/packages/http/src/Session/Session.php @@ -28,6 +28,10 @@ public function __construct( public DateTimeInterface $lastActiveAt, /** @var array */ private(set) array $data = [], + /** + * Whether this session was created during the current request, rather than loaded from storage. + */ + private(set) bool $isNew = false, ) {} /** @@ -140,6 +144,16 @@ public function clear(): void $this->data = []; } + /** + * Marks the session as persisted, so that it is no longer considered new. + * + * @internal + */ + public function markAsPersisted(): void + { + $this->isNew = false; + } + public function __serialize(): array { return [ @@ -158,5 +172,6 @@ public function __unserialize(array $data): void $this->lastActiveAt = DateTime::fromTimestamp($data['last_active_at']); $this->data = $data['data']; $this->expiredKeys = $data['expired_keys']; + $this->isNew = false; } } diff --git a/tests/Integration/Http/ManageSessionMiddlewareTest.php b/tests/Integration/Http/ManageSessionMiddlewareTest.php new file mode 100644 index 0000000000..96d90d79ef --- /dev/null +++ b/tests/Integration/Http/ManageSessionMiddlewareTest.php @@ -0,0 +1,156 @@ +path); + + $this->path = realpath($this->path); + $this->container->get(FrameworkKernel::class)->internalStorage = $this->path; + + $this->config = new FileSessionConfig( + expiration: Duration::hours(2), + cleanupStrategy: CleanupStrategy::DISABLED, + path: 'sessions', + ); + + $this->manager = new FileSessionManager( + clock: $this->container->get(Clock::class), + sessionConfig: $this->config, + ); + } + + #[PostCondition] + protected function cleanup(): void + { + Filesystem\delete_directory($this->path); + } + + #[Test] + public function new_session_without_data_is_not_persisted(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle($session); + + $this->assertFileDoesNotExist($this->sessionPath($session)); + } + + #[Test] + public function new_session_with_only_previous_url_is_not_persisted(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle($session, fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/dashboard')); + + $this->assertFileDoesNotExist($this->sessionPath($session)); + } + + #[Test] + public function new_session_with_data_is_persisted(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle($session, fn (Session $session) => $session->set('user', 'brent')); + + $this->assertFileExists($this->sessionPath($session)); + $this->assertFalse($session->isNew); + } + + #[Test] + public function new_session_with_flash_value_is_persisted(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle($session, fn (Session $session) => $session->flash('success', 'Saved!')); + + $this->assertFileExists($this->sessionPath($session)); + } + + #[Test] + public function existing_session_is_persisted_after_being_cleared(): void + { + $session = $this->manager->getOrCreate(new SessionId('existing-session')); + + $this->handle($session, fn (Session $session) => $session->set('user', 'brent')); + $this->handle($session, fn (Session $session) => $session->clear()); + + $this->assertSame([], $this->manager->getOrCreate(new SessionId('existing-session'))->all()); + } + + #[Test] + public function loaded_session_is_not_new(): void + { + $session = $this->manager->getOrCreate(new SessionId('existing-session')); + $this->assertTrue($session->isNew); + + $this->handle($session, fn (Session $session) => $session->set('user', 'brent')); + + $this->assertFalse($this->manager->getOrCreate(new SessionId('existing-session'))->isNew); + } + + private function handle(Session $session, ?Closure $controller = null): void + { + $middleware = new ManageSessionMiddleware( + sessionManager: $this->manager, + session: $session, + config: $this->config, + deferredTasks: $this->container->get(DeferredTasks::class), + clock: $this->container->get(Clock::class), + ); + + $middleware( + request: new GenericRequest(method: Method::GET, uri: '/'), + next: new HttpMiddlewareCallable(function () use ($session, $controller) { + if ($controller !== null) { + $controller($session); + } + + return new GenericResponse(Status::OK); + }), + ); + } + + private function sessionPath(Session $session): string + { + return $this->path . '/sessions/' . $session->id; + } +} From a486fabac26c590b053b4920d3d480373d76c2cb Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:47:35 +0200 Subject: [PATCH 4/9] perf(http): don't rewrite sessions that didn't change Sessions were serialized and written to storage at the end of every request, even when nothing changed. `Session::isDirty()` now compares a fingerprint of the session's state against the one taken when it was loaded or last persisted, so values that are modified in place are still detected. File sessions that didn't change are only touched, their last activity being read from the modification time when loading. This also prevents an unchanged session from overwriting changes made by a concurrent request. Database sessions that didn't change only have their `last_active_at` updated. Co-Authored-By: Claude Opus 5.5 --- docs/1-essentials/01-routing.md | 2 +- .../src/Session/ManageSessionMiddleware.php | 1 - .../Managers/DatabaseSessionManager.php | 16 +++++ .../Session/Managers/FileSessionManager.php | 37 +++++++++-- .../Session/Managers/RedisSessionManager.php | 2 + packages/http/src/Session/Session.php | 29 ++++++++- tests/Integration/Http/FileSessionTest.php | 45 +++++++++++++ tests/Integration/Http/SessionTest.php | 65 +++++++++++++++++++ 8 files changed, 186 insertions(+), 11 deletions(-) diff --git a/docs/1-essentials/01-routing.md b/docs/1-essentials/01-routing.md index 65ba54888e..962e2be85b 100644 --- a/docs/1-essentials/01-routing.md +++ b/docs/1-essentials/01-routing.md @@ -979,7 +979,7 @@ return new CookieConfig( Sessions in Tempest are managed by the {b`Tempest\Http\Session\Session`} class. It can be injected anywhere needed. As soon as the {b`Tempest\Http\Session\Session`} is injected, it is started behind the scenes. -New sessions are only persisted once something is stored in them. This way, clients that don't keep cookies, such as API consumers or crawlers, don't fill up the session storage. +New sessions are only persisted once something is stored in them. This way, clients that don't keep cookies, such as API consumers or crawlers, don't fill up the session storage. Likewise, sessions that didn't change during a request aren't written again; only their activity is tracked. ```php use Tempest\Http\Session\Session; diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index f4d3910cde..54f2116fea 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -45,7 +45,6 @@ public function __invoke(Request $request, HttpMiddlewareCallable $next): Respon } finally { if ($this->shouldPersist()) { $this->sessionManager->save($this->session); - $this->session->markAsPersisted(); } match ($this->config->cleanupStrategy) { diff --git a/packages/http/src/Session/Managers/DatabaseSessionManager.php b/packages/http/src/Session/Managers/DatabaseSessionManager.php index 75ebcb910e..037021ee27 100644 --- a/packages/http/src/Session/Managers/DatabaseSessionManager.php +++ b/packages/http/src/Session/Managers/DatabaseSessionManager.php @@ -61,6 +61,20 @@ public function save(Session $session): void ) ->execute(); + $session->markAsPersisted(); + + return; + } + + // Unchanged sessions only need their activity to be tracked. + if (! $session->isDirty()) { + query(DatabaseSession::class) + ->update(last_active_at: $session->lastActiveAt) + ->where('id', (string) $session->id) + ->execute(); + + $session->markAsPersisted(); + return; } @@ -71,6 +85,8 @@ public function save(Session $session): void ) ->where('id', (string) $session->id) ->execute(); + + $session->markAsPersisted(); } public function delete(Session $session): void diff --git a/packages/http/src/Session/Managers/FileSessionManager.php b/packages/http/src/Session/Managers/FileSessionManager.php index 6f1e2bfcc4..8994eb07f9 100644 --- a/packages/http/src/Session/Managers/FileSessionManager.php +++ b/packages/http/src/Session/Managers/FileSessionManager.php @@ -5,6 +5,7 @@ namespace Tempest\Http\Session\Managers; use Tempest\Clock\Clock; +use Tempest\DateTime\DateTime; use Tempest\Http\Session\Config\FileSessionConfig; use Tempest\Http\Session\Session; use Tempest\Http\Session\SessionCreated; @@ -45,13 +46,23 @@ public function getOrCreate(SessionId $id): Session public function save(Session $session): void { - $session->lastActiveAt = $this->clock->now(); + $now = $this->clock->now(); + $path = $this->getPath($session->id); - Filesystem\write_file( - filename: $this->getPath($session->id), - content: serialize($session), - flags: LOCK_EX, - ); + $session->lastActiveAt = $now; + + // Unchanged sessions aren't rewritten, their activity is tracked through the file's modification time instead. + if ($session->isDirty() || ! Filesystem\is_file($path)) { + Filesystem\write_file( + filename: $path, + content: serialize($session), + flags: LOCK_EX, + ); + } + + touch($path, $now->getTimestamp()->getSeconds()); + + $session->markAsPersisted(); } public function delete(Session $session): void @@ -104,12 +115,24 @@ private function load(SessionId $id): ?Session return null; } - return unserialize( + $session = unserialize( data: Filesystem\read_locked_file($path), options: [ 'allowed_classes' => true, ], ); + + if (! $session instanceof Session) { + return null; + } + + $lastModifiedAt = filemtime($path); + + if ($lastModifiedAt !== false && $lastModifiedAt > $session->lastActiveAt->getTimestamp()->getSeconds()) { + $session->lastActiveAt = DateTime::fromTimestamp($lastModifiedAt); + } + + return $session; } catch (Throwable) { return null; } diff --git a/packages/http/src/Session/Managers/RedisSessionManager.php b/packages/http/src/Session/Managers/RedisSessionManager.php index 2252bcb737..7b3db7d513 100644 --- a/packages/http/src/Session/Managers/RedisSessionManager.php +++ b/packages/http/src/Session/Managers/RedisSessionManager.php @@ -53,6 +53,8 @@ public function save(Session $session): void value: serialize($session), expiration: $this->config->expiration, ); + + $session->markAsPersisted(); } public function delete(Session $session): void diff --git a/packages/http/src/Session/Session.php b/packages/http/src/Session/Session.php index 1ed46ad81e..ce0be0951b 100644 --- a/packages/http/src/Session/Session.php +++ b/packages/http/src/Session/Session.php @@ -22,6 +22,11 @@ final class Session */ private array $expiredKeys = []; + /** + * Fingerprint of the session's state when it was loaded or last persisted. + */ + private string $persistedFingerprint; + public function __construct( private(set) SessionId $id, private(set) DateTimeInterface $createdAt, @@ -32,7 +37,9 @@ public function __construct( * Whether this session was created during the current request, rather than loaded from storage. */ private(set) bool $isNew = false, - ) {} + ) { + $this->persistedFingerprint = $this->fingerprint(); + } /** * Sets a value in the session. @@ -145,13 +152,30 @@ public function clear(): void } /** - * Marks the session as persisted, so that it is no longer considered new. + * Determines whether the session's data changed since it was loaded or last persisted. + */ + public function isDirty(): bool + { + return $this->fingerprint() !== $this->persistedFingerprint; + } + + /** + * Marks the session as persisted, so that it is no longer considered new or dirty. * * @internal */ public function markAsPersisted(): void { $this->isNew = false; + $this->persistedFingerprint = $this->fingerprint(); + } + + /** + * Values may be objects that are modified in place, so the serialized state is compared instead of tracking changes. + */ + private function fingerprint(): string + { + return hash('xxh128', serialize([$this->data, $this->expiredKeys])); } public function __serialize(): array @@ -173,5 +197,6 @@ public function __unserialize(array $data): void $this->data = $data['data']; $this->expiredKeys = $data['expired_keys']; $this->isNew = false; + $this->persistedFingerprint = $this->fingerprint(); } } diff --git a/tests/Integration/Http/FileSessionTest.php b/tests/Integration/Http/FileSessionTest.php index 98968bf3de..e0d6f32f84 100644 --- a/tests/Integration/Http/FileSessionTest.php +++ b/tests/Integration/Http/FileSessionTest.php @@ -252,6 +252,51 @@ public function delete_expired_sessions_removes_unreadable_files(): void ); } + #[Test] + public function unchanged_session_is_not_rewritten(): void + { + $clock = $this->clock('2025-01-01 00:00:00'); + $id = new SessionId('unchanged'); + $path = Path\normalize($this->path, 'sessions', (string) $id); + + $session = $this->manager->getOrCreate($id); + $session->set('user', 'brent'); + $this->manager->save($session); + + $unchanged = $this->manager->getOrCreate($id); + + // Another request writes to the same session in the meantime + $concurrent = $this->manager->getOrCreate($id); + $concurrent->set('cart', 'book'); + $this->manager->save($concurrent); + + $clock->plus(Duration::minutes(5)); + $this->manager->save($unchanged); + + clearstatcache(); + + $this->assertSame('book', $this->manager->getOrCreate($id)->get('cart')); + $this->assertSame($clock->now()->getTimestamp()->getSeconds(), filemtime($path)); + } + + #[Test] + public function last_active_at_is_read_from_modification_time(): void + { + $clock = $this->clock('2025-01-01 00:00:00'); + $id = new SessionId('active'); + + $session = $this->manager->getOrCreate($id); + $session->set('user', 'brent'); + $this->manager->save($session); + + $clock->plus(Duration::minutes(5)); + $this->manager->save($this->manager->getOrCreate($id)); + + clearstatcache(); + + $this->assertTrue($this->manager->getOrCreate($id)->lastActiveAt->equals($clock->now())); + } + private function saveAtClockTime(Session $session, string $path): void { $this->manager->save($session); diff --git a/tests/Integration/Http/SessionTest.php b/tests/Integration/Http/SessionTest.php index e6d22a9107..812c335965 100644 --- a/tests/Integration/Http/SessionTest.php +++ b/tests/Integration/Http/SessionTest.php @@ -4,8 +4,11 @@ namespace Tests\Tempest\Integration\Http; +use ArrayObject; use PHPUnit\Framework\Attributes\Test; +use Tempest\DateTime\DateTime; use Tempest\Http\Session\Session; +use Tempest\Http\Session\SessionId; use Tests\Tempest\Integration\FrameworkIntegrationTestCase; /** @@ -150,4 +153,66 @@ public function session_is_reset(): void $this->assertNotSame($originalSession, $newSession); } + + #[Test] + public function is_dirty_after_changes(): void + { + $session = $this->createSession(['user' => 'brent']); + $this->assertFalse($session->isDirty()); + + $session->set('user', 'brent'); + $this->assertFalse($session->isDirty()); + + $session->set('user', 'aidan'); + $this->assertTrue($session->isDirty()); + + $session->markAsPersisted(); + $this->assertFalse($session->isDirty()); + + $session->remove('user'); + $this->assertTrue($session->isDirty()); + } + + #[Test] + public function is_dirty_after_values_are_modified_in_place(): void + { + $session = $this->createSession(['cart' => new ArrayObject()]); + + $session->get('cart')->append('book'); + + $this->assertTrue($session->isDirty()); + } + + #[Test] + public function is_dirty_after_flash_values_are_aged(): void + { + $session = $this->createSession(); + $session->flash('success', 'Saved!'); + $session->markAsPersisted(); + + $session->cleanup(); + + $this->assertTrue($session->isDirty()); + } + + #[Test] + public function is_not_dirty_after_being_unserialized(): void + { + $session = $this->createSession(); + $session->set('user', 'brent'); + + $this->assertFalse(unserialize(serialize($session))->isDirty()); + } + + private function createSession(array $data = []): Session + { + $now = DateTime::now(); + + return new Session( + id: new SessionId('dirty-test'), + createdAt: $now, + lastActiveAt: $now, + data: $data, + ); + } } From f86d69ecd138a35507266f71b0b213d4adeb4c36 Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:51:05 +0200 Subject: [PATCH 5/9] test: stop the development server started by the client test The test started the server through `tempest serve`, which runs `php -S` in a child process. Stopping the command left that child running after the test suite finished. Starting the server was also slower than the time the test waits for it, so the tests were usually skipped. The server is now started directly, so it's stopped along with the process. Co-Authored-By: Claude Opus 5.5 --- tests/Integration/Route/ClientTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Integration/Route/ClientTest.php b/tests/Integration/Route/ClientTest.php index 57a74fe97c..467d6a7ccc 100644 --- a/tests/Integration/Route/ClientTest.php +++ b/tests/Integration/Route/ClientTest.php @@ -28,7 +28,12 @@ protected function setUp(): void { parent::setUp(); - $this->server = new Process([root_path('tempest'), 'serve', 'localhost', '8088']); + // The server is started directly instead of through `tempest serve`, which runs it in a child process + // that would keep running after this process is stopped. + $this->server = new Process( + command: [PHP_BINARY, '-S', 'localhost:8088', '-t', 'public/', root_path('packages/router/src/Commands/router.php')], + cwd: root_path(), + ); $this->server->start(); // Server needs to start From b29521344c4979fb9c87dbeceae2d058ab455a92 Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 09:59:23 +0200 Subject: [PATCH 6/9] Cleanup --- packages/http/src/Session/Session.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/http/src/Session/Session.php b/packages/http/src/Session/Session.php index ce0be0951b..e29bd01047 100644 --- a/packages/http/src/Session/Session.php +++ b/packages/http/src/Session/Session.php @@ -33,9 +33,6 @@ public function __construct( public DateTimeInterface $lastActiveAt, /** @var array */ private(set) array $data = [], - /** - * Whether this session was created during the current request, rather than loaded from storage. - */ private(set) bool $isNew = false, ) { $this->persistedFingerprint = $this->fingerprint(); From 71abbcb295242700db4b621e0e881da883ac03c1 Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 12:50:47 +0200 Subject: [PATCH 7/9] fix(http): keep the previous URL for browsers that don't send a referer New sessions that only contained the previous URL weren't persisted, relying on `Back` falling back to the referer. Browsers that don't send one, for example because of a `no-referrer` policy, would lose the previous URL. The previous URL now still persists a new session when a browser navigates between pages (`Sec-Fetch-Mode: navigate`) without sending a referer. Other clients, like crawlers and API consumers, still don't get a session persisted. Co-Authored-By: Claude Opus 5.5 --- .../src/Session/ManageSessionMiddleware.php | 23 +++++++++++-- .../Http/ManageSessionMiddlewareTest.php | 34 +++++++++++++++++-- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index 54f2116fea..27ca1e4350 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -8,6 +8,7 @@ use Tempest\Http\Response; use Tempest\Router\HttpMiddleware; use Tempest\Router\HttpMiddlewareCallable; +use Tempest\Router\SecFetchMode; use Tempest\Support\Filesystem; use Tempest\Support\Priority; use Throwable; @@ -43,7 +44,7 @@ public function __invoke(Request $request, HttpMiddlewareCallable $next): Respon try { return $next($request); } finally { - if ($this->shouldPersist()) { + if ($this->shouldPersist($request)) { $this->sessionManager->save($this->session); } @@ -59,7 +60,7 @@ public function __invoke(Request $request, HttpMiddlewareCallable $next): Respon * Clients that don't send a session cookie, like API consumers, crawlers or load testing tools, get a new session on every request. * Those sessions are only persisted once they contain something worth keeping, so that they don't pile up in storage. */ - private function shouldPersist(): bool + private function shouldPersist(Request $request): bool { if (! $this->session->isNew) { return true; @@ -68,11 +69,27 @@ private function shouldPersist(): bool $data = $this->session->all(); // The previous URL is tracked on every request, so it alone doesn't warrant persisting a new session. - unset($data[PreviousUrl::PREVIOUS_URL_SESSION_KEY]); + if (! $this->needsPreviousUrl($request)) { + unset($data[PreviousUrl::PREVIOUS_URL_SESSION_KEY]); + } return $data !== []; } + /** + * `Back` falls back to the referer when there's no previous URL. Only browsers navigating between pages + * without sending a referer (e.g. because of a `no-referrer` policy) need the previous URL to be kept. + * Other clients, like crawlers and API consumers, don't navigate back and don't send `Sec-Fetch-*` headers. + */ + private function needsPreviousUrl(Request $request): bool + { + if ($request->headers->get('referer') !== null) { + return false; + } + + return SecFetchMode::tryFrom($request->headers->get('sec-fetch-mode', default: '')) === SecFetchMode::NAVIGATE; + } + private function scheduleSessionCleanup(): void { $this->deferredTasks->add( diff --git a/tests/Integration/Http/ManageSessionMiddlewareTest.php b/tests/Integration/Http/ManageSessionMiddlewareTest.php index 96d90d79ef..c1e6f342b1 100644 --- a/tests/Integration/Http/ManageSessionMiddlewareTest.php +++ b/tests/Integration/Http/ManageSessionMiddlewareTest.php @@ -75,7 +75,7 @@ public function new_session_without_data_is_not_persisted(): void } #[Test] - public function new_session_with_only_previous_url_is_not_persisted(): void + public function new_session_with_only_previous_url_is_not_persisted_for_non_browser_clients(): void { $session = $this->manager->getOrCreate(new SessionId('new-session')); @@ -84,6 +84,34 @@ public function new_session_with_only_previous_url_is_not_persisted(): void $this->assertFileDoesNotExist($this->sessionPath($session)); } + #[Test] + public function new_session_with_only_previous_url_is_not_persisted_for_browser_navigation_with_referer(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle( + session: $session, + controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/dashboard'), + headers: ['sec-fetch-mode' => 'navigate', 'referer' => 'https://tempestphp.com/'], + ); + + $this->assertFileDoesNotExist($this->sessionPath($session)); + } + + #[Test] + public function new_session_with_only_previous_url_is_persisted_for_browser_navigation_without_referer(): void + { + $session = $this->manager->getOrCreate(new SessionId('new-session')); + + $this->handle( + session: $session, + controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/dashboard'), + headers: ['sec-fetch-mode' => 'navigate'], + ); + + $this->assertFileExists($this->sessionPath($session)); + } + #[Test] public function new_session_with_data_is_persisted(): void { @@ -127,7 +155,7 @@ public function loaded_session_is_not_new(): void $this->assertFalse($this->manager->getOrCreate(new SessionId('existing-session'))->isNew); } - private function handle(Session $session, ?Closure $controller = null): void + private function handle(Session $session, ?Closure $controller = null, array $headers = []): void { $middleware = new ManageSessionMiddleware( sessionManager: $this->manager, @@ -138,7 +166,7 @@ private function handle(Session $session, ?Closure $controller = null): void ); $middleware( - request: new GenericRequest(method: Method::GET, uri: '/'), + request: new GenericRequest(method: Method::GET, uri: '/', headers: $headers), next: new HttpMiddlewareCallable(function () use ($session, $controller) { if ($controller !== null) { $controller($session); From 9a0ae44242a21cddef300e20abc59e97457c0bad Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 13:02:25 +0200 Subject: [PATCH 8/9] fix(http): claim the session cleanup atomically The last cleanup time was read without a lock, so concurrent requests could all see an outdated time and each schedule a cleanup. It's now read, checked and updated while holding a lock. Requests that can't acquire it immediately skip the cleanup, since another request is already claiming it. Co-Authored-By: Claude Opus 5.5 --- .../src/Session/ManageSessionMiddleware.php | 53 +++++++++++++------ .../Http/SessionCleanupStrategyTest.php | 25 +++++++++ 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index 27ca1e4350..67d42a8c0b 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -9,9 +9,7 @@ use Tempest\Router\HttpMiddleware; use Tempest\Router\HttpMiddlewareCallable; use Tempest\Router\SecFetchMode; -use Tempest\Support\Filesystem; use Tempest\Support\Priority; -use Throwable; use function Tempest\internal_storage_path; @@ -104,24 +102,47 @@ private function maybeScheduleSessionCleanup(): void return; } - // Cleaning up has to go over all sessions, so it is throttled to keep - // its cost from growing along with the amount of sessions and traffic. - $path = internal_storage_path('session-cleanup'); - $now = $this->clock->now()->getTimestamp()->getSeconds(); - - try { - $lastCleanupAt = Filesystem\is_file($path) ? (int) Filesystem\read_file($path) : 0; - } catch (Throwable) { - $lastCleanupAt = 0; + if (! $this->claimSessionCleanup()) { + return; } - if (($now - $lastCleanupAt) < self::RANDOM_REQUESTS_CLEANUP_INTERVAL) { - return; + $this->scheduleSessionCleanup(); + } + + /** + * Cleaning up has to go over all sessions, so it is throttled to keep its cost from growing along with the amount of sessions and traffic. + * The last cleanup time is read, checked and updated while holding a lock, so that concurrent requests can't both claim the same cleanup. + */ + private function claimSessionCleanup(): bool + { + $handle = @fopen(internal_storage_path('session-cleanup'), mode: 'c+'); + + if ($handle === false) { + return false; } - // The cleanup is claimed before it runs, so that concurrent requests don't start one as well. - Filesystem\write_file($path, (string) $now, flags: LOCK_EX); + try { + // Another request holding the lock is already claiming the cleanup, so there's no need to wait for it. + if (! flock($handle, LOCK_EX | LOCK_NB)) { + return false; + } + + $now = $this->clock->now()->getTimestamp()->getSeconds(); + $lastCleanupAt = (int) stream_get_contents($handle); - $this->scheduleSessionCleanup(); + if (($now - $lastCleanupAt) < self::RANDOM_REQUESTS_CLEANUP_INTERVAL) { + return false; + } + + ftruncate($handle, size: 0); + rewind($handle); + fwrite($handle, (string) $now); + fflush($handle); + + return true; + } finally { + flock($handle, LOCK_UN); + fclose($handle); + } } } diff --git a/tests/Integration/Http/SessionCleanupStrategyTest.php b/tests/Integration/Http/SessionCleanupStrategyTest.php index fd32b936c9..762b8766a4 100644 --- a/tests/Integration/Http/SessionCleanupStrategyTest.php +++ b/tests/Integration/Http/SessionCleanupStrategyTest.php @@ -90,6 +90,31 @@ public function random_requests_schedules_cleanup_at_most_once_per_interval(): v $this->assertTrue($this->cleanupScheduledWithin(1000, $config)); } + #[Test] + public function random_requests_does_not_schedule_cleanup_while_another_request_claims_it(): void + { + $this->clock('2025-01-01 00:00:00'); + + $config = new FileSessionConfig( + expiration: Duration::minutes(30), + cleanupStrategy: CleanupStrategy::RANDOM_REQUESTS, + path: 'tests/sessions', + ); + + // Simulates a concurrent request that is in the middle of claiming the cleanup + $handle = fopen(internal_storage_path('session-cleanup'), mode: 'c+'); + flock($handle, LOCK_EX); + + try { + $this->assertFalse($this->cleanupScheduledWithin(1000, $config)); + } finally { + flock($handle, LOCK_UN); + fclose($handle); + } + + $this->assertTrue($this->cleanupScheduledWithin(1000, $config)); + } + private function cleanupScheduledWithin(int $requests, FileSessionConfig $config): bool { for ($i = 0; $i < $requests; $i++) { From 00d81727776377791019c89ec1ba6e82b75cdba4 Mon Sep 17 00:00:00 2001 From: brendt Date: Fri, 23 Oct 2026 13:03:24 +0200 Subject: [PATCH 9/9] fix(http): always keep the previous URL for browser navigations Whether a new session was persisted for its previous URL depended on the current request's referer. But the request that redirects back may not send one, for example when the visitor arrived from a search engine and the site uses a `no-referrer` policy, in which case `Back` redirected to `/`. The previous URL now always persists a new session for browser navigations (`Sec-Fetch-Mode: navigate`). Clients that don't send `Sec-Fetch-*` headers, like crawlers and API consumers, still don't get a session persisted. They can't submit forms that redirect back anyway, since `PreventCrossSiteRequestsMiddleware` requires these headers. Co-Authored-By: Claude Opus 5.5 --- .../src/Session/ManageSessionMiddleware.php | 11 ++++------- .../Http/ManageSessionMiddlewareTest.php | 17 +++++++++-------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/http/src/Session/ManageSessionMiddleware.php b/packages/http/src/Session/ManageSessionMiddleware.php index 67d42a8c0b..6b9aafb98b 100644 --- a/packages/http/src/Session/ManageSessionMiddleware.php +++ b/packages/http/src/Session/ManageSessionMiddleware.php @@ -75,16 +75,13 @@ private function shouldPersist(Request $request): bool } /** - * `Back` falls back to the referer when there's no previous URL. Only browsers navigating between pages - * without sending a referer (e.g. because of a `no-referrer` policy) need the previous URL to be kept. - * Other clients, like crawlers and API consumers, don't navigate back and don't send `Sec-Fetch-*` headers. + * Browsers navigating between pages need the previous URL to be kept, because `Back` can't rely on the referer: + * the request that redirects back may not send one, for example because of a `no-referrer` policy. + * Other clients, like crawlers and API consumers, don't send `Sec-Fetch-*` headers, and can't submit forms + * that redirect back either, since `PreventCrossSiteRequestsMiddleware` requires them. */ private function needsPreviousUrl(Request $request): bool { - if ($request->headers->get('referer') !== null) { - return false; - } - return SecFetchMode::tryFrom($request->headers->get('sec-fetch-mode', default: '')) === SecFetchMode::NAVIGATE; } diff --git a/tests/Integration/Http/ManageSessionMiddlewareTest.php b/tests/Integration/Http/ManageSessionMiddlewareTest.php index c1e6f342b1..dd7033e4d0 100644 --- a/tests/Integration/Http/ManageSessionMiddlewareTest.php +++ b/tests/Integration/Http/ManageSessionMiddlewareTest.php @@ -85,31 +85,32 @@ public function new_session_with_only_previous_url_is_not_persisted_for_non_brow } #[Test] - public function new_session_with_only_previous_url_is_not_persisted_for_browser_navigation_with_referer(): void + public function new_session_with_only_previous_url_is_persisted_for_browser_navigation(): void { $session = $this->manager->getOrCreate(new SessionId('new-session')); + // The referer of this request doesn't tell whether the request redirecting back will send one $this->handle( session: $session, - controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/dashboard'), - headers: ['sec-fetch-mode' => 'navigate', 'referer' => 'https://tempestphp.com/'], + controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/contact'), + headers: ['sec-fetch-mode' => 'navigate', 'referer' => 'https://search.example/'], ); - $this->assertFileDoesNotExist($this->sessionPath($session)); + $this->assertFileExists($this->sessionPath($session)); } #[Test] - public function new_session_with_only_previous_url_is_persisted_for_browser_navigation_without_referer(): void + public function new_session_with_only_previous_url_is_not_persisted_for_browser_subrequests(): void { $session = $this->manager->getOrCreate(new SessionId('new-session')); $this->handle( session: $session, - controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/dashboard'), - headers: ['sec-fetch-mode' => 'navigate'], + controller: fn (Session $session) => $session->set(PreviousUrl::PREVIOUS_URL_SESSION_KEY, '/api/stats'), + headers: ['sec-fetch-mode' => 'cors'], ); - $this->assertFileExists($this->sessionPath($session)); + $this->assertFileDoesNotExist($this->sessionPath($session)); } #[Test]