rinvex / laravel-repositories

⚠️ [ABANDONED] Rinvex Repository is a simple, intuitive, and smart implementation of Active Repository with extremely flexible & granular caching system for Laravel, used to abstract the data layer, making applications more flexible to maintain.
https://rinvex.com
MIT License
666 stars 115 forks source link

Caching relations?! #77

Closed Omranic closed 8 years ago

Omranic commented 8 years ago

While working on rinvex/fort, which uses rinvex/repository we found ourselves dealing too much with relations, which results in direct model interaction and uncached result sets, which means more queries and database hits.

I'm wondering if we can cache relations somehow on the repository level, and limit direct model interactions .. Is it possible? Is it best practice? Any other suggestions?

Omranic commented 8 years ago

Proposal

Usage:

$repository->findRelation(1, 'roles');

Implementation:

/**
 * Get the given relation of the given ID.
 *
 * @param int    $id
 * @param string $relation
 *
 * @return mixed
 */
public function findRelation($id, $relation)
{
    return $this->executeCallback(get_called_class(), __FUNCTION__, func_get_args(), function () use ($id, $relation) {
        return ($model = $this->find($id)) ? $model->$relation : null;
    });
}
ghost commented 8 years ago

Another use case that I found is when I use league/fractal to add a Transformation layer in order to decouple the entities' properties from the database column names.

Using the ?include= request parameter we can embed other relations into the existing model. I know for sure that it eager loads everything so the queries are optimized, but I'm not sure if the relations are really cached. Or the response body itself gets cached.

So even if we call just the findAll method or just find, without calling the with method, using the include param in the query string, it eager loads and embeds the relation into every first level entity.

Gotta test it more to check if everything is cached.

Anyway nice proposal !

Omranic commented 8 years ago

Hmmm, I still don't feel right about this proposal even it's mine 😸 Tastes wrong somehow. Anyway, the raised issues in the first post was gone after re-written parts of the code and used to eagerly load relations at an early stage, so we kinda solved the need to cache relations separately.. I'll close this issue for now, unless there's a reasonable required re-thinking upon solid opinions.