Skip to content

Refactor SwitchTenant task #8623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 2, 2025
Merged

Refactor SwitchTenant task #8623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e9ba76d
Refactor to remove TenantBootstrapper
nolanpro Nov 26, 2025
82d1d47
Cleanup
nolanpro Nov 26, 2025
060c1b0
Fix docker host url
nolanpro Nov 26, 2025
d75aab5
Remove changes to composer files
nolanpro Nov 26, 2025
3902518
Fixes for cursorbot
nolanpro Nov 26, 2025
a64fbed
Allow non-multitenant queues
nolanpro Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions ProcessMaker/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Illuminate\Support\Facades\Config;
use ProcessMaker\Console\Kernel;
use ProcessMaker\Multitenancy\Tenant;
use ProcessMaker\Multitenancy\TenantBootstrapper;

/**
* Class Application.
Expand Down Expand Up @@ -103,15 +102,4 @@ public function registerConfiguredProviders()

parent::registerConfiguredProviders();
}

public function bootstrapWith(array $bootstrappers)
{
// Insert TenantBootstrapper after LoadEnvironmentVariables
if ($bootstrappers[0] !== LoadEnvironmentVariables::class) {
throw new \Exception('LoadEnvironmentVariables is not the first bootstrapper. Did a laravel upgrade change this?');
}
array_splice($bootstrappers, 1, 0, [TenantBootstrapper::class]);

return parent::bootstrapWith($bootstrappers);
}
}
51 changes: 0 additions & 51 deletions ProcessMaker/Console/Commands/HorizonListen.php
View file
Open in desktop

This file was deleted.

115 changes: 94 additions & 21 deletions ProcessMaker/Console/Commands/TenantsVerify.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class TenantsVerify extends Command
*
* @var string
*/
protected $signature = 'tenants:verify';
protected $signature = 'tenants:verify {--json : Output the results as JSON}';

/**
* The console command description.
Expand All @@ -33,20 +33,29 @@ class TenantsVerify extends Command
*
* @return int
*/
private $jsonData = [];

public function handle()
{
if (!config('app.multitenancy')) {
$this->info('Multitenancy is disabled');

return;
}

$errors = [];
$currentTenant = null;
if (app()->has('currentTenant')) {
$currentTenant = app('currentTenant');
}

if (config('app.multitenancy') && !$currentTenant) {
if (!$currentTenant) {
$this->error('Multitenancy enabled but no current tenant found.');

return;
}

$this->info('Current Tenant ID: ' . ($currentTenant?->id ?? 'NONE'));
\Log::warning('TenantsVerify: Current Tenant ID: ' . ($currentTenant?->id ?? 'NONE'));
Copy link

@cursor cursor bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Debug logging to warning level left in diagnostic command

The command now writes \Log::warning() entries for informational output that was previously only displayed via $this->info() and $this->table(). Line 52 replaces console info output with a warning-level log entry, and line 173 adds warning-level logging for every row in the table output. Using warning-level logging for routine informational output in a diagnostic command will create unnecessary noise in production logs every time tenants:verify runs.

Additional Locations (1)

Fix in Cursor Fix in Web


$paths = [
['Storage Path', storage_path()],
Expand All @@ -55,32 +64,44 @@ public function handle()
];

// Display paths in a nice table
$this->table(['Path', 'Value'], $paths);
$this->infoTable(['Path', 'Value'], $paths);

$configs = [
'app.key',
'app.url',
'app.instance',
'cache.prefix',
'database.redis.options.prefix',
'cache.stores.cache_settings.prefix',
'script-runner-microservice.callback',
'database.connections.processmaker.database',
'logging.channels.daily.path',
'filesystems.disks.public.root',
'filesystems.disks.local.root',
'filesystems.disks.lang.root',
'app.key' => null,
'app.url' => null,
'app.instance' => null,
'cache.prefix' => 'tenant_{tenant_id}:',
'database.redis.options.prefix' => null,
'cache.stores.cache_settings.prefix' => 'tenant_{tenant_id}:settings',
'script-runner-microservice.callback' => null,
'database.connections.processmaker.database' => null,
'logging.channels.daily.path' => base_path() . '/storage/tenant_{tenant_id}/logs/processmaker.log',
'filesystems.disks.public.root' => base_path() . '/storage/tenant_{tenant_id}/app/public',
'filesystems.disks.local.root' => base_path() . '/storage/tenant_{tenant_id}/app',
'filesystems.disks.lang.root' => base_path() . '/resources/lang/tenant_{tenant_id}',
];

$configs = array_map(function ($config) {
$configs = array_map(function ($config) use ($configs, $currentTenant, &$errors) {
$ok = '';
if ($configs[$config] !== null) {
$expected = str_replace('{tenant_id}', $currentTenant->id, $configs[$config]);
if (config($config) === $expected) {
$ok = '✓';
} else {
$ok = '✗';
$errors[] = 'Expected: ' . $expected . ' != Actual: ' . config($config);
}
}

return [
$config,
config($config),
$ok,
];
}, $configs);
}, array_keys($configs));

// Display configs in a nice table
$this->table(['Config', 'Value'], $configs);
$this->infoTable(['Config', 'Value', 'OK'], $configs);

$env = EnvironmentVariable::first();
if (!$env) {
Expand All @@ -102,10 +123,62 @@ public function handle()
['Tenant Config Is Cached', File::exists(app()->getCachedConfigPath()) ? 'Yes' : 'No'],
['First username (database check)', User::first()?->username ?? 'No users found'],
['Decrypted check', substr($decrypted, 0, 50)],
['Original App URL (landlord)', $currentTenant?->getOriginalValue('APP_URL') ?? config('app.url')],
// ['Original App URL (landlord)', $currentTenant?->getOriginalValue('APP_URL') ?? config('app.url')],
['config("app.url")', config('app.url')],
['getenv("APP_URL")', getenv('APP_URL')],
['env("APP_URL")', env('APP_URL')],
['$_SERVER["APP_URL"]', $_SERVER['APP_URL'] ?? 'NOT SET'],
['$_ENV["APP_URL"]', $_ENV['APP_URL'] ?? 'NOT SET'],
['Current PID', getmypid()],
];

// Display other in a nice table
$this->table(['Other', 'Value'], $other);
$this->infoTable(['Other', 'Value'], $other);

$checkUrls = [
'config("app.url")' => config('app.url'),
'getenv("APP_URL")' => getenv('APP_URL'),
'env("APP_URL")' => env('APP_URL'),
'$_SERVER["APP_URL"]' => $_SERVER['APP_URL'] ?? 'NOT SET',
'$_ENV["APP_URL"]' => $_ENV['APP_URL'] ?? 'NOT SET',
];

foreach ($checkUrls as $key => $value) {
if ($value !== $currentTenant?->config['app.url']) {
$errors[] = 'Expected: ' . $key . ' to be ' . $currentTenant?->config['app.url'] . ' but got ' . $value;
}
}

$this->finish($errors);
}

private function finish($errors)
{
if (count($errors) > 0) {
$this->error('Errors found');
} else {
$this->info('No errors found');
}

if ($this->option('json')) {
$this->jsonData['Errors'] = $errors;
$this->line(json_encode($this->jsonData, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
}
}

private function infoTable($headers, $rows)
{
if ($this->option('json')) {
$section = [];
foreach ($rows as $row) {
$section[$row[0]] = $row[1];
}
$this->jsonData[$headers[0]] = $section;
} else {
foreach ($rows as $row) {
\Log::warning($row[0] . ': ' . $row[1]);
}
$this->table($headers, $rows);
}
}
}
9 changes: 6 additions & 3 deletions ProcessMaker/Multitenancy/MakeQueueTenantAwareAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
class MakeQueueTenantAwareAction extends BaseMakeQueueTenantAwareAction
{
/**
* We're handling tenant aware queues manually, however, we still need to implement this because for some
* reason the Spatie package calls it in Multitenancy::start(), weather it's a configured action or not.
* Non-multitenant environments shouldn't throw an exception if the tenant is not found.
*/
public function execute() : void
{
// Do nothing
if (!config('app.multitenancy')) {
return;
}

parent::execute();
}
}
32 changes: 32 additions & 0 deletions ProcessMaker/Multitenancy/PrefixCacheTask.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace ProcessMaker\Multitenancy;

use Spatie\Multitenancy\Contracts\IsTenant;
use Spatie\Multitenancy\Tasks\PrefixCacheTask as SpatiePrefixCacheTask;

class PrefixCacheTask extends SpatiePrefixCacheTask
{
private $originalSettingsPrefix;

public function makeCurrent(IsTenant $tenant): void
{
$cachePrefix = 'tenant_' . $tenant->getKey() . ':';
$this->setCachePrefix($cachePrefix);

$this->originalSettingsPrefix = config('cache.stores.cache_settings.prefix');
$tenantSettingsPrefix = 'tenant_' . $tenant->getKey() . ':' . $this->originalSettingsPrefix;
config()->set('cache.stores.cache_settings.prefix', $tenantSettingsPrefix);
$this->storeName = 'cache_settings';
$this->setCachePrefix($cachePrefix);
Copy link

@cursor cursor bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Wrong prefix passed to setCachePrefix for settings store

In makeCurrent(), $tenantSettingsPrefix is calculated as 'tenant_X:' + originalSettingsPrefix (e.g., 'tenant_X:settings') and the config is correctly set, but setCachePrefix() is called with $cachePrefix (just 'tenant_X:') instead. Similarly, in forgetCurrent(), $this->originalPrefix is passed instead of $this->originalSettingsPrefix. This causes the cache_settings store to have a mismatched prefix, potentially breaking cache isolation between tenants.

Additional Locations (1)

Fix in Cursor Fix in Web

}
Copy link

@cursor cursor bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Parent's makeCurrent not called, originalPrefix never initialized

The makeCurrent method completely overrides the parent SpatiePrefixCacheTask without calling parent::makeCurrent($tenant). The parent class sets $this->originalPrefix to save the original cache prefix before applying tenant-specific values. Since the parent is never called, $this->originalPrefix remains uninitialized. When forgetCurrent() is called, it passes $this->originalPrefix (which is null/undefined) to setCachePrefix(), causing the cache prefix to be incorrectly reset instead of restoring the original landlord value.

Additional Locations (1)

Fix in Cursor Fix in Web


public function forgetCurrent(): void
{
$this->setCachePrefix($this->originalPrefix);

config()->set('cache.stores.cache_settings.prefix', $this->originalSettingsPrefix);
$this->storeName = 'cache_settings';
$this->setCachePrefix($this->originalPrefix);
}
Copy link

@cursor cursor bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Default cache store prefix never restored when forgetting tenant

In forgetCurrent(), the first setCachePrefix call on line 26 operates on the wrong cache store. The storeName property is left at 'cache_settings' from the previous makeCurrent() call, so line 26 restores the cache_settings store instead of the default cache store. The default store's prefix is never restored to its original value. The code needs to reset storeName to its default value before the first setCachePrefix call, mirroring the order in makeCurrent() where the default store is handled first.

Fix in Cursor Fix in Web

}
Loading
Loading