laravel-ide-helper icon indicating copy to clipboard operation
laravel-ide-helper copied to clipboard

Table columns not discovered in multi-tenant databases

Open vpratfr opened this issue 3 years ago • 16 comments

Versions:

  • ide-helper Version: 2.9.0
  • Laravel Version: 8.x
  • PHP Version: 8.0

Description:

We have a multi-tenant application where models are split in 2 DB connections : system (app DB) and tenant (each customer has one)

Models on the tenant database do not have their attributes discovered from the database.

Package used to manage tenancy is hyn-multitenant which basically dynamically builds the Laravel DB connection information depending on the active tenant (which could be determined by the subdomain for instance or programmatically)

Issue

The code linked below does not get the proper database which should match a tenant database

https://github.com/barryvdh/laravel-ide-helper/blob/f0959c1184c6f8f9e50afa6d9b6c1eb7691ae3ae/src/Console/ModelsCommand.php#L418-L436

$database stays null

Request

We should have a mecanism to allow discovery of those attributes.

One possible way to allow that could be to have a configuration entry where we could override the database according to the connection name.

Something like

   'db_mapping'=> [
      // connection => database name
      'system' => 'homestead',
      'tenant' => 'my_tenant',
   ],

Then code to get the database could be fixed liked that:

       $connectionName = $model->getConnection()->getName();
       $database = config("ide-helper.db_mapping.$connectionName");
        if (strpos($table, '.')) {
            [$database, $table] = explode('.', $table);
        }

I have tested that on our project. There should be no difference in previous behaviour if the configuration is not specified.

Any thoughts?

vpratfr avatar Jan 11 '21 19:01 vpratfr

If the tenant is dynamically built, how do you decide which one to use for ide-helper (and: I assume we're still talking about a development system and not running this in production…)?

mfn avatar Jan 11 '21 20:01 mfn

Yes, this is run on a local VM (Homestead).

We have at least 2 DB active on the VM: the system DB and a dummy client DB (we actually have several client DB, but they are all the same in schema, so we can use any of them).

the tenant is dynamically built

the whole tenant is not dynamically built, it is just the connection details which are dynamically set according to the tenant properties.

e.g. : when accessing the subdomain acme.example.com, we would setup the tenant connection to access the tenant_acme database with user X and password Y.

This really does not matter in ide-helper context : all those tenant DB have the same schema, so we do not need any dynamic logic, we simply need to point IDE Helper to one valid tenant DB it can explore for those models which use the tenant connection. And still point it to the system database for those models which use the system connection.

vpratfr avatar Jan 12 '21 09:01 vpratfr

Just run the ide-helper:models command for a specific tenant:

https://tenancy.dev/docs/hyn/5.6/commands

I'm using spatie/multitenancy and i had to run the command like this since the ide-helper:models command needs a working connection:

php artisan tenants:artisan "ide-helper:models" --tenant=1

alejandrotrevi avatar Jun 15 '21 23:06 alejandrotrevi

This worked for us using Tenancy for Laravel:

php artisan tenants:run ide-helper:models 🚀

dennisameling avatar Jun 24 '21 11:06 dennisameling

@alejandrotrevi 's suggestion works well for the new tenancy package by the same guys - https://github.com/tenancy/tenancy/issues/238

bretto36 avatar Nov 09 '21 03:11 bretto36

I decided to give it a fresh new look and see why it is working for others and not for us.

We had hyn/multitenant and recently switched to spatie/multitenant. In both cases we see the same issue, this is not a matter of library used to handle tenancy.

This works well for others because probably they do not have a combination of landlord and tenant models. Or they have not noticed that their landlord models where not generated properly.

As I said above

We have at least 2 DB active at the same time on the VM: the system/landlord DB and a tenant DB

That means that IDE Helper needs to inspect the DB Schema in two different connections when run. And it does not, as it is only capable of handling a single connection at a time.

vpratfr avatar Dec 16 '21 10:12 vpratfr

I have opened a PR to allow handling such cases (which are more and more common, judging by the increasing number of packages to handle multi-tenancy)

vpratfr avatar Dec 16 '21 11:12 vpratfr

Solution for Tenancy for Laravel:

  1. Create a custom artisan command: php artisan make:command IdeHelperTenantModels
  2. Create a signature for the ide helper:
    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'ide-helper:tenant-models';
  1. Write in the handle method:

Important: You will need to write attributes to create a temporary tenant or use a factory

    /**
     * Execute the console command.
     *
     * @return int
     */
    public function handle()
    {
        tenancy()->initialize($tenant = Tenant::create(
            // your tenant model attributes
        ));
        $this->call('ide-helper:models');
        $tenant->delete();
        return Command::SUCCESS;
    }
  1. (Bonus) Write in the description attribute the details:
    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'Generate autocompletion for tenant models';

5 Done! ✅, now for generate the tenant models for phpdocs just run php artisan ide-helper:tenant-models

image

This command will create a new tenant database, then calls the command while connected to it, generates the documents with the extracted tenant templates attributes, and then deletes the temporary database.

sbalex27 avatar Jan 03 '22 05:01 sbalex27

@dennisameling it works too, the problem is if i have 8 tenants the command will be executed 8 times

sbalex27 avatar Jan 03 '22 05:01 sbalex27

@dennisameling sorry, but that does not work either, please read the original issue again: that will properly generate helpers for tenant side models. Landlord side models will not be generated properly.

vpratfr avatar Jan 03 '22 06:01 vpratfr

For anyone using the spatie/laravel-multitenancy package with multiple databases, I found that a simple "workaround" was to alter the database.php config file and set a specific connections.{tenant}.database value to one of my test "tenant" databases, before running the command. Means changing it back to null afterwards.

Not sure if something similar would work on other packages?

I'm creating a PR that looks to implement this, but will see if it is accepted.

rabrowne85 avatar Sep 02 '22 10:09 rabrowne85

@rabrowne85 With a different multitenancy package i simply put --tenant= and specify the tenant username. php artisan ide-helper:models "App\Models\TenantModel.php" --tenant=15

Could the same happen for laravel multi tenancy

https://spatie.be/docs/laravel-multitenancy/v2/advanced-usage/making-artisan-commands-tenant-aware

bretto36 avatar Sep 05 '22 19:09 bretto36

@bretto36 Yeah and that seems to work as well on the spatie package too (though a few issues here or there) - just have to keep remembering to run the tenant based command what the tenant ID is every time. Suppose you could write an alias for it 👍

rabrowne85 avatar Sep 06 '22 08:09 rabrowne85

@dennisameling sorry, but that does not work either, please read the original issue again: that will properly generate helpers for tenant side models. Landlord side models will not be generated properly.

Yeah same issue here. I can generate EITHER landlord or tenant just fine but not both.

kdevan avatar Mar 26 '23 03:03 kdevan

Running the following two commands in this order works for me with https://tenancyforlaravel.com/ to cover both central and tenant models, correctly appending/updating docblocks (this assumes you have a tenant set up in development for it to check against).

php artisan ide-helper:models --write --reset
php artisan tenants:run ide-helper:models --option='write=true' --option='smart-reset=true'

27pchrisl avatar Dec 06 '23 15:12 27pchrisl