[4.x] Add batch tenancy queue bootstrapper #874

Riley19280 · 2022-06-08T02:02:58Z

This PR adds a bootstrapper class for working with Laravel job batches. This fixes issues related to #547, where a Call to a member function prepare() on null exception gets thrown when the queue worker attempts to get a batch from the database for a different tenant than the previous one the queue worker processed. This is fixed by temporarily replacing the BatchRepository connection with the correct tenant connection, and then reverting it back when exiting the tenant context.

stancl · 2022-06-08T09:58:13Z

I don't think #547 is related, it's simply the same error message

Riley19280 · 2022-06-14T01:21:13Z

Yeah, that is entirely possible. I still believe there is value in supporting the batch system though.

src/Bootstrappers/BatchTenancyBootstrapper.php

tonning · 2022-08-19T11:51:32Z

Thank you @Riley19280, this was a lifesaver for me trying to figure out why batch jobs would fail with the Call to a member function prepare() on null exception on subsequent dispatches when the first and second dispatch are from different tenants.

For what it's worth @stancl, I added this as a bootstrapper on v3 and it's been work flawless so far.

tests/BatchTest.php

src/Bootstrappers/BatchTenancyBootstrapper.php

assets/config.php

codecov-commenter · 2022-08-31T12:18:01Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.32%. Comparing base (f2c6408) to head (395d859).
Report is 109 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #874      +/-   ##
============================================
- Coverage     87.66%   87.32%   -0.35%     
- Complexity      412      415       +3     
============================================
  Files           112      114       +2     
  Lines          1273     1144     -129     
============================================
- Hits           1116      999     -117     
+ Misses          157      145      -12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/Bootstrappers/BatchTenancyBootstrapper.php

stancl · 2022-09-12T21:59:13Z

@abrardev99 Seems like GH was bugged again and some of my reviews didn't get submitted, they were "Pending". I submitted them now.

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

stancl · 2022-09-22T16:26:33Z

Keep the property name $batchRepository
Since we're using dependency injection, we shouldn't be using the DB:: facade in the code. Use DI for that as well. It should be something like Laravel's DatabaseManager I think

abrardev99 · 2022-09-23T06:05:10Z

Keep the property name $batchRepository
Since we're using dependency injection, we shouldn't be using the DB:: facade in the code. Use DI for that as well. It should be something like Laravel's DatabaseManager I think

Done.

src/Bootstrappers/BatchTenancyBootstrapper.php

assets/config.php

…usConnection logic works correctly

stancl · 2022-09-26T12:09:19Z

src/Bootstrappers/BatchTenancyBootstrapper.php

+    public function bootstrap(Tenant $tenant)
+    {
+        // Update batch repository connection to use the tenant connection
+        $this->previousConnection = $this->batchRepository->getConnection();


I wasn't sure if this wouldn't get overridden when tenants are initialized in a sequence, without tenancy being ended first. So I updated the tests and they still pass. Seems like revert() is called even when we're calling initialize() from another tenant's context.

Thanks 👍

stancl · 2022-09-26T12:14:06Z

Thanks for your work on this guys!

* exclude master from CI * Add batch tenancy queue bootstrapper * add test case * skip tests for old versions * variable docblocks * use Laravel's connection getter and setter * convert test to pest * bottom space * singleton regis in TestCase * Update src/Bootstrappers/BatchTenancyBootstrapper.php Co-authored-by: Samuel Štancl <samuel@archte.ch> * convert batch class resolution to property level * enabled BatchTenancyBootstrapper by default * typehint DatabaseBatchRepository * refactore name * DI DB manager * typehint * Update config.php * use initialize() twice without end()ing tenancy to assert that previousConnection logic works correctly Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com> Co-authored-by: Abrar Ahmad <abrar.dev99@gmail.com> Co-authored-by: Samuel Štancl <samuel@archte.ch>

* Add readied tenants Add config for readied tenants Add `create` and `clear` command Add Readied scope and static functions Add tests * Fix initialize function name * Add readied events * Fix readied column cast * Laravel 6 compatible * Add readied scope tests * Rename config from include_in_scope to include_in_queries * Change terminology to pending * Update CreatePendingTenants.php * Laravel 6 compatible * Update CreatePendingTenants.php * runForMultiple can scope pending tenants * Fix issues * Code and comment style improvements * Change 'tenant' to 'tenants' in command signature * Fix code style (php-cs-fixer) * Rename variables in CreatePendingTenants * Remove withPending from runForMultiple * Update tenants option trait * Update command that use tenants * Fix code style (php-cs-fixer) * Improve getTenants condition * Update config comments * Minor config comment corrections * Grammar fix * Update comments and naming * Correct comments * Improve writing * Remove pending tenant clearing time constraints * Allow using only one time constraint for clearing the pending tenants * phpunit to pest * Fix code style (php-cs-fixer) * Fix code style (php-cs-fixer) * [4.x] Optionally delete storage after tenant deletion (#938) * Add test for deleting storage after tenant deletion * Save `storage_path()` in a variable after initializing tenant in test Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com> * Add DeleteTenantStorage listener * Update test name * Remove storage deletion config key * Remove tenant storage deletion events * Move tenant storage deletion to the DeletingTenant event Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com> * [4.x] Finish incomplete and missing tests (#947) * complete test sqlite manager customize path * complete test seed command works * complete uniqe exists test * Update SingleDatabaseTenancyTest.php * refactor the ternary into if condition * custom path * simplify if condition * random dir name * Update SingleDatabaseTenancyTest.php * Update CommandsTest.php * prefix random DB name with custom_ Co-authored-by: Samuel Štancl <samuel@archte.ch> * [4.x] Add batch tenancy queue bootstrapper (#874) * exclude master from CI * Add batch tenancy queue bootstrapper * add test case * skip tests for old versions * variable docblocks * use Laravel's connection getter and setter * convert test to pest * bottom space * singleton regis in TestCase * Update src/Bootstrappers/BatchTenancyBootstrapper.php Co-authored-by: Samuel Štancl <samuel@archte.ch> * convert batch class resolution to property level * enabled BatchTenancyBootstrapper by default * typehint DatabaseBatchRepository * refactore name * DI DB manager * typehint * Update config.php * use initialize() twice without end()ing tenancy to assert that previousConnection logic works correctly Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com> Co-authored-by: Abrar Ahmad <abrar.dev99@gmail.com> Co-authored-by: Samuel Štancl <samuel@archte.ch> * [4.x] Storage::url() support (modified #689) (#909) * This adds support for tenancy aware Storage::url() method * Trigger CI build * Fixed Link command for Laravel v6, added StorageLink Events, more StorageLink tests, added RemoveStorageSymlinks Job, added Storage Jobs to TenancyServiceProvider stub, renamed misleading config example. * Fix typo * Fix code style (php-cs-fixer) * Update config comments * Format code in Link command, make writing more concise * Change "symLinks" to "symlinks" * Refactor Link command * Fix test name typo * Test fetching files using the public URL * Extract Link command logic into actions * Fix code style (php-cs-fixer) * Check if closure is null in CreateStorageSymlinksAction * Stop using command terminology in CreateStorageSymlinksAction * Separate the Storage::url() test cases * Update url_override comments * Remove afterLink closures, add types, move actions, add usage explanation to the symlink trait * Fix code style (php-cs-fixer) * Update public storage URL test * Fix issue with using str() * Improve url_override comment, add todos * add todo comment * fix docblock style * Add link command tests back * Add types to $tenants in the action handle() methods * Fix typo, update variable name formatting * Add tests for the symlink actions * Change possibleTenantSymlinks not to prefix the paths twice while tenancy is initialized * Fix code style (php-cs-fixer) * Stop testing storage directory existence in symlink test * Don't specify full namespace for Tenant model annotation * Don't specify full namespace in ActionTest * Remove "change to DI" todo * Remove possibleTenantSymlinks return annotation * Remove symlink-related jobs, instantiate and use actions * Revert "Remove symlink-related jobs, instantiate and use actions" This reverts commit 547440c. * Add a comment line about the possible tenant symlinks * Correct storagePath and publicPath variables * Revert "Correct storagePath and publicPath variables" This reverts commit e3aa8e2. * add a todo Co-authored-by: Martin Vlcek <martin@dontfreakout.eu> Co-authored-by: lukinovec <lukinovec@gmail.com> Co-authored-by: PHP CS Fixer <phpcsfixer@example.com> * Use HasTenantOptions in Link * Correct the tenant order in Run command * Fix code style (php-cs-fixer) * Fix formatting issue * Add missing imports * Fix code style (php-cs-fixer) * Use HasTenantOptions instead of the old trait name in Up/Down commands * Fix test name typo * Remove redundant passing of $withPending to runForMultiple in TenantCollection's runForEach * Make `with-pending` default to `config('tenancy.pending.include_in_queries')` in HasTenantOptions * Make `createPending()` return the created tenant * Fix code style (php-cs-fixer) * Remove tenant ordering * Fix code style (php-cs-fixer) * Remove duplicate tenancy bootstrappers config setting * Add and use getWithPendingOption method * Fix code style (php-cs-fixer) * Add optionNotPassedValue property * Test using --with-pending and the include_in_queries config value * Make with-pending VALUE_NONE * use plural in test names * fix test names * add pullPendingTenantFromPool * Add docblock type * Import commands * Fix code style (php-cs-fixer) * Move pending tenant tests to a more appropriate file * Delete queuetest from gitignore * Delete queuetest file * Add queuetest to gitignore * Rename pullPendingTenant to pullPending and don't pass bool to that method * Add a test that checks if pulling a pending tenant removes it from the pool * bump stancl/virtualcolumn to ^1.3 * Update pending tenant pulling test * Dynamically get columns for pending queries * Dynamically get virtual column name in ClearPendingTenants * Fix ClearPendingTenants bug * Make test name more accurate * Update test name * add a todo * Update include in queries test name * Remove `Tenant::query()->delete()` from pending tenant check test * Rename the pending tenant check test name * Update HasPending.php * fix all() call * code style * all() -> get() * Remove redundant `Tenant::all()` call Co-authored-by: j.stein <joristein@gmail.com> Co-authored-by: lukinovec <lukinovec@gmail.com> Co-authored-by: PHP CS Fixer <phpcsfixer@example.com> Co-authored-by: Abrar Ahmad <abrar.dev99@gmail.com> Co-authored-by: Riley19280 <rileyaven88@gmail.com> Co-authored-by: Martin Vlcek <martin@dontfreakout.eu>

MujeebAnwar · 2024-05-01T06:33:43Z

@tonning I'm trying to add this as a bootstrapper on v3. But it's given me an Unresolvable dependency resolving [Parameter #2 [ <required> string $table ]] in class Illuminate\Bus\DatabaseBatchRepository Error. Can you please help me to figure it out?

abhiburk · 2024-08-29T05:21:20Z

On multiple queue jobs for tenant, I am facing a similar issue I tried adding this bootstrapper within v3

Call to a member function prepare() on null {"exception":"[object] (Error(code: 0): Call to a member function prepare() on null

Could someone please help me with this ?

abhiburk · 2024-08-29T05:45:12Z

Ok I think the easiest way was to create a new file in your app BatchTenancyBootstrapper.php and
add it to the config/tenancy.php bootstrappers array

stancl and others added 3 commits June 1, 2022 15:35

exclude master from CI

db9480f

Add batch tenancy queue bootstrapper

abac147

add test case

b0876ae

skip tests for old versions

abd5555

stancl reviewed Jun 20, 2022

View reviewed changes

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

stancl added the v4 label Jun 20, 2022

stancl changed the title ~~Add batch tenancy queue bootstrapper~~ [4.x] Add batch tenancy queue bootstrapper Jun 20, 2022

stancl reviewed Aug 20, 2022

View reviewed changes

tests/BatchTest.php Outdated Show resolved Hide resolved

stancl reviewed Aug 20, 2022

View reviewed changes

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

stancl assigned abrardev99 Aug 20, 2022

variable docblocks

0e72c71

stancl reviewed Aug 25, 2022

View reviewed changes

assets/config.php Outdated Show resolved Hide resolved

use Laravel's connection getter and setter

cc2df5c

abrardev99 added the Don't merge yet Work in progress label Aug 31, 2022

stancl changed the base branch from 3.x to master August 31, 2022 11:42

abrardev99 added 3 commits August 31, 2022 17:11

Merge branch 'master' into pr/874

a7e1a17

convert test to pest

cbdcec2

bottom space

fafe7cf

abrardev99 added 2 commits September 5, 2022 11:37

singleton regis in TestCase

48ed213

Merge branch 'master' into bugfix/batch-bootstrapper

d1a8b74

stancl reviewed Sep 12, 2022

View reviewed changes

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

abrardev99 and others added 4 commits September 13, 2022 11:16

Update src/Bootstrappers/BatchTenancyBootstrapper.php

df172ff

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

convert batch class resolution to property level

8211ddb

enabled BatchTenancyBootstrapper by default

8e919b8

typehint DatabaseBatchRepository

8bbf92a

abrardev99 added 2 commits September 23, 2022 10:57

refactore name

d6947f8

DI DB manager

5617c03

stancl reviewed Sep 23, 2022

View reviewed changes

src/Bootstrappers/BatchTenancyBootstrapper.php Outdated Show resolved Hide resolved

stancl reviewed Sep 23, 2022

View reviewed changes

assets/config.php Show resolved Hide resolved

abrardev99 and others added 3 commits September 26, 2022 11:22

typehint

360643b

Update config.php

989e236

use initialize() twice without end()ing tenancy to assert that previo…

395d859

…usConnection logic works correctly

stancl reviewed Sep 26, 2022

View reviewed changes

stancl merged commit b78320b into archtechx:master Sep 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.x] Add batch tenancy queue bootstrapper #874

[4.x] Add batch tenancy queue bootstrapper #874

Riley19280 commented Jun 8, 2022

stancl commented Jun 8, 2022

Riley19280 commented Jun 14, 2022

tonning commented Aug 19, 2022

codecov-commenter commented Aug 31, 2022 •

edited

Loading

stancl commented Sep 12, 2022

stancl commented Sep 22, 2022 •

edited

Loading

abrardev99 commented Sep 23, 2022

stancl Sep 26, 2022

stancl Sep 26, 2022

abrardev99 Sep 27, 2022

stancl commented Sep 26, 2022

MujeebAnwar commented May 1, 2024

abhiburk commented Aug 29, 2024 •

edited

Loading

abhiburk commented Aug 29, 2024

[4.x] Add batch tenancy queue bootstrapper #874

[4.x] Add batch tenancy queue bootstrapper #874

Conversation

Riley19280 commented Jun 8, 2022

stancl commented Jun 8, 2022

Riley19280 commented Jun 14, 2022

tonning commented Aug 19, 2022

codecov-commenter commented Aug 31, 2022 • edited Loading

Codecov Report

stancl commented Sep 12, 2022

stancl commented Sep 22, 2022 • edited Loading

abrardev99 commented Sep 23, 2022

stancl Sep 26, 2022

Choose a reason for hiding this comment

stancl Sep 26, 2022

Choose a reason for hiding this comment

abrardev99 Sep 27, 2022

Choose a reason for hiding this comment

stancl commented Sep 26, 2022

MujeebAnwar commented May 1, 2024

abhiburk commented Aug 29, 2024 • edited Loading

abhiburk commented Aug 29, 2024

codecov-commenter commented Aug 31, 2022 •

edited

Loading

stancl commented Sep 22, 2022 •

edited

Loading

abhiburk commented Aug 29, 2024 •

edited

Loading