Force third party package's models to specific connection

[isecchin]

Description

I'm not sure if this is actually a bug or just a deprecated function (#620 mentions this, but got no replies and was closed without any extra information, so I'm not quite sure how to deal with it).

But seems like the force-tenant-connection-of-models and force-system-connection-of-models have been deprecated, since they are no longer used anywhere on the package.

Is there an alternative way to force a model to a given connection besides the available traits ?

I guess those configuration values were intended specially for third party packages (which is my goal actually) that I can't apply the trait without having to fork the projects.

I've applied @bkintanar suggestion on #611 of setting the database's default connection to the tenant's one for some other things, and since then, the barryvdh/laravel-translation-manager package, for example, stopped working, since the table is stored on the system's DB and I could not force the model of the translation manager package to use the system connection.

And just a suggestion: if these functionality is really deprecated, maybe we should consider removing the keys entirely from the configuration file, to avoid confusion.

---

Actual behavior

Setting classes on the force-tenant-connection-of-models and force-system-connection-of-models configuration have no effect whatsoever.

Expected behavior

Be able to force a specific connection to third party package's models.

---

Information

• hyn/multi-tenant version: 5.4
• laravel version: 5.8

[open-collective-bot[bot]]

Hey :wave:,

Thank you for using our package.

We firmly believe that open sourcing our code improves the developer experience. In a pursuit to continue our work, help us by donating to our collective! :heart:

Issues opened by backers of our Open Collective will automatically labelled with the "backer" tag for priority response and resolve times.

https://opencollective.com/tenancy

[bkintanar]

@isecchin does the package you're using provide config file for setting the model? If so, you can extend the default 3rd party model and put the UsesTenantConnection trait and use that extended model instead.

[isecchin]

@bkintanar, thanks for the quick reply !

Unfortunately not, I’m doing exactly what you said on the spatie permissions package, but this one in particular does not have a configuration to overwrite the model, and since those keys were even documented as being useful for this particular use case, I had no luck trying to use them.

[bkintanar]

@isecchin well.. if the package developer is as responsive as the spatie guys. maybe you can create a PR for the package to make the config?

see: https://github.com/spatie/laravel-medialibrary/pull/377

[isecchin]

I was preparing to do that when I checked that there is already o PR for this since July, I'll see if they can merge it, otherwise I'll just handle this some other way (maybe forking the project, since at least for now this is the only package that I'm using that is causing an issue).

Just a question though, do you happen to know why this feature of forcing a connection through the configuration file was removed ? I couldn't find that information anywhere.

[bkintanar]

@isecchin, sorry I can't answer that question. But rough guess is that it was a dirty hack.

I personally tried to use it before when I was integrating passport, but failed. I ended up looking for another way.

assigning this to @luceos

[ArlonAntonius]

@isecchin Tests show that this is supposed to be working fine, could you show me what exactly you've entered in that config value? Do you have a repo where this problem is shown? Would love to have a quick look at it for you :)

[isecchin]

@ArlonAntonius, I was trying to set the Translation class from the barryvdh/laravel-translation-manager package to use the system connection.

So basically I just added this to the configuration:

'force-system-connection-of-models' => [
    Barryvdh\TranslationManager\Models\Translation::class,
],

But to no avail, I opened the issue because, strangely enough, if you do a search for the force-system-connection-of-models string on the repo, it only shows 3 files (the assets/configs/tenancy.php itself, the changelog.md and tests/unit-tests/Database/ConnectionTest.php).

So maybe this is why it is passing the tests (they are used there, but nowhere on the package's core).

[bkintanar]

@isecchin I just found the code that refers to this config entry.

For Tenant Connection: https://github.com/hyn/multi-tenant/blob/55ec767472aa6ed871b59b96644290dc23d2dcf0/tests/unit-tests/Database/ConnectionTest.php#L96-L107

For System Connection: https://github.com/hyn/multi-tenant/blob/55ec767472aa6ed871b59b96644290dc23d2dcf0/tests/unit-tests/Database/ConnectionTest.php#L112-L120

[isecchin]

@bkintanar yes, but those are unit tests only, right ? They are not used anywhere on the code itself to force the connection of a model at runtime.

[bkintanar]

@isecchin my mistake, github wasn't playing nice..

here's the correct one (I hope):

https://github.com/hyn/multi-tenant/blob/caf4d08be913cc9b7883b586f56ab020cb49b8df/src/Providers/Tenants/ConnectionProvider.php#L69-L87

[isecchin]

Ooh, I've missed that as well, sorry ! I'll try to do some tests here with that file to see why it isn't working (at least for me), and will post here as soon as I find out !

[bkintanar]

No problem. I missed to mention that file as well. Sorry. Please report what you find out.

Happy coding

[isecchin]

Well, I've had no luck so far trying to figure out the problem.

If I dd((new $class())->getConnection()); on the ConnectionProvider right after setting the connection resolver, it shows correctly the system's connection.

If I go to the model class though, and do exactly the same dd($this->getConnection()); on the class' construct, I get the tenant's connection.

I still couldn't figure out the problem, I'll try to do a fresh install of Laravel and see if the same thing happen. Maybe it's another package that is messing things up.

[isecchin]

I guess I kinda tracked down the issue, but still couldn't figure out a solution...

I'm not sure if anyone ever got this feature working, because apparently (if I'm not mistaken), the setConnectionResolver this relies on, basically set the resolver for all Model instances, and, at least on my case, the Hyn resolver lived until the Illuminate\Database\DatabaseServiceProvider booted (it's being booted afterwards apparently), and then it just overwrites it for all Models to the default $this->app['db'].

My guess is that the tests are passing because they test the config keys individually for a single class only. If you try to set both, for example (so you'd have a tenant resolver and a system resolver, just like you do on the Hyn\Tenancy\Providers\Tenants\ConnectionProvider), then the last one set with the setConnectionResolver would be set for both of them, meaning that while one would work, the other would not.

I don't know if I could explain my thoughts clearly, but if not, please let me know !

[luceos]

Ian thanks for diving in. I was astounded this no longer works, thanks to the team for pointing this out to me. We should probably add a test to see whether enabling both at the same time causes the failure you mentioned and fix it accordingly.

I'll see whether I can push this in asap, unless some else beats me to it ;)

[Nks]

So, I am using one other package which has a lot of models which can't be just simply extended due package related to its models. So, currently, I found only one solution for this but not sure how completely it's clear:

Inside AppServiceProvider just simply add this to the boot method:

$this->app->booted(function () {
      $this->app->resolveProvider(ConnectionProvider::class)->overrideConnectionResolvers();
});

Then when all other items will be booted laravel will run this and method again and will override the connection. I think this can be a bit better than overriding config on fly.

[Bibendus83]

Does anyone have a solution for this? I have been struggling with this issue for 3 days with no results (#912, #913). I tried @Nks solution but it doesn't work with Jobs because overrideConnectionResolvers() is called before the tenant connection is defined.

[bkintanar]

@Bibendus83 do you have a test repository that I can test this on and probably fix it?

[Bibendus83]

@bkintanar I don't have a public one but I suppose I could make one for this purpose

[bkintanar]

@Bibendus83 only if you have time.

[Bibendus83]

@bkintanar I'm quite stuck and until this is fixed my app won't work so I think I'll spend some time setting up the test environment for you

[Bibendus83]

@bkintanar I created a test repository with a test queue and the settings library https://github.com/Bibendus83/laravel-app-multi-tenant-demo/tree/dev Tell me if you need something to set it up

[bkintanar]

Thanks @Bibendus83. I'll check it out.

[Bibendus83]

@bkintanar To reproduce the bug you can do one simple thing. Go in the User model and disable the trait UsesTenantConnection. Then uncomment App\User::class in force-tenant-connection-of-models and visit the homepage, theoretically it should work anyway but it doesnt.

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'tenancy.users' doesn't exist (SQL: select * from `users` where `name` = admin limit 1)

To test force-system-connection-of-models just uncomment \QCod\Settings\Setting\Setting::class, it should throw an error but it doesnt.

Here is my .env file

APP_NAME=Laravel
APP_ENV=local
APP_KEY=xxxxxxxxxxxx
APP_DEBUG=true
APP_URL=http://www.multi-tenant-debug.loc:8000/
ASSET_URL=" "

LOG_CHANNEL=stack

DB_CONNECTION=mysql
DB_HOST=172.16.238.10
DB_PORT=3306
DB_DATABASE=tenancy
DB_USERNAME=root
DB_PASSWORD=

# TENANCY_DEFAULT_CONNECTION=tenant
TENANCY_DRIVER=mysql
TENANCY_HOST=172.16.238.10
TENANCY_PORT=3306
TENANCY_DATABASE=tenancy
TENANCY_USERNAME=root
TENANCY_PASSWORD=
TENANCY_CACHE_TTL=false
TENANCY_AUTO_HOSTNAME_IDENTIFICATION=true
TENANCY_ABORT_WITHOUT_HOSTNAME=true
TENANCY_UPDATE_APP_URL=false
TENANCY_DATABASE_DIVISION_MODE=database
LIMIT_UUID_LENGTH_32=false

BROADCAST_DRIVER=log
CACHE_DRIVER=array
CACHE_EXPIRE=-1
QUEUE_CONNECTION=database 
QUEUE_DRIVER=sync   # dev
SESSION_DRIVER=file
SESSION_LIFETIME=120

REDIS_HOST=127.0.0.1
REDIS_PASSWORD=null
REDIS_PORT=6379

MAIL_DRIVER=smtp
MAIL_HOST=smtp.mailtrap.io
MAIL_PORT=2525
MAIL_USERNAME=xxxxxxxxxxxxxx
MAIL_PASSWORD=yyyyyyyyyyyyyy
MAIL_ENCRYPTION=null
[email protected]
MAIL_FROM_NAME="NoReply"

AWS_ACCESS_KEY_ID=
AWS_SECRET_ACCESS_KEY=
AWS_DEFAULT_REGION=us-east-1
AWS_BUCKET=

PUSHER_APP_ID=
PUSHER_APP_KEY=
PUSHER_APP_SECRET=
PUSHER_APP_CLUSTER=mt1

MIX_PUSHER_APP_KEY="${PUSHER_APP_KEY}"
MIX_PUSHER_APP_CLUSTER="${PUSHER_APP_CLUSTER}"

[bkintanar]

@Bibendus83

can you try this a temporary workaround?

add the following code to your AppServiceProvider's boot method

use Hyn\Tenancy\Environment;



public function boot()
{
    $env = app(Environment::class);

    if ($fqdn = optional($env->hostname())->fqdn) {
        config([
            'database.default' => 'tenant',
        ]);
    }
}

[Bibendus83]

@bkintanar I'm already using this workaround or else I would get the Settings library error even while accessing the tenant website. The problem is that this workaround doesn't work with Jobs as I wrote in #912. However on this ticket I would focus on the fact that force-tenant-connection-of-models and force-system-connection-of-models simply don't work.

[bkintanar]

Apparently Laravel's DatabaseServiceProvider's boot method is overriding the Connection Resolver set by hyn.

[Bibendus83]

@bkintanar So, do you think that disabling auto discovery and changing the order of providers in config/app.php could fix the problem?

[bkintanar]

I still have to check that out. I tried disabling the auto discovery and putting the order to the last, but it didn't work.

[anwarx4u]

any update?

[mattvb91]

Has this been fixed?