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

Migrating from hyn/tenancy #266

Closed
kylekz opened this issue Jan 16, 2020 · 6 comments
Closed

Migrating from hyn/tenancy #266

kylekz opened this issue Jan 16, 2020 · 6 comments

Comments

@kylekz
Copy link

kylekz commented Jan 16, 2020

I've been granted a refactoring period on our core product at work in a few weeks, and came across this package again. I think I've spoken to you on the Laravel Discord a few times in the Vapor channel too :)

At the moment, our app is using latest Laravel and latest hyn/tenancy. The only thing I don't like about it is that tests are extremely fragile to the point where I don't dare mess with anything for risk of breaking everything. I also didn't actually realize just how much boilerplate I was writing until I started looking over the docs of this package. As a result, I'm somewhat interested in migrating over, however the app does have a few requirements:

  • hyn/tenancy has a "system" connection. We currently use this system connection to store keys and settings for each tenant, under a settings table. This includes things like tenant name, enabled feature flags, API keys, OAuth keys and so on.
  • Tenants must be able to be created via HTTP request. Right now we're setting tenants up manually with an Artisan command, however we plan to offer "self serve" soon, which would mean having users, payment info etc etc within the system connection, and tenant creation happen during a request rather than strictly in Artisan. hyn/tenancy allows tenant creation anywhere (I believe) and the newly created tenant gets migrated, seeded and ready to go instantly.
  • Must support Passport or Airlock. Currently using Passport, however I do plan on switching to Airlock as I'm not happy with the workarounds I had to make to get Passport working with the SPA frontend.
  • Must support Vapor. Looks like you're working on any issues, if any, so I'm not too concerned on that one.
  • Laravel Mix must work seamlessly. Our SPA is built on Vue, and we have separate "themes" per tenant, configured by the settings record from the system connection mentioned earlier. It's essentially just changing the path to the js/css files within the root Blade template.

My concerns are:

  • The docs state that tenant info can be stored in a "system" database like hyn. Are tenants able to access this database? For example, my User model is configured to use the tenant connection, but my Settings model is configured to the system connection, so any User model can pull which tenant it's part of from hyn's tenant manager, then use that information to pull from the Settings table on the separate connection.
  • In the event/hook system you have, am I able to pass arguments in those? For example, I hook into the database created event to create the initial user, how do I get user-specified data into those callbacks? Currently I can swap to the newly created tenant on the fly, create a new User, and return the info back.
  • How does the scheduler work? I've just changed my defined commands to be scheduled, to have tenancy:run in front of the actual command, so they run on all tenants. Probably isn't the way to go, and in the future I'd like to be able to configure whether or not a command is even scheduled based on its system config, to save on Vapor invoc time.

Hopefully I've not missed anything. Package looks extremely clean and I'm definitely interested in making the switch.

@stancl
Copy link
Member

stancl commented Jan 16, 2020

Hi,

I'll address your points like this:

Central connection

You can create a central/system connection, where tenants will be stored, and use it even inside the tenant app when you need to (make specific models use that connection, or just do DB::connection('central')->table(...)->.... See this page of docs: https://tenancy.samuelstancl.me/docs/v2/storage-drivers/#database

HTTP requests

You can call Tenant::new()->withData([...])->save() from anywhere, including controllers.

Passport/Airlock

Passport integration is documented and since Airlock is simpler, I expect it to work without issues too.

Vapor support

The package makes sure to make the injected ASSET_URL tenant-aware, and there isn't probably anything else that needs special treatment when Vapor is used.

Mix

Not sure exactly what you mean, can you elaborate on what you need here from the package?

Hooks

For example, I hook into the database created event to create the initial user, how do I get user-specified data into those callbacks

You can set the initial user's credentials in the tenant's data and then use that in your hook. See this: #194 (comment)

Schedule

Scheduled jobs runs centrally, so anything that should be done for all tenants should be wrapped in a loop and $tenant->run()

@kylekz
Copy link
Author

kylekz commented Jan 22, 2020

re: Mix: I was hoping it played nice in terms of URLs generated etc, but after looking into it a bit more, it's all driven by configs so it's fine.

Thanks for the write up! Might have a tinker this weekend.

@kylekz kylekz closed this as completed Jan 22, 2020
@kylekz
Copy link
Author

kylekz commented Mar 2, 2020

Hey, I don't mean to necropost or anything, I was just looking at taking this a little more seriously after spending a few hours today wrangling with trying to fix 10 second unit tests with hyn. One basic user register unit test was running 700 db queries in total...

I started a clean copy of Laravel + this package and wrote a basic user model create test, and seemed to get a time of 400ms. This would be perfect. However I couldn't find any remnants of the test DB or any test data, despite removing any cleanup code, so I'm not sure it was actually running on the db driver with all the migrations.

Would you happen to have a recommended approach to migrating existing tenant data over to the new package? I just noticed the $data feature on tenants and how you can automatically map them to config variables, which is a brilliant idea and basically replaces 90% of my "system" connection usage.

My approach would be:

  1. Install stancl/tenancy
  2. Replace hyn/multi-tenant code refs with stancl/tenancy equivalents
  3. Remove hyn/multi-tenant
  4. Migrate stancl/tenancy tables
  5. Write a command to convert Website, Hostname & Settings records into Tenant record
  6. Link up new Tenant records with the existing databases

Steps 1-5 are somewhat straight forward, albeit step 2 being tedious with how ingrained hyn is. As for step 6, hyn uses separate MySQL users for each database, so it's not as simple as renaming databases to swap tenants around. I'm running on Vapor, so having this all run in one process would be ideal.

Thanks.

@kylekz kylekz reopened this Mar 2, 2020
@stancl
Copy link
Member

stancl commented Mar 2, 2020

This would be perfect. However I couldn't find any remnants of the test DB or any test data, despite removing any cleanup code

How did you set up your test environment? Did you follow the testing guide in the docs? I think if you use sqlite for testing, you should see tenant DB files in database/.

step 2 being tedious with how ingrained hyn is

Right, this would be the hardest part. I guess you have to go through the whole codebase and look for anything "tenant-aware" and replace it with normal Laravel code.

@kylekz
Copy link
Author

kylekz commented Mar 6, 2020

I did follow the testing guide, everything seemed to be connected to the db driver and MySQL, perhaps I missed something. Thanks.

@kylekz kylekz closed this as completed Mar 6, 2020
@OliverWich
Copy link

Hi @kylekz and sorry to everyone for raising this issue from the dead again :D

Due to the terrible maintenance state of hyn/tenancy, we are also looking to migrate to this.
The grass looks so much greener on the other side, but we are a bit stumped as to where to start the migration and lacking a bit of a migration guide of sorts.

For a bit of context: This is a Laravel application which got stuck at Laravel 7 and is just now finally getting updated to 11 and given a fresh coat of paint which should ensure the future maintainability. This is the reason we are very, very interested in this, but proper and painless migration seems like a big hurdle...
As our software is the basis for many systems (and used by us as a composer dependency in those) our goal is to require as little changes as possible from all the systems which use the main package. Most of the work required should be done in the main project and then just work automatically with as little manual change as possible in the system projects. I assume this will be fine as the only thing that is really shared is the storage/ directory, apart from that, no real php source lives in the separate projects (ignoring a 10 line, mostly default laravel bootstrap file).

However, we are a bit stumped by said migration of the main project. Basically, we need a hand in getting started with changing over a Laravel 11 project (with a bunch of slightly legacy Laravel 6/7 code) to archtex/tenancy.

I thought since you (@kylekz) performed a similar migration, you could elaborate a bit on what you did, esp. some more details to step 2:

Replace hyn/multi-tenant code refs with stancl/tenancy equivalents

would be greatly appreciated!


And if anyone else is knowledgeable in this, some tips on the following points would be awesome:

  1. How does one properly migrate from a multi database setup with hyn to a multi database setup with this? For now, we don't need any of the fancy features, this might come later, we just want to translate the same state over to this. This would be: one 'meta' database which contains the laravel jobs (although those could be moved to the individual DBs if needed) along with the 'systems' (i.e. what hyn called 'hostnames'). Then the tenancy DBs contain some Models which are the same for each along with some custom tables build with custom logic (see 7. for some more details). We do not share anything from the individual tenant DBs between the tenants.
  2. How to handle storage with folders and files which currently reside in storage/app/tenancy/tenants/<tenant_name>/files. One thing of note: we have no special/individual configs for each tenant, the storage is really just files (and directories).
  3. Ensuring that data and users (including their auth data, we use passport) survive the transition. I assume we would need to either manually write migrations for the tenant DBs from the hyn schema to the setup this uses (which would be painful) or is the multi DB version of this package able to be configured to work with a (originally) hyn multi tenant DB with less effort?
  4. How is the laravel WebPush channel handled? Do we need to have any concerns with stuff like VAPID keys and/or the integration?
  5. Are changes to the log needed? We currently have logs from our workers (via supervisor) in one file and general logs in another. (with cli logs in a third one)
  6. How do workers handle the new tenancy? Any special changes needed for them?
  7. How do our migration commands on updates change? We currently just use artisan migrate as our tenant DB is build (at least partially and apart from some default tables whose migrations live in database/tenant/migrations) with custom logic. I assume/hope we won't need to touch the custom logic / tenant migrations? The tenant migrations contain mostly standard models which are extended via the custom logic which also adds tables for "dynamically" created models.
  8. What are the core changes we need to make to our models to mark them as either "use for each tenant" or "save this in the meta database"?

Thank you everyone for guiding us and thanks @stancl for your awesome work on this package! We really hope we can get a successful migration going and are looking forward to finally using a well maintained and modern packages as the backbone of our application!

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

No branches or pull requests

3 participants