Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] [PHP 7.4] Using Typed properties classes on SerializesModels causes error due Illuminate\Contracts\Database\ModelIdentifier #30494

Closed
eusonlito opened this issue Nov 1, 2019 · 7 comments · Fixed by #30604

Comments

@eusonlito
Copy link
Contributor

eusonlito commented Nov 1, 2019

Using this class definition:

<?php declare(strict_types=1);

namespace App\Mails\User;

use Illuminate\Bus\Queueable;
use Illuminate\Mail\Mailable;
use Illuminate\Queue\SerializesModels;
use App\Models;

class Confirm extends Mailable
{
    use Queueable, SerializesModels;

    /**
     * @var \App\Models\User
     */
    public Models\User $user;

    /**
     * @param \App\Models\User $user
     *
     * @return self
     */
    public function __construct(Models\User $user)
    {
        $this->user = $user;
    }
}

Queue can not serialize properties due Typed property App\Mails\User\Confirm::$user.

Error:

[2019-11-01 12:10:31] local.ERROR: Typed property App\Mails\User\Confirm::$user must be an instance of App\Models\User, Illuminate\Contracts\Database\ModelIdentifier used {"userId":1,"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Typed property App\\Mails\\User\\Confirm::$user must be an instance of App\\Models\\User, Illuminate\\Contracts\\Database\\ModelIdentifier used at /vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php:23)
[stacktrace]
#0 /vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php(23): ReflectionProperty->setValue()
#1 [internal function]: App\\Mails\\MailAbstract->__sleep()
#2 /vendor/laravel/framework/src/Illuminate/Queue/Queue.php(139): serialize()
#3 /vendor/laravel/framework/src/Illuminate/Queue/Queue.php(110): Illuminate\\Queue\\Queue->createObjectPayload()
#4 /vendor/laravel/framework/src/Illuminate/Queue/Queue.php(88): Illuminate\\Queue\\Queue->createPayloadArray()
#5 /vendor/laravel/framework/src/Illuminate/Queue/SyncQueue.php(37): Illuminate\\Queue\\Queue->createPayload()
#6 /vendor/laravel/framework/src/Illuminate/Queue/Queue.php(44): Illuminate\\Queue\\SyncQueue->push()
#7 /vendor/laravel/framework/src/Illuminate/Mail/Mailable.php(180): Illuminate\\Queue\\Queue->pushOn()
#8 /vendor/laravel/framework/src/Illuminate/Mail/Mailer.php(388): Illuminate\\Mail\\Mailable->queue()
#9 /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(239): Illuminate\\Mail\\Mailer->queue()
#10 /app/Services/Mail/Mailer.php(53): Illuminate\\Support\\Facades\\Facade::__callStatic()
#11 /app/Services/Mail/Mailer.php(19): App\\Services\\Mail\\Mailer::queue()
#12 /app/Services/User/Confirm.php(53): App\\Services\\Mail\\Mailer::userConfirm()
#13 /app/Http/Controllers/Api/UserSignup.php(39): App\\Services\\User\\Confirm::start()
#14 [internal function]: App\\Http\\Controllers\\Api\\UserSignup->confirmStart()
#15 /vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): call_user_func_array()
#16 /vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(45): Illuminate\\Routing\\Controller->callAction()
#17 /vendor/laravel/framework/src/Illuminate/Routing/Route.php(219): Illuminate\\Routing\\ControllerDispatcher->dispatch()
#18 /vendor/laravel/framework/src/Illuminate/Routing/Route.php(176): Illuminate\\Routing\\Route->runController()
#19 /vendor/laravel/framework/src/Illuminate/Routing/Router.php(680): Illuminate\\Routing\\Route->run()
#20 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(130): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}()
#21 /app/Http/Middlewares/LoggerRequest.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#22 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): App\\Http\\Middlewares\\LoggerRequest->handle()
#23 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(105): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#24 /vendor/laravel/framework/src/Illuminate/Routing/Router.php(682): Illuminate\\Pipeline\\Pipeline->then()
#25 /vendor/laravel/framework/src/Illuminate/Routing/Router.php(657): Illuminate\\Routing\\Router->runRouteWithinStack()
#26 /vendor/laravel/framework/src/Illuminate/Routing/Router.php(623): Illuminate\\Routing\\Router->runRoute()
#27 /vendor/laravel/framework/src/Illuminate/Routing/Router.php(612): Illuminate\\Routing\\Router->dispatchToRoute()
#28 /vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(176): Illuminate\\Routing\\Router->dispatch()
#29 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(130): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}()
#30 /app/Http/Middlewares/Language.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#31 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): App\\Http\\Middlewares\\Language->handle()
#32 /vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/CheckForMaintenanceMode.php(62): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#33 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode->handle()
#34 /vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(105): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#35 /vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(151): Illuminate\\Pipeline\\Pipeline->then()
#36 /vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(116): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter()
#37 /public/index.php(55): Illuminate\\Foundation\\Http\\Kernel->handle()
#38 {main}
@driesvints
Copy link
Member

Will need a little bit more code to reproduce the problem. How are you instantiating the mailable? Can you show how you're sending the mailable?

@eusonlito
Copy link
Contributor Author

eusonlito commented Nov 1, 2019

Ok @driesvints.

This code is currenty working under PHP 7.3.11-1+ubuntu18.04.1+deb.sury.org+1 and I'm working to migrate to PHP 7.4 adding new features as Typed Properties, Arrow Functions, etc...

Here the trace (with some non related code removed):

# /app/Models/User.php

<?php declare(strict_types=1);

namespace App\Models;

use Illuminate\Auth\Authenticatable;
use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
use Illuminate\Database\Eloquent\Model;
use Tymon\JWTAuth\Contracts\JWTSubject;

class User extends Model implements AuthenticatableContract, JWTSubject
{
    use Authenticatable;

    /**
     * @var string
     */
    protected $table = 'user';

    /**
     * @var string
     */
    public static string $foreign = 'user_id';

    /**
     * @var array
     */
    protected $hidden = ['password'];
}
# /app/Services/User/Confirm.php

<?php declare(strict_types=1);

namespace App\Services\User;

use App\Models;
use App\Services;

class Confirm
{
    /**
     * @param string $user
     *
     * @return \App\Models\User
     */
    public static function start(string $user): Models\User
    {
        $user = Models\User::where('user', $user)->firstOrFail();

        Services\Mail\Mailer::userConfirm($user);

        return $user;
    }
}
# /app/Services/Mail/Mailer.php

<?php declare(strict_types=1);

namespace App\Services\Mail;

use Illuminate\Support\Facades\Mail;
use App\Mails;
use App\Models;

class Mailer
{
    /**
     * @param \App\Models\User $user
     *
     * @return void
     */
    public static function userConfirm(Models\User $user)
    {
        static::queue(new Mails\User\Confirm($user), [$user->user]);
    }

    /**
     * @param \App\Mails\MailAbstract $mail
     * @param array $emails
     *
     * @return void
     */
    protected static function queue(Mails\MailAbstract $mail, array $emails)
    {
        Mail::queue(static::options($mail, $emails));
    }

    /**
     * @param \App\Mails\MailAbstract $mail
     * @param array $emails
     *
     * @return \App\Mails\MailAbstract
     */
    protected static function options(Mails\MailAbstract $mail, array $emails): Mails\MailAbstract
    {
        $mail->to(static::filter($emails));

        return $mail;
    }

    /**
     * @param array $emails
     *
     * @return array
     */
    protected static function filter(array $emails): array
    {
        return array_values(array_unique(array_filter($emails, static function (string $value): bool {
            return $value && filter_var($value, FILTER_VALIDATE_EMAIL);
        })));
    }
}
# /app/Mails/User/Confirm.php

<?php declare(strict_types=1);

namespace App\Mails\User;

use App\Mails\MailAbstract;
use App\Models;

class Confirm extends MailAbstract
{
    /**
     * @var \App\Models\User
     */
    public Models\User $user;

    /**
     * @var string
     */
    public $subject = '';

    /**
     * @var string
     */
    public $view = 'user.confirm';

    /**
     * @param \App\Models\User $user
     *
     * @return self
     */
    public function __construct(Models\User $user)
    {
        $this->user = $user;
    }
}
# /app/Mails/MailAbstract.php

<?php declare(strict_types=1);

namespace App\Mails;

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

abstract class MailAbstract extends Mailable
{
    use Queueable, SerializesModels;

    /**
     * Build the message.
     *
     * @return self
     */
    public function build(): self
    {
        return $this->view('emails.pages.'.$this->view);
    }
}

Thanks!

@mfn
Copy link
Contributor

mfn commented Nov 1, 2019

Will need a little bit more code to reproduce the problem.

No, everything is here.

But it's hard to say if it's a "bug", it's how the current implementation works.

Serializable classes us a proxy object \Illuminate\Contracts\Database\ModelIdentifier to handle models and model collections, see \Illuminate\Queue\SerializesAndRestoresModelIdentifiers::getSerializedPropertyValue.

The reason is simple: models are complex and have database connection stuff (Eloquent is ActiveRecord, after all) and can't be easily serialized => so a proxy object is used instead.

Which means temporarily this property will receive a different value and this, as it stands right now, property type declarations can't be used for serializable objects in Laravel.

I'm curious if this is even fixable without change of paradigm; but I don't know the code that well.

As PHP 7.4 starts to get released and more people start using this new features, more stuff like this will pop up.

@eusonlito
Copy link
Contributor Author

Is easy to reproduce in any PHP 7.4 environment. You only need to set a typed property to a model on a Queueable and SerializesModels event.

It will be a bug when Laravel officially supports PHP 7.4. But, can be fixed before that moment?

@driesvints
Copy link
Member

I've attempted to fix this here: #30604

Any feedback is greatly appreciated.

@mohabdelaziz95
Copy link

@driesvints also got this error [laravel 6.16], [PHP 7.4.3]

"message": "Type of Illuminate\\Database\\Eloquent\\Relations\\Pivot::$incrementing must be bool (as in class Illuminate\\Database\\Eloquent\\Model)",
    "exception": "Symfony\\Component\\Debug\\Exception\\FatalErrorException",
    "file": "/home/web/www/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Relations/Pivot.php",
    "line": 8

@cdjenkins
Copy link

I ran into this issue today migrating to PHP 7.4. Thanks for posting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants