multi-tenant icon indicating copy to clipboard operation
multi-tenant copied to clipboard

Package events not firing / not being captured

Open DennisOng opened this issue 7 years ago • 22 comments

Description

Seems like this package's events are not firing for me, or rather, I'm unsuccessful in capturing those events.


Actual behavior

Events not captured

Expected behavior

Events captured


Information

  • hyn/multi-tenant version: 5.2.6
  • laravel version: 5.6.29
  • database driver and version: mysql
  • php version: 7.2

tenancy.php config

    'models' => [
        /**
         * Specify different models to be used for the global, system database
         * connection. These are also used in their relationships. Models
         * used have to implement their respective contracts and
         * either extend the SystemModel or use the trait
         * UsesSystemConnection.
         */

        // Must implement \Hyn\Tenancy\Contracts\Hostname
        'hostname' => \Hyn\Tenancy\Models\Hostname::class,

        // Must implement \Hyn\Tenancy\Contracts\Website
        'website' => \Hyn\Tenancy\Models\Website::class
    ],
    'website' => [
        /**
         * Each website has a short random hash that identifies this entity
         * to the application. By default this id is randomized and fully
         * auto-generated. In case you want to force your own logic for
         * when you need to have a better overview of the complete
         * tenant folder structure, disable this and implement
         * your own id generation logic.
         */
        'disable-random-id' => false,

        /**
         * The random Id generator is responsible for creating the hash as mentioned
         * above. You can override what generator to use by modifying this value
         * in the configuration.
         *
         * @warn This won't work if disable-random-id is true.
         */
        'random-id-generator' => Hyn\Tenancy\Generators\Uuid\ShaGenerator::class,

        /**
         * Enable this flag in case you're using a driver that does not support
         * database username or database name with a length of more than 32 characters.
         *
         * This should be enabled for MySQL, but not for MariaDB and PostgreSQL.
         */
        'uuid-limit-length-to-32' => env('LIMIT_UUID_LENGTH_32', true),

        /**
         * Specify the disk you configured in the filesystems.php file where to store
         * the tenant specific files, including media, packages, routes and other
         * files for this particular website.
         *
         * @info If not set, will revert to the default filesystem.
         * @info If set to false will disable all tenant specific filesystem auto magic
         *       like the config, vendor overrides.
         */
        'disk' => 's3',

        /**
         * Automatically generate a tenant directory based on the random id of the
         * website. Uses the above disk to store files to override system-wide
         * files.
         *
         * @info set to false to disable.
         */
        'auto-create-tenant-directory' => true,

        /**
         * Automatically rename the tenant directory when the random id of the
         * website changes. This should not be too common, but in case it happens
         * we automatically want to move files accordingly.
         *
         * @info set to false to disable.
         */
        'auto-rename-tenant-directory' => true,

        /**
         * Automatically deletes the tenant specific directory and all files
         * contained within.
         *
         * @see
         * @info set to true to enable.
         */
        'auto-delete-tenant-directory' => false,

        /**
         * Time to cache websites in minutes. Set to false to disable.
         */
        'cache' => false,
    ],
    'hostname' => [
        /**
         * If you want the multi tenant application to fall back to a default
         * hostname/website in case the requested hostname was not found
         * in the database, complete in detail the default hostname.
         *
         * @warn this must be a FQDN, these have no protocol or path!
         */
        'default' => env('TENANCY_DEFAULT_HOSTNAME'),
        /**
         * The package is able to identify the requested hostname by itself,
         * disable to get full control (and responsibility) over hostname
         * identification. The hostname identification is needed to
         * set a specific website as currently active.
         *
         * @see src/Jobs/HostnameIdentification.php
         */
        'auto-identification' => env('TENANCY_AUTO_HOSTNAME_IDENTIFICATION', true),

        /**
         * In case you want to have the tenancy environment set up early,
         * enable this flag. This will run the tenant identification
         * inside a middleware. This will eager load tenancy.
         *
         * A good use case is when you have set "tenant" as the default
         * database connection.
         */
        'early-identification' => env('TENANCY_EARLY_IDENTIFICATION', true),

        /**
         * Abort application execution in case no hostname was identified. This will throw a
         * 404 not found in case the tenant hostname was not resolved.
         */
        'abort-without-identified-hostname' => env('TENANCY_ABORT_WITHOUT_IDENTIFIED_HOSTNAME', true),

        /**
         * Time to cache hostnames in minutes. Set to false to disable.
         */
        'cache' => false,

        /**
         * Automatically update the app.url configured inside Laravel to match
         * the tenant FQDN whenever a hostname/tenant was identified.
         *
         * This will resolve issues with password reset mails etc using the
         * correct domain.
         */
        'update-app-url' => true,
    ],
    'db' => [
        /**
         * The default connection to use; this overrules the Laravel database.default
         * configuration setting. In Laravel this is normally configured to 'mysql'.
         * You can set a environment variable to override the default database
         * connection to - for instance - the tenant connection 'tenant'.
         */
        'default' => env('TENANCY_DEFAULT_CONNECTION'),
        /**
         * Used to give names to the system and tenant database connections. By
         * default we configure 'system' and 'tenant'. The tenant connection
         * is set up automatically by this package.
         *
         * @see src/Database/Connection.php
         * @var system-connection-name The database connection name to use for the global/system database.
         * @var tenant-connection-name The database connection name to use for the tenant database.
         */
        'system-connection-name' => env('TENANCY_SYSTEM_CONNECTION_NAME', Connection::DEFAULT_SYSTEM_NAME),
        'tenant-connection-name' => env('TENANCY_TENANT_CONNECTION_NAME', Connection::DEFAULT_TENANT_NAME),

        /**
         * The tenant division mode specifies to what database websites will be
         * connecting. The default setup is to use a new database per tenant.
         * In case you prefer to use the same database with a table prefix,
         * set the mode to 'prefix'.
         *
         * @see src/Database/Connection.php
         */
        'tenant-division-mode' => env('TENANCY_DATABASE_DIVISION_MODE', 'database'),

        /**
         * The database password generator takes care of creating a valid hashed
         * string used for tenants to connect to the specific database. Do
         * note that this will only work in 'division modes' that set up
         * a connection to a separate database.
         */
        'password-generator' => Hyn\Tenancy\Generators\Database\DefaultPasswordGenerator::class,

        /**
         * The tenant migrations to be run during creation of a tenant. Specify a directory
         * to run the migrations from. If specified these migrations will be executed
         * whenever a new tenant is created.
         *
         * @info set to false to disable auto migrating.
         *
         * @warn this has to be an absolute path, feel free to use helper methods like
         * base_path() or database_path() to set this up.
         */
        'tenant-migrations-path' => database_path('migrations/tenant'),

        /**
         * The default Seeder class used on newly created databases and while
         * running artisan commands that fire seeding.
         *
         * @info requires tenant-migrations-path in order to seed newly created websites.
         *
         * @warn specify a valid fully qualified class name.
         */
        'tenant-seed-class' => Database\Seeder\Tenant\TenantDatabaseSeeder::class,
//      eg an admin seeder under `app/Seeders/AdminSeeder.php`:
//        'tenant-seed-class' => App\Seeders\AdminSeeder::class,

        /**
         * Automatically generate a tenant database based on the random id of the
         * website.
         *
         * @info set to false to disable.
         */
        'auto-create-tenant-database' => true,

        /**
         * Automatically generate the user needed to access the database.
         *
         * @info Useful in case you use root or another predefined user to access the
         *       tenant database.
         * @info Only creates in case tenant databases are set to be created.
         *
         * @info set to false to disable.
         */
        'auto-create-tenant-database-user' => true,

        /**
         * Automatically rename the tenant database when the random id of the
         * website changes. This should not be too common, but in case it happens
         * we automatically want to move databases accordingly.
         *
         * @info set to false to disable.
         */
        'auto-rename-tenant-database' => true,

        /**
         * Automatically deletes the tenant specific database and all data
         * contained within.
         *
         * @info set to true to enable.
         */
        'auto-delete-tenant-database' => env('TENANCY_DATABASE_AUTO_DELETE', false),

        /**
         * Automatically delete the user needed to access the tenant database.
         *
         * @info Set to false to disable.
         * @info Only deletes in case tenant database is set to be deleted.
         */
        'auto-delete-tenant-database-user' => env('TENANCY_DATABASE_AUTO_DELETE_USER', false),

        /**
         * Define a list of classes that you wish to force onto the tenant or system connection.
         * The connection will be forced when the Model has booted.
         *
         * @info Useful for overriding the connection of third party packages.
         */
        'force-tenant-connection-of-models' => [
//            App\User::class
        ],
        'force-system-connection-of-models' => [
//            App\User::class
        ],
    ],

    /**
     * Global tenant specific routes.
     * Making it easier to distinguish between landing and tenant routing.
     *
     * @info only works with `tenancy.hostname.auto-identification`.
     */
    'routes' => [
        /**
         * Routes file to load whenever a tenant was identified.
         *
         * @info Set to false or null to disable.
         */
        'path' => base_path('routes/tenants.php'),

        /**
         * Set to true to flush all global routes before setting the routes from the
         * tenants.php routes file.
         */
        'replace-global' => false,
    ],

    /**
     * Folders configuration specific per tenant.
     * The following section relates to configuration to files inside the tenancy/<uuid>
     * tenant directory.
     */
    'folders' => [
        'config' => [
            /**
             * Merge configuration files from the config directory
             * inside the tenant directory with the global configuration files.
             */
            'enabled' => true,

            /**
             * List of configuration files to ignore, preventing override of crucial
             * application configurations.
             */
            'blacklist' => ['database', 'tenancy', 'webserver'],
        ],
        'routes' => [
            /**
             * Allows adding and overriding URL routes inside the tenant directory.
             */
            'enabled' => false,

            /**
             * Prefix all tenant routes.
             */
            'prefix' => null,
        ],
        'trans' => [
            /**
             * Allows reading translation files from a trans directory inside
             * the tenant directory.
             */
            'enabled' => false,

            /**
             * Will override the global translations with the tenant translations.
             * This is done by overriding the laravel default translator with the new path.
             */
            'override-global' => true,

            /**
             * In case you disabled global override, specify a namespace here to load the
             * tenant translation files with.
             */
            'namespace' => 'tenant',
        ],
        'vendor' => [
            /**
             * Allows using a custom vendor (composer driven) folder inside
             * the tenant directory.
             */
            'enabled' => false,
        ],
        'media' => [
            /**
             * Mounts the assets directory with (static) files for public use.
             */
            'enabled' => false,
        ],
        'views' => [
            /**
             * Adds the vendor directory of the tenant inside the application.
             */
            'enabled' => false,

            /**
             * Specify a namespace to use with which to load the views.
             *
             * @eg setting `tenant` will allow you to use `tenant::some.blade.php`
             * @info set to null to add to the global namespace.
             */
            'namespace' => null,

            /**
             * If `namespace` is set to null (thus using the global namespace)
             * make it override the global views. Disable to
             */
            'override-global' => true,
        ]
    ]

Relevant Code

EventServiceProvider.php

protected $listen = [
        // Hyn Tenancy Events
        Identified::class => [
            ActivatesPublicDisk::class
        ],
        NoneFound::class => [
            ActivatesPublicDisk::class
        ],
        Switched::class => [
            ActivatesPublicDisk::class
        ],
];

    public function boot()
    {
        parent::boot();

        Event::listen('Hyn\Tenancy\Events\*', function ($eventName, array $data) {
              // Never gets in here
              print_r($eventName);
        });
        Event::listen('*', function ($eventName, array $data) {
            // Other non hyn/multi-tenant events show, but nothing from this package
            print_r($eventName);
        });
    }

EventServiceProvider.php

class ActivatesPublicDisk
{
    public function __construct(Factory $filesystem)
    {
        dd('ccc'); // This never shows
        $this->filesystem = $filesystem;
    }
}

I've also tried adding adding some dd() inside WebsiteEvent.php and Identified.php, and those are triggered and show.

Also, I do get data from the tenant tables, so the tenant idenfication part must be working.

Hopefully that's enough information, I'll update as I find more, thanks!

DennisOng avatar Jul 30 '18 17:07 DennisOng

Just a wild guess but:

Event::listen('Hyn\Tenancy\Events\*', function ($eventName, array $data) {
              // Never gets in here
              print_r($eventName);
        });

If I'm not mistaken FQCN in strings are prefixed with a \. Can you try:

Event::listen('\Hyn\Tenancy\Events\*', function ($eventName, array $data) {
              // Never gets in here
              print_r($eventName);
        });

luceos avatar Jul 31 '18 08:07 luceos

Unfortunately tried that, but still no luck, thanks though!

DennisOng avatar Jul 31 '18 18:07 DennisOng

Can you post your .env values (except database information).

luceos avatar Jul 31 '18 20:07 luceos

Sure, they are pretty much the default:

TENANCY_DEFAULT_HOSTNAME=demo.api.whispli.test

TENANCY_CONNECTION=system TENANCY_HOST=127.0.0.1 TENANCY_PORT=3306 TENANCY_DATABASE=xxx TENANCY_USERNAME=xxx TENANCY_PASSWORD=xxx

DennisOng avatar Jul 31 '18 22:07 DennisOng

Hey @luceos I seem to have found the solution for my case:

If I disable package auto-discovery for this package, and manually register Hyn\Tenancy\Providers\TenancyProvider::class in config/app.php after the EventServiceProvider then it works

Might be a bug with the auto-discovery here

DennisOng avatar Aug 01 '18 05:08 DennisOng

This.. is extremely confusing! 😂

luceos avatar Aug 01 '18 11:08 luceos

Hi @DennisOng and @luceos ,

Thankfully I am not the only one with this kind of problem. For some reason the events fired by this package are not being captured in the App\Providers\EventServiceProvider. Not out-of-the-box anyways.

When I discover more I report back. Or perhaps I read an answer here in time... ^_^

Success!

ametad avatar Aug 01 '18 15:08 ametad

I am sorry I was not correct in my last post: the Hyn\*\Events are fired and can be listened for in \App\Providers\EventServiceProvider.

It is only confusing in what order, when is what fired!?

This is what I observe: in App\Providers\EventServiceProvider

    public function boot()
    {
        parent::boot();

        Event::listen('Hyn\*', function ($eventName, array $data) {

            dump($eventName, $data);
        });
    }

Two urls are pointing to the webserver. One url is defined as Hostname for a Website, the other url is not defined and so will not be identified by the tenant system.

The none identified tenant url does show events fired by Hyn\Tenancy:

image

The identified tenant url on the other hand, shows no events from Hyn\Tenancy!

This package holds a lot of secrets for me yet... Perhaps @luceos you notice something that is helpful here?

ametad avatar Aug 02 '18 08:08 ametad

Further more, var_dumping from \Illuminate\Events\Dispatcher in the fire() method, the Hyn\Tenancy events are showing up correctly!

So this must have something to do with the order in which all things are flowing...

ametad avatar Aug 02 '18 10:08 ametad

I looked at this last night but found no cause yet..

luceos avatar Aug 02 '18 11:08 luceos

Do you also experience the behavior where the events are not 'heared' in App\Providers\EventServiceProvider?

ametad avatar Aug 02 '18 12:08 ametad

@ametad can you compare your tenancy.php with the one from @DennisOng to check for overlap? So that I can see what you both configured to get at this state?

luceos avatar Aug 06 '18 06:08 luceos

Yes of course, here it is:

LIMIT_UUID_LENGTH_32=true
TENANCY_DEFAULT_CONNECTION=tenant
TENANCY_EARLY_IDENTIFICATION=true

Most of config/tenancy.php is the same, the things that are different in my file:

    'website' => [
        'disk' => null,
    ],
    'hostname' => [
        'abort-without-identified-hostname' => false,
    ],

To be thorough: website.cache and hostname.cache are both false.

ametad avatar Aug 06 '18 07:08 ametad


```
/**
         * In case you want to have the tenancy environment set up early,
         * enable this flag. This will run the tenant identification
         * inside a middleware. This will eager load tenancy.
         *
         * A good use case is when you have set "tenant" as the default
         * database connection.
         */
        'early-identification' => env('TENANCY_EARLY_IDENTIFICATION', true),
```

~~~So perhaps in the above described middleware?~~~

NEVER MIND, also with 'early idetification` off events cannot be listened for...

ametad avatar Aug 06 '18 07:08 ametad

@DennisOng Hope this helps you:

I was so blindly staring at the boot() method, but I think registering listeners in the register() method of a Provider would do the trick.

<?php

namespace App\Listeners;

use Illuminate\Contracts\Events\Dispatcher;

class TestListener
{
    /**
     * Create the event listener.
     *
     * @return void
     */
    public function __construct()
    {
        //
    }

    public function subscribe(Dispatcher $events)
    {
        $events->listen('*', [$this, 'test']);
    }

    public function test($event)
    {
        var_dump($event);
    }
}

<?php

namespace App\Providers;

use App\Listeners\TestListener;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Support\Facades\Event;
use Illuminate\Foundation\Support\Providers\EventServiceProvider as ServiceProvider;


class EventServiceProvider extends ServiceProvider
{
    /**
     * The event listener mappings for the application.
     *
     * @var array
     */
    protected $listen = [
        'App\Events\Event' => [
            'App\Listeners\EventListener',
        ],
    ];

    /**
     * The subscriber classes to register.
     *
     * @var array
     */
    protected $subscribe = [
//        TestListener::class, // This is NOT working!
    ];

    /**
     * Register any events for your application.
     *
     * @return void
     */
    public function boot()
    {
        parent::boot();

        //
    }

    public function register()
    {
        $this->app[Dispatcher::class]->subscribe(TestListener::class);
    }
}

I think perhaps this is not te preferred way of Laravel reading the documentation.... But perhaps this is okay after all.

Or can anyone point me in a better/right direction?

ametad avatar Aug 14 '18 13:08 ametad

I've spend some time with @ametad trying to validate two candidate causes of this issue.

  • Listeners might be returning false, blocking the event from travelling further down the chain.
  • Another dispatcher is rebound somewhere in the startup cycle.

What we discovered was that once the Laravel app is booted, the number of listeners for the "Identified" amount reset back to 0, which before (during registration of providers) was 7.

Keep you posted.

luceos avatar Aug 21 '18 13:08 luceos

It looks like there is a work around; don't auto discover hyn/multi-tenant. Like this:

composer.json
    "extra": {
        "laravel": {
            "dont-discover": [
                "hyn/multi-tenant"
            ]
        }
    },

config/app.php

    'providers' => [

        /*
         * Laravel Framework Service Providers...
         */
        Illuminate\Auth\AuthServiceProvider::class,
    // .. etc ..
        Illuminate\View\ViewServiceProvider::class,

        /*
         * Package Service Providers...
         */
        \Hyn\Tenancy\Providers\TenancyProvider::class,

        /*
         * Application Service Providers...
         */
        App\Providers\AppServiceProvider::class,
        App\Providers\AuthServiceProvider::class,
        // App\Providers\BroadcastServiceProvider::class,
        App\Providers\EventServiceProvider::class,
        App\Providers\RouteServiceProvider::class,

    ],

ametad avatar Aug 22 '18 14:08 ametad

I see this work around is mentioned before @DennisOng . Credits to you! But why the need of declaring after the EventServiceProvider? Registered Listeners/Subscribers in third party package can listen to Hyn\\* events with config above.

ametad avatar Aug 22 '18 14:08 ametad

Hmm that's interesting @ametad , I had assumed that the issue was caused by the order these are booted/registered, but from what you're saying this is not really the case, good to know!

DennisOng avatar Aug 22 '18 18:08 DennisOng

I'm thoroughly confused by this issue if the order doesn't matter but disabling auto-discover works...

Providers are by Laravel in the following order:

  1. config/app.php providers array IF they start with Illuminate
  2. auto-discovered providers from other packages
  3. the rest of the config/app.php providers array

Disabling auto-discovery for Hyn and adding TenancyProvider before any/all App\* providers should do exactly nothing about the order (ignoring other 3rd party packages at least). I don't see how that could possibly change anything.

fletch3555 avatar Aug 22 '18 20:08 fletch3555

To be clear: I did NOT ignore 3rd party. Because my test of listening to Hyn* events was done in a package that was auto discovered. I call this package 3rd party, because it was required by Composer.

(Although the code is of my own and resides in a subdirectory, Composer require does make a symlink to this code and place it in the vendor directory. Hence I call it 3rd party)

ametad avatar Aug 23 '18 12:08 ametad

I run into the same issue, disabling auto-discover worked for me as DennisOng described. For me it worked only when I load the TenancyProvider after EventServiceProvider too.

For me it looks like some kind of loading order issue. Multi-tenant seems to call all the events before the App\Providers\EventServiceProvider has been booted. So my listener, for example Hyn\Tenancy\Events\Database\ConfigurationLoaded, is registered too late.

If I use console commands, the loading order seems to be right and it will work even with auto-discover.

X-Coder avatar Aug 24 '18 07:08 X-Coder