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.

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

How to tell if a tenant was fully created (db/migration/seeding), to stop "bootstrapped" hooks from firing for non-ready tenant? #270

Closed
tamkeen-tms opened this issue Jan 19, 2020 · 7 comments
Assignees
Labels

Comments

@tamkeen-tms
Copy link

Consider the following scenario: you have a "bootstrapped" hook that loads something from the tenant's db. Somewhere else in the application, Tenant::create() is used to created a tenant (through a controller in the "central" app, not in the CLI environment).

This new tenant will first be added to the tenants tables, then the "bootstrapped" event will be triggered ... while the db creation, migration, and seeding is not yet done! This of course triggers the hook described above, which will attempt to find something from a db table that doesn't exist yet!

I figured I should use isPersisted() inside that hook block, but in fact the new tenant was marked persisted at this point in the code.

// SettingsServiceProvider
    public function boot()
    {
	    tenancy()->hook('bootstrapped', function($manager, Tenant $tenant){
		    // Get something from table xyz
	    });
    }
// NewTenantController
Tenant::create(...); // When created will trigger the hook above, while the db is not created yet!

Is there away to tell if a tenant is fully created to control if such hooks can be triggered?

@tamkeen-tms tamkeen-tms changed the title Triggering "bootstrapped" event while the tenant is still being created How to tell if a tenant was fully created (db/migration/seeding), to stop "bootstrapped" hooks from firing for non-ready tenant? Jan 19, 2020
@stancl
Copy link
Member

stancl commented Jan 19, 2020

Is this a separate issue or can we close this in favor of #271?

@tamkeen-tms
Copy link
Author

tamkeen-tms commented Jan 19, 2020

I think the two are separate issues. My solution for the above problem was to flag the tenant with "ready" when created successfully. And check if this flag exists within the "bootstrapped" hook.

tenancy()->hook('tenant.created', function($manager, $tenant){
$tenant->persisted(true) // Mark persisted first #271 
->put('ready', true); // Mark as "ready" = db created, migrated, and seeded
});
tenancy()->hook('bootstrapped', function($manager, $tenant){
if($tenant->get('ready') === true){ // Check if was "ready"
// Do something with the tenant's database
}
});

Or else, the "bootstrapped" hook will be called, after the tenant was created, and BEFORE the db was created and ready, thus resulting in a "table x doesnot exist" exception.

@stancl
Copy link
Member

stancl commented Jan 19, 2020

This new tenant will first be added to the tenants tables, then the "bootstrapped" event will be triggered

The tenant.created event will be triggered.

... while the db creation, migration, and seeding is not yet done! This of course triggers the hook described above, which will attempt to find something from a db table that doesn't exist yet!

Tenant being created does not mean that the tenant is ready. In production, DB creation, migration, seeding, etc will happen asynchronously, using queued jobs.

Look at this, specifically at the initial_migration_complete: #194 (comment)

I have a queued job that's executed after the DB creation/migration/seeding is finished and it sets the flag to true. When you try to visit a tenant site while that flag is false, or while the database does not exist yet, an exception is thrown and caught by the ExceptionHandler which returns a view with a "Your site is being created ..." message.

@tamkeen-tms
Copy link
Author

Great. So me thinking that a "flag" (for the tenant being ready or not yet) is necessary was correct?! I feel like this MUST be implemented in the library by default, agree?

The tenants table should have "ready" column, which is set to true automatically when all the migration and seeding is actually done. And calling any "bootstrapped" should be blocked until the "ready" flag is true. And yes, the exception for a "being created" tenant should be added.

What do you think?

@stancl
Copy link
Member

stancl commented Jan 19, 2020

How would you do that? If you add a job that creates a superuser, the tenant is only ready after that, not before, so implementing this is up to you.

And yes, the exception for a "being created" tenant should be added

You care about a database being created, not a tenant being created. Tenant is created synchronously, DB can be created asynchronously. Handling this exception will work:

https://github.com/stancl/tenancy/blob/2.x/src/Exceptions/TenantDatabaseDoesNotExistException.php

And the only reason you need to do that is because the initial_migration_complete thing will work only for created but not-yet-migrated/seeded databases.

I don't think everyone needs this and it's pretty straightforward to implement, so doing something like adding an extra job at the end of the queue chain that's executed when a tenant is created, just to add a flag about tenant being created is just unnecessary complexity.

@tamkeen-tms
Copy link
Author

I understand. I believe that this in fact wont be an issue for non-SaaS apps, which will probably use the command line to create new tenants, but for a SaaS app ... this is a must, I think.

But yes, I understand it's straightforward to implement, I just expected this to be natively supported by the library, because my app is SaaS, and a flag for "not yet ready" was necessary for the user.

Thanks for the support :)

If I create a PR for this feature (plus the documentation), would you add it?

@stancl
Copy link
Member

stancl commented Jan 19, 2020

I plan on making an app skeleton with the boilerplate that most SaaS's would need, and it would include things like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants