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

Trying to load non-existing relationships on deserialisation of an object with polymorphic children #26126

Closed
jsphpl opened this issue Oct 15, 2018 · 5 comments
Labels

Comments

@jsphpl
Copy link

jsphpl commented Oct 15, 2018

  • Laravel Version: 5.7.7
  • PHP Version: 7.2
  • Database Driver & Version: Postgres

Description:

Relationships that were loaded on a model before serialisation are restored during deserialisation, eg. when using a model in a queued event or job. During deserialisation, an error occurs if a polymorphic relationship was loaded on the model, where one of the polymorphic types has in turn a child relationship loaded that does not exist on one of the other polymorphic types. The issue occurs when the polymorphic child that has the relationship is first in the relationship and the nested relationship is loaded during serialisation.

I assume this method in Eloquent\Collection is part of the issue by picking the the first item in the collection to determine which child relationships exist:

    /**
     * Get the relationships of the entities being queued.
     *
     * @return array
     */
    public function getQueueableRelations()
    {
        return $this->isNotEmpty() ? $this->first()->getQueueableRelations() : [];
    }

Steps To Reproduce:

1. Have the following model tree:

class Parent extends Model
{
    public function children()
    {
        return $this->morphTo();
    }
}

class ChildTypeOne extends Model
{
    public function parent()
    {
        return $this->morphMany(Parent::class, 'parentable');
    }

    public function nestedRelationship()
    {
        // Note: This relationship does not exist on ChildTypeTwo
        $this->hasMany(AnyOtherModel::class);
    }
}

class ChildTypeTwo extends Model
{
    public function parent()
    {
        return $this->morphMany(Parent::class, 'parentable');
    }
}

2. Create some instances

3. ->load('children') on a Parent instance, ensuring that the first loaded item is a ChildTypeOne

4. ->load('nestedRelationship') on the first of the ChildTypeOne instanced loaded on the parent

5. Serialize the Parent instance, eg. by setting it as a property of a queued job or event

6. Deserialize it by handling the job.

@staudenmeir
Copy link
Contributor

I don't see a feasible way to restore the exact eager loading in a case like this.

We can fix it in Collection::getQueueableRelations() by getting the relationships from all models (instead of only the first one) and then calculating the intersection.

@derekmd
Copy link
Contributor

derekmd commented Oct 17, 2018

I had the same polymorphic problem along with infinite recursion when serializing circular relationships.

Since I don't require event-broadcast payloads (that this feature was added for), I've just disabled queued relationship serialization altogether in a base model class:

namespace App;

use Illuminate\Database\Eloquent\Model as BaseModel;

class Model extends BaseModel
{
    /**
     * Disable SerializesAndRestoresModelIdentifiers@getSerializedPropertyValue().
     *
     * @return array
     */
    public function getQueueableRelations()
    {
        return [];
    }
}

Awhile back I had a brief look at actually fixing polymorphic relations but as already mentioned, it's not an easy problem to solve.

Maybe the framework should instead support a way to disable queuing mixed/polymorphic relationship IDs?

@driesvints driesvints added the bug label Nov 9, 2018
@stemis
Copy link
Contributor

stemis commented Nov 28, 2018

This change was introduced from 5.5 to 5.6

I can confirm this is also happening on 5.6 and this is the reason we are still staying on 5.5 for now.

The issue occurs when we have a relation which has many in combination with morph
Ie:
booking.points.pointable.X
Where points is a hasMany relation
Where pointable is a Morph relation
Where X is a hasOne relation

A booking contains multiple points and when we load booking.points[0].pointable.X
Upon deserialization, it also tries to load pointable.X on all the other points.pointable's, which may not always be there.

Secondly, I don't think it is clean to pass a different serialization object depending on what happened to the object before it was passed on to the job. IMO it should always be the same as a job should be a stand-alone isolated runnable program. I am very much interested in the reason to include relations in the serialization of job models as it can create different situations depending on what is loaded and what is not

The related commit is the following:
230eeef#diff-955cb662f7bf33d1dec2eee598ce800d

@michael-ow
Copy link

Currently having this problem on a 5.7 project. Does anyone know if it's been resolved since then?

@taylorotwell
Copy link
Member

Fixed by #32613

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

No branches or pull requests

7 participants