barryvdh / laravel-ide-helper

IDE Helper for Laravel
MIT License
14.14k stars 1.16k forks source link

Table columns not discovered in multi-tenant databases #1141

Open vpratfr opened 3 years ago

vpratfr commented 3 years ago

Versions:

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?

mfn commented 3 years ago

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…)?

vpratfr commented 3 years ago

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.

alejandrotrevi commented 3 years ago

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

dennisameling commented 3 years ago

This worked for us using Tenancy for Laravel:

php artisan tenants:run ide-helper:models 🚀

bretto36 commented 2 years ago

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

vpratfr commented 2 years ago

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 commented 2 years ago

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)

sbalex27 commented 2 years ago

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';
  3. 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;
    }
  4. (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 commented 2 years ago

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

vpratfr commented 2 years ago

@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.

rabrowne85 commented 2 years ago

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.

bretto36 commented 2 years ago

@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

rabrowne85 commented 2 years ago

@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 👍

kdevan commented 1 year ago

@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.

27pchrisl commented 9 months ago

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'
joelstein commented 4 months ago

Inspired by https://github.com/barryvdh/laravel-ide-helper/issues/1141#issuecomment-1003880977, I offer the following solution, which works perfectly for us.

We namespace the central and tenant models like so:

- App\Central\Model
- App\Tenant\Model

First, publish the config and set model_locations to an empty array. This prevents this package from scanning the entire "app" directory with the ide-helper:models command. See https://github.com/barryvdh/laravel-ide-helper/issues/137#issuecomment-2115661531.

Then, create a custom command:

php artisan make:command IdeHelperTenancyModels

And use the following code. It first generates central model docs. Then, it creates a tenant, generates its model docs, and deletes it.

class IdeHelperTenancyModels extends Command
{
    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'ide-helper:tenancy-models';

    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'Generates IDE helper docs for central and tenant models.';

    /**
     * Execute the console command.
     */
    public function handle()
    {
        // Ignore if not in the local environment for some reason.
        if (!App::isLocal()) {
            return;
        }

        // Write the central model docs.
        $this->call('ide-helper:models', [
            '--dir' => ['app/Central/Models'],
            '--write' => true,
            '--reset' => true,
        ]);

        // Create a tenant, generate the tenant model docs, and delete the
        // tenant. Slick!
        $tenant = Tenant::create();
        $tenant->run(function () {
            $this->call('ide-helper:models', [
                '--dir' => ['app/Tenant/Models'],
                '--write' => true,
                '--reset' => true,
            ]);
        });
        $tenant->delete();
    }
}

Enjoy!