Provide a way to define default values for entity fields

#1803064: Horizontal extensibility of Fields: introduce the concept of behavior plugins gives an example use-case of "prepopulating" values.

Closely related: #1253820-34: It's impossible to submit no value for a field that has a default value.

I think we have a really huge Drupalism/DrupalWTF problem if an essential OO concept like default class property values ceases to work. Replacing all object properties with class instances is a very weird concept already, but it is borderline acceptable as long as PHP developers only need minimal adjustments to cater for it.

Destroying the native concept of default values of class properties, however, is really too much IMHO. It's the tipping point at which PHP developers will reasonably blame Drupal for "Re-inventing OO in user-land PHP code" and a PHP class is no longer a PHP class. That's definitely detrimental for Drupal's health and future. A lot of the TypedData abstraction essentially exists to work around PHP's lack of primitive data type support, and PHP developers will already blame us for inventing and doing that; one doesn't have to look very far to find answers from PHP developers who strongly recommend to NOT do what we've done, for plethora of very good reasons; performance being a major one, and as you know, we're experiencing the consequences in various benchmarks already. Also, following the entire logic, one can only guess that one of the next steps is to invent polymorphism and mixins in this user-space OO layer (because, really, what prevents us from doing it? Nothing!) - ultimately rewriting PHP's native C code for OO support in PHP code.

I believe that this should ring all available alarm bells, and instead of abstracting even more, we should try hard to scale things back where possible.

Why can't we do something like this?

added tests for default language & uuids.

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldDefaultValueTest.phpundefined
@@ -0,0 +1,73 @@
+    // Initiate the generator. This will lazy-load uuid.inc.
+    $this->uuid = new Uuid();

Not sure I understand that comment, there's no such file?

re #13, yeah I just copied this comment over together with the uuid-related code borrowed from UuidUnitTest

Can you try without those two lines? Shouldn't be necessary.

Suppose uuid hinks should be filed in other issue

Coming from #1979260: Automatically populate the author default value with the current user, I also want this to work:

$values = array('title' => 'test node', 'type' => 'article');
$node = entity_create('node', $values);
$node->save();

That throws Exceptions currently if comment module is enabled, because the node author is not set.

Raising to major because of #27. It's a major (if not critical) bug for non-form-based creation of nodes to throw errors.

#1957888: Exception when a field is empty on programmatic creation is already major

See the interdiff's in #1818570-38: Convert users to the new Entity Field API and comment #40 for a fix for the failing forum tests.

Funny, for #1969728: Implement Field API "field types" as TypedData Plugins I needed to add a Field::filterEmpty() method that's basically the cleanValue() method added here.
Mine was a bit more straightforward though:

  public function filterEmpty() {
    if (isset($this->list)) {
      $this->list = array_values(array_filter($this->list, function ($item) {
        return !$item->isEmpty();
      }));
    }
  }

(+ may I bikeshed that maybe 'filter empty" is more explicit than "clean" as to what the function does ? filterEmptyValues() maybe ?)

Also, most probably not for this issue, but I'm a bit perplexed at the "default NULL values for all field items" behavior - i.e, an "empty field" is a Field *with* a FieldItem, that happens to have NULL properties - why is that exactly ?

BTW, both the cleanValue() method in the current patch and the alternative code proposed in #33 will leave an "empty" Field with an empty $this->list - which breaks the behavior mentioned above (an "empty field" is a Field *with* a FieldItem, that happens to have NULL properties).
"Sometimes there is an empty FieldItem, sometimes there isn't" sounds wrong ?

+++ b/core/lib/Drupal/Core/TypedData/Type/Map.php
@@ -230,4 +230,14 @@ public function onChange($property_name) {
+  /**
+   * {@inheritdoc}
+   */
+  public function applyDefaultValue($notify = TRUE) {
+    // Apply the default value of all properties.
+    foreach ($this->getProperties() as $property) {
+      $property->applyDefaultValue(FALSE);
+    }
+  }

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php
@@ -1134,4 +1134,11 @@ public function setContext($name = NULL, TypedDataInterface $parent = NULL) {
+  /**
+   * Implements \Drupal\Core\TypedData\TypedDataInterface::applyDefaultValue().
+   */
+  public function applyDefaultValue($notify = TRUE) {
+    $this->storage->applyDefaultValue($notify);
+  }

Isn't here a return $this; missing? According to the function documentation it should allow chaining.

+++ b/core/modules/forum/forum.module
@@ -334,10 +334,13 @@ function forum_node_presave(EntityInterface $node) {
+      // Only do a shadow copy check if this is not a new node.
+      if (!$node->isNew()) {

Shouldn't that be "Only do a shadow copy if this is not a new node."?
The word "check" seems to be a leftover from an earlier sentence.

That's all I've found so far :)

As discussed in #1957888: Exception when a field is empty on programmatic creation, the check there is actually correct. And this issue will need a re-roll once the other one is in :)

It's not an improvement :)

The previous comment is correct as explained in the other issue, in case the node is not new, we are *checking* if we have to create a shadow copy.

+++ b/core/lib/Drupal/Core/Entity/EntityNG.phpundefined
@@ -486,8 +482,7 @@ public function createDuplicate() {
+      $duplicate->{$entity_info['entity_keys']['uuid']}->applyDefaultValue();

Talking about applying a default value for UUID is a bit weird as we're actually generating a new one here but can't do much about this.

+++ b/core/lib/Drupal/Core/Entity/Field/Type/Field.phpundefined
@@ -21,6 +21,9 @@
+ * Supported settings (below the definition's 'settings' key) are:
+ * - default_value: (optional) If set, the default value to apply to the field.

This would allow to both set the default for the first field item and also multiple values. Not very intuitive, should we document this better here?

+++ b/core/modules/text/lib/Drupal/text/Type/TextItem.phpundefined
@@ -54,6 +54,16 @@ public function getPropertyDefinitions() {
+    // Default to a simple check_plain().
+    // @todo: Add in the filter default format here.
+    $this->setValue(array('format' => NULL), $notify);

In case of configurable text fields, it depends on the configuration of the field if it should be NULL (check_plain(), or filter_default_format(). Should we respect that here?

We can also do it in a follow-up, but then @todo should be slightly reworded I think.

Patch nicely shows the code duplication of entity reference/term/file/image fields. There's an RTBC patch that converts term to extend from entity and use target_id and a green needs review patch for files/images. Would be nice if that could go in first I think, I hope that happens soon.

About the default value setting, I think we should also try to find a better place to document the settings a certain class supports. Possibly simply define them with default value in the plugin definition? As we move to annotations, that would be almost the same place but then it's also obvious to search there for documentation.

This all looks very good to me, fixed the old language constant in the test case.

Patch looks really good and I'm tempted to set it to RTBC but I found some nitpicky stuff:

+++ b/core/lib/Drupal/Core/Entity/Field/Type/LanguageItem.php
@@ -51,14 +51,28 @@ public function getPropertyDefinitions() {
+    // Default to LANGUAGE_NOT_SPECIFIED.

I think that should be adjusted to Language::LANGCODE_NOT_SPECIFIE now, right?

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldDefaultValueTest.php
@@ -0,0 +1,70 @@
+      'entity_test_mulrev_property_revision'

Missing trailing comma.

+++ b/core/modules/text/lib/Drupal/text/Type/TextItem.php
@@ -54,6 +54,16 @@ public function getPropertyDefinitions() {
+    // @todo: Add in the filter default format here.

Do we need / have a follow up issue for that? This was mentioned already by Berdir in #48.

+++ b/core/lib/Drupal/Core/Entity/Field/Type/LanguageItem.php
@@ -51,14 +51,28 @@ public function getPropertyDefinitions() {
+  public function applyDefaultValue($notify = TRUE) {
+    // Default to LANGCODE_NOT_SPECIFIED.
+    $this->setValue(array('value' => \Drupal\Core\Language\Language::LANGCODE_NOT_SPECIFIED), $notify);

Do not use the fully qualified namespace here, always use use statements.

Fixed that.

According to the interdiffs all my nitpicky stuff was addressed and since I'm afraid that klausi will punch me if I don't set it to RTBC now - here we go. :P

This would be nice addition for #731724: Convert comment settings into a field to make them work with CMI and non-node entities

+1 to RTBC

Why is this only changed for DatabaseStorageController? Is there a follow-up to keep ConfigStorageController up to date?

Hm, we just forgot to keep the config storage controller in sync. I think that should not hold up this issue, because we need this urgently elsewhere. So I created this follow-up for the config stuff: #2004756: Defining default values for config entities.

I haven't reviewed this patch, but have confirmed that it has no actual code changes relative to #59, so back to RTBC.

default values for User seem to be not removed?

+++ b/core/lib/Drupal/Core/Entity/Field/Type/LanguageItem.phpundefined
@@ -51,14 +51,28 @@ public function getPropertyDefinitions() {
+  public function applyDefaultValue($notify = TRUE) {
+    // Default to LANGCODE_NOT_SPECIFIED.
+    $this->setValue(array('value' => Language::LANGCODE_NOT_SPECIFIED), $notify);

Outdated comment.

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldDefaultValueTest.phpundefined
@@ -0,0 +1,70 @@
+class EntityFieldDefaultValueTest extends EntityUnitTestBase  {
...
+    $this->installSchema('entity_test', array(
+      'entity_test_mul',
+      'entity_test_mul_property_data',
+      'entity_test_rev',
+      'entity_test_rev_revision',
+      'entity_test_mulrev',
+      'entity_test_mulrev_property_data',
+      'entity_test_mulrev_property_revision',

Do we need the tables as we don't seem the save the entities here?

Also, might be a useful helper function for EntityUnitTestBase for which we could create a novice issue.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
@@ -1135,4 +1135,11 @@ public function setContext($name = NULL, TypedDataInterface $parent = NULL) {
+   * Implements \Drupal\Core\TypedData\TypedDataInterface::applyDefaultValue().

@inheritdoc. Also not consistent with EntityBCDecorator, but at last that one will go away so it doesn't matter :)

#76: d8_default.patch queued for re-testing.

Gave this another visual review. I'd say RTBC again.

Unrelated but while reviewing I came across following code in UserStorageController::baseFieldDefinitions(), is it intention to use "Name" for all the labels?

    $properties['name'] = array(
      'label' => t('Name'),
      'description' => t('The name of this user'),
      'type' => 'string_field',
      'settings' => array('default_value' => ''),
    );
    $properties['pass'] = array(
      'label' => t('Name'),
      'description' => t('The password of this user (hashed)'),
      'type' => 'string_field',
    );
    $properties['mail'] = array(
      'label' => t('Name'),
      'description' => t('The e-mail of this user'),
      'type' => 'string_field',
      'settings' => array('default_value' => ''),
    );
    $properties['signature'] = array(
      'label' => t('Name'),
      'description' => t('The signature of this user'),
      'type' => 'string_field',
    );
    $properties['signature_format'] = array(
      'label' => t('Name'),
      'description' => t('The signature format of this user'),
      'type' => 'string_field',
    );

Shall I open a novice follow-up for a clean-up? --> Follow-up: #2019741: Set appropriate labels in the user entity base field definition

#76: d8_default.patch queued for re-testing.

Simple reroll, no other changes, so straight back to RTBC assuming the bot is happy.

Merge remote-tracking branch 'origin/8.x' into entity-defaults-1777956

Conflicts:
core/lib/Drupal/Core/Entity/EntityBCDecorator.php
core/modules/views_ui/lib/Drupal/views_ui/ViewUI.php

#86: entity-default-1777956-86.patch queued for re-testing.

#86: entity-default-1777956-86.patch queued for re-testing.

Committed 851d7d0 and pushed to 8.x. Thanks!

We probably need to create or update some change notices for this.

Closely related #1919834: Field instance got no default value when created in field UI

fix tags

Change record created: https://drupal.org/node/2031221

Not sure the wording is correct like "type data definition", please fix where you see fit.

Excellent, so let's call this fixed.

Removing tag

Removing sprint tag.

Note : #2050801: Unify handling of default values between base and configurable fields has a patch to move the handling of configurable field default values.

Reminding everyone that while I asked for this to be done equally to both config and content entities, it is still undone for config entities: #2004756: Defining default values for config entities