Skip to content

Commit

Permalink
[4.x] Queue logic refactor (#1289)
Browse files Browse the repository at this point in the history
* simplify QueueTenancyBootstrapper

* wip: add persistent queue bootstrapper, minor testcase refactor

* ci: run persistent queue tests

* simplify persistent queue bootstrapper

* Fix code style (php-cs-fixer)

* phpstan fixes, clarify previousTenant use

* remove false positive regression test

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
stancl and github-actions[bot] authored Jan 14, 2025
1 parent 0e223e0 commit 8f958d5
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 114 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/queue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ jobs:
cd tenancy-queue-tester
TENANCY_VERSION=${VERSION_PREFIX}#${GITHUB_SHA} ./setup.sh
./test.sh
./alternative_config.sh
PERSISTENT=1 ./test.sh
146 changes: 146 additions & 0 deletions src/Bootstrappers/PersistentQueueTenancyBootstrapper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
<?php

declare(strict_types=1);

namespace Stancl\Tenancy\Bootstrappers;

use Illuminate\Config\Repository;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Contracts\Foundation\Application;
use Illuminate\Queue\Events\JobFailed;
use Illuminate\Queue\Events\JobProcessed;
use Illuminate\Queue\Events\JobProcessing;
use Illuminate\Queue\Events\JobRetryRequested;
use Illuminate\Queue\QueueManager;
use Illuminate\Support\Testing\Fakes\QueueFake;
use Stancl\Tenancy\Contracts\TenancyBootstrapper;
use Stancl\Tenancy\Contracts\Tenant;

class PersistentQueueTenancyBootstrapper implements TenancyBootstrapper
{
/** @var Repository */
protected $config;

/** @var QueueManager */
protected $queue;

/**
* The normal constructor is only executed after tenancy is bootstrapped.
* However, we're registering a hook to initialize tenancy. Therefore,
* we need to register the hook at service provider execution time.
*/
public static function __constructStatic(Application $app): void
{
static::setUpJobListener($app->make(Dispatcher::class), $app->runningUnitTests());
}

public function __construct(Repository $config, QueueManager $queue)
{
$this->config = $config;
$this->queue = $queue;

$this->setUpPayloadGenerator();
}

protected static function setUpJobListener(Dispatcher $dispatcher, bool $runningTests): void
{
$previousTenant = null;

$dispatcher->listen(JobProcessing::class, function ($event) use (&$previousTenant) {
$previousTenant = tenant();

static::initializeTenancyForQueue($event->job->payload()['tenant_id'] ?? null);
});

$dispatcher->listen(JobRetryRequested::class, function ($event) use (&$previousTenant) {
$previousTenant = tenant();

static::initializeTenancyForQueue($event->payload()['tenant_id'] ?? null);
});

// If we're running tests, we make sure to clean up after any artisan('queue:work') calls
$revertToPreviousState = function ($event) use (&$previousTenant, $runningTests) {
if ($runningTests) {
static::revertToPreviousState($event->job->payload()['tenant_id'] ?? null, $previousTenant);

// We don't need to reset $previousTenant since the value will be set again when a job is processed.
}

// If we're not running tests, we remain in the tenant's context. This makes other JobProcessed
// listeners able to deserialize the job, including with SerializesModels, since the tenant connection
// remains open.
};

$dispatcher->listen(JobProcessed::class, $revertToPreviousState); // artisan('queue:work') which succeeds
$dispatcher->listen(JobFailed::class, $revertToPreviousState); // artisan('queue:work') which fails
}

protected static function initializeTenancyForQueue(string|int|null $tenantId): void
{
if (! $tenantId) {
// The job is not tenant-aware
if (tenancy()->initialized) {
// Tenancy was initialized, so we revert back to the central context
tenancy()->end();
}

return;
}

// Re-initialize tenancy between all jobs even if the tenant is the same
// so that we don't work with an outdated tenant() instance in case it
// was updated outside the queue worker.
tenancy()->end();

/** @var Tenant $tenant */
$tenant = tenancy()->find($tenantId);
tenancy()->initialize($tenant);
}

protected static function revertToPreviousState(string|int|null $tenantId, ?Tenant $previousTenant): void
{
// The job was not tenant-aware
if (! $tenantId) {
return;
}

// Revert back to the previous tenant
if (tenant() && $previousTenant && $previousTenant->isNot(tenant())) {
tenancy()->initialize($previousTenant);
}

// End tenancy
if (tenant() && (! $previousTenant)) {
tenancy()->end();
}
}

protected function setUpPayloadGenerator(): void
{
$bootstrapper = &$this;

if (! $this->queue instanceof QueueFake) {
$this->queue->createPayloadUsing(function ($connection) use (&$bootstrapper) {
return $bootstrapper->getPayload($connection);
});
}
}

public function getPayload(string $connection): array
{
if (! tenancy()->initialized) {
return [];
}

if ($this->config["queue.connections.$connection.central"]) {
return [];
}

return [
'tenant_id' => tenant()->getTenantKey(),
];
}

public function bootstrap(Tenant $tenant): void {}
public function revert(): void {}
}
82 changes: 15 additions & 67 deletions src/Bootstrappers/QueueTenancyBootstrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper
/** @var QueueManager */
protected $queue;

/**
* Don't persist the same tenant across multiple jobs even if they have the same tenant ID.
*
* This is useful when you're changing the tenant's state (e.g. properties in the `data` column) and want the next job to initialize tenancy again
* with the new data. Features like the Tenant Config are only executed when tenancy is initialized, so the re-initialization is needed in some cases.
*
* @var bool
*/
public static $forceRefresh = false;

/**
* The normal constructor is only executed after tenancy is bootstrapped.
* However, we're registering a hook to initialize tenancy. Therefore,
Expand Down Expand Up @@ -68,9 +58,12 @@ protected static function setUpJobListener(Dispatcher $dispatcher): void
static::initializeTenancyForQueue($event->payload()['tenant_id'] ?? null);
});

// If we're running tests, we make sure to clean up after any artisan('queue:work') calls
$revertToPreviousState = function ($event) use (&$previousTenant) {
static::revertToPreviousState($event, $previousTenant);
// In queue worker context, this reverts to the central context.
// In dispatchSync context, this reverts to the previous tenant's context.
// There's no need to reset $previousTenant here since it's always first
// set in the above listeners and the app is reverted back to that context.
static::revertToPreviousState($event->job->payload()['tenant_id'] ?? null, $previousTenant);
};

$dispatcher->listen(JobProcessed::class, $revertToPreviousState); // artisan('queue:work') which succeeds
Expand All @@ -79,61 +72,25 @@ protected static function setUpJobListener(Dispatcher $dispatcher): void

protected static function initializeTenancyForQueue(string|int|null $tenantId): void
{
if ($tenantId === null) {
// The job is not tenant-aware
if (tenancy()->initialized) {
// Tenancy was initialized, so we revert back to the central context
tenancy()->end();
}

return;
}

if (static::$forceRefresh) {
// Re-initialize tenancy between all jobs
if (tenancy()->initialized) {
tenancy()->end();
}

/** @var Tenant $tenant */
$tenant = tenancy()->find($tenantId);
tenancy()->initialize($tenant);

if (! $tenantId) {
return;
}

if (tenancy()->initialized) {
// Tenancy is already initialized
if (tenant()->getTenantKey() === $tenantId) {
// It's initialized for the same tenant (e.g. dispatchSync was used, or the previous job also ran for this tenant)
return;
}
}

// Tenancy was either not initialized, or initialized for a different tenant.
// Therefore, we initialize it for the correct tenant.

/** @var Tenant $tenant */
$tenant = tenancy()->find($tenantId);
tenancy()->initialize($tenant);
}

protected static function revertToPreviousState(JobProcessed|JobFailed $event, ?Tenant &$previousTenant): void
protected static function revertToPreviousState(string|int|null $tenantId, ?Tenant $previousTenant): void
{
$tenantId = $event->job->payload()['tenant_id'] ?? null;

// The job was not tenant-aware
// The job was not tenant-aware so no context switch was done
if (! $tenantId) {
return;
}

// Revert back to the previous tenant
if (tenant() && $previousTenant?->isNot(tenant())) {
tenancy()->initialize($previousTenant);
}

// End tenancy
if (tenant() && (! $previousTenant)) {
// End tenancy when there's no previous tenant
// (= when running in a queue worker, not dispatchSync)
if (tenant() && ! $previousTenant) {
tenancy()->end();
}
}
Expand All @@ -149,16 +106,6 @@ protected function setUpPayloadGenerator(): void
}
}

public function bootstrap(Tenant $tenant): void
{
//
}

public function revert(): void
{
//
}

public function getPayload(string $connection): array
{
if (! tenancy()->initialized) {
Expand All @@ -169,10 +116,11 @@ public function getPayload(string $connection): array
return [];
}

$id = tenant()->getTenantKey();

return [
'tenant_id' => $id,
'tenant_id' => tenant()->getTenantKey(),
];
}

public function bootstrap(Tenant $tenant): void {}
public function revert(): void {}
}
7 changes: 7 additions & 0 deletions src/TenancyServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Stancl\Tenancy;

use Closure;
use Illuminate\Cache\CacheManager;
use Illuminate\Database\Console\Migrations\FreshCommand;
use Illuminate\Routing\Events\RouteMatched;
Expand All @@ -18,9 +19,15 @@

class TenancyServiceProvider extends ServiceProvider
{
public static Closure|null $configure = null;

/* Register services. */
public function register(): void
{
if (static::$configure) {
(static::$configure)();
}

$this->mergeConfigFrom(__DIR__ . '/../assets/config.php', 'tenancy');

$this->app->singleton(Database\DatabaseManager::class);
Expand Down
6 changes: 0 additions & 6 deletions tests/Features/NoAttachTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@
use Stancl\Tenancy\Tests\Etc\Tenant;

test('sqlite ATTACH statements can be blocked', function (bool $disallow) {
try {
readlink(base_path('vendor'));
} catch (\Throwable) {
symlink(base_path('vendor'), '/var/www/html/vendor');
}

if (php_uname('m') == 'aarch64') {
// Escape testbench prison. Can't hardcode /var/www/html/extensions/... here
// since GHA doesn't mount the filesystem on the container's workdir
Expand Down
Loading

0 comments on commit 8f958d5

Please sign in to comment.