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
(0cf7043),
and the changes mentioned above
(5e65c67)
make these tests pass.

---------

Co-authored-by: Samuel Stancl <samuel@archte.ch>

Author: lukinovec

src/Bootstrappers/CacheTenancyBootstrapper.php

@@ -16,6 +16,15 @@
 
 /**
  * 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
 {

src/Bootstrappers/DatabaseCacheBootstrapper.php

@@ -21,11 +21,15 @@
  *
  * 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
 {

src/TenancyServiceProvider.php

@@ -86,14 +86,30 @@ public function register(): void
             // 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);
             }

tests/CachedTenantResolverTest.php

@@ -165,6 +165,7 @@
     ['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) {

tests/GlobalCacheTest.php

@@ -165,6 +165,7 @@
     ['redis', [CacheTagsBootstrapper::class]],
     ['redis', [CacheTenancyBootstrapper::class]],
     ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]],
+    ['database', [DatabaseTenancyBootstrapper::class, CacheTenancyBootstrapper::class]],
 ])->with([
     'helper',
     'facade',