diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 73f6355b..5ddcd8a6 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -8,14 +8,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Check for todo0 - run: '! grep -r "todo0" --exclude-dir=workflows .' - if: always() - - name: Check for todo1 - run: '! grep -r "todo1" --exclude-dir=workflows .' - if: always() - - name: Check for todo2 - run: '! grep -r "todo2" --exclude-dir=workflows .' + - name: Check for priority todos + run: '! grep -r "todo[0-9]" --exclude-dir=workflows .' if: always() - name: Check for non-todo skip()s in tests run: '! grep -r "skip(" --exclude-dir=workflows tests/ | grep -v "todo"' diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index c0fe775c..d4ce4e41 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -21,7 +21,6 @@ $rules = [ 'blank_line_before_statement' => [ 'statements' => ['return'] ], - 'braces' => true, 'cast_spaces' => true, 'class_definition' => true, 'concat_space' => [ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d256f42..ee451d20 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,3 +43,9 @@ If you need to rebuild the container for any reason (e.g. a change in `Dockerfil ## PHPStan Use `composer phpstan` to run our phpstan suite. + +## PhpStorm + +Create `.env` with `PROJECT_PATH=/full/path/to/this/directory`. Configure a Docker-based interpreter for tests (with exec, not run). + +If you want to use XDebug, use `composer docker-rebuild-with-xdebug`. diff --git a/Dockerfile b/Dockerfile index 0d51244e..fb1620cd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,4 +30,16 @@ RUN echo "apc.enable_cli=1" >> "$PHP_INI_DIR/php.ini" # Only used on GHA COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composer +# Conditionally install and configure Xdebug (last step for faster rebuilds) +ARG XDEBUG_ENABLED=false +RUN if [ "$XDEBUG_ENABLED" = "true" ]; then \ + pecl install xdebug && docker-php-ext-enable xdebug && \ + echo "xdebug.mode=debug" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini" && \ + echo "xdebug.start_with_request=yes" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini" && \ + echo "xdebug.client_host=host.docker.internal" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini" && \ + echo "xdebug.client_port=9003" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini" && \ + echo "xdebug.discover_client_host=true" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini" && \ + echo "xdebug.log=/var/log/xdebug.log" >> "$PHP_INI_DIR/conf.d/docker-php-ext-xdebug.ini"; \ +fi + WORKDIR /var/www/html diff --git a/assets/config.php b/assets/config.php index 5e231f2a..ba503aad 100644 --- a/assets/config.php +++ b/assets/config.php @@ -93,11 +93,14 @@ return [ * Identification middleware tenancy recognizes as path identification middleware. * * This is used for determining if a path identification middleware is used - * during operations specific to path identification, e.g. forgetting the tenant parameter in ForgetTenantParameter. + * during operations specific to path identification. + * + * This is used for forgetting the tenant parameter using the ForgetTenantParameter listener. + * The listener only has an effect when path identification middleware + * is used in the global middleware stack and certain other conditions are met. * * If you're using a custom path identification middleware, add it here. * - * @see \Stancl\Tenancy\Actions\CloneRoutesAsTenant * @see \Stancl\Tenancy\Listeners\ForgetTenantParameter */ 'path_identification_middleware' => [ diff --git a/composer.json b/composer.json index 3d3bd3eb..2eab8837 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "illuminate/support": "^12.0", "laravel/tinker": "^2.0", "ramsey/uuid": "^4.7.3", - "stancl/jobpipeline": "2.0.0-rc5", + "stancl/jobpipeline": "2.0.0-rc6", "stancl/virtualcolumn": "^1.5.0", "spatie/invade": "*", "laravel/prompts": "0.*" @@ -63,7 +63,14 @@ "docker-up": "docker compose up -d", "docker-down": "docker compose down", "docker-restart": "docker compose down && docker compose up -d", - "docker-rebuild": "PHP_VERSION=8.4 docker compose up -d --no-deps --build", + "docker-rebuild": [ + "Composer\\Config::disableProcessTimeout", + "PHP_VERSION=8.4 docker compose up -d --no-deps --build" + ], + "docker-rebuild-with-xdebug": [ + "Composer\\Config::disableProcessTimeout", + "PHP_VERSION=8.4 XDEBUG_ENABLED=true docker compose up -d --no-deps --build" + ], "docker-m1": "ln -s docker-compose-m1.override.yml docker-compose.override.yml", "testbench-unlink": "rm ./vendor/orchestra/testbench-core/laravel/vendor", "testbench-link": "ln -s /var/www/html/vendor ./vendor/orchestra/testbench-core/laravel/vendor", @@ -72,10 +79,22 @@ "phpstan": "vendor/bin/phpstan --memory-limit=256M", "phpstan-pro": "vendor/bin/phpstan --memory-limit=256M --pro", "cs": "PHP_CS_FIXER_IGNORE_ENV=1 php-cs-fixer fix --config=.php-cs-fixer.php", - "test": "./test --no-coverage", - "test-full": "./test", - "act": "act -j tests --matrix 'laravel:^11.0'", - "act-input": "act -j tests --matrix 'laravel:^11.0' --input" + "test": [ + "Composer\\Config::disableProcessTimeout", + "./test --no-coverage" + ], + "test-full": [ + "Composer\\Config::disableProcessTimeout", + "./test" + ], + "act": [ + "Composer\\Config::disableProcessTimeout", + "act -j tests --matrix 'laravel:^11.0'" + ], + "act-input": [ + "Composer\\Config::disableProcessTimeout", + "act -j tests --matrix 'laravel:^11.0' --input" + ] }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/docker-compose.yml b/docker-compose.yml index 9d5eb6c8..2d7a6e9f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -2,6 +2,8 @@ services: test: build: context: . + args: + XDEBUG_ENABLED: ${XDEBUG_ENABLED:-false} depends_on: mysql: condition: service_healthy @@ -18,7 +20,8 @@ services: dynamodb: condition: service_healthy volumes: - - .:/var/www/html:cached + - .:${PROJECT_PATH:-$PWD}:cached + working_dir: ${PROJECT_PATH:-$PWD} environment: DOCKER: 1 DB_PASSWORD: password @@ -30,6 +33,8 @@ services: TENANCY_TEST_SQLSRV_HOST: mssql TENANCY_TEST_SQLSRV_USERNAME: sa TENANCY_TEST_SQLSRV_PASSWORD: P@ssword + extra_hosts: + - "host.docker.internal:host-gateway" stdin_open: true tty: true mysql: diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index c5818878..ec60d880 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -30,6 +30,11 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * By providing a callback to shouldClone(), you can change how it's determined if a route should be cloned if you don't want to use middleware flags. * * Cloned routes are prefixed with '/{tenant}', flagged with 'tenant' middleware, and have their names prefixed with 'tenant.'. + * + * The addition of the tenant parameter can be controlled using addTenantParameter(true|false). Note that if you decide to disable + * tenant parameter addition, the routes MUST differ in domains. This can be controlled using the domain(string|null) method. The + * default behavior is to NOT set any domains on cloned routes -- unless specified otherwise using that method. + * * The parameter name and prefix can be changed e.g. to `/{team}` and `team.` by configuring the path resolver (tenantParameterName and tenantRouteNamePrefix). * Routes with names that are already prefixed won't be cloned - but that's just the default behavior. * The cloneUsing() method allows you to completely override the default behavior and customize how the cloned routes will be defined. @@ -43,8 +48,8 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * * Example usage: * ``` - * Route::get('/foo', fn () => true)->name('foo')->middleware('clone'); - * Route::get('/bar', fn () => true)->name('bar')->middleware('universal'); + * Route::get('/foo', ...)->name('foo')->middleware('clone'); + * Route::get('/bar', ...)->name('bar')->middleware('universal'); * * $cloneAction = app(CloneRoutesAsTenant::class); * @@ -54,10 +59,20 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * // Clone bar route as /{tenant}/bar and name it tenant.bar ('universal' middleware won't be present in the cloned route) * $cloneAction->cloneRoutesWithMiddleware(['universal'])->handle(); * - * Route::get('/baz', fn () => true)->name('baz'); + * Route::get('/baz', ...)->name('baz'); * * // Clone baz route as /{tenant}/bar and name it tenant.baz ('universal' middleware won't be present in the cloned route) * $cloneAction->cloneRoute('baz')->handle(); + * + * Route::domain('central.localhost')->get('/no-tenant-parameter', ...)->name('no-tenant-parameter')->middleware('clone'); + * + * // Clone baz route as /no-tenant-parameter and name it tenant.no-tenant-parameter (the route won't have the tenant parameter) + * // This can be useful with domain identification. Importantly, the original route MUST have a domain set. The domain of the + * // cloned route can be customized using domain(string|null). By default, the cloned route will not be scoped to a domain, + * // unless a domain() call is used. It's important to keep in mind that: + * // 1. When addTenantParameter(false) is used, the paths will be the same, thus domains must differ. + * // 2. If the original route (with the same path) has no domain, the cloned route will never be used due to registration order. + * $cloneAction->addTenantParameter(false)->cloneRoutesWithMiddleware(['clone'])->cloneRoute('no-tenant-parameter')->handle(); * ``` * * Calling handle() will also clear the $routesToClone array. @@ -70,6 +85,8 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; class CloneRoutesAsTenant { protected array $routesToClone = []; + protected bool $addTenantParameter = true; + protected string|null $domain = null; protected Closure|null $cloneUsing = null; // The callback should accept Route instance or the route name (string) protected Closure|null $shouldClone = null; protected array $cloneRoutesWithMiddleware = ['clone']; @@ -78,6 +95,7 @@ class CloneRoutesAsTenant protected Router $router, ) {} + /** Clone routes. This resets routesToClone() but not other config. */ public function handle(): void { // If no routes were specified using cloneRoute(), get all routes @@ -102,15 +120,37 @@ class CloneRoutesAsTenant $route = $this->router->getRoutes()->getByName($route); } - $this->copyMiscRouteProperties($route, $this->createNewRoute($route)); + $this->createNewRoute($route); } // Clean up the routesToClone array after cloning so that subsequent calls aren't affected $this->routesToClone = []; $this->router->getRoutes()->refreshNameLookups(); + $this->router->getRoutes()->refreshActionLookups(); } + /** + * Should a tenant parameter be added to the cloned route. + * + * The name of the parameter is controlled using PathTenantResolver::tenantParameterName(). + */ + public function addTenantParameter(bool $addTenantParameter): static + { + $this->addTenantParameter = $addTenantParameter; + + return $this; + } + + /** The domain the cloned route should use. Set to null if it shouldn't be scoped to a domain. */ + public function domain(string|null $domain): static + { + $this->domain = $domain; + + return $this; + } + + /** Provide a custom callback for cloning routes, instead of the default behavior. */ public function cloneUsing(Closure|null $cloneUsing): static { $this->cloneUsing = $cloneUsing; @@ -118,6 +158,7 @@ class CloneRoutesAsTenant return $this; } + /** Specify which middleware should serve as "flags" telling this action to clone those routes. */ public function cloneRoutesWithMiddleware(array $middleware): static { $this->cloneRoutesWithMiddleware = $middleware; @@ -125,6 +166,10 @@ class CloneRoutesAsTenant return $this; } + /** + * Provide a custom callback for determining whether a route should be cloned. + * Overrides the default middleware-based detection. + * */ public function shouldClone(Closure|null $shouldClone): static { $this->shouldClone = $shouldClone; @@ -132,6 +177,7 @@ class CloneRoutesAsTenant return $this; } + /** Clone an individual route. */ public function cloneRoute(Route|string $route): static { $this->routesToClone[] = $route; @@ -171,28 +217,31 @@ class CloneRoutesAsTenant $action->put('as', PathTenantResolver::tenantRouteNamePrefix() . $name); } - $action - ->put('middleware', $middleware) - ->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}'); + if ($this->domain) { + $action->put('domain', $this->domain); + } elseif ($action->offsetExists('domain')) { + $action->offsetUnset('domain'); + } + + $action->put('middleware', $middleware); + + if ($this->addTenantParameter) { + $action->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}'); + } /** @var Route $newRoute */ $newRoute = $this->router->$method($uri, $action->toArray()); - return $newRoute; - } - - /** - * Copy misc properties of the original route to the new route. - */ - protected function copyMiscRouteProperties(Route $originalRoute, Route $newRoute): void - { + // Copy misc properties of the original route to the new route. $newRoute - ->setBindingFields($originalRoute->bindingFields()) - ->setFallback($originalRoute->isFallback) - ->setWheres($originalRoute->wheres) - ->block($originalRoute->locksFor(), $originalRoute->waitsFor()) - ->withTrashed($originalRoute->allowsTrashedBindings()) - ->setDefaults($originalRoute->defaults); + ->setBindingFields($route->bindingFields()) + ->setFallback($route->isFallback) + ->setWheres($route->wheres) + ->block($route->locksFor(), $route->waitsFor()) + ->withTrashed($route->allowsTrashedBindings()) + ->setDefaults($route->defaults); + + return $newRoute; } /** Removes top-level cloneRoutesWithMiddleware and adds 'tenant' middleware. */ diff --git a/src/Bootstrappers/DatabaseCacheBootstrapper.php b/src/Bootstrappers/DatabaseCacheBootstrapper.php new file mode 100644 index 00000000..ae547471 --- /dev/null +++ b/src/Bootstrappers/DatabaseCacheBootstrapper.php @@ -0,0 +1,123 @@ +scopedStoreNames(); + + foreach ($stores as $storeName) { + $this->originalConnections[$storeName] = $this->config->get("cache.stores.{$storeName}.connection"); + $this->originalLockConnections[$storeName] = $this->config->get("cache.stores.{$storeName}.lock_connection"); + + $this->config->set("cache.stores.{$storeName}.connection", 'tenant'); + $this->config->set("cache.stores.{$storeName}.lock_connection", 'tenant'); + + $this->cache->purge($storeName); + } + + if (static::$adjustGlobalCacheManager) { + // Preferably we'd try to respect the original value of this static property -- store it in a variable, + // pull it into the closure, and execute it there. But such a naive approach would lead to existing callbacks + // *from here* being executed repeatedly in a loop on reinitialization. For that reason we do not do that + // (this is our only use of $adjustCacheManagerUsing anyway) but ideally at some point we'd have a better solution. + $originalConnections = array_combine($stores, array_map(fn (string $storeName) => [ + 'connection' => $this->originalConnections[$storeName] ?? config('tenancy.database.central_connection'), + 'lockConnection' => $this->originalLockConnections[$storeName] ?? config('tenancy.database.central_connection'), + ], $stores)); + + TenancyServiceProvider::$adjustCacheManagerUsing = static function (CacheManager $manager) use ($originalConnections) { + foreach ($originalConnections as $storeName => $connections) { + /** @var DatabaseStore $store */ + $store = $manager->store($storeName)->getStore(); + + $store->setConnection(DB::connection($connections['connection'])); + $store->setLockConnection(DB::connection($connections['lockConnection'])); + } + }; + } + } + + public function revert(): void + { + foreach ($this->originalConnections as $storeName => $originalConnection) { + $this->config->set("cache.stores.{$storeName}.connection", $originalConnection); + $this->config->set("cache.stores.{$storeName}.lock_connection", $this->originalLockConnections[$storeName]); + + $this->cache->purge($storeName); + } + + TenancyServiceProvider::$adjustCacheManagerUsing = null; + } + + protected function scopedStoreNames(): array + { + return array_filter( + static::$stores ?? array_keys($this->config->get('cache.stores', [])), + function ($storeName) { + $store = $this->config->get("cache.stores.{$storeName}"); + + if (! $store) return false; + if (! isset($store['driver'])) return false; + + return $store['driver'] === 'database'; + } + ); + } +} diff --git a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php index fb371d6a..12b49f78 100644 --- a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php +++ b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php @@ -86,7 +86,6 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper protected function useTenantRoutesInFortify(Tenant $tenant): void { if (static::$passQueryParameter) { - // todo@tests $tenantParameterName = RequestDataTenantResolver::queryParameterName(); $tenantParameterValue = RequestDataTenantResolver::payloadValue($tenant); } else { diff --git a/src/Commands/Migrate.php b/src/Commands/Migrate.php index 5348b509..6ecd6e14 100644 --- a/src/Commands/Migrate.php +++ b/src/Commands/Migrate.php @@ -51,13 +51,24 @@ class Migrate extends MigrateCommand return 1; } - if ($this->getProcesses() > 1) { - return $this->runConcurrently($this->getTenantChunks()->map(function ($chunk) { - return $this->getTenants($chunk); - })); + $originalTemplateConnection = config('tenancy.database.template_tenant_connection'); + + if ($database = $this->input->getOption('database')) { + config(['tenancy.database.template_tenant_connection' => $database]); } - return $this->migrateTenants($this->getTenants()) ? 0 : 1; + if ($this->getProcesses() > 1) { + $code = $this->runConcurrently($this->getTenantChunks()->map(function ($chunk) { + return $this->getTenants($chunk); + })); + } else { + $code = $this->migrateTenants($this->getTenants()) ? 0 : 1; + } + + // Reset the template tenant connection to the original one + config(['tenancy.database.template_tenant_connection' => $originalTemplateConnection]); + + return $code; } protected function childHandle(mixed ...$args): bool diff --git a/src/Commands/MigrateFresh.php b/src/Commands/MigrateFresh.php index 4e89cefd..d4733552 100644 --- a/src/Commands/MigrateFresh.php +++ b/src/Commands/MigrateFresh.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Commands; +use Illuminate\Console\ConfirmableTrait; use Illuminate\Database\Console\Migrations\BaseCommand; use Illuminate\Database\QueryException; use Illuminate\Support\LazyCollection; @@ -17,7 +18,7 @@ use Symfony\Component\Console\Output\OutputInterface as OI; class MigrateFresh extends BaseCommand { - use HasTenantOptions, DealsWithMigrations, ParallelCommand; + use HasTenantOptions, DealsWithMigrations, ParallelCommand, ConfirmableTrait; protected $description = 'Drop all tables and re-run all migrations for tenant(s)'; @@ -27,6 +28,7 @@ class MigrateFresh extends BaseCommand $this->addOption('drop-views', null, InputOption::VALUE_NONE, 'Drop views along with tenant tables.', null); $this->addOption('step', null, InputOption::VALUE_NONE, 'Force the migrations to be run so they can be rolled back individually.'); + $this->addOption('force', null, InputOption::VALUE_NONE, 'Force the command to run when in production.', null); $this->addProcessesOption(); $this->setName('tenants:migrate-fresh'); @@ -34,6 +36,10 @@ class MigrateFresh extends BaseCommand public function handle(): int { + if (! $this->confirmToProceed()) { + return 1; + } + $success = true; if ($this->getProcesses() > 1) { diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index d7fb8da2..64b96fc1 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -46,7 +46,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager * tenant instances passed to $closeInMemoryConnectionUsing closures, * if you're setting that property as well. * - * @property Closure(PDO, string)|null + * @var Closure(PDO, string)|null */ public static Closure|null $persistInMemoryConnectionUsing = null; @@ -59,7 +59,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager * NOTE: The parameter provided to the closure is the Tenant * instance, not a PDO connection. * - * @property Closure(Tenant)|null + * @var Closure(Tenant)|null */ public static Closure|null $closeInMemoryConnectionUsing = null; diff --git a/src/Facades/GlobalCache.php b/src/Facades/GlobalCache.php index b8b5ce99..d1a182aa 100644 --- a/src/Facades/GlobalCache.php +++ b/src/Facades/GlobalCache.php @@ -8,6 +8,9 @@ use Illuminate\Support\Facades\Cache; class GlobalCache extends Cache { + /** Make sure this works identically to global_cache() */ + protected static $cached = false; + protected static function getFacadeAccessor() { return 'globalCache'; diff --git a/src/Listeners/BootstrapTenancy.php b/src/Listeners/BootstrapTenancy.php index 50f38208..957949a4 100644 --- a/src/Listeners/BootstrapTenancy.php +++ b/src/Listeners/BootstrapTenancy.php @@ -20,6 +20,10 @@ class BootstrapTenancy $tenant = $event->tenancy->tenant; $bootstrapper->bootstrap($tenant); + + if (! in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) { + $event->tenancy->initializedBootstrappers[] = $bootstrapper::class; + } } event(new TenancyBootstrapped($event->tenancy)); diff --git a/src/Listeners/ForgetTenantParameter.php b/src/Listeners/ForgetTenantParameter.php index 0b1d1440..d159b967 100644 --- a/src/Listeners/ForgetTenantParameter.php +++ b/src/Listeners/ForgetTenantParameter.php @@ -11,18 +11,18 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; // todo@earlyIdReview /** - * Remove the tenant parameter from the matched route when path identification is used globally. + * Conditionally removes the tenant parameter from matched routes when using kernel path identification. * - * While initializing tenancy, we forget the tenant parameter (in PathTenantResolver), - * so that the route actions don't have to accept it. + * When path identification middleware is in the global stack, + * the tenant parameter is initially forgotten during tenancy initialization in PathTenantResolver. + * However, because kernel identification occurs before route matching, the route still contains + * the tenant parameter when RouteMatched is fired. This listener removes it to prevent route + * actions from needing to accept an unwanted tenant parameter. * - * With kernel identification, tenancy gets initialized before the route gets matched. - * The matched route gets the tenant parameter again, so we have to forget the parameter again on RouteMatched. - * - * We remove the {tenant} parameter from the matched route when - * 1) the InitializeTenancyByPath middleware is in the global stack, AND - * 2) the matched route does not have identification middleware (so that {tenant} isn't forgotten when using route-level identification), AND - * 3) the route isn't in the central context (so that {tenant} doesn't get accidentally removed from central routes). + * The {tenant} parameter is removed from the matched route only when ALL of these conditions are met: + * 1) A path identification middleware is in the global middleware stack (kernel identification) + * 2) The matched route does NOT have its own identification middleware (route-level identification takes precedence) + * 3) The route is in tenant or universal context (central routes keep their tenant parameter) */ class ForgetTenantParameter { diff --git a/src/Listeners/RevertToCentralContext.php b/src/Listeners/RevertToCentralContext.php index 0a680532..312346ac 100644 --- a/src/Listeners/RevertToCentralContext.php +++ b/src/Listeners/RevertToCentralContext.php @@ -15,7 +15,9 @@ class RevertToCentralContext event(new RevertingToCentralContext($event->tenancy)); foreach (array_reverse($event->tenancy->getBootstrappers()) as $bootstrapper) { - $bootstrapper->revert(); + if (in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) { + $bootstrapper->revert(); + } } event(new RevertedToCentralContext($event->tenancy)); diff --git a/src/Resolvers/Contracts/CachedTenantResolver.php b/src/Resolvers/Contracts/CachedTenantResolver.php index cd982d2f..017f4b04 100644 --- a/src/Resolvers/Contracts/CachedTenantResolver.php +++ b/src/Resolvers/Contracts/CachedTenantResolver.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Stancl\Tenancy\Resolvers\Contracts; -use Illuminate\Contracts\Cache\Factory; use Illuminate\Contracts\Cache\Repository; +use Illuminate\Contracts\Foundation\Application; use Illuminate\Database\Eloquent\Model; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Contracts\TenantCouldNotBeIdentifiedException; @@ -13,12 +13,14 @@ use Stancl\Tenancy\Contracts\TenantResolver; abstract class CachedTenantResolver implements TenantResolver { - /** @var Repository */ - protected $cache; + protected Repository $cache; - public function __construct(Factory $cache) + public function __construct(Application $app) { - $this->cache = $cache->store(static::cacheStore()); + // globalCache should generally not be injected, however in this case + // the class is always created from scratch when calling invalidateCache() + // meaning the global cache stores are also resolved from scratch. + $this->cache = $app->make('globalCache')->store(static::cacheStore()); } /** diff --git a/src/Resolvers/RequestDataTenantResolver.php b/src/Resolvers/RequestDataTenantResolver.php index 4d8b3277..48ee45ee 100644 --- a/src/Resolvers/RequestDataTenantResolver.php +++ b/src/Resolvers/RequestDataTenantResolver.php @@ -31,7 +31,6 @@ class RequestDataTenantResolver extends Contracts\CachedTenantResolver public function getPossibleCacheKeys(Tenant&Model $tenant): array { - // todo@tests return [ $this->formatCacheKey(static::payloadValue($tenant)), ]; diff --git a/src/Tenancy.php b/src/Tenancy.php index f96c0a51..66173cba 100644 --- a/src/Tenancy.php +++ b/src/Tenancy.php @@ -35,6 +35,20 @@ class Tenancy */ public static array $findWith = []; + /** + * A list of bootstrappers that have been initialized. + * + * This is used when reverting tenancy, mainly if an exception + * occurs during bootstrapping, to ensure we don't revert + * bootstrappers that haven't been properly initialized + * (bootstrapped for the first time) previously. + * + * @internal + * + * @var list> + */ + public array $initializedBootstrappers = []; + /** Initialize tenancy for the passed tenant. */ public function initialize(Tenant|int|string $tenant): void { @@ -192,7 +206,6 @@ class Tenancy /** * Run a callback for multiple tenants. - * More performant than running $tenant->run() one by one. * * @param array|array|\Traversable|string|int|null $tenants */ diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 4059479e..557306b2 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -20,6 +20,11 @@ use Stancl\Tenancy\Resolvers\DomainTenantResolver; class TenancyServiceProvider extends ServiceProvider { public static Closure|null $configure = null; + public static bool $registerForgetTenantParameterListener = true; + public static bool $migrateFreshOverride = true; + + /** @internal */ + public static Closure|null $adjustCacheManagerUsing = null; /* Register services. */ public function register(): void @@ -79,7 +84,29 @@ class TenancyServiceProvider extends ServiceProvider }); $this->app->bind('globalCache', function ($app) { - return new CacheManager($app); + // We create a separate CacheManager to be used for "global" cache -- cache that + // is always central, regardless of the current context. + // + // Importantly, we use a regular binding here, not a singleton. Thanks to that, + // any time we resolve this cache manager, we get a *fresh* instance -- an instance + // that was not affected by any scoping logic. + // + // 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. + // + // For that reason, we also adjust the relevant stores on this new CacheManager + // using the callback below. It is set by DatabaseCacheBootstrapper. + $manager = new CacheManager($app); + + if (static::$adjustCacheManagerUsing !== null) { + (static::$adjustCacheManagerUsing)($manager); + } + + return $manager; }); } @@ -104,9 +131,11 @@ class TenancyServiceProvider extends ServiceProvider Commands\CreateUserWithRLSPolicies::class, ]); - $this->app->extend(FreshCommand::class, function ($_, $app) { - return new Commands\MigrateFreshOverride($app['migrator']); - }); + if (static::$migrateFreshOverride) { + $this->app->extend(FreshCommand::class, function ($_, $app) { + return new Commands\MigrateFreshOverride($app['migrator']); + }); + } $this->publishes([ __DIR__ . '/../assets/config.php' => config_path('tenancy.php'), @@ -152,6 +181,14 @@ class TenancyServiceProvider extends ServiceProvider Route::middlewareGroup('tenant', []); Route::middlewareGroup('central', []); - Event::listen(RouteMatched::class, ForgetTenantParameter::class); + if (static::$registerForgetTenantParameterListener) { + // Ideally, this listener would only be registered when kernel-level + // path identification is used, however doing that check reliably + // at this point in the lifecycle isn't feasible. For that reason, + // rather than doing an "outer" check, we do an "inner" check within + // that listener. That also means the listener needs to be registered + // always. We allow for this to be controlled using a static property. + Event::listen(RouteMatched::class, ForgetTenantParameter::class); + } } } diff --git a/tests/Bootstrappers/CacheTagsBootstrapperTest.php b/tests/Bootstrappers/CacheTagsBootstrapperTest.php index 660be1a7..f07a0f3f 100644 --- a/tests/Bootstrappers/CacheTagsBootstrapperTest.php +++ b/tests/Bootstrappers/CacheTagsBootstrapperTest.php @@ -56,7 +56,7 @@ test('tags separate cache properly', function () { $tenant1 = Tenant::create(); tenancy()->initialize($tenant1); - cache()->put('foo', 'bar', 1); + cache()->put('foo', 'bar'); expect(cache()->get('foo'))->toBe('bar'); $tenant2 = Tenant::create(); @@ -64,7 +64,7 @@ test('tags separate cache properly', function () { expect(cache('foo'))->not()->toBe('bar'); - cache()->put('foo', 'xyz', 1); + cache()->put('foo', 'xyz'); expect(cache()->get('foo'))->toBe('xyz'); }); @@ -72,7 +72,7 @@ test('invoking the cache helper works', function () { $tenant1 = Tenant::create(); tenancy()->initialize($tenant1); - cache(['foo' => 'bar'], 1); + cache(['foo' => 'bar']); expect(cache('foo'))->toBe('bar'); $tenant2 = Tenant::create(); @@ -80,7 +80,7 @@ test('invoking the cache helper works', function () { expect(cache('foo'))->not()->toBe('bar'); - cache(['foo' => 'xyz'], 1); + cache(['foo' => 'xyz']); expect(cache('foo'))->toBe('xyz'); }); @@ -88,7 +88,7 @@ test('cache is persisted', function () { $tenant1 = Tenant::create(); tenancy()->initialize($tenant1); - cache(['foo' => 'bar'], 10); + cache(['foo' => 'bar']); expect(cache('foo'))->toBe('bar'); tenancy()->end(); @@ -102,7 +102,7 @@ test('cache is persisted when reidentification is used', function () { $tenant2 = Tenant::create(); tenancy()->initialize($tenant1); - cache(['foo' => 'bar'], 10); + cache(['foo' => 'bar']); expect(cache('foo'))->toBe('bar'); tenancy()->initialize($tenant2); diff --git a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php index 63f0f2a0..bfc835ee 100644 --- a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php +++ b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php @@ -1,6 +1,5 @@ [FortifyRouteBootstrapper::class]]); + config([ + // Parameter name (RequestDataTenantResolver::queryParameterName()) + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => 'team_query', + // Parameter value (RequestDataTenantResolver::payloadValue() gets the tenant model column value) + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => 'company', + ]); + + config([ + // Parameter name (PathTenantResolver::tenantParameterName()) + 'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team_path', + // Parameter value (PathTenantResolver::tenantParameterValue() gets the tenant model column value) + 'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'name', + ]); + $originalFortifyHome = config('fortify.home'); $originalFortifyRedirects = config('fortify.redirects'); - Route::get('/home', function () { - return true; - })->name($homeRouteName = 'home'); - - Route::get('/welcome', function () { - return true; - })->name($welcomeRouteName = 'welcome'); + Route::get('/home', fn () => true)->name($homeRouteName = 'home'); + Route::get('/welcome', fn () => true)->name($welcomeRouteName = 'welcome'); FortifyRouteBootstrapper::$fortifyHome = $homeRouteName; FortifyRouteBootstrapper::$fortifyRedirectMap['login'] = $welcomeRouteName; @@ -43,21 +53,53 @@ test('fortify route tenancy bootstrapper updates fortify config correctly', func expect(config('fortify.home'))->toBe($originalFortifyHome); expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); - FortifyRouteBootstrapper::$passTenantParameter = true; - tenancy()->initialize($tenant = Tenant::create()); - expect(config('fortify.home'))->toBe('http://localhost/home?tenant=' . $tenant->getTenantKey()); - expect(config('fortify.redirects'))->toEqual(['login' => 'http://localhost/welcome?tenant=' . $tenant->getTenantKey()]); + /** + * When $passQueryParameter is true, the bootstrapper uses + * the RequestDataTenantResolver config for generating the Fortify URLs + * - tenant parameter is 'team_query' + * - parameter value is the tenant's company ('Acme') + * + * When $passQueryParameter is false, the bootstrapper uses + * the PathTenantResolver config for generating the Fortify URLs + * - tenant parameter is 'team_path' + * - parameter value is the tenant's name ('Foo') + */ + $tenant = Tenant::create([ + 'company' => 'Acme', + 'name' => 'Foo', + ]); - tenancy()->end(); - expect(config('fortify.home'))->toBe($originalFortifyHome); - expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); + // The bootstrapper generates and overrides the URLs in the Fortify config correctly + // (= the generated URLs have the correct tenant parameter + parameter value) + // The bootstrapper should use RequestDataTenantResolver while generating the URLs (default) + FortifyRouteBootstrapper::$passQueryParameter = true; - FortifyRouteBootstrapper::$passTenantParameter = false; tenancy()->initialize($tenant); - expect(config('fortify.home'))->toBe('http://localhost/home'); - expect(config('fortify.redirects'))->toEqual(['login' => 'http://localhost/welcome']); + expect(config('fortify.home'))->toBe('http://localhost/home?team_query=Acme'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome?team_query=Acme']); + + // The bootstrapper restores the original Fortify config when ending tenancy tenancy()->end(); + expect(config('fortify.home'))->toBe($originalFortifyHome); expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); + + // The bootstrapper should use PathTenantResolver while generating the URLs now + FortifyRouteBootstrapper::$passQueryParameter = false; + + tenancy()->initialize($tenant); + + expect(config('fortify.home'))->toBe('http://localhost/home?team_path=Foo'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome?team_path=Foo']); + + tenancy()->end(); + + // The bootstrapper can override the home and redirects config without the tenant parameter being passed + FortifyRouteBootstrapper::$passTenantParameter = false; + + tenancy()->initialize($tenant); + + expect(config('fortify.home'))->toBe('http://localhost/home'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome']); }); diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 558fb345..920c95a1 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -5,71 +5,95 @@ declare(strict_types=1); use Illuminate\Database\Schema\Blueprint; use Illuminate\Routing\Route; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Redis; use Stancl\Tenancy\Tests\Etc\Tenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Stancl\Tenancy\Resolvers\DomainTenantResolver; use Illuminate\Support\Facades\Route as RouteFacade; use Illuminate\Support\Facades\Schema; +use Stancl\Tenancy\Bootstrappers\CacheTagsBootstrapper; +use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; use Stancl\Tenancy\Contracts\TenantCouldNotBeIdentifiedException; +use Stancl\Tenancy\Events\TenancyEnded; +use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; +use Stancl\Tenancy\Listeners\BootstrapTenancy; +use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Middleware\InitializeTenancyByPath; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use function Stancl\Tenancy\Tests\pest; +use function Stancl\Tenancy\Tests\withCacheTables; +use function Stancl\Tenancy\Tests\withTenantDatabases; -test('tenants can be resolved using cached resolvers', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); +beforeEach($cleanup = function () { + Tenant::$extraCustomColumns = []; +}); - $tenant->domains()->create(['domain' => $tenantKey]); +afterEach($cleanup); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); +test('tenants can be resolved using cached resolvers', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + + $tenant->createDomain($tenant->{$tenantModelColumn}); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('the underlying resolver is not touched when using the cached resolver', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); +test('the underlying resolver is not touched when using the cached resolver', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); - $tenant->createDomain($tenantKey); + $tenant->createDomain($tenant->{$tenantModelColumn}); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => false]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); pest()->assertNotEmpty(DB::getQueryLog()); // not empty config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); // empty })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('cache is invalidated when the tenant is updated', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); - $tenant->createDomain($tenantKey); +test('cache is invalidated when the tenant is updated', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + + $tenant->createDomain($tenant->{$tenantModelColumn}); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); // Tenant cache gets invalidated when the tenant is updated @@ -77,41 +101,101 @@ test('cache is invalidated when the tenant is updated', function (string $resolv DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the tenant was retrieved from the DB })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('cache is invalidated when the tenant is deleted', function (string $resolver) { - DB::statement('SET FOREIGN_KEY_CHECKS=0;'); // allow deleting the tenant - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); - $tenant->createDomain($tenantKey); +// Only testing update here - presumably if this works, deletes (and other things we test here) +// will work as well. The main unique thing about this test is that it makes the change from +// *within* the tenant context. +test('cache is invalidated when tenant is updated from within the tenant context', function (string $cacheStore, array $bootstrappers) { + config([ + 'cache.default' => $cacheStore, + 'tenancy.bootstrappers' => $bootstrappers, + ]); + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + if ($cacheStore === 'database') { + withCacheTables(); + withTenantDatabases(); + } + + $resolver = PathTenantResolver::class; + + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn(true) => 'acme']); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); + expect(DB::getQueryLog())->not()->toBeEmpty(); + DB::flushQueryLog(); + + // Different cache context + tenancy()->initialize($tenant); + + // Tenant cache gets invalidated when the tenant is updated + $tenant->touch(); + + // If we use 'globalCache' in CachedTenantResolver's constructor (correct) this line has no effect - in either case the global cache is used. + // If we use 'cache' (regression) this line DOES have an effect. If we don't end tenancy, the tenant's cache is used, which + // wasn't populated before, so the test succeeds - a query is made. But if we do end tenancy, the central cache is used, + // which was populated BUT NOT INVALIDATED which makes the test fail. + // + // The actual resolver would only ever run in the central context, but this detail makes it easier to reason about how this test + // confirms the fix. + tenancy()->end(); + + DB::flushQueryLog(); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); + + expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the tenant was retrieved from the DB +})->with([ + ['redis', [CacheTenancyBootstrapper::class]], + ['redis', [CacheTagsBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], +]); + +test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { + DB::statement('SET FOREIGN_KEY_CHECKS=0;'); // allow deleting the tenant + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + $tenant->createDomain($tenant->{$tenantModelColumn}); + + DB::enableQueryLog(); + + config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); $tenant->delete(); DB::flushQueryLog(); - expect(fn () => app($resolver)->resolve(getResolverArgument($resolver, $tenantKey)))->toThrow(TenantCouldNotBeIdentifiedException::class); + expect(fn () => app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn)))->toThrow(TenantCouldNotBeIdentifiedException::class); expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the DB was queried })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); test('cache is invalidated when a tenants domain is changed', function () { @@ -307,23 +391,49 @@ test('PathTenantResolver properly separates cache for each tenant column', funct pest()->get("/x/bar/b")->assertSee('foo'); expect(count(DB::getRawQueryLog()))->toBe(4); // unchanged expect(count($redisKeys()))->toBe(4); // unchanged - - Tenant::$extraCustomColumns = []; // reset }); /** - * Return the argument for the resolver – tenant key, or a route instance with the tenant parameter. + * This method is used in generic tests to ensure that caching works correctly both with default and custom resolver config. * - * PathTenantResolver uses a route instance with the tenant parameter as the argument, - * unlike other resolvers which use a tenant key as the argument. - * - * This method is used in the tests where we test all the resolvers - * to make getting the resolver arguments less repetitive (primarily because of the PathTenantResolver). + * If $configureTenantModelColumn is false, the tenant model column is 'id' (default) -- don't configure anything, keep the defaults. + * If $configureTenantModelColumn is true, the tenant model column should be 'name' (custom) -- configure tenant_model_column in the resolvers. */ -function getResolverArgument(string $resolver, string $tenantKey): string|Route +function tenantModelColumn(bool $configureTenantModelColumn): string { + // Default tenant model column is 'id' + $tenantModelColumn = 'id'; + + if ($configureTenantModelColumn) { + // Use 'name' as the custom tenant model column + $tenantModelColumn = 'name'; + + Tenant::$extraCustomColumns = [$tenantModelColumn]; + + Schema::table('tenants', function (Blueprint $table) use ($tenantModelColumn) { + $table->string($tenantModelColumn)->unique(); + }); + + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + } + + return $tenantModelColumn; +} + +/** + * This method is used in generic tests where we test all the resolvers + * to make getting the resolver arguments less repetitive (primarily because of PathTenantResolver). + * + * For PathTenantResolver, return a route instance with the value retrieved using $tenant->{$parameterColumn} as the parameter. + * For RequestDataTenantResolver and DomainTenantResolver, return the value retrieved using $tenant->{$parameterColumn}. + * + * Tenant model column is 'id' by default, but in the generic tests, + * we also configure that to 'name' to ensure everything works both with default and custom config. + */ +function getResolverArgument(string $resolver, Tenant $tenant, string $parameterColumn = 'id'): string|Route { - // PathTenantResolver uses a route instance as the argument if ($resolver === PathTenantResolver::class) { + // PathTenantResolver uses a route instance as the argument $routeName = 'tenant-route'; // Find or create a route instance for the resolver @@ -332,17 +442,21 @@ function getResolverArgument(string $resolver, string $tenantKey): string|Route ->prefix('{tenant}') ->middleware(InitializeTenancyByPath::class); - // To make the tenant available on the route instance - // Make the 'tenant' route parameter the tenant key - // Setting the parameter on the $route->parameters property is required - // Because $route->setParameter() throws an exception when $route->parameters is not set yet - $route->parameters[PathTenantResolver::tenantParameterName()] = $tenantKey; + /** + * To make the tenant available on the route instance, + * set the 'tenant' route parameter to the tenant model column value ('id' or 'name'). + * + * Setting the parameter on the $route->parameters property is required + * because $route->setParameter() throws an exception when $route->parameters isn't set yet. + */ + $route->parameters['tenant'] = $tenant->{$parameterColumn}; - // Return the route instance with the tenant key as the 'tenant' parameter + // Return the route instance with 'id' or 'name' as the 'tenant' parameter return $route; } - // Resolvers other than PathTenantResolver use the tenant key as the argument - // Return the tenant key - return $tenantKey; + // Assuming that: + // - with RequestDataTenantResolver, the tenant model column value is the payload value + // - with DomainTenantResolver, the tenant has a domain with name equal to the tenant model column value (see the createDomain() calls in various tests) + return $tenant->{$parameterColumn}; } diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 866babb5..656ad327 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -6,6 +6,7 @@ use Stancl\Tenancy\Actions\CloneRoutesAsTenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Illuminate\Support\Facades\Route as RouteFacade; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Routing\Exceptions\UrlGenerationException; test('CloneRoutesAsTenant action clones routes with clone middleware by default', function () { config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team']); @@ -337,3 +338,54 @@ test('clone action can be used fluently', function() { expect(collect(RouteFacade::getRoutes()->get())->map->getName()) ->toContain('tenant.foo', 'tenant.bar', 'tenant.baz'); }); + +test('the cloned route can be scoped to a specified domain', function () { + RouteFacade::domain('foo.localhost')->get('/foo', fn () => in_array('tenant', request()->route()->middleware()) ? 'tenant' : 'central')->name('foo')->middleware('clone'); + + // Importantly, we CANNOT add a domain to the cloned route *if the original route didn't have a domain*. + // This is due to the route registration order - the more strongly scoped route (= route with a domain) + // must be registered first, so that Laravel tries that route first and only moves on if the domain check fails. + $cloneAction = app(CloneRoutesAsTenant::class); + // To keep the test simple we don't even need a tenant parameter + $cloneAction->domain('bar.localhost')->addTenantParameter(false)->handle(); + + expect(route('foo'))->toBe('http://foo.localhost/foo'); + expect(route('tenant.foo'))->toBe('http://bar.localhost/foo'); +}); + +test('tenant parameter addition can be controlled by setting addTenantParameter', function (bool $addTenantParameter) { + RouteFacade::domain('central.localhost') + ->get('/foo', fn () => in_array('tenant', request()->route()->middleware()) ? 'tenant' : 'central') + ->name('foo') + ->middleware('clone'); + + // By default this action also removes the domain + $cloneAction = app(CloneRoutesAsTenant::class); + $cloneAction->addTenantParameter($addTenantParameter)->handle(); + + $clonedRoute = RouteFacade::getRoutes()->getByName('tenant.foo'); + + // We only use the route() helper here, since once a request is made + // the URL generator caches the request's domain and it affects route + // generation for routes that do not have domain() specified (tenant.foo) + expect(route('foo'))->toBe('http://central.localhost/foo'); + if ($addTenantParameter) + expect(route('tenant.foo', ['tenant' => 'abc']))->toBe('http://localhost/abc/foo'); + else + expect(route('tenant.foo'))->toBe('http://localhost/foo'); + + // Original route still works + $this->withoutExceptionHandling()->get(route('foo'))->assertSee('central'); + + if ($addTenantParameter) { + expect($clonedRoute->uri())->toContain('{tenant}'); + + $this->withoutExceptionHandling()->get('http://localhost/abc/foo')->assertSee('tenant'); + $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); + } else { + expect($clonedRoute->uri())->not()->toContain('{tenant}'); + + $this->withoutExceptionHandling()->get('http://localhost/foo')->assertSee('tenant'); + $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); + } +})->with([true, false]); diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index 7ebb07a8..a5b3b856 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -27,6 +27,7 @@ use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException; use function Stancl\Tenancy\Tests\pest; +use Stancl\Tenancy\Events\MigratingDatabase; beforeEach(function () { if (file_exists($schemaPath = 'tests/Etc/tenant-schema-test.dump')) { @@ -95,6 +96,60 @@ test('migrate command works with tenants option', function () { expect(Schema::hasTable('users'))->toBeTrue(); }); +test('migrate command uses the passed database option as the template tenant connection', function () { + $originalTemplateConnection = config('tenancy.database.template_tenant_connection'); + + // Add a custom connection that will be used as the template for the tenant connection + // Identical to the default (mysql), just with different charset and collation + config(['database.connections.custom_connection' => [ + "driver" => "mysql", + "url" => "", + "host" => "mysql", + "port" => "3306", + "database" => "main", + "username" => "root", + "password" => "password", + "unix_socket" => "", + "charset" => "latin1", // Different from the default (utf8mb4) + "collation" => "latin1_swedish_ci", // Different from the default (utf8mb4_unicode_ci) + "prefix" => "", + "prefix_indexes" => true, + "strict" => true, + "engine" => null, + "options" => [] + ]]); + + $templateConnectionDuringMigration = null; + $tenantConnectionDuringMigration = null; + + Event::listen(MigratingDatabase::class, function() use (&$templateConnectionDuringMigration, &$tenantConnectionDuringMigration) { + $templateConnectionDuringMigration = config('tenancy.database.template_tenant_connection'); + $tenantConnectionDuringMigration = DB::connection('tenant')->getConfig(); + }); + + // The original tenant template connection config remains default + expect(config('tenancy.database.template_tenant_connection'))->toBe($originalTemplateConnection); + + Tenant::create(); + + // The original template connection is used when the --database option is not passed + pest()->artisan('tenants:migrate'); + expect($templateConnectionDuringMigration)->toBe($originalTemplateConnection); + + Tenant::create(); + + // The migrate command temporarily uses the connection passed in the --database option + pest()->artisan('tenants:migrate', ['--database' => 'custom_connection']); + expect($templateConnectionDuringMigration)->toBe('custom_connection'); + + // The tenant connection during migration actually used custom_connection's config + expect($tenantConnectionDuringMigration['charset'])->toBe('latin1'); + expect($tenantConnectionDuringMigration['collation'])->toBe('latin1_swedish_ci'); + + // The tenant template connection config is restored to the original after migrating + expect(config('tenancy.database.template_tenant_connection'))->toBe($originalTemplateConnection); +}); + test('migrate command only throws exceptions if skip-failing is not passed', function() { Tenant::create(); @@ -311,6 +366,21 @@ test('migrate fresh command works', function () { expect(DB::table('users')->exists())->toBeFalse(); }); +test('migrate fresh command respects force option in production', function () { + // Set environment to production + app()->detectEnvironment(fn() => 'production'); + + Tenant::create(); + + // Without --force in production, command should prompt for confirmation + pest()->artisan('tenants:migrate-fresh') + ->expectsConfirmation('Are you sure you want to run this command?'); + + // With --force, command should succeed without prompting + pest()->artisan('tenants:migrate-fresh', ['--force' => true]) + ->assertSuccessful(); +}); + test('run command with array of tenants works', function () { $tenantId1 = Tenant::create()->getTenantKey(); $tenantId2 = Tenant::create()->getTenantKey(); diff --git a/tests/DatabaseCacheBootstrapperTest.php b/tests/DatabaseCacheBootstrapperTest.php new file mode 100644 index 00000000..87ca91ca --- /dev/null +++ b/tests/DatabaseCacheBootstrapperTest.php @@ -0,0 +1,193 @@ + 'central', // Explicitly set cache DB connection name in config + 'cache.stores.database.lock_connection' => 'central', // Also set lock connection name + 'cache.default' => 'database', + 'tenancy.bootstrappers' => [ + DatabaseTenancyBootstrapper::class, + DatabaseCacheBootstrapper::class, // Used instead of CacheTenancyBootstrapper + ], + ]); +}); + +afterEach(function () { + DatabaseCacheBootstrapper::$stores = null; +}); + +test('DatabaseCacheBootstrapper switches the database cache store connections correctly', function () { + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.database.lock_connection'))->toBe('central'); + expect(Cache::store()->getConnection()->getName())->toBe('central'); + expect(Cache::lock('foo')->getConnectionName())->toBe('central'); + + tenancy()->initialize(Tenant::create()); + + expect(config('cache.stores.database.connection'))->toBe('tenant'); + expect(config('cache.stores.database.lock_connection'))->toBe('tenant'); + expect(Cache::store()->getConnection()->getName())->toBe('tenant'); + expect(Cache::lock('foo')->getConnectionName())->toBe('tenant'); + + tenancy()->end(); + + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.database.lock_connection'))->toBe('central'); + expect(Cache::store()->getConnection()->getName())->toBe('central'); + expect(Cache::lock('foo')->getConnectionName())->toBe('central'); +}); + +test('cache is separated correctly when using DatabaseCacheBootstrapper', function() { + // We need the prefix later for lower-level assertions. Let's store it + // once now and reuse this variable rather than re-fetching it to make + // it clear that the scoping does NOT come from a prefix change. + + $cachePrefix = config('cache.prefix'); + $getCacheUsingDbQuery = fn (string $cacheKey) => + DB::selectOne("SELECT * FROM `cache` WHERE `key` = '{$cachePrefix}{$cacheKey}'")?->value; + + $tenant = Tenant::create(); + $tenant2 = Tenant::create(); + + // Write to cache in central context + cache()->set('foo', 'central'); + expect(Cache::get('foo'))->toBe('central'); + // The value retrieved by the DB query is formatted like "s:7:"central";". + // We use toContain() because of this formatting instead of just toBe(). + expect($getCacheUsingDbQuery('foo'))->toContain('central'); + + tenancy()->initialize($tenant); + + // Central cache doesn't leak to tenant context + expect(Cache::has('foo'))->toBeFalse(); + expect($getCacheUsingDbQuery('foo'))->toBeNull(); + + cache()->set('foo', 'bar'); + expect(Cache::get('foo'))->toBe('bar'); + expect($getCacheUsingDbQuery('foo'))->toContain('bar'); + + tenancy()->initialize($tenant2); + + // Assert one tenant's cache doesn't leak to another tenant + expect(Cache::has('foo'))->toBeFalse(); + expect($getCacheUsingDbQuery('foo'))->toBeNull(); + + cache()->set('foo', 'xyz'); + expect(Cache::get('foo'))->toBe('xyz'); + expect($getCacheUsingDbQuery('foo'))->toContain('xyz'); + + tenancy()->initialize($tenant); + + // Assert cache didn't leak to the original tenant + expect(Cache::get('foo'))->toBe('bar'); + expect($getCacheUsingDbQuery('foo'))->toContain('bar'); + + tenancy()->end(); + + // Assert central 'foo' cache is still the same ('central') + expect(Cache::get('foo'))->toBe('central'); + expect($getCacheUsingDbQuery('foo'))->toContain('central'); +}); + +test('DatabaseCacheBootstrapper auto-detects all database driver stores by default', function() { + config([ + 'cache.stores.database' => [ + 'driver' => 'database', + 'connection' => 'central', + 'table' => 'cache', + ], + 'cache.stores.sessions' => [ + 'driver' => 'database', + 'connection' => 'central', + 'table' => 'sessions_cache', + ], + 'cache.stores.redis' => [ + 'driver' => 'redis', + 'connection' => 'default', + ], + 'cache.stores.file' => [ + 'driver' => 'file', + 'path' => '/foo/bar', + ], + ]); + + // Here, we're using auto-detection (default behavior) + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.sessions.connection'))->toBe('central'); + expect(config('cache.stores.redis.connection'))->toBe('default'); + expect(config('cache.stores.file.path'))->toBe('/foo/bar'); + + tenancy()->initialize(Tenant::create()); + + // Using auto-detection (default behavior), + // all database driver stores should be configured, + // and stores with non-database drivers are ignored. + expect(config('cache.stores.database.connection'))->toBe('tenant'); + expect(config('cache.stores.sessions.connection'))->toBe('tenant'); + expect(config('cache.stores.redis.connection'))->toBe('default'); // unchanged + expect(config('cache.stores.file.path'))->toBe('/foo/bar'); // unchanged + + tenancy()->end(); + + // All database stores should be reverted, others unchanged + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.sessions.connection'))->toBe('central'); + expect(config('cache.stores.redis.connection'))->toBe('default'); + expect(config('cache.stores.file.path'))->toBe('/foo/bar'); +}); + +test('manual $stores configuration takes precedence over auto-detection', function() { + // Configure multiple database stores + config([ + 'cache.stores.sessions' => [ + 'driver' => 'database', + 'connection' => 'central', + 'table' => 'sessions_cache', + ], + 'cache.stores.redis' => [ + 'driver' => 'redis', + 'connection' => 'default', + ], + ]); + + // Specific store overrides (including non-database stores) + DatabaseCacheBootstrapper::$stores = ['sessions', 'redis']; // Note: excludes 'database' + + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.sessions.connection'))->toBe('central'); + expect(config('cache.stores.redis.connection'))->toBe('default'); + + tenancy()->initialize(Tenant::create()); + + // Manual config takes precedence: only 'sessions' is configured + // - redis filtered out by driver check + // - database store not included in $stores + expect(config('cache.stores.database.connection'))->toBe('central'); // Excluded in manual config + expect(config('cache.stores.sessions.connection'))->toBe('tenant'); // Included and is database driver + expect(config('cache.stores.redis.connection'))->toBe('default'); // Included but filtered out (not database driver) + + tenancy()->end(); + + // Only the manually configured stores' config will be reverted + expect(config('cache.stores.database.connection'))->toBe('central'); + expect(config('cache.stores.sessions.connection'))->toBe('central'); + expect(config('cache.stores.redis.connection'))->toBe('default'); +}); diff --git a/tests/DatabasePreparationTest.php b/tests/DatabasePreparationTest.php index d5641af4..1f3a4f09 100644 --- a/tests/DatabasePreparationTest.php +++ b/tests/DatabasePreparationTest.php @@ -9,7 +9,6 @@ use Stancl\Tenancy\Events\TenantCreated; use Stancl\Tenancy\Jobs\CreateDatabase; use Stancl\Tenancy\Jobs\MigrateDatabase; use Stancl\Tenancy\Jobs\SeedDatabase; -use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; use Stancl\Tenancy\Tests\Etc\Tenant; use Illuminate\Foundation\Auth\User as Authenticable; use Stancl\Tenancy\Tests\Etc\TestSeeder; diff --git a/tests/GlobalCacheTest.php b/tests/GlobalCacheTest.php index 72ba5ebd..016ad2a4 100644 --- a/tests/GlobalCacheTest.php +++ b/tests/GlobalCacheTest.php @@ -11,6 +11,11 @@ use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\CacheTagsBootstrapper; use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; + +use function Stancl\Tenancy\Tests\withCacheTables; +use function Stancl\Tenancy\Tests\withTenantDatabases; beforeEach(function () { config([ @@ -20,26 +25,40 @@ beforeEach(function () { Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + withCacheTables(); }); -test('global cache manager stores data in global cache', function (string $bootstrapper) { - config(['tenancy.bootstrappers' => [$bootstrapper]]); +test('global cache manager stores data in global cache', function (string $store, array $bootstrappers) { + config([ + 'cache.default' => $store, + 'tenancy.bootstrappers' => $bootstrappers, + ]); + + if ($store === 'database') withTenantDatabases(true); expect(cache('foo'))->toBe(null); - GlobalCache::put(['foo' => 'bar'], 1); + GlobalCache::put('foo', 'bar'); expect(GlobalCache::get('foo'))->toBe('bar'); $tenant1 = Tenant::create(); tenancy()->initialize($tenant1); expect(GlobalCache::get('foo'))->toBe('bar'); - GlobalCache::put(['abc' => 'xyz'], 1); - cache(['def' => 'ghi'], 10); + GlobalCache::put('abc', 'xyz'); + cache(['def' => 'ghi']); expect(cache('def'))->toBe('ghi'); - // different stores, same underlying connection. the prefix is set ON THE STORE - expect(cache()->store()->getStore() === GlobalCache::store()->getStore())->toBeFalse(); - expect(cache()->store()->getStore()->connection() === GlobalCache::store()->getStore()->connection())->toBeTrue(); + // different stores + expect(cache()->store()->getStore() !== GlobalCache::store()->getStore())->toBeTrue(); + if ($store === 'redis') { + // same underlying connection. the prefix is set ON THE STORE + expect(cache()->store()->getStore()->connection() === GlobalCache::store()->getStore()->connection())->toBeTrue(); + } else { + // different connections + expect(cache()->store()->getStore()->getConnection()->getName())->toBe('tenant'); + expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + } tenancy()->end(); expect(GlobalCache::get('abc'))->toBe('xyz'); @@ -51,25 +70,129 @@ test('global cache manager stores data in global cache', function (string $boots expect(GlobalCache::get('abc'))->toBe('xyz'); expect(GlobalCache::get('foo'))->toBe('bar'); expect(cache('def'))->toBe(null); - cache(['def' => 'xxx'], 1); + cache(['def' => 'xxx']); expect(cache('def'))->toBe('xxx'); tenancy()->initialize($tenant1); expect(cache('def'))->toBe('ghi'); })->with([ - CacheTagsBootstrapper::class, - CacheTenancyBootstrapper::class, + ['redis', [CacheTagsBootstrapper::class]], + ['redis', [CacheTenancyBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], ]); -test('the global_cache helper supports the same syntax as the cache helper', function (string $bootstrapper) { - config(['tenancy.bootstrappers' => [$bootstrapper]]); +test('global cache facade is not persistent', function () { + $oldId = spl_object_id(GlobalCache::getFacadeRoot()); + + $_ = new class {}; + + expect(spl_object_id(GlobalCache::getFacadeRoot()))->not()->toBe($oldId); +}); + +test('global cache is always central', function (string $store, array $bootstrappers, string $initialCentralCall) { + config([ + 'cache.default' => $store, + 'tenancy.bootstrappers' => $bootstrappers, + ]); + + if ($store === 'database') { + withTenantDatabases(true); + } + + // This tells us which "accessor" for the global cache should be instantiated first, before we go + // into the tenant context. We make sure to not touch the other one here. This tests that whether + // a particular accessor is used "early" makes no difference in the later behavior. + if ($initialCentralCall === 'helper') { + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + global_cache()->put('central-helper', true); + } else if ($initialCentralCall === 'facade') { + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + GlobalCache::put('central-facade', true); + } else if ($initialCentralCall === 'both') { + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + global_cache()->put('central-helper', true); + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + GlobalCache::put('central-facade', true); + } $tenant = Tenant::create(); $tenant->enter(); - // different stores, same underlying connection. the prefix is set ON THE STORE - expect(cache()->store()->getStore() === global_cache()->store()->getStore())->toBeFalse(); - expect(cache()->store()->getStore()->connection() === global_cache()->store()->getStore()->connection())->toBeTrue(); + // Here we use both the helper and the facade to ensure the value is accessible via either one + if ($initialCentralCall === 'helper') { + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + expect(global_cache('central-helper'))->toBe(true); + expect(GlobalCache::get('central-helper'))->toBe(true); + } else if ($initialCentralCall === 'facade') { + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + expect(global_cache('central-facade'))->toBe(true); + expect(GlobalCache::get('central-facade'))->toBe(true); + } else if ($initialCentralCall === 'both') { + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + expect(global_cache('central-helper'))->toBe(true); + expect(GlobalCache::get('central-helper'))->toBe(true); + expect(global_cache('central-facade'))->toBe(true); + expect(GlobalCache::get('central-facade'))->toBe(true); + } + + global_cache()->put('tenant-helper', true); + GlobalCache::put('tenant-facade', true); + + tenancy()->end(); + + if ($store === 'database') expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + if ($store === 'database') expect(GlobalCache::store()->getStore()->getConnection()->getName())->toBe('central'); + + expect(global_cache('tenant-helper'))->toBe(true); + expect(GlobalCache::get('tenant-helper'))->toBe(true); + expect(global_cache('tenant-facade'))->toBe(true); + expect(GlobalCache::get('tenant-facade'))->toBe(true); + + if ($initialCentralCall === 'helper') { + expect(GlobalCache::get('central-helper'))->toBe(true); + } else if ($initialCentralCall === 'facade') { + expect(global_cache('central-facade'))->toBe(true); + } else if ($initialCentralCall === 'both') { + expect(global_cache('central-helper'))->toBe(true); + expect(GlobalCache::get('central-helper'))->toBe(true); + expect(global_cache('central-facade'))->toBe(true); + expect(GlobalCache::get('central-facade'))->toBe(true); + } +})->with([ + ['redis', [CacheTagsBootstrapper::class]], + ['redis', [CacheTenancyBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], +])->with([ + 'helper', + 'facade', + 'both', + 'none', +]); + +test('the global_cache helper supports the same syntax as the cache helper', function (string $store, array $bootstrappers) { + config([ + 'cache.default' => $store, + 'tenancy.bootstrappers' => $bootstrappers, + ]); + + if ($store === 'database') withTenantDatabases(true); + + $tenant = Tenant::create(); + $tenant->enter(); + + // different stores + expect(cache()->store()->getStore() !== GlobalCache::store()->getStore())->toBeTrue(); + if ($store === 'redis') { + // same underlying connection. the prefix is set ON THE STORE + expect(cache()->store()->getStore()->connection() === global_cache()->store()->getStore()->connection())->toBeTrue(); + } else { + // different connections + expect(cache()->store()->getStore()->getConnection()->getName())->toBe('tenant'); + expect(global_cache()->store()->getStore()->getConnection()->getName())->toBe('central'); + } expect(cache('foo'))->toBe(null); // tenant cache is empty @@ -81,6 +204,7 @@ test('the global_cache helper supports the same syntax as the cache helper', fun expect(cache('foo'))->toBe(null); // tenant cache is not affected })->with([ - CacheTagsBootstrapper::class, - CacheTenancyBootstrapper::class, + ['redis', [CacheTagsBootstrapper::class]], + ['redis', [CacheTenancyBootstrapper::class]], + ['database', [DatabaseTenancyBootstrapper::class, DatabaseCacheBootstrapper::class]], ]); diff --git a/tests/InitializedBootstrappersTest.php b/tests/InitializedBootstrappersTest.php new file mode 100644 index 00000000..549c527f --- /dev/null +++ b/tests/InitializedBootstrappersTest.php @@ -0,0 +1,87 @@ + [ + Initialized_DummyBootstrapperFoo::class, + Initialized_DummyBootstrapperBar::class, + Initialized_DummyBootstrapperBaz::class, + ]]); + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + // Only needs to be done in tests + app()->singleton(Initialized_DummyBootstrapperFoo::class); + app()->singleton(Initialized_DummyBootstrapperBar::class); + app()->singleton(Initialized_DummyBootstrapperBaz::class); + + $tenant = TenantModel::create(); + + try { + $tenant->run(fn() => null); + // Should throw an exception + expect(true)->toBe(false); + } catch (Exception $e) { + // NOT 'baz fail in revert' as was the behavior before + // the commit that added this test + expect($e->getMessage())->toBe('bar fail in bootstrap'); + } + + expect(tenancy()->initializedBootstrappers)->toBe([ + Initialized_DummyBootstrapperFoo::class, + ]); +}); + +class Initialized_DummyBootstrapperFoo implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + $this->bootstrapped = 'bootstrapped'; + } + + public function revert(): void + { + $this->bootstrapped = 'reverted'; + } +} + +class Initialized_DummyBootstrapperBar implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + throw new Exception('bar fail in bootstrap'); + } + + public function revert(): void + { + $this->bootstrapped = 'reverted'; + } +} + +class Initialized_DummyBootstrapperBaz implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + $this->bootstrapped = 'bootstrapped'; + } + + public function revert(): void + { + throw new Exception('baz fail in revert'); + } +} diff --git a/tests/Pest.php b/tests/Pest.php index cd18d174..a4517f09 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -2,21 +2,52 @@ namespace Stancl\Tenancy\Tests; +use Illuminate\Database\Schema\Blueprint; use Stancl\Tenancy\Tests\TestCase; use Stancl\JobPipeline\JobPipeline; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Schema; +use Stancl\Tenancy\Events\TenancyEnded; +use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Jobs\CreateDatabase; use Stancl\Tenancy\Events\TenantCreated; +use Stancl\Tenancy\Jobs\MigrateDatabase; +use Stancl\Tenancy\Listeners\BootstrapTenancy; +use Stancl\Tenancy\Listeners\RevertToCentralContext; uses(TestCase::class)->in(__DIR__); -function withTenantDatabases() +function withBootstrapping() { - Event::listen(TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) { + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); +} + +function withTenantDatabases(bool $migrate = false) +{ + Event::listen(TenantCreated::class, JobPipeline::make($migrate + ? [CreateDatabase::class, MigrateDatabase::class] + : [CreateDatabase::class] + )->send(function (TenantCreated $event) { return $event->tenant; })->toListener()); } +function withCacheTables() +{ + Schema::create('cache', function (Blueprint $table) { + $table->string('key')->primary(); + $table->mediumText('value'); + $table->integer('expiration'); + }); + + Schema::create('cache_locks', function (Blueprint $table) { + $table->string('key')->primary(); + $table->string('owner'); + $table->integer('expiration'); + }); +} + function pest(): TestCase { return \Pest\TestSuite::getInstance()->test; diff --git a/tests/RequestDataIdentificationTest.php b/tests/RequestDataIdentificationTest.php index 0f635bf1..63940ed5 100644 --- a/tests/RequestDataIdentificationTest.php +++ b/tests/RequestDataIdentificationTest.php @@ -9,6 +9,7 @@ use Stancl\Tenancy\Middleware\InitializeTenancyByRequestData; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use Stancl\Tenancy\Tests\Etc\Tenant; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Cookie\CookieValuePrefix; beforeEach(function () { config([ @@ -88,6 +89,11 @@ test('cookie identification works', function (string|null $tenantModelColumn) { // Default cookie name $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test')->assertSee($tenant->id); + // Manually encrypted cookie (encrypt the cookie exactly like MakesHttpRequests does in prepareCookiesForRequest()) + $encryptedPayload = encrypt(CookieValuePrefix::create('tenant', app('encrypter')->getKey()) . $payload, false); + + $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $encryptedPayload)->get('test')->assertSee($tenant->id); + // Custom cookie name config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.cookie' => 'custom_tenant_id']); $this->withoutExceptionHandling()->withUnencryptedCookie('custom_tenant_id', $payload)->get('test')->assertSee($tenant->id); @@ -97,10 +103,7 @@ test('cookie identification works', function (string|null $tenantModelColumn) { expect(fn () => $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test'))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); })->with([null, 'slug']); -// todo@tests encrypted cookie - test('an exception is thrown when no tenant data is provided in the request', function () { pest()->expectException(TenantCouldNotBeIdentifiedByRequestDataException::class); $this->withoutExceptionHandling()->get('test'); }); - diff --git a/tests/TestCase.php b/tests/TestCase.php index 0999c117..0aca40e4 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -23,8 +23,10 @@ use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper; use Stancl\Tenancy\Bootstrappers\BroadcastingConfigBootstrapper; use Stancl\Tenancy\Bootstrappers\BroadcastChannelPrefixBootstrapper; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; -use function Stancl\Tenancy\Tests\pest; use Stancl\Tenancy\Bootstrappers\LogTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseCacheBootstrapper; + +use function Stancl\Tenancy\Tests\pest; abstract class TestCase extends \Orchestra\Testbench\TestCase { @@ -39,6 +41,10 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase ini_set('memory_limit', '1G'); + TenancyServiceProvider::$registerForgetTenantParameterListener = true; + TenancyServiceProvider::$migrateFreshOverride = true; + TenancyServiceProvider::$adjustCacheManagerUsing = null; + Redis::connection('default')->flushdb(); Redis::connection('cache')->flushdb(); Artisan::call('cache:clear memcached'); // flush memcached @@ -181,6 +187,7 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase // to manually register bootstrappers as singletons here. $app->singleton(RedisTenancyBootstrapper::class); $app->singleton(CacheTenancyBootstrapper::class); + $app->singleton(DatabaseCacheBootstrapper::class); $app->singleton(BroadcastingConfigBootstrapper::class); $app->singleton(BroadcastChannelPrefixBootstrapper::class); $app->singleton(PostgresRLSBootstrapper::class);