fix: refactor Request scope cache feature by smarcet · Pull Request #415 · OpenStackweb/summit-api

smarcet · 2025-10-15T18:18:07Z

ref: https://app.clickup.com/t/86b736x8d
remove unnecessary middle-ware

Copilot

Pull Request Overview

Refactors the request-scoped caching mechanism by removing the middleware-based tag flushing and moving to a per-request keying strategy with a configurable cache store.

Add configurable cache store for request-scoped caching.
Replace tag-based, middleware-flushed cache with per-request key + short TTL.
Remove RequestScopedCacheMiddleware and its registration in Kernel.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
config/cache.php	Introduces request_scope_cache_store config to choose cache driver for request scope.
app/ModelSerializers/Traits/RequestScopedCache.php	Adds store() helper and refactors cache() to use per-request keys with TTL. Removes JSON compression and tag usage.
app/Http/Middleware/RequestScopedCacheMiddleware.php	Deletes middleware that previously flushed tag-scoped cache per request.
app/Http/Kernel.php	Removes RequestScopedCacheMiddleware from the global middleware stack.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-15T18:20:22Z

app/ModelSerializers/Traits/RequestScopedCache.php

-        //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s res %s adding to cache.", $scope, $key, $json));
-        Cache::tags($scope)->add($key, gzdeflate($json, 9));
-        return $res;
+        self::store()->rememberForever($key, fn() => $callback());


The unscoped rememberForever call stores the value under the global key permanently, breaking the request-scoped behavior and potentially leaking data across requests. Remove line 70 so only the request-scoped key with a short TTL is used.

Suggested change

self::store()->rememberForever($key, fn() => $callback());

// Removed global rememberForever call to ensure request-scoped caching only

Copilot · 2025-10-15T18:20:23Z

app/ModelSerializers/Traits/RequestScopedCache.php

-        //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s res %s adding to cache.", $scope, $key, $json));
-        Cache::tags($scope)->add($key, gzdeflate($json, 9));
-        return $res;
+        self::store()->rememberForever($key, fn() => $callback());


On a cache miss, the callback is executed twice: once by rememberForever (line 70) and again by remember (line 73). Remove the rememberForever call to avoid double computation and unnecessary writes.

Suggested change

self::store()->rememberForever($key, fn() => $callback());

Copilot · 2025-10-15T18:20:23Z

config/cache.php

+    'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),
 ];


Document the purpose and acceptable values for request_scope_cache_store (e.g., 'array', 'redis', 'memcached') and how it affects request-scoped caching behavior, especially differences in persistence and TTL handling.

Suggested change

'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

];

/*

|--------------------------------------------------------------------------

| Request-Scoped Cache Store

|--------------------------------------------------------------------------

|

| This option determines which cache store is used for request-scoped caching.

| Acceptable values are 'array', 'redis', 'memcached', or any other defined

| cache store. The 'array' store is non-persistent and resets on every request,

| making it suitable for temporary, in-memory caching with no TTL enforcement.

| 'redis' and 'memcached' are persistent stores, allowing cache data to survive

| across requests and support TTL (expiration) handling. Choose the store that

| best fits your application's request-scoped caching needs.

|

*/

'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

remove unnecessary middleware that was affecting performance

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-15T20:21:56Z

app/ModelSerializers/Traits/RequestScopedCache.php

+        $key   = self::getScopeId().':'.$key;
+        $store = self::store();

-        $scope = self::getScopeId();
+        $computed = false;

-        //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s.", $scope, $key));
+        $value = $store->remember($key, now()->addSeconds(30), function () use ($callback, &$computed, $key) {
+            $computed = true; // closure ran => MISS
+            Log::debug('RequestScopedCache MISS', ['key' => $key]);
+            return $callback();
+        });

-        $res = Cache::tags($scope)->get($key);
-        if(!empty($res)){
-            $json_res = gzinflate($res);
-            $res = json_decode($json_res,true);
-            //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s cache hit res %s.", $scope, $key, $json_res));
-            return $res;
+        if (!$computed) {
+            Log::debug('RequestScopedCache HIT', ['key' => $key]);
        }

-        $res = $callback();
-        $json = json_encode($res);
-        //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s res %s adding to cache.", $scope, $key, $json));
-        Cache::tags($scope)->add($key, gzdeflate($json, 9));
-        return $res;
+        return $value;


Cache::remember will not persist and subsequently hit for null values; a null result will cause the callback to run on every access within the same request. To correctly cache nulls, use has()/get()/put() semantics: check $store->has($key), log HIT, otherwise compute, log MISS, and put the value with the TTL.

Copilot · 2025-10-15T20:21:56Z

app/ModelSerializers/Traits/RequestScopedCache.php

+        $computed = false;

-        //Log::debug(sprintf("RequestScopedCache::cache scope %s key %s.", $scope, $key));
+        $value = $store->remember($key, now()->addSeconds(30), function () use ($callback, &$computed, $key) {


[nitpick] The 30-second TTL is a magic number. Make it configurable (e.g., config('cache.request_scope_cache_ttl', 30)) so it can be tuned per environment without code changes.

Suggested change

$value = $store->remember($key, now()->addSeconds(30), function () use ($callback, &$computed, $key) {

$ttl = config('cache.request_scope_cache_ttl', 30);

$value = $store->remember($key, now()->addSeconds($ttl), function () use ($callback, &$computed, $key) {

Copilot · 2025-10-15T20:21:57Z

config/cache.php


    'prefix' => 'laravel',
-
+    'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),


[nitpick] If you externalize the TTL (see trait comment), add a sibling config key such as 'request_scope_cache_ttl' => env('REQUEST_SCOPE_CACHE_TTL', 30) here, and consume it in the trait.

Suggested change

'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

'request_scope_cache_ttl' => env('REQUEST_SCOPE_CACHE_TTL', 30),

Copilot · 2025-10-15T20:21:57Z

app/ModelSerializers/Traits/RequestScopedCache.php

 trait RequestScopedCache
 {

+    static function store(){


[nitpick] Declare method visibility explicitly and add a return type for clarity and static analysis support, e.g., public static function store(): \Illuminate\Contracts\Cache\Repository.

Suggested change

static function store(){

public static function store(): \Illuminate\Contracts\Cache\Repository {

smarcet force-pushed the chore/refactor-request-scope branch from d684412 to a806634 Compare October 15, 2025 18:18

smarcet requested a review from Copilot October 15, 2025 18:18

Copilot AI reviewed Oct 15, 2025

View reviewed changes

smarcet force-pushed the chore/refactor-request-scope branch 2 times, most recently from 3940103 to c95709d Compare October 15, 2025 20:03

fix: refactor Request scope cache feature

65125f5

remove unnecessary middleware that was affecting performance

smarcet force-pushed the chore/refactor-request-scope branch from c95709d to 65125f5 Compare October 15, 2025 20:14

smarcet requested a review from Copilot October 15, 2025 20:19

Copilot AI reviewed Oct 15, 2025

View reviewed changes

smarcet merged commit 71ff0fe into main Oct 15, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: refactor Request scope cache feature#415

fix: refactor Request scope cache feature#415
smarcet merged 1 commit intomainfrom
chore/refactor-request-scope

smarcet commented Oct 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	self::store()->rememberForever($key, fn() => $callback());
	// Removed global rememberForever call to ensure request-scoped caching only

		'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),
		];

-    'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),
-];
+    /*
+    |--------------------------------------------------------------------------
+    | Request-Scoped Cache Store
+    |--------------------------------------------------------------------------
+    |
+    | This option determines which cache store is used for request-scoped caching.
+    | Acceptable values are 'array', 'redis', 'memcached', or any other defined
+    | cache store. The 'array' store is non-persistent and resets on every request,
+    | making it suitable for temporary, in-memory caching with no TTL enforcement.
+    | 'redis' and 'memcached' are persistent stores, allowing cache data to survive
+    | across requests and support TTL (expiration) handling. Choose the store that
+    | best fits your application's request-scoped caching needs.
+    |
+    */
+    'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

	$value = $store->remember($key, now()->addSeconds(30), function () use ($callback, &$computed, $key) {
	$ttl = config('cache.request_scope_cache_ttl', 30);
	$value = $store->remember($key, now()->addSeconds($ttl), function () use ($callback, &$computed, $key) {


		'prefix' => 'laravel',

		'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),

	'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),
	'request_scope_cache_store' => env('REQUEST_SCOPE_CACHE_STORE', 'array'),
	'request_scope_cache_ttl' => env('REQUEST_SCOPE_CACHE_TTL', 30),

	static function store(){
	public static function store(): \Illuminate\Contracts\Cache\Repository {

Conversation

smarcet commented Oct 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants