akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

ManyToMany (BelongsToMany) Relationships issues #644

Closed Skullbock closed 7 years ago

Skullbock commented 7 years ago

Hi @nikosdion as per our twitter messages, i post here my issue regarding the ManyToMany (BelongsToMany) Relationships in FOF3, alongside my findings.

Issue

In nearly every custom component i write, i have several Many 2 Many relationships. As per the docs, i usually deal with them in this way:


class  Roles extends DataModel {
    public function __construct(Container $container, array $config = array())  {
        parent::__construct($container, $config);
        $this->belongsToMany('users', 'Users');
    }
}

class  Users extends DataModel {
    public function __construct(Container $container, array $config = array())  {
        parent::__construct($container, $config);
        $this->belongsToMany('roles', 'Roles');
    }
}

Of course, with a many to many relationship, i expect to be able to "link" the two models with each other and access the data from both sides.

If i do this, though, FOF3 fails with an infinite loop issue. After a bit of xdebugging, i found out that, if i call

   $this->belongsToMany('users', 'Users');

without the extra configuration parameters, FOF30 tries to guess the primary keys, the glue table, etc by itself. This data comes from the foreign model, which is built using tmpInstance().

Basically what happens is that it tries to create the foreign model, which has a relationship with the current model, so it tries to get the foreign model (the current model), therefore it will try to contruct it, and finding the relationship in the contructor, will go into the infamous infinite loop.

Possible Solution

I looked into eloquent (Laravel orm) since i know you were inspired by it for fof3. THis doesn't happen there because the call is not in the constructor, but is exposed as a method

public function roles()
{
    $this->belongsToMany('roles');
}

Is therefore a way in fof to add a relation not in the constructor? Would it make sense to support this the way eloquent does?

The current ugly fix

Currently i'm fixing the issue by being completely explicity in the belongsToMany call. If i give every config parameter by hand, the foreign model is never constructed and therefore no infinite loop is created.

Bad, but works

nikosdion commented 7 years ago

I can't do what Eloquent does. Allowing you to set up relations in the fof.xml file means that I have to set them up in the constructor. If you dive deeper you'll see that this also lends to a much more simplified implementation of eager loading. The downside is that many-to-many relations can't be set up implicitly in both models.

The solution you have found is actually the intended way to use this type of relationship. I haven't stated it in the documentation but all of the examples include all the parameters. I think all we can do is change the documentation to NOT implicitly describe the localKey and foreignKey parameters as optional. The latter three parameters should work implicitly.

What do you think?

Skullbock commented 7 years ago

Ok I see. I'll try and check if I can change the way the model is constructed in that particular relation (maybe passing the current model through?) and have both ways Il giorno gio 19 gen 2017 alle 19:01 Nicholas K. Dionysopoulos < notifications@github.com> ha scritto:

I can't do what Eloquent does. Allowing you to set up relations in the fof.xml file means that I have to set them up in the constructor. If you dive deeper you'll see that this also lends to a much more simplified implementation of eager loading. The downside is that many-to-many relations can't be set up implicitly in both models.

The solution you have found is actually the intended way to use this type of relationship. I haven't stated it in the documentation but all of the examples include all the parameters. I think all we can do is change the documentation https://github.com/akeeba/fof/wiki/Model-relations#many-to-many-belongstomany-relation to NOT implicitly describe the localKey and foreignKey parameters as optional. The latter three parameters should work implicitly.

What do you think?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/akeeba/fof/issues/644#issuecomment-273850423, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDY0yZaQBXRX9a-rMsNugau0-65vKG5ks5rT6TxgaJpZM4LoZ1d .

-- Daniele Rosario CTO Via F.Filzi, 56/A 36051 Creazzo (Vicenza) Tel.: +39 0444 1454934 Mob: +39 328 3017134 Immagine in linea 1

nikosdion commented 7 years ago

It's a chicken and egg problem. You'd need to know which table is used by the foreign model and which field is the primary key without instantiating the foreign model. However, this information is set up in the constructor. The relations are also set up in the constructor. So all you can do is pass the information that would otherwise require a model instance (table name and primary key field) in the relation setup in the constructor. I don't see any way around it which would work universally (both fof.xml model setup and custom code in the constructor).

Skullbock commented 7 years ago

Yeah, i tried but feels even more awkward. Ok, thanks! I'll stick to the old way and update the docs

Skullbock commented 7 years ago

https://github.com/akeeba/fof/wiki/Model-relations