From 6a047d36866a180cf1a4cdfe6eea6ff7fe975097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 7 Jan 2025 15:37:37 +0100 Subject: [PATCH] add persistence, sync, and forceRefresh assertions --- README.md | 37 ++++- alternative_config.sh | 7 + setup/AppServiceProvider.php | 28 ++++ setup/LogAbcJob.php | 20 +++ setup/_tenancy_setup.sh | 4 + setup/alternative/AppServiceProvider.php | 35 +++++ setup/alternative/_alternative_setup.sh | 14 ++ setup/alternative/cache.php | 114 ++++++++++++++ setup/alternative/database.php | 191 +++++++++++++++++++++++ test.sh | 141 ++++++++++++++--- 10 files changed, 562 insertions(+), 29 deletions(-) create mode 100755 alternative_config.sh create mode 100755 setup/AppServiceProvider.php create mode 100755 setup/LogAbcJob.php create mode 100755 setup/alternative/AppServiceProvider.php create mode 100755 setup/alternative/_alternative_setup.sh create mode 100755 setup/alternative/cache.php create mode 100755 setup/alternative/database.php diff --git a/README.md b/README.md index 804f56c..c5840b7 100755 --- a/README.md +++ b/README.md @@ -4,12 +4,43 @@ In addition to the tests we can write using testbench, we have this repository w 1. Creates a new Laravel application 2. Sets up Tenancy 3. Creates a sample job -4. Asserts that the queue worker is working as expected +4. Asserts that the queue worker is working as expected -- running in the correct context and responding to restart signals This is mostly due to some past bugs that were hard to catch in our test suite. With this repo, we can have a separate CI job validating queue behavior _in a real application_. -## TODOs +## Persistence tests -- Verify how `queue:restart` works in v4 +Additionally, we can also test for _queue worker persistence_. This refers to the worker staying in the context of the tenant +used in the last job. The benefit of that is significantly better third-party package support (especially in cases where said +packages unserialize job payloads on e.g. `JobProcessed`). + +In versions prior to v4: +- 3.8.5 handles restarts correctly but is not persistent +- 3.8.4 is persistent but doesn't respond to restarts correctly (if the last processed job was in the tenant context) + +In v4, there's `QueueTenancyBootstrapper` that works similarly to 3.8.5 and `PersistentQueueTenancyBootstrapper` that works +similarly to 3.8.4. + +For the different setups: +- 3.x should have only warns on missing persistence + - 3.8.4 fails the restart-related assertions. The alternative config (./alternative_config.sh) makes them pass. + - 3.8.4 fails the FORCEREFRESH-related assertions. Either run with FORCEREFRESH=0 or set `QueueTenancyBootstrapper::$forceRefresh = true` in a service provider. +- 4.x should only show warns on missing persistence + - With the alternative config, it should pass ALL tests without any warnings. + +3.x (3.8.5+) tests: +```bash +./setup.sh +./test.sh +``` + +4.x tests: +```bash +./setup.sh +./test.sh + +./alternative_config.sh +PERSISTENT=1 ./test.sh +``` diff --git a/alternative_config.sh b/alternative_config.sh new file mode 100755 index 0000000..ce28e1e --- /dev/null +++ b/alternative_config.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +# Sets up the "alternative config" mentioned here https://github.com/archtechx/tenancy/issues/1260#issuecomment-2572951587 + +set -e + +docker run --rm -v .:/var/www/html tenancy-queue-test-cli bash setup/alternative/_alternative_setup.sh diff --git a/setup/AppServiceProvider.php b/setup/AppServiceProvider.php new file mode 100755 index 0000000..f8e5acc --- /dev/null +++ b/setup/AppServiceProvider.php @@ -0,0 +1,28 @@ +app->extend(RestartCommand::class, function ($_, $app) { + return new RestartCommand($app['cache']->store('global_redis')); + }); + $this->app->extend(WorkCommand::class, function ($_, $app) { + return new WorkCommand($app['queue.worker'], $app['cache']->store('global_redis')); + }); + } + + /** + * Bootstrap any application services. + */ + public function boot(): void + { + Event::listen(JobProcessed::class, function () { + file_put_contents(base_path('jobprocessed_context'), tenant() ? ('tenant_' . tenant('id')) : 'central'); + }); + } +} diff --git a/setup/alternative/_alternative_setup.sh b/setup/alternative/_alternative_setup.sh new file mode 100755 index 0000000..a28e894 --- /dev/null +++ b/setup/alternative/_alternative_setup.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +set -e + +cd src/ + +rm config/cache.php +cp ../setup/alternative/cache.php config/cache.php + +rm config/database.php +cp ../setup/alternative/database.php config/database.php + +rm app/Providers/AppServiceProvider.php +cp ../setup/alternative/AppServiceProvider.php app/Providers/AppServiceProvider.php diff --git a/setup/alternative/cache.php b/setup/alternative/cache.php new file mode 100755 index 0000000..5b28b46 --- /dev/null +++ b/setup/alternative/cache.php @@ -0,0 +1,114 @@ + env('CACHE_STORE', 'database'), + + /* + |-------------------------------------------------------------------------- + | Cache Stores + |-------------------------------------------------------------------------- + | + | Here you may define all of the cache "stores" for your application as + | well as their drivers. You may even define multiple stores for the + | same cache driver to group types of items stored in your caches. + | + | Supported drivers: "array", "database", "file", "memcached", + | "redis", "dynamodb", "octane", "null" + | + */ + + 'stores' => [ + + 'array' => [ + 'driver' => 'array', + 'serialize' => false, + ], + + 'database' => [ + 'driver' => 'database', + 'connection' => env('DB_CACHE_CONNECTION'), + 'table' => env('DB_CACHE_TABLE', 'cache'), + 'lock_connection' => env('DB_CACHE_LOCK_CONNECTION'), + 'lock_table' => env('DB_CACHE_LOCK_TABLE'), + ], + + 'file' => [ + 'driver' => 'file', + 'path' => storage_path('framework/cache/data'), + 'lock_path' => storage_path('framework/cache/data'), + ], + + 'memcached' => [ + 'driver' => 'memcached', + 'persistent_id' => env('MEMCACHED_PERSISTENT_ID'), + 'sasl' => [ + env('MEMCACHED_USERNAME'), + env('MEMCACHED_PASSWORD'), + ], + 'options' => [ + // Memcached::OPT_CONNECT_TIMEOUT => 2000, + ], + 'servers' => [ + [ + 'host' => env('MEMCACHED_HOST', '127.0.0.1'), + 'port' => env('MEMCACHED_PORT', 11211), + 'weight' => 100, + ], + ], + ], + + 'redis' => [ + 'driver' => 'redis', + 'connection' => env('REDIS_CACHE_CONNECTION', 'cache'), + 'lock_connection' => env('REDIS_CACHE_LOCK_CONNECTION', 'default'), + ], + + 'global_redis' => [ + 'driver' => 'redis', + 'connection' => 'global_cache', + 'lock_connection' => env('REDIS_CACHE_LOCK_CONNECTION', 'default'), + ], + + 'dynamodb' => [ + 'driver' => 'dynamodb', + 'key' => env('AWS_ACCESS_KEY_ID'), + 'secret' => env('AWS_SECRET_ACCESS_KEY'), + 'region' => env('AWS_DEFAULT_REGION', 'us-east-1'), + 'table' => env('DYNAMODB_CACHE_TABLE', 'cache'), + 'endpoint' => env('DYNAMODB_ENDPOINT'), + ], + + 'octane' => [ + 'driver' => 'octane', + ], + + ], + + /* + |-------------------------------------------------------------------------- + | Cache Key Prefix + |-------------------------------------------------------------------------- + | + | When utilizing the APC, database, memcached, Redis, and DynamoDB cache + | stores, there might be other applications using the same cache. For + | that reason, you may prefix every cache key to avoid collisions. + | + */ + + 'prefix' => env('CACHE_PREFIX', Str::slug(env('APP_NAME', 'laravel'), '_').'_cache_'), + +]; diff --git a/setup/alternative/database.php b/setup/alternative/database.php new file mode 100755 index 0000000..e5616e1 --- /dev/null +++ b/setup/alternative/database.php @@ -0,0 +1,191 @@ + env('DB_CONNECTION', 'sqlite'), + + /* + |-------------------------------------------------------------------------- + | Database Connections + |-------------------------------------------------------------------------- + | + | Below are all of the database connections defined for your application. + | An example configuration is provided for each database system which + | is supported by Laravel. You're free to add / remove connections. + | + */ + + 'connections' => [ + + 'sqlite' => [ + 'driver' => 'sqlite', + 'url' => env('DB_URL'), + 'database' => env('DB_DATABASE', database_path('database.sqlite')), + 'prefix' => '', + 'foreign_key_constraints' => env('DB_FOREIGN_KEYS', true), + 'busy_timeout' => null, + 'journal_mode' => null, + 'synchronous' => null, + ], + + 'mysql' => [ + 'driver' => 'mysql', + 'url' => env('DB_URL'), + 'host' => env('DB_HOST', '127.0.0.1'), + 'port' => env('DB_PORT', '3306'), + 'database' => env('DB_DATABASE', 'laravel'), + 'username' => env('DB_USERNAME', 'root'), + 'password' => env('DB_PASSWORD', ''), + 'unix_socket' => env('DB_SOCKET', ''), + 'charset' => env('DB_CHARSET', 'utf8mb4'), + 'collation' => env('DB_COLLATION', 'utf8mb4_unicode_ci'), + 'prefix' => '', + 'prefix_indexes' => true, + 'strict' => true, + 'engine' => null, + 'options' => extension_loaded('pdo_mysql') ? array_filter([ + PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'), + ]) : [], + ], + + 'mariadb' => [ + 'driver' => 'mariadb', + 'url' => env('DB_URL'), + 'host' => env('DB_HOST', '127.0.0.1'), + 'port' => env('DB_PORT', '3306'), + 'database' => env('DB_DATABASE', 'laravel'), + 'username' => env('DB_USERNAME', 'root'), + 'password' => env('DB_PASSWORD', ''), + 'unix_socket' => env('DB_SOCKET', ''), + 'charset' => env('DB_CHARSET', 'utf8mb4'), + 'collation' => env('DB_COLLATION', 'utf8mb4_unicode_ci'), + 'prefix' => '', + 'prefix_indexes' => true, + 'strict' => true, + 'engine' => null, + 'options' => extension_loaded('pdo_mysql') ? array_filter([ + PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'), + ]) : [], + ], + + 'pgsql' => [ + 'driver' => 'pgsql', + 'url' => env('DB_URL'), + 'host' => env('DB_HOST', '127.0.0.1'), + 'port' => env('DB_PORT', '5432'), + 'database' => env('DB_DATABASE', 'laravel'), + 'username' => env('DB_USERNAME', 'root'), + 'password' => env('DB_PASSWORD', ''), + 'charset' => env('DB_CHARSET', 'utf8'), + 'prefix' => '', + 'prefix_indexes' => true, + 'search_path' => 'public', + 'sslmode' => 'prefer', + ], + + 'sqlsrv' => [ + 'driver' => 'sqlsrv', + 'url' => env('DB_URL'), + 'host' => env('DB_HOST', 'localhost'), + 'port' => env('DB_PORT', '1433'), + 'database' => env('DB_DATABASE', 'laravel'), + 'username' => env('DB_USERNAME', 'root'), + 'password' => env('DB_PASSWORD', ''), + 'charset' => env('DB_CHARSET', 'utf8'), + 'prefix' => '', + 'prefix_indexes' => true, + // 'encrypt' => env('DB_ENCRYPT', 'yes'), + // 'trust_server_certificate' => env('DB_TRUST_SERVER_CERTIFICATE', 'false'), + ], + + ], + + /* + |-------------------------------------------------------------------------- + | Migration Repository Table + |-------------------------------------------------------------------------- + | + | This table keeps track of all the migrations that have already run for + | your application. Using this information, we can determine which of + | the migrations on disk haven't actually been run on the database. + | + */ + + 'migrations' => [ + 'table' => 'migrations', + 'update_date_on_publish' => true, + ], + + /* + |-------------------------------------------------------------------------- + | Redis Databases + |-------------------------------------------------------------------------- + | + | Redis is an open source, fast, and advanced key-value store that also + | provides a richer body of commands than a typical key-value system + | such as Memcached. You may define your connection settings here. + | + */ + + 'redis' => [ + + 'client' => env('REDIS_CLIENT', 'phpredis'), + + 'options' => [ + 'cluster' => env('REDIS_CLUSTER', 'redis'), + 'prefix' => env('REDIS_PREFIX', Str::slug(env('APP_NAME', 'laravel'), '_').'_database_'), + ], + + 'default' => [ + 'url' => env('REDIS_URL'), + 'host' => env('REDIS_HOST', '127.0.0.1'), + 'username' => env('REDIS_USERNAME'), + 'password' => env('REDIS_PASSWORD'), + 'port' => env('REDIS_PORT', '6379'), + 'database' => env('REDIS_DB', '0'), + ], + + 'queue' => [ + 'url' => env('REDIS_URL'), + 'host' => env('REDIS_HOST', '127.0.0.1'), + 'username' => env('REDIS_USERNAME'), + 'password' => env('REDIS_PASSWORD'), + 'port' => env('REDIS_PORT', '6379'), + 'database' => env('REDIS_DB', '0'), + ], + + 'cache' => [ + 'url' => env('REDIS_URL'), + 'host' => env('REDIS_HOST', '127.0.0.1'), + 'username' => env('REDIS_USERNAME'), + 'password' => env('REDIS_PASSWORD'), + 'port' => env('REDIS_PORT', '6379'), + 'database' => env('REDIS_CACHE_DB', '1'), + ], + + 'global_cache' => [ + 'url' => env('REDIS_URL'), + 'host' => env('REDIS_HOST', '127.0.0.1'), + 'username' => env('REDIS_USERNAME'), + 'password' => env('REDIS_PASSWORD'), + 'port' => env('REDIS_PORT', '6379'), + 'database' => 3, + ], + + ], + +]; diff --git a/test.sh b/test.sh index 7172ba0..c4f8fd4 100755 --- a/test.sh +++ b/test.sh @@ -2,6 +2,9 @@ set -e +PERSISTENT=${PERSISTENT:-"0"} +FORCEREFRESH=${FORCEREFRESH:-"1"} # No config needed for this from 3.8.5/4.0 on + assert_queue_worker_running() { if docker compose ps -a --format '{{.Status}}' queue | grep -q "Exited"; then echo "ERR: Queue worker has exited!" @@ -28,14 +31,15 @@ assert_no_queue_failures() { assert_tenant_users() { assert_no_queue_failures - local expected_count=$1 - test "$(sqlite3 src/database/tenantfoo.sqlite 'SELECT count(*) from USERS')" -eq "$expected_count" || { echo "ERR: Tenant DB expects $expected_count user(s)."; exit 1; } + local tenant=$1 + local expected_count=$2 + test "$(sqlite3 src/database/tenant${tenant}.sqlite 'SELECT count(*) from users')" -eq "$expected_count" || { echo "ERR: Tenant DB $tenant expects $expected_count user(s)."; exit 1; } } assert_central_users() { assert_no_queue_failures local expected_count=$1 - test "$(sqlite3 src/database/database.sqlite 'SELECT count(*) from USERS')" -eq "$expected_count" || { echo "ERR: Central DB expects $expected_count user(s)."; exit 1; } + test "$(sqlite3 src/database/database.sqlite 'SELECT count(*) from users')" -eq "$expected_count" || { echo "ERR: Central DB expects $expected_count user(s)."; exit 1; } } without_queue_assertions() { @@ -58,21 +62,39 @@ dispatch_central_job() { } dispatch_tenant_job() { - echo "Dispatching job from tenant context..." - docker compose exec -T queue php artisan tinker --execute "App\\Models\\Tenant::first()->run(function () { dispatch(new App\Jobs\FooJob); });" + local tenant=$1 + echo "Dispatching job from tenant ${tenant} context..." + docker compose exec -T queue php artisan tinker --execute "App\\Models\\Tenant::find('${tenant}')->run(function () { dispatch(new App\Jobs\FooJob); });" sleep 5 } +expect_worker_context() { + expected_context="$1" + + actual_context=$(cat src/jobprocessed_context) + + if [ "$actual_context" = "$expected_context" ]; then + echo "OK: JobProcessed context is $expected_context" + else + if [ "$PERSISTENT" -eq 1 ]; then + echo "ERR: JobProcessed context is NOT $expected_context" + exit 1 + else + echo "WARN: JobProcessed context is NOT $expected_context" + fi + fi +} ###################################### SETUP ###################################### rm -f src/database.sqlite rm -f src/database/tenantfoo.sqlite +rm -f src/database/tenantbar.sqlite docker compose up -d redis # in case it's not running - the below setup code needs Redis to be running docker compose run --rm queue php artisan migrate:fresh >/dev/null -docker compose run --rm queue php artisan tinker -v --execute "App\\Models\\Tenant::create(['id' => 'foo', 'tenancy_db_name' => 'tenantfoo.sqlite']);" +docker compose run --rm queue php artisan tinker -v --execute "App\\Models\\Tenant::create(['id' => 'foo', 'tenancy_db_name' => 'tenantfoo.sqlite']);App\\Models\\Tenant::create(['id' => 'bar', 'tenancy_db_name' => 'tenantbar.sqlite']);" docker compose down; docker compose up -d --wait docker compose logs -f queue & @@ -80,27 +102,46 @@ docker compose logs -f queue & # Kill any log watchers that may still be alive trap "docker compose stop queue" EXIT -echo "Setup complete, starting tests...\n" +echo "Setup complete, starting tests..." ################### BASIC PHASE: Assert jobs use the right context ################### +echo +echo "-------- BASIC PHASE --------" +echo -dispatch_tenant_job -assert_tenant_users 1 +dispatch_tenant_job foo +assert_tenant_users foo 1 +assert_tenant_users bar 0 assert_central_users 0 -echo "OK: User created in tenant\n" +echo "OK: User created in tenant foo" +expect_worker_context tenant_foo + +# Assert that the worker correctly distinguishes not just between tenant and central +# contexts, but also between different tenants. +dispatch_tenant_job bar +assert_tenant_users foo 1 +assert_tenant_users bar 1 +assert_central_users 0 +echo "OK: User created in tenant bar" +expect_worker_context tenant_bar dispatch_central_job -assert_tenant_users 1 +assert_tenant_users foo 1 +assert_tenant_users bar 1 assert_central_users 1 -echo "OK: User created in central\n" +echo "OK: User created in central" +expect_worker_context central ############# RESTART PHASE: Assert the worker always responds to signals ############# +echo +echo "-------- RESTART PHASE --------" +echo echo "Running queue:restart (after a central job)..." docker compose exec -T queue php artisan queue:restart >/dev/null sleep 5 assert_queue_worker_exited -echo "OK: Queue worker has exited\n" +echo "OK: Queue worker has exited" echo "Starting queue worker again..." docker compose restart queue @@ -109,7 +150,7 @@ docker compose logs -f queue & echo -dispatch_tenant_job +dispatch_tenant_job foo # IMPORTANT: # If the worker remains in the tenant context after running a job # it not only fails the final assertion here by not responding to queue:restart. @@ -119,12 +160,13 @@ dispatch_tenant_job # Then, if the queue worker has shut down, we simply start it up again and continue # with the tests. That said, if the warning has been printed, it should be pretty much # guaranteed that the assertion about queue:restart post-tenant job will fail too. -without_queue_assertions assert_tenant_users 2 +without_queue_assertions assert_tenant_users foo 2 without_queue_assertions assert_central_users 1 -echo "OK: User created in tenant\n" +echo "OK: User created in tenant foo" +expect_worker_context tenant_foo if docker compose ps -a --format '{{.Status}}' queue | grep -q "Exited"; then - echo "WARN: Queue worker restarted after running a tenant job post-restart (https://github.com/archtechx/tenancy/issues/1229#issuecomment-2566111616) following assertions will likely fail." + echo "WARN: Queue worker restarted after running a tenant job post-restart (https://github.com/archtechx/tenancy/issues/1229#issuecomment-2566111616), following assertions will likely fail." docker compose start queue # Start the worker back up sleep 5 docker compose logs -f queue & @@ -140,14 +182,16 @@ fi # This time, just to add more context, we can try to dispatch a central job first # in case it changes anything. But odds are that in broken setups we'll see both warnings. dispatch_central_job -without_queue_assertions assert_tenant_users 2 +without_queue_assertions assert_tenant_users foo 2 without_queue_assertions assert_central_users 2 -echo "OK: User created in central\n" +echo "OK: User created in central" +expect_worker_context central -dispatch_tenant_job -without_queue_assertions assert_tenant_users 3 +dispatch_tenant_job foo +without_queue_assertions assert_tenant_users foo 3 without_queue_assertions assert_central_users 2 -echo "OK: User created in tenant\n" +echo "OK: User created in tenant foo" +expect_worker_context tenant_foo if docker compose ps -a --format '{{.Status}}' queue | grep -q "Exited"; then echo "WARN: ANOTHER extra restart took place after running a tenant job" @@ -167,16 +211,61 @@ docker compose exec redis redis-cli -n 1 DEL laravel_database_illuminate:queue:r # Also make the queue worker reload the value from cache docker compose restart queue -docker compose logs -f queue & +# restart doesn't kill log watchers, so we don't need to create another one # Finally, we dispatch a tenant job *immediately* before a restart. -dispatch_tenant_job -assert_tenant_users 4 +dispatch_tenant_job foo +assert_tenant_users foo 4 assert_central_users 2 -echo "OK: User created in tenant\n" +echo "OK: User created in tenant foo" +expect_worker_context tenant_foo echo "Running queue:restart (after a tenant job)..." docker compose exec -T queue php artisan queue:restart >/dev/null sleep 5 assert_queue_worker_exited echo "OK: Queue worker has exited" + +############# SYNC PHASE: Assert that dispatching sync jobs doesn't affect outer context ############# +echo +echo "-------- SYNC PHASE --------" +echo + +docker compose run --rm queue php artisan tinker -v --execute "tenancy()->initialize('foo'); App\Jobs\FooJob::dispatchSync(); file_put_contents('sync_context', tenant() ? ('tenant_' . tenant('id')) : 'central');" +without_queue_assertions assert_tenant_users foo 5 +without_queue_assertions assert_tenant_users bar 1 +without_queue_assertions assert_central_users 2 + +if grep -q 'tenant_foo' src/sync_context; then + echo "OK: Sync dispatch preserved context" +else + echo "ERR: Sync dispatch changed context" + exit 1 +fi + +######## REFRESH PHASE: Assert that the worker doesn't hold on to an outdated tenant instance ######## +echo +echo "-------- REFRESH PHASE --------" +echo + +docker compose start queue +sleep 5 +docker compose logs -f queue & +dispatch_tenant_job bar +assert_tenant_users bar 2 +assert_central_users 2 +echo "OK: User created in tenant bar" + +docker compose exec -T queue php artisan tinker --execute "\$tenant = App\Models\Tenant::find('bar'); \$tenant->update(['abc' => 'def']); \$tenant->run(function () { dispatch(new App\Jobs\LogAbcJob); });" +sleep 5 + +if grep -q 'def' src/abc; then + echo "OK: Worker notices changes made to the current tenant outside the worker" +else + if [ "$FORCEREFRESH" -eq 1 ]; then + echo "ERR: Worker does NOT notice changes made to the current tenant outside the worker" + exit 1 + else + echo "WARN: Worker does NOT notice changes made to the current tenant outside the worker" + fi +fi