Skip to content
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

[3.x] Add method to run a callback on the central application #512

Closed
wants to merge 5 commits into from
Closed

[3.x] Add method to run a callback on the central application #512

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5700e7
Add runGlobal helper
morloderex Oct 24, 2020
4ad4e34
Rename method to `central`
morloderex Oct 24, 2020
49e3273
add description
morloderex Oct 24, 2020
b33366e
Improve the tests for central helper
morloderex Oct 24, 2020
0c3fe61
Changed description wording
morloderex Oct 24, 2020
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
23 changes: 23 additions & 0 deletions src/Tenancy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,29 @@ public function getBootstrappers(): array
return array_map('app', $resolve($this->tenant));
}

/**
morloderex marked this conversation as resolved.
Show resolved Hide resolved
* Run a callback using the central environment.
morloderex marked this conversation as resolved.
Show resolved Hide resolved
*
* @param callable $callback
* @return mixed
*/
public function central(callable $callback)
{
$oldTenant = $this->tenant;

if ($this->initialized) {
$this->end();
}

$result = $callback();

if ($oldTenant) {
$this->initialize($oldTenant);
}

return $result;
}

public function query(): Builder
{
return $this->model()->query();
Expand Down
52 changes: 52 additions & 0 deletions tests/AutomaticModeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,57 @@ public function context_is_switched_when_tenancy_is_reinitialized()

$this->assertSame('foobar', app('tenancy_initialized_for_tenant'));
}

/** @test */
public function running_the_central_tenancy_helper_with_tenant_already_initialized()
{
config(['tenancy.bootstrappers' => [
MyBootstrapper::class,
]]);

$tenant = Tenant::create([
'id' => 'acme',
]);

tenancy()->initialize($tenant);

$this->assertSame('acme', app('tenancy_initialized_for_tenant'));

tenancy()->central(function () {
$this->assertTrue(app('tenancy_ended'));
$this->assertEmpty(app('tenancy_initialized_for_tenant'));
});

$this->assertSame('acme', app('tenancy_initialized_for_tenant'));
}

/** @test */
public function running_the_central_tenancy_helper_with_tenant_not_already_initialized()
{
config(['tenancy.bootstrappers' => [
MyBootstrapper::class,
]]);

$runned = 0;

$this->assertFalse(tenancy()->initialized);
$this->assertFalse(app()->bound('tenancy_ended'));
$this->assertFalse(app()->bound('tenancy_initialized_for_tenant'));

tenancy()->central(function () use (&$runned) {
$this->assertFalse(app()->bound('tenancy_initialized_for_tenant'));
$this->assertFalse(app()->bound('tenancy_ended'));
$this->assertFalse(tenancy()->initialized);

$runned = 1;
Comment on lines +105 to +116
Copy link
Member

Choose a reason for hiding this comment

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

What I meant is to:

  1. Check that tenancy was initialized by seeing that config('database.default') was tenant and is not anymore.
  2. Do app()->instance('central_callback_executed', true) inside this closure.

I can just rewrite the whole thing if you want.

It should

  • set it to false at the beginning of the test
  • check that it's false right before the central() call
  • set it to true inside the central call
  • check that it's true after the central call

Copy link
Contributor Author

@morloderex morloderex Oct 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@stancl We have 2 different tests here which tests different scenarios seperatly.

One where the tenant is already initialized and one where a tenant is not already initialized.

The test you are commenting on is the use case where a tenant has not been initialized so i want to make sure that the TenancyManager does not call the end method before running the callback.

However, am not quite sure why you don't see this logic as sufficient? I am not quite sure why we would need to go through the entire Laravel container class to check that a veriable is changed within a callback? Using a variable passed in by reference does the same thing here and the assertions called within the callback tests wether or not the bootstrapper provided has been executed. Can you ellabrorate on throughs here?

});

$this->assertSame(1, $runned);

$this->assertFalse(app()->bound('tenancy_ended'));
$this->assertFalse(app()->bound('tenancy_initialized_for_tenant'));
$this->assertFalse(tenancy()->initialized);
}
}

class MyBootstrapper implements TenancyBootstrapper
Expand All @@ -82,6 +133,7 @@ public function bootstrap(\Stancl\Tenancy\Contracts\Tenant $tenant)

public function revert()
{
app()->instance('tenancy_initialized_for_tenant', '');
app()->instance('tenancy_ended', true);
}
}