Problem/Motivation

  • ContentEntityInterface::bundleFieldDefinitions() and hook_entity_bundle_field_info() can return either bundle-specific overrides of base field definitions, or definitions of fields that are not base fields. In this issue, let's call the latter case "bundle fields".
  • HEAD's current example of hook_entity_bundle_field_info() creates a bundle field via BaseFieldDefinition::create(), but that is incorrect, because BaseFieldDefinition::isBaseField() returns TRUE, which is incorrect for a bundle field.
  • Core's only real implementation of bundle fields is via field_entity_bundle_field_info(), which implements them via config entities.
  • So, in theory, contrib modules can implement non-config-based bundle fields, but we don't provide an implementation class with which to do so.
  • Additionally, we have 'base_field_override' config entities for using config to override a code-defined base field, but do not have 'bundle_field_override' config entities for overriding code-defined bundle fields. So a contrib module attempting to implement a bundle field implementation would also need to implement a getConfig() method so that content_translation and contrib extensions like field_permissions could work on them.
  • And hook_entity_bundle_field_info_alter() has the same problem as #2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods.

Proposed resolution

Remaining tasks

Figure out how to fix all of the above.

User interface changes

API changes

Issue fork drupal-2346347

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

effulgentsia’s picture

Issue tags: +API change
xjm’s picture

Priority: Critical » Major
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Contributed project soft blocker

Discussed with @alexpott and @effulgentsia. This would be critical if it were considered a hard blocker for a contributed project that required code-defined bundle fields. The workaround at present would be for such a project to use a configurable field instead, which is not necessarily ideal but would still allow the contributed module to move forward. Based on this, the issue should be major; see #2350615-4: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?.

If someone has information about a specific contributed module or usecase, we can then assess its impact, disruption, etc. to see if this change can still be made during the beta phase. If it is actually a hard blocker for a particular contributed module, then we can reassess the priority.

bojanz’s picture

One problematic use case is the one used by Comment:

  /**
   * {@inheritdoc}
   */
  public static function bundleFieldDefinitions(EntityTypeInterface $entity_type, $bundle, array $base_field_definitions) {
    if ($comment_type = CommentType::load($bundle)) {
      $fields['entity_id'] = clone $base_field_definitions['entity_id'];
      $fields['entity_id']->setSetting('target_type', $comment_type->getTargetEntityTypeId());
      return $fields;
    }
    return array();
  }

It allows Comment to define an entity_id reference field, but set the target_type based on the comment type setting.
This means that an entity can reference any entity type that's suitable (implements an interface, etc), and is a workaround for the fact that target_type is a storage level setting and not a field level one. Commerce uses the same trick (line items referencing purchasable entities such as product variations or product bundles).

Now, what happens if we want to have the reference field only for specific bundles (based on a setting again)? It makes sense to move the entityreference to a configurable field, but that can't work because target_type is no longer malleable. Sacrifices have to be made (keep the field, but hide it for those bundles).

This isn't an argument for changing the issue priority, just wanted to document my thoughts.

kristiaanvandeneynde’s picture

Status: Postponed (maintainer needs more info) » Active

Just like the Comment module, I've run into this issue with the Group module. The use case I have is that a GroupContent bundle is created for every entity that could possibly be added to a Group.

This allows users to provide metadata (through configurable fields) about why they added a Node or any other entity to a Group. The catch is that they can add different fields to each relationship. So for Article nodes they could tell the Group why they found it interesting and for Page nodes they could add "review notes" or something.

Technically, I therefore need to provide two entityreference fields on the GroupContent entity: one to the Group, which doesn't change, and one to the groupable entity, which obviously needs to have a different target type based on the configuration. Much like the Comment example above.

So in short: I'm doing it the way Comment does now, but if this API were to change or improve, I'd definitely need to update the Group module as well.

kristiaanvandeneynde’s picture

Also, I don't know if this is a bug, but the way Comment implements it allows for a crash if copied by other modules.

In EntityReferenceItem::defaultStorageSettings() If no target type is specified and node is enabled, it will fall back to node. If node is disabled, it will fall back to user.

In EntityReferenceItem::schema(), the schema returned is based on the 'target_type' setting of the ER field:

  • If the target is a content entity, the schema will reference the entities with an INT column
  • If the target is a config entity, the schema will reference the entities with a VARCHAR column

The above two combined can lead to the following:

  1. Comment does not mention a target_type in its base field definitions
  2. It does mention them in its bundle field definitions
  3. When a database table is created for an entity type, only base fields are taken into account
  4. This will cause the entity_id column to be of type INT (due to lack of 'target_type' in base field definitions and the fallback to node/user)
  5. This means that if you override the base field in your bundle field definitions to target a config entity, Drupal will try to store a VARCHAR inside an INT column, leading to a crash

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sukanya.ramakrishnan’s picture

Any updates on this issue please?

Thanks,
Sukanya

apaderno’s picture

Version: 8.1.x-dev » 8.3.x-dev
apaderno’s picture

Correct me if I am wrong, but could not hook_entity_bundle_info() use the BaseFieldOverride class? The example shown in the documentation for hook_entity_bundle_info() would become the following.

function hook_entity_bundle_field_info(\Drupal\Core\Entity\EntityTypeInterface $entity_type, $bundle, array $base_field_definitions) {
  // Add a property only to nodes of the 'article' bundle.
  if ($entity_type->id() == 'node' && $bundle == 'article') {
    $fields = [];
    $base_field_definition = BaseFieldDefinition::create('string')
      ->setLabel(t('More text'))
      ->setComputed(TRUE)
      ->setClass('\Drupal\mymodule\EntityComputedMoreText');
    $fields['mymodule_text_more'] = BaseFieldOverride::createFromBaseFieldDefinition($base_field_definition, $bundle);
    return $fields;
  }
}

If the problem is just BaseFieldDefinition::create()returning a field that is reported to be a base field, the class resolves it.

Anonymous’s picture

kiamlaluno: I've used BaseFieldOverride::createFromBaseFieldDefinition in my bundleFieldDefinitions but it didn't do anything.

And in order to run installFieldStorageDefinition from entity update manager the override does not implement FieldStorageDefinitionInterface so it will fail.

I have also copied the \Drupal\entity_test\FieldStorageDefinition and used it instead of the origina lclas but didn't worked either.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ohthehugemanatee’s picture

@kiamlaluno That's close, but BaseFieldOverride assumes that there is a base field underneath it. So you get odd fatal errors, for example when anything tries to get display options. Note the usage of BaseFieldOverride::getBaseFieldDefinition()... all over the class, and who
knows where else.

If code defined "bundle fields" are going to be a thing, we actually do need a new BundleFieldDefinition class.

Meanwhile, I guess the recommended approach is to add a Base field only when it's a computed field (because you can't uninstall a module which provides a base field with data in it. See #1498720-86: [meta] Make the entity storage system handle changes in the entity and field schema definitions ). Otherwise use a config field, like this:

/**
 * Implements hook_entity_bundle_field_info().
 *
 * Add the example_field field to appropriate entity bundles.
 *
 * {@inheritdoc}
 */
function example_entity_bundle_field_info(EntityTypeInterface $entity_type,
$bundle, array $base_field_definitions) {
  $fields = [];
  // If the field doesn't otherwise exist.
  $field_config_storage = \Drupal::getContainer()->get('entity_type.manager')->getStorage('field_config');
    $existing_field = $field_config_storage->loadByProperties([
      'entity_type' => $entity_type->id(),
      'bundle' => $bundle,
      'field_name' => 'example_field',
    ]);
    if (empty($existing_field)) {
      $fields = [];

      $fields['example_field'] = FieldConfig::create([
        'entity_type' => $entity_type->id(),
        'field_name' => 'example_field',
        'bundle' => $bundle,
      ]);
    }
  }
  return $fields;
}

/**
 * Implements hook_entity_field_storage_info().
 *
 * {@inheritdoc}
 */
function example_entity_field_storage_info(EntityTypeInterface $entity_type) {
  $fields = [];
  // Make sure the field storage doesn't already exist.
  $field_storage_storage =
\Drupal::getContainer()->get('entity_type.manager')->getStorage('field_storage_config');
  $field_storage = $field_storage_storage->load('example_field');
  if (empty($field_storage)) {
    $fields['example_field'] = FieldStorageConfig::create([
      'field_name' => 'example_field',
      'entity_type' => $entity_type->id(),
      'type' => 'string',
      'locked' => TRUE,
      'settings' => [
        'max_length' => 255,
        'is_ascii' => FALSE,
        'case_sensitive' => FALSE,
      ],
    ]);
  }
  return $fields;
}

Note that there are a few things you can do with Base Fields (and the mythical "bundle fields"), which you cannot do with Config fields. My personal pain point is setting displayConfigurable=FALSE. The whole reason I'm doing this in code is because I don't want it dependent on anything in the config store, and with displayConfigurable=TRUE, every entity that receives the field gets a new entry in its displays. :|

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

heddn’s picture

Commerce in #3 still references this issue and implements it this way:

class BundleFieldDefinition extends BaseFieldDefinition {

  /**
   * {@inheritdoc}
   */
  public function isBaseField() {
    return FALSE;
  }

}
benjy’s picture

I have the same implementation in a few places. Core even has it in tests Drupal\entity_test\FieldStorageDefinition

heddn’s picture

Status: Active » Needs review
Issue tags: +Needs change record
FileSize
1.86 KB

Here's something to get us started. It is the most simple implementation I could think of. We'll still need to add a CR and link it into the deprecation.

apaderno’s picture

The FieldStorageDefinition class just defines isBaseField(), so it could be replaced by the proposed BundleFieldDefinition class: Since there is already #2280639: Add the FieldStorageDefinition class to define field storage definitions in hook_entity_field_storage_info() for that class, I would rather leave to that issue the task to replace the class with a proper one, especially because the comment describing the FieldStorageDefinition class:

For convenience we extend from BaseFieldDefinition although this should not implement FieldDefinitionInterface.

I think it would be an error to first replace it with the FieldStorageDefinition class, and then with a class that doesn't implement the FieldDefinitionInterface.

apaderno’s picture

Essentially, the tasks to do are (ordering them by what I feel it's the priority):

☐ Fix the problem hook_entity_bundle_field_info_alter() and hook_entity_base_field_info_alter() have (#2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods)
☐ Define a new class for bundle field definitions (#2935932: Add a FieldDefinition class for defining bundle fields in code.)
☐ Correct the example given in hook_entity_bundle_field_info() (#2935932: Add a FieldDefinition class for defining bundle fields in code.)
☐ Create bundle_field_override config entities (#2935978: Create a bundle_field_override configuration entity to override a bundle field with configuration)
☐ Update the documentation where necessary
☐ Update the code and remove the comments pointing to this issue, once this marked as fixed

kristiaanvandeneynde’s picture

Re #20 as long as we can still dynamically (i.e.: through code) change a bundle field's target entity type, I'm okay with it. We need to make sure that we maintain support for the behavior as currently implemented in Comment, Commerce (#3) and Group (#4)

apaderno’s picture

I think that #2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods should be fixed first, since that should take to the definition of a new interface that is then implemented by the BaseFieldDefinition class. In this case, the hierarchy of classes/interfaces would change.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

claudiu.cristea’s picture

There's also another aspect related to hook_entity_bundle_field_info() defined fields. The result of \Drupal::service('entity_field.manager')->getFieldStorageDefinitions($entity_type_id) doesn't return fields defined in this way. But it returns entity base fields together with bundle configurable fields. Is this normal?

Berdir’s picture

Not sure what you mean. getFieldStorageDefinitions() returns both base fields (which implement both the field and field storage interfaces) als well as FieldStorageConfig objects. As well as any other object that modules might define that implements that interface. That is by design yes.

claudiu.cristea’s picture

@Berdir, so it's by design that getFieldStorageDefinitions() returns bundle configurable fields but not bundle "code" fields?

Berdir’s picture

it returns what it says, field storage definitions. I'm not sure what you mean by bundle code fields. All fields must have a field storage definitions and field storage definitions are *not* by bundle. See \field_entity_field_storage_info() and \field_entity_bundle_field_info(), you must implement both to define per-bundle fields.

jibran’s picture

Sam152’s picture

It would be great to see some movement on this issue. Having implemented bundle fields in code, you hit a few problems like this. My only comments below:

  1. +++ b/core/lib/Drupal/Core/Field/BundleFieldDefinition.php
    @@ -0,0 +1,24 @@
    +/**
    + * A class for defining entity bundle fields.
    + */
    

    I think this would be a good place to expand on the docs for how one goes about actually using bundle field definitions outside the context of field.module config.

  2. +++ b/core/lib/Drupal/Core/Field/BundleFieldDefinition.php
    index 1a37d63..1cb4658 100644
    --- a/core/modules/system/tests/modules/entity_test/src/FieldStorageDefinition.php
    
    --- a/core/modules/system/tests/modules/entity_test/src/FieldStorageDefinition.php
    +++ b/core/modules/system/tests/modules/entity_test/src/FieldStorageDefinition.php
    
    +++ b/core/modules/system/tests/modules/entity_test/src/FieldStorageDefinition.php
    @@ -12,14 +12,8 @@
    + * @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0.
    + *   Use \Drupal\Core\Field\BundleFieldDefinition instead.
    

    Hm, do we really need to deprecate things in entity_test?

Not sure how this got into "Needs work" from #16, but would be good to see if anyone has any other feedback on this.

I'm hyped to get this in, since it's such a small change that affords a lot of power.

Sam152’s picture

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

moshe weitzman’s picture

We have bumped into this as well.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mikeryan’s picture

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Majdi’s picture

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bradjones1’s picture

Adding a pointer to Commerce's ConfigurableFieldManager, added since the 2015 pointers to that ecosystem.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

alexandersluiter’s picture

One of the issues with Commerce's ConfigurableFieldManager is that it requires field storage. Computed fields are not possible with that approach. With bundle classes being a thing now, could bundle field definitions be handled there? I've had success with this approach inside a base entity classes bundleFieldDefinitions() method. Added custom bundle class entity handlers works pretty well.

$class = $entity_type->getHandlerClass('bundle_classes', $bundle);

if ($class && is_subclass_of($class, EntityBundleClassInterface::class)) {
  $fields = call_user_func($class . '::bundleClassFieldDefinitions', $entity_type, $bundle, $base_field_definitions) + $fields;
}

From there, each bundle class can define it's own fields.

joachim’s picture

ConfigurableFieldManager looks like some useful functions for manipulating config fields.

I don't think that's really relevant to this issue, which is about declaring bundle fields in code.

longwave’s picture

Version: 9.5.x-dev » 10.1.x-dev

New API will have to go into 10.1.x since 9.5.0-beta1 is released.

D34dMan made their first commit to this issue’s fork.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.