From ecf031237d5eca044a54325f96cb346ba7e51bcb Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sun, 28 Jun 2026 03:30:01 +0200 Subject: [PATCH] Make globalCache always use central conn with DB cache stores (#1462) `globalCache` should always use the central connection, but when using a `database`-driver cache store with `DatabaseTenancyBootstrapper`, it does not (with the exception of `DatabaseCacheBootstrapper`, explained below). `globalCache` creates a fresh `CacheManager` each time it's resolved (it's a `bind`, not a `singleton`). A freshly-created manager builds its database stores using the current default DB connection. When `DatabaseTenancyBootstrapper` is active, that default is `tenant`. So `globalCache` in tenant context points at the tenant DB. Specifically, `CachedTenantResolver` stores cached tenant lookups via `globalCache`. When a domain is deleted in tenant context, the invalidation logic calls `globalCache->forget(...)`, but that hits the tenant DB, while the resolver cache entry is in the central DB. `globalCache->forget(...)` doesn't actually do anything in that case. With `DatabaseCacheBootstrapper`, this is already handled. `globalCache` is always central because it sets `TenancyServiceProvider::$adjustCacheManagerUsing` to a callback that explicitly restores the central connection on `globalCache`'s stores. To fix this, after constructing the fresh CacheManager in the globalCache binding, explicitly set the connection of every database-driver store to its configured connection value, **falling back to central_connection** when the config value is null (null value = inherits whatever the current default DB connection is). This is sufficient for `CacheTenancyBootstrapper` and any other bootstrapper that doesn't explicitly set the store's DB connection. For `DatabaseCacheBootstrapper` specifically, this alone is not enough since it explicitly sets the store's config connection to 'tenant'. That's why DatabaseCacheBootstrapper's `$adjustCacheManagerUsing` callback runs after and overrides those stores back to the original (central) connection. > In short: `makeDatabaseCacheStoresCentral()` handles stores with a `null` connection config (falls back to central). `$adjustCacheManagerUsing` handles the `DatabaseCacheBootstrapper` case where the config is explicitly set to 'tenant'. Added datasets that use the database cache store + CacheTenancyBootstrapper to the relevant tests (globalCache and invalidation) to test regression (https://github.com/archtechx/tenancy/commit/0cf7043b733848b6d139de967df676e30247323e), and the changes mentioned above (https://github.com/archtechx/tenancy/pull/1462/commits/5e65c67ea0daf98f57f2a6a7b0e1937bbc397a56) make these tests pass. --------- Co-authored-by: Samuel Stancl --- .../CacheTenancyBootstrapper.php | 9 +++++++ .../DatabaseCacheBootstrapper.php | 8 ++++-- src/TenancyServiceProvider.php | 26 +++++++++++++++---- tests/CachedTenantResolverTest.php | 1 + tests/GlobalCacheTest.php | 1 + 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/Bootstrappers/CacheTenancyBootstrapper.php b/src/Bootstrappers/CacheTenancyBootstrapper.php index 97bd7d24..74fc8490 100644 --- a/src/Bootstrappers/CacheTenancyBootstrapper.php +++ b/src/Bootstrappers/CacheTenancyBootstrapper.php @@ -16,6 +16,15 @@ use Stancl\Tenancy\Contracts\Tenant; /** * Makes cache tenant-aware by applying a prefix. + * + * Using this bootstrapper together with DatabaseTenancyBootstrapper + * with a database cache store will result in "double scoping". The store will be scoped + * by the DB connection (entries will go into the tenant's database) *and* by the prefix. + * This is harmless in most cases, but is important to be aware of. + * + * If you only use database cache stores, consider using DatabaseCacheBootstrapper instead. + * + * @see Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper */ class CacheTenancyBootstrapper implements TenancyBootstrapper { diff --git a/src/Bootstrappers/DatabaseCacheBootstrapper.php b/src/Bootstrappers/DatabaseCacheBootstrapper.php index 0e41849f..81611d0a 100644 --- a/src/Bootstrappers/DatabaseCacheBootstrapper.php +++ b/src/Bootstrappers/DatabaseCacheBootstrapper.php @@ -21,11 +21,15 @@ use Stancl\Tenancy\TenancyServiceProvider; * * By default, this bootstrapper scopes ALL cache stores that use the database driver. If you only * want to scope SOME stores, set the static $stores property to an array of names of the stores - * you want to scope. These stores must use 'database' as their driver. + * you want to scope. Those stores must use 'database' as their driver. * * Notably, this bootstrapper sets TenancyServiceProvider::$adjustCacheManagerUsing to a callback * that ensures all affected stores still use the central connection when accessed via global cache - * (typicaly the GlobalCache facade or global_cache() helper). + * (typically the GlobalCache facade or global_cache() helper). The code in TenancyServiceProvider + * that uses `extend()` callbacks to make database stores on the global cache manager use the central + * connection only corrects stores scoped by the Database*Tenancy*Bootstrapper. This bootstrapper + * also changes the stores' connection in the *config* to 'tenant' which doesn't let that callback + * change the connection back to central on the global cache manager. */ class DatabaseCacheBootstrapper implements TenancyBootstrapper { diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index afd20fb6..23d1ffab 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -86,14 +86,30 @@ class TenancyServiceProvider extends ServiceProvider // This works great for cache stores that are *directly* scoped, like Redis or // any other tagged or prefixed stores, but it doesn't work for the database driver. // - // When we use the DatabaseTenancyBootstrapper, it changes the default connection, - // and therefore the connection of the database store that will be created when - // this new CacheManager is instantiated again. + // When DatabaseTenancyBootstrapper is used, it changes the default DB connection + // to 'tenant'. A freshly created CacheManager would therefore instantiate database + // stores with the tenant connection. // - // For that reason, we also adjust the relevant stores on this new CacheManager - // using the callback below. It is set by DatabaseCacheBootstrapper. + // For that reason, we override the 'database' driver creator on this manager so that + // database stores are built with the central connection, and we run the + // $adjustCacheManagerUsing callback below (set by DatabaseCacheBootstrapper). $manager = new CacheManager($app); + // When DatabaseTenancyBootstrapper is used, database stores whose 'connection' + // config is null fall back to the default DB connection ('tenant'). Reset each + // such store to its explicitly configured connection, or fall back to central. + $centralConnection = $app['config']['tenancy.database.central_connection']; + $manager->extend('database', function ($_app, array $config) use ($centralConnection) { + $config['connection'] ??= $centralConnection; + + /** @var CacheManager $this */ + return $this->createDatabaseDriver($config); // @phpstan-ignore method.protected + }); + + // DatabaseCacheBootstrapper explicitly writes 'tenant' into each store's 'connection' + // config. The extend() closure above would then read 'tenant' as the configured value + // (not null) and use it directly, so the central connection fallback wouldn't be used. + // This callback is used to correct those connections back to central for globalCache. if (static::$adjustCacheManagerUsing !== null) { (static::$adjustCacheManagerUsing)($manager); } diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 920c95a1..fc6cfb79 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -165,6 +165,7 @@ test('cache is invalidated when tenant is updated from within the tenant context ['redis', [CacheTenancyBootstrapper::class]], ['redis', [CacheTagsBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ]); test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { diff --git a/tests/GlobalCacheTest.php b/tests/GlobalCacheTest.php index 016ad2a4..4cda8b74 100644 --- a/tests/GlobalCacheTest.php +++ b/tests/GlobalCacheTest.php @@ -165,6 +165,7 @@ test('global cache is always central', function (string $store, array $bootstrap ['redis', [CacheTagsBootstrapper::class]], ['redis', [CacheTenancyBootstrapper::class]], ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]], ])->with([ 'helper', 'facade',