Issue #2256679 by alexpott, tim.plunkett: Fixed Use config schema to determine...

Issue #2256679 by alexpott, tim.plunkett: Fixed Use config schema to determine which config entity properties to export regardless of whether they are public or protected.

core/config/schema/core.data_types.schema.yml

@@ -235,15 +235,9 @@ config_dependencies:
 config_entity:
   type: mapping
   mapping:
-    id:
-      type: string
-      label: 'ID'
     uuid:
       type: string
       label: 'UUID'
-    label:
-      type: label
-      label: 'Label'
     langcode:
       type: string
       label: 'Default language'

core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php

@@ -10,6 +10,7 @@
 use Drupal\Component\Plugin\ConfigurablePluginInterface;
 use Drupal\Component\Utility\String;
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\Config\Schema\SchemaIncompleteException;
 use Drupal\Core\Entity\Entity;
 use Drupal\Core\Config\ConfigDuplicateUUIDException;
 use Drupal\Core\Entity\EntityStorageInterface;
@@ -240,19 +241,35 @@ public static function sort(ConfigEntityInterface $a, ConfigEntityInterface $b)
    * {@inheritdoc}
    */
   public function toArray() {
-    // Configuration objects do not have a schema. Extract all key names from
-    // class properties.
-    $class_info = new \ReflectionClass($this);
     $properties = array();
-    foreach ($class_info->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
-      $name = $property->getName();
-      $properties[$name] = $this->get($name);
+    $config_name = $this->getEntityType()->getConfigPrefix() . '.' . $this->id();
+    $definition = $this->getTypedConfig()->getDefinition($config_name);
+    if (!isset($definition['mapping'])) {
+      throw new SchemaIncompleteException(String::format('Incomplete or missing schema for @config_name', array('@config_name' => $config_name)));
+    }
+    $id_key = $this->getEntityType()->getKey('id');
+    foreach (array_keys($definition['mapping']) as $name) {
+      // Special handling for IDs so that computed compound IDs work.
+      // @see \Drupal\entity\EntityDisplayBase::id()
+      if ($name == $id_key) {
+        $properties[$name] = $this->id();
+      }
+      else {
+        $properties[$name] = $this->get($name);
+      }
     }
-    // Add protected dependencies property.
-    $properties['dependencies'] = $this->dependencies;
     return $properties;
   }
 
+  /**
+   * Gets the typed config manager.
+   *
+   * @return \Drupal\Core\Config\TypedConfigManagerInterface
+   */
+  protected function getTypedConfig() {
+    return \Drupal::service('config.typed');
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/block/config/schema/block.schema.yml

@@ -4,6 +4,9 @@ block.block.*:
   type: config_entity
   label: 'Block'
   mapping:
+    id:
+      type: string
+      label: 'ID'
     theme:
       type: string
       label: 'Theme'

core/modules/block/src/Entity/Block.php

@@ -33,8 +33,7 @@
  *   admin_permission = "administer blocks",
  *   fieldable = FALSE,
  *   entity_keys = {
- *     "id" = "id",
- *     "label" = "label"
+ *     "id" = "id"
  *   },
  *   links = {
  *     "delete-form" = "block.admin_block_delete",
@@ -134,25 +133,6 @@ public function label() {
     }
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function toArray() {
-    $properties = parent::toArray();
-    $names = array(
-      'theme',
-      'region',
-      'weight',
-      'plugin',
-      'settings',
-      'visibility',
-    );
-    foreach ($names as $name) {
-      $properties[$name] = $this->get($name);
-    }
-    return $properties;
-  }
-
   /**
    * Sorts active blocks by weight; sorts inactive blocks by name.
    */

core/modules/block/src/Tests/BlockStorageUnitTest.php

@@ -90,13 +90,15 @@ protected function createTests() {
 
     // Ensure that default values are filled in.
     $expected_properties = array(
-      'id' => 'test_block',
-      'weight' => NULL,
-      'status' => TRUE,
       'langcode' => \Drupal::languageManager()->getDefaultLanguage()->id,
+      'status' => TRUE,
       'dependencies' => array('module' => array('block_test'), 'theme' => array('stark')),
+      'id' => 'test_block',
       'theme' => 'stark',
       'region' => '-1',
+      'weight' => NULL,
+      'provider' => NULL,
+      'visibility' => NULL,
       'plugin' => 'test_html',
       'settings' => array(
         'id' => 'test_html',
@@ -108,8 +110,8 @@ protected function createTests() {
           'contexts' => array(),
         ),
       ),
-      'visibility' => NULL,
     );
+
     $this->assertIdentical($actual_properties, $expected_properties);
 
     $this->assertTrue($entity->getPlugin() instanceof TestHtmlBlock, 'The entity has an instance of the correct block plugin.');

core/modules/block_content/config/schema/block_content.schema.yml

@@ -4,6 +4,12 @@ block_content.type.*:
   type: config_entity
   label: 'Custom block type settings'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     revision:
       type: integer
       label: 'Create new revision'

core/modules/breakpoint/config/schema/breakpoint.schema.yml

@@ -3,6 +3,12 @@ breakpoint.breakpoint.*.*.*:
   type: config_entity
   label: 'Defines the Breakpoint entity'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     name:
       type: string
       label: 'Machine name'
@@ -29,6 +35,12 @@ breakpoint.breakpoint_group.*.*.*:
   type: config_entity
   label: 'Breakpoint group settings'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     name:
       type: string
       label: 'Machine name'

core/modules/breakpoint/src/Entity/BreakpointGroup.php

@@ -211,29 +211,6 @@ public function getBreakpointById($id) {
     return FALSE;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function toArray() {
-    $names = array(
-      'id',
-      'uuid',
-      'name',
-      'label',
-      'breakpoint_ids',
-      'source',
-      'sourceType',
-      'status',
-      'langcode',
-      'dependencies',
-    );
-    $properties = array();
-    foreach ($names as $name) {
-      $properties[$name] = $this->get($name);
-    }
-    return $properties;
-  }
-
   /**
    * {@inheritdoc}
    */

core/modules/comment/comment.module

@@ -120,7 +120,7 @@ function comment_uri(CommentInterface $comment) {
 function comment_entity_extra_field_info() {
   $return = array();
   foreach (CommentType::loadMultiple() as $comment_type) {
-    $return['comment'][$comment_type->id] = array(
+    $return['comment'][$comment_type->id()] = array(
       'form' => array(
         'author' => array(
           'label' => t('Author'),

core/modules/comment/config/schema/comment.schema.yml

@@ -47,6 +47,12 @@ comment.type.*:
   type: config_entity
   label: 'Comment type settings'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     target_entity_type_id:
       type: string
       label: 'Target Entity Type ID'

core/modules/comment/src/CommentTypeForm.php

@@ -82,7 +82,7 @@ public function form(array $form, array &$form_state) {
 
     $form['description'] = array(
       '#type' => 'textarea',
-      '#default_value' => $comment_type->description,
+      '#default_value' => $comment_type->getDescription(),
       '#description' => t('Describe this comment type. The text will be displayed on the <em>Comment types</em> administration overview page'),
       '#title' => t('Description'),
     );

core/modules/comment/src/Entity/CommentType.php

@@ -47,28 +47,28 @@ class CommentType extends ConfigEntityBundleBase implements CommentTypeInterface
    *
    * @var string
    */
-  public $id;
+  protected $id;
 
   /**
    * The comment type label.
    *
    * @var string
    */
-  public $label;
+  protected $label;
 
   /**
    * The description of the comment type.
    *
    * @var string
    */
-  public $description;
+  protected $description;
 
   /**
    * The target entity type.
    *
    * @var string
    */
-  public $target_entity_type_id;
+  protected $target_entity_type_id;
 
   /**
    * {@inheritdoc}

core/modules/config/src/Tests/ConfigDiffTest.php

@@ -111,11 +111,12 @@ function testDiff() {
 
     $diff = \Drupal::service('config.manager')->diff($active, $staging, 'config_test.dynamic.' . $new_test_entity_id, $config_name);
     $edits = $diff->getEdits();
-    $this->assertEqual($edits[0]->type, 'change',  'The second item in the diff is a copy.');
-    $this->assertEqual($edits[0]->orig, array('id: ' . $new_test_entity_id));
-    $this->assertEqual($edits[0]->closing, array('id: ' . $test_entity_id));
-    $this->assertEqual($edits[1]->type, 'copy',  'The second item in the diff is a copy.');
-    $this->assertEqual(count($edits), 2, 'There are two items in the diff.');
+    $this->assertEqual($edits[0]->type, 'copy',  'The first item in the diff is a copy.');
+    $this->assertEqual($edits[1]->type, 'change',  'The second item in the diff is a change.');
+    $this->assertEqual($edits[1]->orig, array('id: ' . $new_test_entity_id));
+    $this->assertEqual($edits[1]->closing, array('id: ' . $test_entity_id));
+    $this->assertEqual($edits[2]->type, 'copy',  'The third item in the diff is a copy.');
+    $this->assertEqual(count($edits), 3, 'There are three items in the diff.');
   }
 
   /**

core/modules/config/src/Tests/ConfigEntityTest.php

@@ -46,7 +46,6 @@ function testCRUD() {
     $default_langcode = \Drupal::languageManager()->getDefaultLanguage()->id;
     // Verify default properties on a newly created empty entity.
     $empty = entity_create('config_test');
-    $this->assertIdentical($empty->id, NULL);
     $this->assertTrue($empty->uuid);
     $this->assertIdentical($empty->label, NULL);
     $this->assertIdentical($empty->style, NULL);
@@ -105,7 +104,6 @@ function testCRUD() {
       'label' => $this->randomString(),
       'style' => $this->randomName(),
     ));
-    $this->assertIdentical($config_test->id, $expected['id']);
     $this->assertTrue($config_test->uuid);
     $this->assertNotEqual($config_test->uuid, $empty->uuid);
     $this->assertIdentical($config_test->label, $expected['label']);
@@ -159,7 +157,7 @@ function testCRUD() {
     try {
       $id_length_config_test->save();
       $this->pass(String::format("config_test entity with ID length @length was saved.", array(
-        '@length' => strlen($id_length_config_test->id))
+        '@length' => strlen($id_length_config_test->id()))
       ));
     }
     catch (ConfigEntityIdLengthException $e) {
@@ -173,7 +171,7 @@ function testCRUD() {
     try {
       $id_length_config_test->save();
       $this->pass(String::format("config_test entity with ID length @length was saved.", array(
-        '@length' => strlen($id_length_config_test->id),
+        '@length' => strlen($id_length_config_test->id()),
       )));
     }
     catch (ConfigEntityIdLengthException $e) {
@@ -187,13 +185,13 @@ function testCRUD() {
     try {
       $status = $id_length_config_test->save();
       $this->fail(String::format("config_test entity with ID length @length exceeding the maximum allowed length of @max saved successfully", array(
-        '@length' => strlen($id_length_config_test->id),
+        '@length' => strlen($id_length_config_test->id()),
         '@max' => static::MAX_ID_LENGTH,
       )));
     }
     catch (ConfigEntityIdLengthException $e) {
       $this->pass(String::format("config_test entity with ID length @length exceeding the maximum allowed length of @max failed to save", array(
-        '@length' => strlen($id_length_config_test->id),
+        '@length' => strlen($id_length_config_test->id()),
         '@max' => static::MAX_ID_LENGTH,
       )));
     }
@@ -221,7 +219,7 @@ function testCRUD() {
       $this->assertIdentical($config_test->getOriginalId(), $old_id);
 
       // Rename.
-      $config_test->id = $new_id;
+      $config_test->set('id', $new_id);
       $this->assertIdentical($config_test->id(), $new_id);
       $status = $config_test->save();
       $this->assertIdentical($status, SAVED_UPDATED);

core/modules/config/src/Tests/ConfigImportUITest.php

@@ -56,15 +56,15 @@ function testImport() {
 
     // Create new config entity.
     $original_dynamic_data = array(
+      'uuid' => '30df59bd-7b03-4cf7-bb35-d42fc49f0651',
+      'langcode' => \Drupal::languageManager()->getDefaultLanguage()->id,
+      'status' => TRUE,
+      'dependencies' => array(),
       'id' => 'new',
       'label' => 'New',
       'weight' => 0,
       'style' => '',
       'test_dependencies' => array(),
-      'status' => TRUE,
-      'uuid' => '30df59bd-7b03-4cf7-bb35-d42fc49f0651',
-      'langcode' => \Drupal::languageManager()->getDefaultLanguage()->id,
-      'dependencies' => array(),
       'protected_property' => '',
     );
     $staging->write($dynamic_name, $original_dynamic_data);
@@ -349,31 +349,31 @@ function testImportErrorLog() {
     $uuid = $this->container->get('uuid');
 
     $values_primary = array(
+      'uuid' => $uuid->generate(),
+      'langcode' => 'en',
+      'status' => TRUE,
+      'dependencies' => array(),
       'id' => 'primary',
       'label' => 'Primary',
       'weight' => 0,
       'style' => NULL,
       'test_dependencies' => array(),
-      'status' => TRUE,
-      'uuid' => $uuid->generate(),
-      'langcode' => 'en',
-      'dependencies' => array(),
       'protected_property' => null,
     );
     $staging->write($name_primary, $values_primary);
     $values_secondary = array(
-      'id' => 'secondary',
-      'label' => 'Secondary Sync',
-      'weight' => 0,
-      'style' => NULL,
-      'test_dependencies' => array(),
-      'status' => TRUE,
       'uuid' => $uuid->generate(),
       'langcode' => 'en',
+      'status' => TRUE,
       // Add a dependency on primary, to ensure that is synced first.
       'dependencies' => array(
         'entity' => array($name_primary),
       ),
+      'id' => 'secondary',
+      'label' => 'Secondary Sync',
+      'weight' => 0,
+      'style' => NULL,
+      'test_dependencies' => array(),
       'protected_property' => null,
     );
     $staging->write($name_secondary, $values_secondary);

core/modules/config/src/Tests/ConfigImporterTest.php

@@ -170,15 +170,15 @@ function testNew() {
 
     // Create new config entity.
     $original_dynamic_data = array(
+      'uuid' => '30df59bd-7b03-4cf7-bb35-d42fc49f0651',
+      'langcode' => \Drupal::languageManager()->getDefaultLanguage()->id,
+      'status' => TRUE,
+      'dependencies' => array(),
       'id' => 'new',
       'label' => 'New',
       'weight' => 0,
       'style' => '',
       'test_dependencies' => array(),
-      'status' => TRUE,
-      'uuid' => '30df59bd-7b03-4cf7-bb35-d42fc49f0651',
-      'langcode' => \Drupal::languageManager()->getDefaultLanguage()->id,
-      'dependencies' => array(),
       'protected_property' => '',
     );
     $staging->write($dynamic_name, $original_dynamic_data);

core/modules/config/src/Tests/ConfigSchemaTest.php

@@ -135,12 +135,8 @@ function testSchemaMapping() {
     $expected['label'] = 'Image style';
     $expected['class'] = '\Drupal\Core\Config\Schema\Mapping';
     $expected['mapping']['name']['type'] = 'string';
-    $expected['mapping']['id']['label'] = 'ID';
-    $expected['mapping']['id']['type'] = 'string';
     $expected['mapping']['uuid']['type'] = 'string';
     $expected['mapping']['uuid']['label'] = 'UUID';
-    $expected['mapping']['label']['type'] = 'label';
-    $expected['mapping']['label']['label'] = 'Label';
     $expected['mapping']['langcode']['type'] = 'string';
     $expected['mapping']['langcode']['label'] = 'Default language';
     $expected['mapping']['status']['type'] = 'boolean';
@@ -148,6 +144,8 @@ function testSchemaMapping() {
     $expected['mapping']['dependencies']['type'] = 'config_dependencies';
     $expected['mapping']['dependencies']['label'] = 'Dependencies';
     $expected['mapping']['name']['type'] = 'string';
+    $expected['mapping']['label']['type'] = 'label';
+    $expected['mapping']['label']['label'] = 'Label';
     $expected['mapping']['effects']['type'] = 'sequence';
     $expected['mapping']['effects']['sequence'][0]['type'] = 'mapping';
     $expected['mapping']['effects']['sequence'][0]['mapping']['id']['type'] = 'string';

core/modules/config/tests/config_test/config/schema/config_test.schema.yml

 # Schema for the configuration files of the Configuration Test module.
 
-config_test.dynamic.*:
-  type: mapping
-  label: 'Config test entity'
+config_test_dynamic:
+  type: config_entity
   mapping:
     id:
       type: string
-      label: 'Machine name'
+      label: 'ID'
     label:
       type: label
       label: 'Label'
-    weight:
-      type: integer
-      label: 'Weight'
-    style:
-      type: string
-      label: 'Image style'
-    protected_property:
-      type: string
-      label: 'Protected property'
-
-config_test_dynamic:
-  type: config_entity
-  mapping:
     weight:
       type: integer
       label: 'Weight'
     style:
       type: string
       label: 'style'
+    test_dependencies:
+      type: config_dependencies
+      label: 'Configuration dependencies'
     protected_property:
       type: string
       label: 'Protected property'
@@ -41,6 +30,24 @@ config_test.dynamic.*.*:
   type: config_test_dynamic
   label: 'Config test dynamic settings'
 
+config_test.query.*:
+  type: config_entity
+  mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
+    array:
+      type: sequence
+      label: 'Array'
+      sequence:
+        - type: string
+    number:
+      type: integer
+      label: 'number'
+
 config_test.types:
   type: mapping
   label: 'Configuration type'

core/modules/config/tests/config_test/src/Entity/ConfigTest.php

@@ -48,7 +48,7 @@ class ConfigTest extends ConfigEntityBase implements ConfigTestInterface {
    *
    * @var string
    */
-  public $id;
+  protected $id;
 
   /**
    * The human-readable name of the configuration entity.
@@ -85,20 +85,6 @@ class ConfigTest extends ConfigEntityBase implements ConfigTestInterface {
    */
   protected $protected_property;
 
-  /**
-   * {@inheritdoc}
-   */
-  public function toArray() {
-    $properties = parent::toArray();
-    $protected_names = array(
-      'protected_property',
-    );
-    foreach ($protected_names as $name) {
-      $properties[$name] = $this->get($name);
-    }
-    return $properties;
-  }
-
   /**
    * Overrides \Drupal\Core\Config\Entity\ConfigEntityBase::sort().
    */

core/modules/contact/config/schema/contact.schema.yml

@@ -4,6 +4,12 @@ contact.category.*:
   type: config_entity
   label: 'Contact category'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     recipients:
       type: sequence
       label: 'Recipients'

core/modules/editor/src/Entity/Editor.php

@@ -176,21 +176,4 @@ public function setImageUploadSettings(array $image_upload_settings) {
     return $this;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function toArray() {
-    $properties = parent::toArray();
-    $names = array(
-      'format',
-      'editor',
-      'settings',
-      'image_upload',
-    );
-    foreach ($names as $name) {
-      $properties[$name] = $this->get($name);
-    }
-    return $properties;
-  }
-
 }

core/modules/entity/config/schema/entity.schema.yml

@@ -33,6 +33,12 @@ entity.form_mode.*.*:
   type: config_entity
   label: 'Entity form mode settings'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     targetEntityType:
       type: string
       label: 'Target entity type'
@@ -45,6 +51,12 @@ entity.view_display.*.*.*:
   type: config_entity
   label: 'Entity display'
   mapping:
+    id:
+      type: string
+      label: 'ID'
+    label:
+      type: label
+      label: 'Label'
     targetEntityType:
       type: string
       label: 'Target entity type'
@@ -71,6 +83,9 @@ entity.form_display.*.*.*:
   type: config_entity
   label: 'Entity form display'
   mapping:
+    id:
+      type: string
+      label: 'ID'
     targetEntityType:
       type: string
       label: 'Target entity type'
@@ -80,9 +95,12 @@ entity.form_display.*.*.*:
     mode:
       type: string
       label: 'View or form mode machine name'
+    status:
+      type: boolean
+      label: 'Enabled'
     content:
       type: sequence
-      label: 'Field form display formatters'
+      label: 'Field display formatters'
       sequence:
         - type: entity_form_display.field.[type]
     hidden:
@@ -91,6 +109,9 @@ entity.form_display.*.*.*:
       sequence:
         - type: boolean
           label: 'Component'
+    dependencies:
+      type: config_dependencies
+      label: 'Dependencies'
 
 # Default schema for entity display field with undefined type.
 entity_view_display.field.*:

core/modules/entity/src/EntityDisplayBase.php

@@ -200,23 +200,7 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
    * {@inheritdoc}
    */
   public function toArray() {
-    $names = array(
-      'uuid',
-      'targetEntityType',
-      'bundle',
-      'mode',
-      'content',
-      'hidden',
-      'status',
-      'dependencies'
-    );
-    $properties = array(
-      'id' => $this->id(),
-    );
-    foreach ($names as $name) {
-      $properties[$name] = $this->get($name);
-    }
-
+    $properties = parent::toArray();
     // Do not store options for fields whose display is not set to be
     // configurable.
     foreach ($this->getFieldDefinitions() as $field_name => $definition) {

core/modules/field/config/schema/field.schema.yml

@@ -12,6 +12,9 @@ field.field.*.*:
   type: config_entity
   label: 'Field'
   mapping:
+    id:
+      type: string
+      label: 'ID'
     name:
       type: string
       label: 'Name'
@@ -49,6 +52,12 @@ field.instance.*.*.*:
   type: config_entity
   label: 'Field instance'
   mapping:
+    id:
+      type: string
+      label: 'ID'