Problem/Motivation

Declaring the wrapped entities and the typed entity repositories inside of the service container separate from the actual classes makes for a poor DX. This leads to confusion and a sense of magic is happening

Proposed resolution

We could move the metadata from the service container to the actual class using annotations.

A custom plugin type for typed entity repositories may be a good approach for this. This would take care of the typed repository's discovery.

Ideas:

/**
 * Typed Entity Wrapper for a Product Page node.
 *
 * @TypedEntityRepository(
 *   entityTypeId = "node",
 *   bundle = "product_page",
 *   wrapper = "Drupal\custom_module\WrappedEntities\ProductPage"
 * )
 */
final class ProductPageRepository extends TypedEntityRepositoryBase {
  // Leave empty or implement additional logic depending of your needs.
}

A more elaborate case:

/**
 * Typed Entity Wrapper for a Product Page node.
 *
 * @TypedEntityRepository(
 *   entityTypeId = "node",
 *   bundle = "product_page",
 *   wrappers = [
 *     @WrappedEntityVariant(
 *       "wrapper_class" = "Drupal\custom_module\WrappedEntities\ProductPageFoo",
 *       "condition" = @VariantCondition(
 *         "Drupal\typed_entity\WrappedEntityVariants\FieldValueVariantCondition",
 *         "field_foo",
 *         "expected value",
 *         "FALSE"
 *       )
 *     ),
 *     "Drupal\custom_module\WrappedEntities\ProductPage",
 *   ],
 *   renderers: [
 *     "Drupal\custom_module\WrappedEntities\Render\ProductPageTeaser",
 *     "Drupal\custom_module\WrappedEntities\Render\ProductPageFull",
 *   ]
 * )
 */
final class ProductPageRepository extends TypedEntityRepositoryBase {
  // Leave empty or implement additional logic depending of your needs.
}

Challenges

1. How do we declare wrapped entity classes without a custom typed entity repository? How would the wrapped entities be discovered?
2. The service container allows us to execute arbitrary methods in the typed entity repository (via calls:). We rely on this to register renderers for the wrapped entity.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

Issue summary: View changes
e0ipso’s picture

Issue summary: View changes
hawkeye.twolf’s picture

Awesome. Thank you for making an issue for this.

To keep the annotations from getting too unwieldy, could we remove the conditions/variants list and just leverage an applies() method like we did for the renderers? I think I can see a way to obviate the need for Typed Entity Repositories... If each wrapper annotation has entity type ID and bundle keys, the "Typed Entity Manager" service can load all the plugins/annotations that match given keys, and the loop over them until we find one that applies(). Would also need a "weight" key to optionally allow setting precedence.

The downside to this approach compared to the service definition is that the repository has to load _all_ plugin/annotation definitions to find out which ones apply ('cause we don't have the nice calls: compiler pass functionality you mentioned). However, it doesn't have to instantiate the wrapper classes, just load the definition array, so I don't think the performance will be much hindered.

e0ipso’s picture

I think I can see a way to obviate the need for Typed Entity Repositories... If each wrapper annotation has entity type ID and bundle keys, the "Typed Entity Manager" service can load all the plugins/annotations that match given keys

Yeah, there is room for improvement in the DX. On one hand I like the idea of simplification. On the other hand I think the repositories should not be sidestepped because they play a crucial role. Whenever you have the temptation to have a static method in the wrapped entity, that means that you need a method in your repository.

To keep the annotations from getting too unwieldy, could we remove the conditions/variants list and just leverage an applies() method like we did for the renderers?

That is also interesting, and it definitely helps with consistency. Just to say it out loud these can be combined, see the example below:

class ThemeCloud extends Base {

  /**
   * {@inheritdoc}
   *
   * TRUE when the node should be rendered with the Acme Cloud subtheme.
   */
  public static function applies(TypedEntityRenderContext $context): bool {
    $condition = new WebSegmentThemeCondition('acme_cloud', '');
    // Add the custom requirements to evaluate the conditions.
    $condition->setContext('entity', $context->offsetGet('entity'));
    $condition->setContext('theme_manager', \Drupal::service('web_segment_companion.theme_manager'));
    return $condition->evaluate();
  }

}

Drupal core has examples of both patterns: condition objects for block visibility, and applies method in the serialization selection. The problem with any of the approaches is that today we cannot easily cache the variant selection.

----

Let's say we go with the applies() approach. We would end up with something like:

/**
 * Typed Entity Repository for a Product Page node.
 *
 * @TypedEntityRepository(
 *   entityTypeId = "node",
 *   bundle = "product_page",
 *   wrappers = @ClassWithVariants(
 *     fallback = "Drupal\custom_module\WrappedEntities\ProductPage",
 *     variants = [
 *       "Drupal\custom_module\WrappedEntities\ProductPageFoo"
 *     ]
 *   )
 *   renderers: @ClassWithVariants(
 *     fallback = "Drupal\custom_module\WrappedEntities\Render\Base",
 *     variants = [
 *       "Drupal\custom_module\WrappedEntities\Render\Teaser",
 *       "Drupal\custom_module\WrappedEntities\Render\Full",
 *     ]
 *   )
 * )
 */
final class ProductPageRepository extends TypedEntityRepositoryBase {
  // Leave empty or implement additional logic depending of your needs.
}

I think I like this one better! ☝

e0ipso’s picture

Version: 3.x-dev » 4.x-dev
e0ipso’s picture

e0ipso’s picture

Status: Active » Needs review

Go testbot!

  • e0ipso committed 300cf6c on 4.x
    Issue #3203581: Use comment annotations to declare the typed entities
    
e0ipso’s picture

Status: Needs review » Fixed
hawkeye.twolf’s picture

👏 yay!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.