Framework: Queueing notifications and using multiple database connections for $notifiable

[michaeldzjap]

• Laravel Version: 5.4.24
• PHP Version: 7.1
• Database Driver & Version: MySQL 5.7.18

Description:

I am running into problems when I want to queue a notification by calling notify() on a user fetched through a custom database connection. This doesn't seem to be supported. If the notification is not queue'd everything works fine and the mail is send to the correct user.

Steps To Reproduce:

In routes/web.php I have the following test route:

$router->get('/test', function () {
    // A user with id 14 exists in the 2nd database, not in the 1st (default connection) db
    $user = \App\User::on('mysql-custom')->find(14);
    // dd($user) gives the correct user
    $user->notify(new \App\Notifications\PasswordGenerated('secret'));
});

My notification:

<?php

namespace App\Notifications;

use App\Mail\PasswordGenerated as PasswordGeneratedMailable;
use Illuminate\Bus\Queueable;
use Illuminate\Notifications\Notification;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Notifications\Messages\MailMessage;

class PasswordGenerated extends Notification implements ShouldQueue
{
    use Queueable;

    /**
     * The user's password.
     *
     * @var string
     */
    private $password;

    /**
     * Create a new notification instance.
     *
     * @return void
     */
    public function __construct(string $password)
    {
        $this->password = $password;
    }

    /**
     * Get the notification's delivery channels.
     *
     * @param  mixed  $notifiable
     * @return array
     */
    public function via($notifiable)
    {
        return ['mail'];
    }

    /**
     * Get the mail representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return \Illuminate\Notifications\Messages\MailMessage
     */
    public function toMail($notifiable)
    {
        return (new PasswordGeneratedMailable($this->password))->to($notifiable->email);
    }

    /**
     * Get the array representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return array
     */
    public function toArray($notifiable)
    {
        return [
            //
        ];
    }
}

and my mailable:

<?php

namespace App\Mail;

use Illuminate\Bus\Queueable;
use Illuminate\Mail\Mailable;
use Illuminate\Queue\SerializesModels;
use Illuminate\Contracts\Queue\ShouldQueue;

class PasswordGenerated extends Mailable
{
    use Queueable, SerializesModels;

    /**
     * The user's password.
     *
     * @var string
     */
    private $password;

    /**
     * Create a new message instance.
     *
     * @param  User $user
     * @return void
     */
    public function __construct(string $password)
    {
        $this->password = $password;
    }

    /**
     * Build the message.
     *
     * @return $this
     */
    public function build()
    {
        return $this->markdown('emails.accounts.password')
                    ->with(['password' => $this->password]);
    }
}

The email doesn't get send. However, no exceptions are thrown and the notification is processed by the queue (using the database driver) successfully.

Next, changing routes/web.php to the following:

$router->get('/test', function () {
    // A user with id 1 exists in both databases. They have different email addresses
    $user = \App\User::on('mysql-custom')->find(1);
    // dd($user) gives the correct user
    $user->notify(new \App\Notifications\PasswordGenerated('secret'));
});

Now an email does get send. However, it is send to the user with id 1 in the 1st database (default connection) instead of the user I have fetched through \App\User::on('mysql-custom')->find(1).

Once again, when App\Notifications\PasswordGenerated doesn't implement ShouldQueue there is no problem confusing the different db connections and the mail is send to the right user.

[marcvdm]

Having two different users with the same id in different databases is in itself a problem.

Better solution would be to use a different model for the second database where you set the connection manually

class SecondUser extends User {

    /**
     * The connection name for the model.
     *
     * @var string
     */
    protected $connection;

}

[michaeldzjap]

Hmm... I wanted to avoid having to use different user models to be honest. They're essentially identical, just from different db's so seems most logical to have one model instead of maintaining two. But perhaps that is the only option.

Why would users with the same id pose a problem? As long as you use different connections that shouldn't be an issue no? And it still doesn't explain to me why without queue'ing the notification things work perfectly fine.

[marcvdm]

The Notification class uses the SerializesModles trait which when queued wil only send the model class and the id with the queue.

https://laravel.com/docs/5.4/queues#creating-jobs

In this example, note that we were able to pass an Eloquent model directly into the queued job's constructor. Because of the SerializesModels trait that the job is using, Eloquent models will be gracefully serialized and unserialized when the job is processing. If your queued job accepts an Eloquent model in its constructor, only the identifier for the model will be serialized onto the queue. When the job is actually handled, the queue system will automatically re-retrieve the full model instance from the database. It's all totally transparent to your application and prevents issues that can arise from serializing full Eloquent model instances.

[michaeldzjap]

I see. That explains it. I guess I could pass along the db connection and user id to my notification class and then fetch the user from the right db in toMail(). Worth a try. If not, I'll go the two model way. Thanks for the help.

[michaeldzjap]

Hmm... So instead of relying on $notifiable I modified my notification by passing along a user id and then fetching the user myself in tomail() on the custom db connection, but it still doesn't work. That doesn't make sense to me. I guess the only way to find out what really is going on is by tracking each step in the notification process. I will give that a go myself. So for now I re-open this

[michaeldzjap]

Ok, I have traced down where it goes wrong. It has to do with the notifiable, i.e. the entity you call notify() on, in my case this is:

$user = \App\User::on('mysql-custom')->find(14);
$user->notify(new \App\Notifications\PasswordGenerated('secret'));

In NotificationSender's queueNotification() function this is passed as the first argument to SendQueuedNotifications. There it is "gracefully" serialized and so I end up with an empty Collection of $notifiables.

I am not sure if you can classify this as a bug though. Kinda feels it is in between a conscious design decision and a limitation of the current implementation. Whatever the case may be it is rather unfortunate and unexpected behavior. Personally, I think there should at least be a warning about this in the documentation for Notifications (Sending Notifications section).

[themsaid]

Hello, this PR should fix your issue: https://github.com/laravel/framework/pull/19521

But backporting it to 5.4 would be a breaking change, so I'd say that you consider your scenario as a limitation in 5.4

Sorry :)

[michaeldzjap]

Ok, thanks very much.