Add logic to determine which bundle class should take effect when multiple classes extend the same bundle

Created on 6 July 2024, 10 months ago

Problem/Motivation

In a complex site/development scenario, where a site happens to have multiple bundle classes that are vying to be the bundle class for the same entity type/bundle combination, the chosen bundle class is not deterministic.

A scenario I have is a downstream site, that brings in common classes from a codebase that is shared.

Steps to reproduce

Bundle classes: A & B, both trying to implement node:page

Two problematic scenarios

- A extends B extends Node
- A & B extend Node. But they dont extend eachother.

Proposed resolution

- In the case where a clear common hierarchy exists, BCA should determine the class at the top of the heirarchy and choose that.
- In the case where there is no common hierarchy, such that there are multiple branches in a tree, choose the winner based on a priority (weight) system. If the priority is the same for all branches in the tree, then throw an exception.

Remaining tasks

Tests + update for BCA 2 (Im currently using this with stable 1.x)

User interface changes

Nil

API changes

Modify internal IDs of bundle plugin [definitions], such that the ID becomes the class the bundle attribute is attached to, rather than a combined entitytype:bundle string. This will allow for discovery of multiple plugins per bundle.

New optional `priority` property added to the Bundle attribute

Internal only changes to plugin manager and discovery.

Data model changes

None

Feature request
Status

Active

Version

1.1

Component

Code

Created by

🇦🇺Australia dpi Perth, Australia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @dpi
  • Merge request !14Deterministic bundles → (Open) created by dpi
  • Issue was unassigned.
  • Status changed to Needs review 10 months ago
  • 🇦🇺Australia dpi Perth, Australia
  • Pipeline finished with Failed
    10 months ago
    Total: 139s
    #217384
  • Pipeline finished with Canceled
    10 months ago
    #217390
  • 🇦🇺Australia dpi Perth, Australia

    Sample exception:

    BCA found Drupal\Module1\Entity\Document and Drupal\Module2\Entity\Media\Document
    vying to be the bundle class media:document, but one does not extend the other, so a winner
    could not be determined. Use the `priority` property on Drupal\bca\Attribute\Bundle with a
    value larger than 0 to select which bundle should take priority.

  • Pipeline finished with Success
    10 months ago
    Total: 139s
    #217391
  • 🇦🇺Australia dpi Perth, Australia
  • Pipeline finished with Success
    10 months ago
    Total: 139s
    #217392
  • 🇦🇺Australia mstrelan

    I think I'd like to get Make $entityType optional: infer from class heirarchy Needs review in first. That refactors the hook implementation out of bca.module and in to its own class. It also has simple logic for setting the plugin ID to the class name, so perhaps we wouldn't need BcaDiscovery?

    I think the priority stuff makes sense, I guess without BCA you'd have to put the logic in your own hook_entity_bundle_info_alter and probably also hook_module_implements_alter.

  • 🇦🇺Australia mstrelan

    Adding Use class names instead of plugin IDs Active as related, let's shoot for the moon.

  • 🇦🇺Australia dpi Perth, Australia

    No problem about timing, I just need it for selfish reasons right now. Plan to revisit with whatever goodness we get elsewhere + tests.

  • 🇨🇭Switzerland berdir Switzerland

    Drive-by review without actually using this module.

    IMHO, bundle classes should be used by the module that "owns" a certain bundle. E.g. forum could be a use case in contrib, or book. And mostly in custom distributions and custom modules, because it's also a BC problem. And custom module may customize it.

    > - A extends B extends Node
    This is then a valid scenario, for example a custom module that adds another function to a book bundle class. Or a distribution providing a default and a specific project extending it.

    > - A & B extend Node. But they dont extend eachother.
    This however is not. This can't work. Both module a and b are expecting their class and their methods to be there, one will be broken. This is similar to two modules replacing a service. One of them is going to be broken: 🐛 Fatal error after installing the module (incompatibility with Context & Menu position module) Needs work . Either subtly in that features just don't work because an instanceof check is FALSE or with a fatal error.

    A priority system won't solve that. In theory, a decorator pattern could be used, but properly using a decorator pattern is hard and complex and really only works if there is a decorator trait/base class that by default passes all methods through to the inner one, or adding a new class on the parent is going to break.

    That's why this should IMHO not be supported and we should fail early with an exception. You can still use an alter hook, that's on you, can't prevent that except in the specific alter hook.

Production build 0.71.5 2024