SortableRepository is final?

[AlexeyKosov]

Extending the SortableRepository class now throws a warning: The "Gedmo\Sortable\Entity\Repository\SortableRepository" class is considered final since gedmo/doctrine-extensions 3.11. It may change without further notice as of its next major version. You should not extend it from "App\Repository\YourOwnSortableRepository"

What would be the proper way of using the sortable repository and extending it?

[mbabker]

What would be the proper way of using the sortable repository and extending it?

Wait for #2574 to be released

[AlexeyKosov]

@mbabker Thanks, that makes sense. Just as an idea of a workaround option - instead of having to extend the SortableRepository, maybe just add a Trait (e.g. SortableRepositoryTrait) with all the needed functionality, and an Interface (SortableRepositoryInterface) which has the same methods as the trait. And use a compiler pass to inject the listener, config, and metadata to the repository. This way, we could extend our own repositories from whatever we want, e.g. ServiceEntityRepository or even combine with other repositories from this package.

[phansys]

v3.11.1 was just released, including #2574.

Closing this issue. Thank you so much!

[phansys]

@AlexeyKosov, @mbabker, @franmomu, @aleho, @CreativeNative, @develth; please, correct me if I'm wrong in some of these premises:

1. SortableRepository (like the others) is set in the user implementations through mapping (attributes, annotations, XML, etc) in their own models;
2. If users need to replace the default implementation provided by this library, they can use the mapping config in their models.

If these assumptions are correct, I think there is no issue with marking the default implementations as final, as the extension points are already available.

[mbabker]

To use SortableRepository today, one can either:

• Set it as the entity repository directly in their mapping (i.e. #[ORM\Entity(repositoryClass: SortableRepository::class)]), no extra code required if they're making an entity repository with zero extra functionality (rare but it happens)
• Use their own repository extending from SortableRepository (if they're adding code to their entity repositories)

The issue for most of my apps isn't in needing to replace the functionality from this class (or the other repository classes this package provides), it's needing the functionality to be available to me somewhere. That means either I'm inlining all of the code from these repository classes to my app or I'm extending from them, as of today there isn't a sane middle ground with a decorator type of approach because the repository classes require a Doctrine\Persistence\Mapping\ClassMetadata object that holds information about a single mapped class.

Doctrine's design pattern with Doctrine\Persistence\ObjectRepository objects is to support inheritance, and it's very clearly documented in the Doctrine\ORM\EntityRepository and Doctrine\ODM\MongoDB\Repository\DocumentRepository base classes that this is the intention.

IMO, as long as these are repository classes (i.e. implementing Doctrine\Persistence\ObjectRepository), it's just better for everyone to leave them open for inheritance. If the intention is for the 4.0 API to not support inheritance anywhere in this package, the best two options are either providing traits (which just masks the inheritance bit anyway) that can be added into downstream classes or introducing new service helper type classes that aren't repository classes.

[aleho]

Just to add my two cents, I'm with @mbabker on this.

Please correct me if I'm wrong, but wouldn't a combination of an interface, a trait and a final repository class work well?

That way users can either reference the final repository directly or use their own repo (e.g. extending from ServiceEntityRepository) and implement an interface via the SortableRepositoryTrait.

[phansys]

First of all, thanks for sharing these points.

Use their own repository extending from SortableRepository (if they're adding code to their entity repositories)

AFAIK, this kind of extension point was never documented, nor covered by our BC promise, nor exposed as part of an API:

https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/sortable.md#sortable-mapping

---

Doctrine's design pattern with Doctrine\Persistence\ObjectRepository objects is to support inheritance, and it's very clearly documented in the Doctrine\ORM\EntityRepository and Doctrine\ODM\MongoDB\Repository\DocumentRepository base classes that this is the intention.

I'm not proposing to forbid the inheritance of Doctrine\Persistence\ObjectRepository nor Doctrine\ODM\MongoDB\Repository\DocumentRepository classes.

Description from ObjectRepository

NOTE: IIUC, "this class" is referencing to direct inheritance, not suggesting nor recommending a long ancestral implementation.

An EntityRepository serves as a repository for entities with generic as well as business specific methods for retrieving entities.

This class is designed for inheritance and users can subclass this class to write their own repositories with business-specific methods to locate entities.

At the end of the story, I'm proposing to avoid the inheritance for the classes provided by this library those are not meant to be used as extension points to keep the API as useful, explicit, documented and closed as possible; in order to ease maintenance and let us provide a better DX.

---

Please correct me if I'm wrong, but wouldn't a combination of an interface, a trait and a final repository class work well?

Regarding decoration, traits, etc; I think we should discuss about additional extension points in a new RFC issue.

[mbabker]

In all honesty, I really think making final repository classes just makes using this package harder and introduces added userland complexity for little gain. With a final repository, I have to go through extra steps in my application to essentially wire two repository classes up for a single entity and expose that second repository's functionality through an extra step. Providing that functionality to userland via traits doesn't really change anything; those traits still rely on my classes inheriting from a base class and my class then has to take extra steps to initialize their functionality.

I'm generally all for closing API footprints and the purity of composition over inheritance. But, I honestly just don't see the benefit for this specific set of classes.

And again, at least for me, it's not about being able to extend or modify the behaviors from the repository classes in this package. I just want to be able to use them with as little friction as possible.