Create a generic way to decorate Entity classes

We probably want to run this by the entity subsystem maintainers.

I was at first taken aback by the sheer number of methods, but the reason for that is (of course) EntityInterface itself.

+++ b/core/lib/Drupal/Core/Entity/EntityDecorator.php
@@ -0,0 +1,403 @@
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(array $values = []) {
+    throw new \LogicException('Static methods cannot be called on the decorated entity');
+  }

I guess this is because the static methods are conceptually incompatible with decorating something?

I wonder, would it make sense to document that more clearly about the static methods? Not sure how to without a ton of copypaste -- maybe a paragraph on the class docblock? (Which could also use more detail overall.)

I wonder also if we should have a followup to move the static methods to their own interface that EntityInterface can extend (for now), but that we'd decouple in the future (or deprecate entirely when a better solution for their usecase of "don't make it so hard to load an entity" comes around). Not in scope here of course.

Yeah I think the class docblock needs to have some more extended information, including:

• The purpose/usecase of an abstracted entity proxy.
• The bit about why the static methods throw exceptions.
• An @ingroup for the entity API group.

I always thought the Views UI thing was kind of a workaround we had for historical reasons, in order to not rewrite/refactor it a fifth time. (Or maybe only a fourth time for the UI.) So it'd be good to know why this pattern of wrapping all the things is useful and desirable. I can see how getting rid of the boilerplate is good, but I've not been clear on why we're doing it this way in the first place.

Thanks!

+++ b/core/lib/Drupal/Core/Entity/EntityDecorator.php
@@ -0,0 +1,358 @@
+  public function __construct(EntityInterface $subject) {
+    $this->subject = $subject instanceof static ? $subject->getEntity() : $subject;

Is there a specific reason why we unwrap in the constructor?

Decorators are by design supposed to be added in multiple layers, this line means you can't re-decorate with the same class, but you can decorate a different class even if that class then again decorates your class.

https://3v4l.org/tNU9S

Just wondering if that's "just" an optimization or if it has a specific reason and if the above is really what you'd expect to happen.

And yes, there are a lot of methods on EntityInterface, but apart from the deprecated methods and the magic stuff, I'm not sure what we could do differently really? Caching adds a lot of methods, but that's a critical part of entities at this point, The config system also adds a few and so on. And having more of those on separate interfaces would in reality just mean that there are even more methods that entity-type-specific decorators like ViewsUI would still need to implement :)

+++ b/core/lib/Drupal/Core/Entity/EntityDecorator.php
@@ -6,6 +6,10 @@
+ * @see https://en.wikipedia.org/wiki/Decorator_pattern

This is not documentation. ;) I didn't mean "explain to me what a decorator is in the class docblock"; I meant "document why this class exists and what it should be used for, possibly with an example".

Like, I know what a decorator is. (I learned that from @tim.plunkett I think like five years ago when we were working on the plugin system or something.) :) However, simply naming it that does not explain to the developer why it's useful or where it fits in the massive public API the entity system provides.

I think what it's being used for in both layouts and Views UI is to provide user interface representations of a thing that are decoupled from the thing, or etc. There could be other usecases.

Basically, a docblock for FooBar that simply contains Provides a Bar for Foo isn't usually sufficient.

Okay, @tim.plunkett and I had a vigorous discussion yesterday about what the role of this is in the layout initiative and why we should provide a reusable API for it.

For now, for that initiative it's an internal implementation of layout section storage (that I am not on board with, but out of scope here). I think we should not add this as a generic API until we have a third usecase for it beyond the Views UI (which I'd always assumed we'd eventually change to remove the decoration, but also out of scope here) and the proposed layout entity section stuff.

So postponing on such a third usecase, because as it is I'm not quite convinced yet that decorating entities is a good pattern for core.

If you're also taking contrib use cases into account: In the Search API we have this pattern, too. Basically, we allow editing search indexes the same way as views, by making temporary changes and then letting the admin save (or discard) all of them at once.
So, this class would be quite useful to us.

Whilst looking for a decorator solution for our project, I saw several other people are actually trying to do this kind of stuff, maybe a combined solution can be forged and put into core.

solomonrothman's https://www.drupal.org/sandbox/solomonrothman/2977603 (based on this thread)
Steve Jones' https://www.drupal.org/project/2992933/issues/2994042 (automatic solution)

Before I saw Steve's solution, I was also thinking into implementing something amongst those lines.

As for the comment by @Berdir in #12
"Decorators are by design supposed to be added in multiple layers, this line means you can't re-decorate with the same class, but you can decorate a different class even if that class then again decorates your class."

A nicer implementation to allow re-decoration can be found here:
http://jrgns.net/decorator-pattern-implemented-properly-in-php/index.html

Came across this issue as I was looking to decorate an entity from a different module.

The patch in #14 needs a reroll for 8.7.x I believe which if I get time I will do.

Other than that, the constructor is incorrect as it just contains $this->subject; which does nothing, needs to be $this->subject = $subject; - again shall fix if I get time to do a reroll as there's no point submitting a patch that won't apply against the current version.

To use the entity decorator, you can either implement the static methods load/loadMultiple/etc in your decorator class, or extends the EntityDecorator class to add another abstract method to return the entity type class to proxy those methods to and implement then in a generic way in the EntityDecorator class.

Seems like a few more use case/examples have surfaced. Taking this out of postponed for now.

Here's how to decorate entities.

core has a generate-proxy-class script which gets very close to writing the decorator for us. Example:

php ./core/scripts/generate-proxy-class.php 'Drupal\somemodule\Entity\SomeEntityClass' ./
modules/contrib/mymodule/src

. After generating do the following:

1. Move the file to ./modules/contrib/mymodule/src -- it lands in ./modules/contrib/mymodule/src/ProxyClass/Entity which we hardly need.
2. Fix namespace to be just Drupal\mymodule.
3. change namespace {} notation to the usual namespace ;
4. Reformat the file to adhere to Drupal code style.
5. Delete everything in the class until and including the lazyLoadItself() method.
6. search-replace lazyLoadItself() with subject.
7. Add a ENTITY_TYPE constant
8. Add the code from this gist.
9. Finally, replace the static methods calling OriginalEntityClass:: and ancestors up to and including EntityBase:: using the load method in the gist as a pattern. It's really simple.

To hook it in, one can implement hook_entity_type_alter:

function mymodule_entity_type_alter(array &$entity_types) {
  $entity_type = $entity_types['my_entity_type'];
  $entity_type->set(MyEntityDecorator::class, $entity_type->getClass());
  $entity_type->setClass(MyEntityDecorator::class);
}

While $property for EntityType::set() is a completely arbitrary string, a namespaced class name is pretty much guaranteed not to be used by anything else. The getDecoratedClass method uses the class saved here.

Nice one, I think this is still relevant. To add a use-case:

In a contrib module I'd like to extend entities with further API methods, for example for a user hasSubscription(): bool or getSubscriptions(): array for developer convenience.

@Anybody: No, that's a different party. In simple words: If you add methods to a decorating class, the next decorator can wrap and hide your methods.

I guess you want BundleClasses: https://www.drupal.org/node/3191609

;-)

Thanks @geek-merlin! Great reminder, I had already forgotten that I read about it once!
[Oh man, the Drupal('s great features) documentation. Probably the most important area left behind (by us all as community).] BTT!

> In a contrib module

No, in a *contrib* module you don't want to use bundle classes for user or any other entity type that's not your own IMHO, exactly because these are not decorators. You can only have exactly one bundle class active for a given bundle. So if you have two contrib modules doing that, one of them would win and the other would not be used, breaking code that depends on it.

Bundle classes are for custom modules and I guess possibly your own entity types/bundles in contrib modules if you have predefined, specific bundles.

If we want to allow contrib modules to add some methods to other entity type classes then we would indeed need such a decorator system, but that IMHO comes at a price of complexity, decorators need to be correctly implemented and add extra method calls for every single decorator that is there, as each method call is passed through all decorated classes, which also makes things even harder to debug around entities.

A better approach for that might be an Adapter pattern IMHO, so if you have subscriber related logic for a user, you can do new SubscriberUserAdapter($user), and then use your methods and just those on a user when you need them. It's an extra line in your code and then only exposes the specific methods you defined, but it's also not overhead to pass through every single time anyone does something with a user.

@Berdir: Thanks for the additional explanation! Like you wrote, there's no silver bullet for such cases and all solutions come with pro's and con's.

What I currently use is kind of Proxy pattern. And I think my case isn't that far away from what the issue summary described. But the DX downside is that you have to implement all methods and delegate them to the proxied object, which causes lots of boilerplate and is problematic in evolving systems like Drupal, where the Interface may be subject to change and requires to proxy to adapt these changes.

I was also thinking about, if Traits might help, but it then seems to end up similar to extending the base class... so I think let's come back to the original issue here and keep the cases like described in #34 in mind. Being able to "extend" entity classes is surely helpful, and already possible in some ways. Perhaps we'll come to better out of the box solutions and patterns for Drupal here in the future.
Thanks a lot for your feedback and support, much appreciated!

@Anybody: There is a package for that too: https://packagist.org/packages/geeks4change/composable-inheritance
(Feel free to contact me on another channel if it's useful for you.)

Thank you very much @geek-merlin that looks super interesting! :)
I'm done with my project where I ran into this, but will definitely consider it in the future!