johnnyfreeman / laravel-custom-relation

A custom relation for when stock relations aren't enough.
78 stars 28 forks source link

Added preliminary eager loading support #4

Closed Miguel-Serejo closed 4 years ago

Miguel-Serejo commented 7 years ago

Added support for eager loading relations on the target model

Example working eager/lazy loaded relation:

Models: User Group

There is a many-to-many membership relation between User and Group.

The following is a custom relation that retrieves all the users which share at least one group with a specified user:

class User extends Model{
use HasCustomRelations;

public function colleagues()
{
    return $this->custom(
        User::class,
        function ($relation) {
            $relation->getQuery()
            ->select('users.*', 'gu2.user_id as pivot_users_id', 'gu1.group_id as pivot_group_id')
            ->join('group_user as gu1', 'gu1.user_id', '=', 'users.id')
            ->join('group_user as gu2', 'gu2.group_id', '=', 'gu1.group_id')
            ->whereColumn('gu2.user_id', '!=', 'users.id')
            ->where('gu2.user_id', '=', $this->id)
            ->distinct()
            ;
        },
        function ($relation, $models) {
            $relation->getQuery()
            ->select('users.*', 'gu2.user_id as pivot_users_id', 'gu1.group_id as pivot_group_id')
            ->join('group_user as gu1', 'gu1.user_id', '=', 'users.id')
            ->join('group_user as gu2', 'gu2.group_id', '=', 'gu1.group_id')
            ->whereColumn('gu2.user_id', '!=', 'users.id')
            ->whereIn('gu2.user_id', array_column($models, 'id'))
            ->distinct()
            ;
        },
        null,
        'pivot_users_id'
    );
}
}

Here's an example using a morph relation: A Bonus can be assigned to an individual user or a Group. Group bonuses apply to all users of that group, as well as members of descendant groups (group hierarchy implemented with nested sets) Users can have multiple individual bonuses and multiple group bonuses.

public function activeBonuses()
    {
        return $this->custom(Bonus::class,
        function ($relation) {
            $relation->getQuery()
            ->join('group_user as gu', function($join){
                $join->orOn(function($join){
                    $join->on('bonuses.target_id', '=', 'gu.user_id')
                    ->where('bonuses.target_type', '=', static::class)
                    ;
                })
                ->orOn(function($join){
                    $join->on('bonuses.target_id', '=', 'gu.group_id')
                    ->where('bonuses.target_type', '=', Group::class)
                    ;
                })
                ;
            })
            ->join('groups as g1', 'gu.group_id', '=', 'g1.id')
            ->join('groups as g2',function($join){
                $join->on('g1._lft', '>=', 'g2._lft')
                ->on('g1._lft', '<=', 'g2._rgt')
                ;
            })
            ->where('gu.user_id', $this->id)
            ->distinct()
            ;
        },
        function ($relation, $models) {
            $relation->getQuery()
            ->select('bonuses.*', 'gu.user_id as pivot_users_id')
            ->join('group_user as gu', function($join){
                $join->orOn(function($join){
                    $join->on('bonuses.target_id', '=', 'gu.user_id')
                    ->where('bonuses.target_type', '=', static::class)
                    ;
                })
                ->orOn(function($join){
                    $join->on('bonuses.target_id', '=', 'gu.group_id')
                    ->where('bonuses.target_type', '=', Group::class)
                    ;
                })
                ;
            })
            ->join('groups as g1', 'gu.group_id', '=', 'g1.id')
            ->join('groups as g2',function($join){
                $join->on('g1._lft', '>=', 'g2._lft')
                ->on('g1._lft', '<=', 'g2._rgt')
                ;
            })
            ->whereIn('gu.user_id', array_column($models, 'id'))
            ->distinct()
            ;
        },
        null,
        'pivot_users_id'
    );
}
Miguel-Serejo commented 7 years ago

This should fix #3, but does change the way you're meant to define your eager loading Closure. Effectively all this does is wrap a query builder instance in a relation object, which is about as custom as you can get.

johnnyfreeman commented 7 years ago

Ok thanks! I'll try it out tonight when I get home.

johnnyfreeman commented 7 years ago

Been really busy lately... I'll really try to check this out tomorrow and get back to you. :)

SpinyMan commented 7 years ago

it doesn't work with morph relation because it uses different column names. Example: $child->parent_id = $parent->id // this is normal behaviour $child->tagable_id && $child->tagable_type = $tag->id // morph So his function buildDictionary uses only first example...

protected function buildDictionary(Collection $results, $models)
{
    // First we will build a dictionary of child models keyed by the foreign key
    // of the relation so that we will easily and quickly match them to their
    // parents without having a possibly slow inner loops for every models.
    $dictionary = [];
    foreach($models as $model){
        $dictionary[$model->getKey()] = $results->where($model->getKeyName(), $model->getKey())->toArray();
    }
    return $dictionary;
}

Here $model->getKeyName() returns $parent->id but I need tagable_id... So I think you have to add one more optional param to custom function that means you should use some other key for compare: before $dictionary[$model->getKey()] = $results->where($model->getKeyName(), $model->getKey())->toArray(); after $dictionary[$model->getKey()] = $results->where($this->localKey ?: $model->getKeyName(), $model->getKey())->toArray(); where $this->localKey is the key that have to be compared. It's just an example, so try to find the best way to implement it. Thanx)

Miguel-Serejo commented 7 years ago

I pulled the dictionary function from the BelongsToMany relation source code and then had to change it a bit since we don't have a $pivot property in the custom relation. This was just a first attempt at getting eager loading to do something instead of generating errors, and as such is quite limited in functionality. The example I gave originally doesn't even work, and I've since stopped using custom relations for eager loading in my project as I had to invest my time elsewhere. I'll see if I can fix it up properly this week, maybe I'll start with a quick fix for morphs and work from there.

From a quick look at the built-in morph relations, they use the same dictionary function as BelongsToMany, which works because the pivot table gets set to whatever relation is actually being queried. In our case it might not be so simple, since, in theory, this should allow as many pivot tables as the user wants.

I see two paths to success here: Change the way custom relations are created to include every intermediate model involved, or try to fudge with how the dictionary is created and used to, for example, key by ["$table_name|$table_key|$model_id"] or [$table_key][$id].

Keep you posted when I get some time to actually work on it.

Miguel-Serejo commented 7 years ago

@SpinyMan could you give me an example of how you'd use a custom relation with a morph relation? I've been having a go at it but I think my use cases are either too complex/varied for a single custom relation class to handle all of them or just not practical to solve via sql queries as opposed to pulling all the data and then filtering it with php.

SpinyMan commented 7 years ago

Schema

CREATE TABLE `meta` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `form_field_id` int(10) unsigned NOT NULL,
  `object_id` int(10) unsigned NOT NULL,
  `object_type` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `value` text COLLATE utf8mb4_unicode_ci,
  PRIMARY KEY (`id`),
  KEY `object` (`object_id`,`object_type`),
  KEY `FK_META_FORM_FIELD_ID` (`form_field_id`),
  CONSTRAINT `FK_META_FORM_FIELD_ID` FOREIGN KEY (`form_field_id`) REFERENCES `form_fields` (`id`) ON DELETE CASCADE
)

My model

public function meta()
{
    return $this->custom(
        Meta::class,

        // add constraints
        function($relation){
            /** @var  \Illuminate\Database\Eloquent\Builder  $relation */
            $relation->getQuery()
                ->select(['meta.*', 'ff.name', 'ffi.label'])
                ->join('form_fields AS ff', 'ff.id', '=', 'meta.form_field_id')
                ->leftJoin('form_fields_i18n AS ffi', function($join){
                    $join->on('ffi.form_field_id', '=', 'ff.id')
                        ->where('ffi.locale', app()->getLocale());
                })
                ->where([
                    //'object_id' => $pageId,
                    'object_type' => $this->getTable()
                ])
                ->whereIn('meta.object_id', [$relation->getParent()->id]);
        },

        // add eager constraints
        function($relation, $models){
            /** @var  \Illuminate\Database\Eloquent\Builder  $relation */
            $relation->getQuery()
                ->select(['meta.*', 'ff.name', 'ffi.label'])
                ->join('form_fields AS ff', 'ff.id', '=', 'meta.form_field_id')
                ->leftJoin('form_fields_i18n AS ffi', function($join){
                    $join->on('ffi.form_field_id', '=', 'ff.id')
                        ->where('ffi.locale', app()->getLocale());
                })
                ->where('object_type', $this->getTable())
                ->whereIn('meta.object_id', array_column($models, 'id'));
        },
        null,
        'object_id' //it's just a field name to compare
    );
}

HasCustomRelations.php

public function custom($related, Closure $baseConstraints, Closure $eagerConstraints, array $eagerParentRelations = null, $localKey = null)
{
    $instance = $this->newRelatedInstance($related);
    $query = $instance->newQuery();

    return new Custom($query, $this, $baseConstraints, $eagerConstraints, $eagerParentRelations, $localKey);
}

Custom.php

public function __construct(Builder $query, Model $parent, Closure $baseConstraints, Closure $eagerConstraints, array $eagerParentRelations = null, $localKey = '')
{
    $this->localKey = $localKey;
    $this->baseConstraints = $baseConstraints;
    $this->eagerConstraints = $eagerConstraints;
    if(isset($eagerParentRelations))
        $parent->load($eagerParentRelations);

    parent::__construct($query, $parent);
}
protected function buildDictionary(Collection $results, $models)
{
    // First we will build a dictionary of child models keyed by the foreign key
    // of the relation so that we will easily and quickly match them to their
    // parents without having a possibly slow inner loops for every models.
    $dictionary = [];
    foreach($models as $model){
        $dictionary[$model->getKey()] = $results->where($this->localKey ?: $model->getKeyName(), $model->getKey())->toArray();
    }
    return $dictionary;
}

It's just a primitive example that could be implemented and it works. But I think u will find a better way to do it much better ;)

Miguel-Serejo commented 7 years ago

Trying to figure out the best way to make this work in the general case, right now I'm thinking we need to add a [model => key] array parameter to the constructor, so you'd do something like $this->custom($relatedModel, $closure, $eagerClosure [, $intermediateModelKeys])

I think I've been looking at the dictionary problem backwards (doing things like dictionary[$model][$key] = $results rather than what you did), which basically caused me to get stuck. Gonna see if I can get some progress on this today. Thanks for your help!

Miguel-Serejo commented 7 years ago

I now believe a pivot element would be best. I can't devote more time to this today, so I'm using @SpinyMan's localKey solution for now. I suggest aliasing the localKey to prevent duplicate column conflicts and the likes.

Also, there was a weird issue with relation collections being inside another collection with only one element, like this:

User{
  id: 1,
  colleagues: Illuminate\Database\Eloquent\Collection {
    all: [
       0 => Illuminate\Database\Eloquent\Collection {
          all: [
                 (actual collection)
         ]
        }
     ]
}

So I fixed that.

Future development: Substitute this $localKey parameter with some proper pivot interactions. I was condiering using the InteractsWithPivotTable trait, but considering there may not be an actual table to interact with, the pivot would only exist in the context of the relation, it might be best to develop another solution.

For now, I'm able to use this with morph relations as well as any other type of relation that I've been able to test.

I'm updating the sample usage scenario in my first post to one that actually works and demonstrates how to use the new localKey parameter.

SpinyMan commented 6 years ago

Question: how to save custom relation? For example hasOne relation: $user->meta()->save(...); But when I try to save the same way custom relation it crashes with Call to undefined method Illuminate\Database\Query\Builder::save() Any live hack?

johnnyfreeman commented 6 years ago

Not yet. But if you want to take the charge on that. I'd gladly merge a pull request for it. :)

On Oct 29, 2017 8:36 AM, "Andrew" notifications@github.com wrote:

Question: how to save custom relation? For example hasOne relation: $user->meta()->save(...); But when I try to save the same way custom relation it crashes with Call to undefined method Illuminate\Database\Query\Builder::save() Any live hack?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/johnnyfreeman/laravel-custom-relation/pull/4#issuecomment-340259318, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWrGZJeJ_w1OztlIq1EeCTk3JoiINqCks5sxHFEgaJpZM4NtDIP .

NicksonYap commented 5 years ago

@johnnyfreeman any updates on this?

I think we've all hit Laravel's limitation on relationships. Would be great when we can start using this with eagerLoading

johnnyfreeman commented 4 years ago

I think I like what's proposed in #8. Instead of passing in column names that work with the buildDictionary method, you just pass a closure and write your own.

What do you guys think?

Miguel-Serejo commented 4 years ago

I haven't looked at this code in over two years. I also haven't used this package since, as the project I was using it for fell through.

Comparing my PR with #8, broadly it would seem my PR is simpler to use, but may not be as flexible. I don't really have the time to make a detailed analysis.

johnnyfreeman commented 4 years ago

Closing this in favor of #8. Thank you for the discussion and effort here everyone.