Skip to content

feat: Refresh Sushi connection on each Octane request via event listener#131

Open
tkaratug wants to merge 3 commits intocalebporzio:mainfrom
tkaratug:feature/octane-request-refresh
Open

feat: Refresh Sushi connection on each Octane request via event listener#131
tkaratug wants to merge 3 commits intocalebporzio:mainfrom
tkaratug:feature/octane-request-refresh

Conversation

@tkaratug
Copy link

Related work

PR #128 by @alimorgaan previously addressed the same problem by tracking the current request ID inside resolveConnection() using spl_object_hash(app('request')). That PR was not merged, and the subsequent v2.5.4 release introduced Laravel 13 compatibility changes that altered the booting mechanism (adding the whenBooted deferral). This PR reopens the feature request with a different implementation approach that is compatible with v2.5.4 and avoids the issues described below.

Problem

When running under Laravel Octane, a single application process handles many HTTP requests. Sushi's SQLite connection (and the data behind it) is configured once at boot and reused for the lifetime of that process. This is the right default, but it means models whose getRows() returns dynamic data (e.g. pulled from cache or a database) will serve stale rows for every request after the first.

Solution

This PR adds an opt-in shouldRefreshDataOnEachRequest() hook. When a model overrides it to return true, Sushi registers a listener on Octane's RequestReceived event at boot time. On every new request the listener calls clearSushiConnection(), nulling out the static connection. The next query triggers a lazy configureSushiConnection() call in resolveConnection(), which re-runs getRows() and rebuilds the in-memory SQLite table.

Under traditional FPM deployments (or any environment where the Laravel\Octane\Events\RequestReceived class does not exist) the listener is never registered, $sushiConnection is never nulled, and there is zero overhead compared to the current behaviour.

Why not the request-ID approach from PR #128?

The request-ID approach had two problems:

  1. It performed a container lookup (app('request')) on every single query, adding overhead even for models that never refresh.
  2. Under FrankenPHP's fiber-based concurrency, spl_object_hash() can return the same hash for different request objects, and the REQUEST_TIME_FLOAT fallback introduces a race condition under high traffic.

The event listener approach moves request detection entirely out of the hot path. The listener fires once per request at the Octane level, and resolveConnection() only pays a === null check — which is always false for non-refreshing models after boot.

Usage

class LiveSettings extends Model
{
    use \Sushi\Sushi;

    public function getRows(): array
    {
        return cache()->get('live-settings', []);
    }

    protected static function shouldRefreshDataOnEachRequest(): bool
    {
        return true;
    }
}

What changed

src/Sushi.php

  • Added protected static $sushiOctaneListenerRegistered = false to prevent duplicate listener registration when a model is booted more than once (e.g. after clearBootedModels()).
  • Added protected static function shouldRefreshDataOnEachRequest(): bool — returns false by default, making this entirely opt-in and backwards-compatible.
  • Updated resolveConnection() to lazily call configureSushiConnection() when the connection is null and the model has opted in to per-request refresh. The double condition (=== null && shouldRefreshDataOnEachRequest()) short-circuits immediately for the common case (refresh disabled), so there is no per-query overhead for standard models.
  • Updated bootSushi() to call registerOctaneRefreshListener() after configureSushiConnection(), in both the whenBooted and the legacy path.
  • Added protected static function registerOctaneRefreshListener(): void — guards with three early returns (refresh disabled / already registered / Octane not present), then registers a static closure on app('events') that calls clearSushiConnection() for this model class.
  • Added public static function clearSushiConnection(): void — sets $sushiConnection = null, making it available as a public API so callers (and tests) can simulate an Octane request boundary without depending on the event system.

tests/SushiTest.php

  • Added OctaneRefreshModel — a test model that overrides shouldRefreshDataOnEachRequest() and counts how many times getRows() has been called, returning one extra row per call.
  • Added test_model_with_refresh_reruns_get_rows_after_connection_is_cleared — verifies that calling clearSushiConnection() (what the Octane event listener does) causes the next query to re-run getRows() and return fresh data.
  • Added test_model_without_refresh_does_not_register_octane_listener — verifies that $sushiOctaneListenerRegistered remains false after boot for a model with the default shouldRefreshDataOnEachRequest() = false, proving that the Octane listener is never registered and the connection is therefore never cleared between requests.
  • Updated resetStatics() on Foo, Bar, and OctaneRefreshModel to reset $sushiOctaneListenerRegistered between tests.
  • Fixed setUp() to reset Bar::$hasBeenAccessedBefore unconditionally (it was only reset inside individual tests, which could leave state across test runs).

README.md

  • Added a "Refreshing Data on Each Request (Laravel Octane)" section documenting the opt-in hook, a usage example, and a note clarifying that the feature has no effect under FPM.

* Add `shouldRefreshDataOnEachRequest()` hook and wire it to Octane's `RequestReceived` event. On each new request, `clearSushiConnection()` nulls out `$sushiConnection` so the next query re-runs `getRows()` and rebuilds the in-memory table.
* Under FPM the listener is never registered and `$sushiConnection` is never nulled, so there is zero overhead for non-Octane deployments.
* Updated `test_model_without_refresh_does_not_rerun_get_rows_on_new_request` to `test_model_without_refresh_does_not_register_octane_listener`.
* Adjusted assertions to verify that the Octane listener is not registered when `shouldRefreshDataOnEachRequest()` is false.
…sertions

* Updated the test to set the `sushiOctaneListenerRegistered` property as accessible.
* This change allows for proper validation of the listener registration state in the tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant