Doctrine has a separation between the storage handler and the repository, which contains business logic specific to the entity type.
The storage handler is not extended, the repository is.
Reference: http://www.doctrine-project.org/api/orm/2.2/class-Doctrine.ORM.EntityRep...

In Drupal we currently have no such separation. Core and contrib alike extends the storage when it needs to add new business logic.
This makes it hard for storage to be overriden by modules such as Multiversion, and it makes it hard for alternative storage to be used.

So, let's create an EntityRepository and an EntityRepositoryInterface. Each entity type can define its own repository in the annotation (under handlers), otherwise the default one is used.

Comments

bojanz created an issue.

Berdir’s picture

Hm. I have to say I don't know doctrine.

Our separation for business is entity class (preSave() etc.) vs. storage. Storage is only extended when actually adding storage related functionality, like specific queries, mass updates, etc.? that's tied to storage anyway, so I'm not sure what we'd gain by that and how that would work for multiversion exactly?

timmillwood’s picture

This would be cool!

I think it would need properly mapping out, maybe a job for a BoF in New Orleans.

bojanz’s picture

@Berdir
Custom loading methods have nowhere to go but on the storage currently.
loadByX(), loadDefault() and a matching markAsDefault(), other examples.

All of these just use the existing storage methods (and/or EntityQuery), which means that they are not tied to a specific storage.

Crell’s picture

Put me down as another "Yes please!" I've run into the "where do I put this getByX() method or that isY() method" several times now, and a proper repository object is the perfect place for it. Often it's just a wrapper around an entity query or a loadMultiple() call that pulls data off another object; that's a good thing! Wrap that stuff up and give it a nice descriptive name, and the code elsewhere gets cleaner and easier to follow.

(Some of the weirdness of the Entity API makes it more complex than that often, as well, which is all the more reason to put it in a common location with easy access to the Entity API key services.)

Berdir’s picture

Having a standardized place for custom methods like that sounds useful, agreed.

But what would the default one then do exactly? Just wondering if we really need that as a standard generic handler, maybe we just want to introduce it as a common per-entity type pattern/service, maybe with a pattern for the name, like entity.repository.node. The problem with $entity_type_manager->getSomething($entity_type_id) is that you can't get a useful type hint anyway and you can also not directly inject it.

Crell’s picture

If we have to choose, I'd favor a standardized service over a standardized handler. Going through ETM means you have to annotate the type, which is annoying, and it creates a deeper, and therefore harder to unit test, dependency tree. (Demeter and all that.)

So then, the question for me would be how we can make entity.repository.$foo as easy to write as possible, and as common/similar as possible (to make them more straightforward to use).

timmillwood’s picture

We already have EntityRepository https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

So I guess doing this in a non-api breaking way is going to be a little tricky.

Berdir’s picture

Actually no, if we just introduce the concept of having per entity type repository services. EntityRepository has common methods that are the same for all, like loadByUuid().

And then we'd just add UserRepository, that for example has a loadByName()/loadByMail() method that replaces the current functions.

bojanz’s picture

I agree with #9, that would work fine.

slashrsm’s picture

Having per entity type repository services sounds good. It would also define a pattern contrib should follow too. We ran into similar problems when porting Relation and CRM core.

+1 for #9.

bojanz’s picture

As discussed as a part of #2697879: Add function that returns array of entity labels, I'd expect to be able to get a repository for any entity type, regardless if the entity type is using the generic repository, or a custom one. That requires a service builder that depends on entity type information, perhaps non-possible. In which case we're back to handlers.

Andrej Galuf’s picture

As this is something that's of interest to me as well and as this topic has stalled for the last year, let me throw my two cent into the debate:

Would this not be as simple as adding an EntityHandlerInterface to the EntityRepository and setting it as a new handler to all entities? Now load the default EntityRepository unless the handler specifies a custom class for that.

That of course means the code will be a bit harder to test due to dependency chain on Entity Type Manager, but I already have that dependency on storage, so I'm likely loading it from a helper method anyway. As a plus, that's the same pattern that Symfony uses for Doctrine's Repositories, so the developers will probably be accustomed to it.

I tested this by setting a custom "repository" Handler class in hook_entity_type_alter(), then calling the "getHandler" on EntityTypeManager and loading a couple of users. Since we can use "createInstance" due to EntityHandlerInterface, the repository can also get any available service as needed.

I've set up a tiny concept on Github:
https://github.com/tufy/entity_repository

Adapting that for global use in Drupal would be easy and would in my humble opinion be better than a separate service per entity.

Peter Majmesku’s picture

I'm glad that such discussion exists in the Drupal 8 community. I have developed a year with Symfony full-stack and I like the Doctrine repositories. Although they cannot be decorated, like services.

@Andrej Galuf: I have taken a look into "tufy/entity_repository". Thanks for starting effort on this. I was taking a look on the repository retrieval.

$repository = $this->entityTypeManager()->getHandler('user', 'repository');

Honestly I do not like such a way. This lacks IDE integration. You are much slower that way. Because you have to look always manually for the right string in a class and such. That's PHP 4 style and not the shiny object-oriented PHP world. Symfony 3 does it that way:

$product = $this->getDoctrine()
        ->getRepository(Product::class)
        ->findCheaperSold($productId);

See: https://symfony.com/doc/current/doctrine.html. Any IDE will quickly auto-complete for yout the right entity type by class name and provide the namespace (e.g. Product::class).

Another thin example for a repository in Symfony:

<?php
namespace App\MyProjectBundle\Entity\Repository;


use App\MyProjectBundle\Entity\Entity\Store;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\NoResultException;
use App\MyProjectBundle\Entity\StoreProspectLink;

/**
 * Class StoreProspectLinkRepository
 *
 * @package App\MyProjectBundle\Entity\Repository
 */
class StoreProspectLinkRepository extends EntityRepository
{
    /**
     * @param Store $store
     *
     * @return StoreProspectLink
     */
    public function findOrCreateByStore(Store $store): StoreProspectLink
    {
        try {
            $storeProspectLink =  $this->createQueryBuilder('spl')
                ->where('spl.store = :store')
                ->setParameter('store', $store)
                ->getQuery()
                ->getSingleResult();
        } catch (NoResultException $e) {
            $storeProspectLink = new StoreProspectLink();
            $storeProspectLink->setStore($store);
        }

        return $storeProspectLink;
    }
}

The repository in Doctrine under Symfony Full-Stack is nothing else than

  • a "limited" service, which has only resposibility for entity type related queries
  • implements EntityRepository which gives you quick access to the query builder related methods (e.g. $this->createQueryBuilder)

Repositories are an implementation of the separation of concerns principle. The repositories in Symfony Full-Stack are also declared in the services.yml file, like any other services.

So how about having this limited "service type" to provide separation of concerns with coupling to the entity manager like in Symfony? I think that is not a big deal. Since repositories aren't doing much. They just provide separation and let me as a dev know better "where to put things to" (according to #5 from Crell).

Andrej Galuf’s picture

@Peter Majmesku: exactly. The difference with ->getRepository() and ->getHandler() is that ->getRepository() does not exist in current EntityTypeManager (because the concept of a Repository doesn't exist), whereas ->getHandler() is a generic function that already works today (even if it isn't pretty).

Drupal already has the concept of Storage, which can be called using the method ->getStorage(@class) on EntityTypeManager, which in turn calls ->getHandler(@class, 'storage'); The same principle could be introduced for Repositories by adding a single method to EntityTypeManager and by adding a handler to the entity.

In principle, much like you I see a Repository as a layer above the Storage, i.e. a layer where I put whatever logic I need to search for the entity/-ies. The only difference is that I already use the Repositories by using custom handlers, even if Drupal currently does not yet support them from core.

JeroenT’s picture

IMO it makes sense to have repositories per bundle.

An example: When you have a blog article node type, you may write a query getByCategory(). This query is specific to blog articles and may contain joins to fields that don't exist for other node types.

When Repository classes are provided per Entity type, this will probably result in long repository classes with a lot of methods like getBlogArticlesByCategory(), get[BUNDLE]By[FIELD], ...