-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[12.x]: Cache Session Driver #56887
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
[12.x]: Cache Session Driver #56887
Conversation
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
Can you provide a use case? Seems odd to have a TTL on a piece of session data. |
@browner12 For example, in my application the SESSION_LIFETIME is 60 minutes, but I want this information to be saved for only 10 minutes, not 60 minutes, that is, I want the callback to be checked in the same session each 10 minutes. |
Laminas has a session-backed cache store. That could be an alternative. Cache::driver('session')->remember('key', ttl: 5, callback: fn () => \rand(1, 5)); |
I can give a use case for this feature. See my blog post Sticky database connections across requests with Laravel We have a write with replicated read database connection. We use The problem lies with redirects. After a redirect, the next request will start using the read connection. There is the chance that the data has not yet been replicated to the read connection. Imagine you create a record and redirect to view the record. The record you just created may not be in the read database yet and result in a So I want sticky, but I want stick across requests, not just within the current request. Here is the custom implementation that uses the session to indicate if the write connection should be used. It remembers the value for 10 seconds. If the write connection continues to be used, the duration continues to be extended. Once the value expires, we forget the value in the session. <?php
namespace App\Http\Middleware;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Context;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\Facades\DB;
use Symfony\Component\HttpFoundation\Response;
class StickyWriteConnections
{
public function handle(Request $request, Closure $next): Response
{
if (! $request->hasSession()) {
return $next($request);
}
$expiry = $request->session()->has('use_write_connection_until')
? Date::parse($request->session()->get('use_write_connection_until'))
: null;
$expiry?->isFuture()
&& DB::connection('pgsql')->useWriteConnectionWhenReading()
&& Context::addHidden('use_write_connection_until', $expiry->toDateTimeString();
$response = $next($request);
$expiry = (DB::getConnections()['pgsql'] ?? null)?->hasModifiedRecords()
? Date::now()->addSeconds(10)
: $expiry;
$expiry?->isFuture()
? $request->session()->put('use_write_connection_until', $expiry->toDateTimeString())
: $request->session()->forget('use_write_connection_until');
return $response;
}
} Side note: the implementation we cooked up also uses Laravel Context to push the value across jobs in the queue, so the same principle applies for jobs: if the write connection is used in the request or the job, it will continue to be used in queued jobs that spawn from the request or job. Refactoring this to the implementation, with a API I think would be nice on the request object: <?php
namespace App\Http\Middleware;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Context;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\Facades\DB;
use Symfony\Component\HttpFoundation\Response;
class StickyWriteConnections
{
public function handle(Request $request, Closure $next): Response
{
if (! $request->hasSession()) {
return $next($request);
}
$expiry = $request->session()->cache()->get('use_write_connection_until');
$expiry?->isFuture()
&& DB::connection('pgsql')->useWriteConnectionWhenReading()
&& Context::addHidden('use_write_connection_until', $expiry);
$response = $next($request);
(DB::getConnections()['pgsql'] ?? null)?->hasModifiedRecords()
&& $request->session()->cache()->remember('use_write_connection_until', 10);
return $response;
}
} This refactor is the best case API scenario which assumes the cache mechanism serializes objects, otherwise it wouldn't be as clean. However, (de)serializing anything to and from the session could be dangerous. I wonder if this should store the value in the session itself or in a cache, as @rodrigopedra suggested. That way the data doesn't actually live in the session. The cache entries are instead tied to the session ID. So the <?php
class Store
{
// ...
public function cache()
{
// e.g., redis with pre-configured prefix of the session id
return Cache::store('session');
}
} |
@timacdonald I can try to implement the session driver in Laravel Cache, because it doesn't actually exist. |
@joaopalopes24, I'd hold off and see what Taylor thinks. I can see it both ways and just wanted to share my thoughts for consideration, not to point the PR in a specific direction. If it is a feature the framework wants, it could even be that you can have a session-based session cache and a cache-based session cache! |
I think the |
@timacdonald What do you think about the Cache session store, use only redis driver or configure for all drivers? |
Or save the cache in session payload? |
@timacdonald I made a commit, what did you think, would it be something like that? Or do we just take the default cache and change the prefix? |
Although I've provided some feedback on the locks, I don't think you should include cache locks. I don't really understand the use case for a session-based lock. Locks are good for cross process communication, but the session is explicitly scoped to one user's session. So I would recommend stripping that out and keep the feature sharp and focused for Taylor's review. Lastly, I still question whether we should support serializing in the session driver. If you are using an unencrypted cookie session driver, you expose yourself to vulnerabilities. Not saying you should change this in your PR, I'm just questioning it. Could be good to get Taylor's opinion on that one. |
@joaopalopes24 I am testing @timacdonald suggested stick keys middleware with this store implementation: <?php
namespace App\Support\Cache;
use Illuminate\Cache\RetrievesMultipleKeys;
use Illuminate\Contracts\Cache\Store;
use Illuminate\Contracts\Session\Session;
use Illuminate\Support\Carbon;
class SessionStore implements Store
{
use RetrievesMultipleKeys;
protected readonly string $prefix;
public function __construct(
protected readonly Session $session,
string $prefix,
) {
$this->prefix = \filled($prefix) ? $prefix : '_cache';
}
public function get($key)
{
if (\is_null($item = $this->session->get($this->cacheKey($key)))) {
return null;
}
if ($this->isExpired($item['expiresAt'] ?? 0)) {
$this->forget($key);
return null;
}
return $item['value'] ?? null;
}
public function put($key, $value, $seconds): bool
{
$this->session->put($this->cacheKey($key), [
'value' => $value,
'expiresAt' => $this->toTimestamp($seconds),
]);
return true;
}
public function increment($key, $value = 1): int
{
$this->forever($key, $incremented = $value + \intval($this->get($key) ?? 0));
return $incremented;
}
public function decrement($key, $value = 1): int
{
return $this->increment($key, $value * -1);
}
public function forever($key, $value): bool
{
return $this->put($key, $value, 0);
}
public function forget($key): bool
{
$this->session->forget($this->cacheKey($key));
return true;
}
public function flush(): bool
{
$this->session->forget($this->getPrefix());
return true;
}
public function getPrefix(): string
{
return $this->prefix;
}
protected function cacheKey(string $key): string
{
return $this->getPrefix() . '.' . $key;
}
protected function toTimestamp(int $seconds): int
{
if ($seconds > 0) {
return \intval(Carbon::now()->getPreciseTimestamp(3) / 1000) + $seconds;
}
return 0;
}
protected function isExpired(int $expiresAt): bool
{
return $expiresAt !== 0
&& (Carbon::now()->getPreciseTimestamp(3) / 1000) >= $expiresAt;
}
} As Tim said, I didn't handle serialization or locks. Also, I leveraged the Maybe it can help with some of Tim's observations. EDIT: Added |
I will do the corrections tomorrow, I thanks for the feedback. |
@timacdonald I made the requested changes, and among the corrections mentioned above, I forgot to remove the lock part, sorry, but I removed it now. |
I personally find the term "prefix" a bit confusing here in general. It seems to be the key of the item in the session that is going to hold the array of session cache data, is that correct? |
@taylorotwell You're right about the prefix. Any suggestions for changing the config name? |
@joaopalopes24 to me that configuration option should be called "key" or something. |
@taylorotwell Done! |
Thanks! |
Would it be possible to have this global attached to the user? When a session expires, the whole shopping cart (just to name an example, or switches from devices) is gone, right? Or did I miss something with this PR? Like is it possible to do The session cache is already really cool, just wanted to ask of this PR introduced this as well. |
@francoism90 Exactly that |
@joaopalopes24 Do you mean I should submit a feature requests for the user? :) |
@francoism90 If you're in the renewal session, your data is already disabled. But it's best to test this. |
* feat(session): adding new method to remeber value for a given time * wip * feat(cache): adding session driver skeleton * feat(cache): finish cache session driver * test(cache): adding tests for cache session driver * wip * wip * wip * wip * refactor(cache): some adjusts * wip * formatting * refactor(cache): some adjusts in cache session driver * Update cache.php * Update CacheManager.php * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
New method to remember one value in session for a given time.