Issue #2788637 by mglaman, Berdir, bradjones1, effulgentsia, alexpott,...

Issue #2788637 by mglaman, Berdir, bradjones1, effulgentsia, alexpott, amateescu, borisson_: Values in shared table for SQL content entity storage do not get unserialized

core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php

@@ -463,6 +463,7 @@ protected function mapFromStorageRecords(array $records, $load_from_revision = F
       $field_names = array_unique(array_merge($field_names, $this->tableMapping->getFieldNames($this->revisionTable)));
     }
 
+    $storage_definitions = $this->entityFieldManager->getFieldStorageDefinitions($this->entityTypeId);
     $values = [];
     foreach ($records as $id => $record) {
       $values[$id] = [];
@@ -473,9 +474,10 @@ protected function mapFromStorageRecords(array $records, $load_from_revision = F
         $field_columns = $this->tableMapping->getColumnNames($field_name);
         // Handle field types that store several properties.
         if (count($field_columns) > 1) {
+          $definition_columns = $storage_definitions[$field_name]->getColumns();
           foreach ($field_columns as $property_name => $column_name) {
             if (property_exists($record, $column_name)) {
-              $values[$id][$field_name][LanguageInterface::LANGCODE_DEFAULT][$property_name] = $record->{$column_name};
+              $values[$id][$field_name][LanguageInterface::LANGCODE_DEFAULT][$property_name] = !empty($definition_columns[$property_name]['serialize']) ? unserialize($record->{$column_name}) : $record->{$column_name};
               unset($record->{$column_name});
             }
           }
@@ -484,7 +486,9 @@ protected function mapFromStorageRecords(array $records, $load_from_revision = F
         else {
           $column_name = reset($field_columns);
           if (property_exists($record, $column_name)) {
-            $values[$id][$field_name][LanguageInterface::LANGCODE_DEFAULT] = $record->{$column_name};
+            $columns = $storage_definitions[$field_name]->getColumns();
+            $column = reset($columns);
+            $values[$id][$field_name][LanguageInterface::LANGCODE_DEFAULT] = !empty($column['serialize']) ? unserialize($record->{$column_name}) : $record->{$column_name};
             unset($record->{$column_name});
           }
         }
@@ -578,6 +582,7 @@ protected function loadFromSharedTables(array &$values, array &$translations, $l
         $all_fields = $table_mapping->getFieldNames($this->dataTable);
       }
 
+      $storage_definitions = $this->entityFieldManager->getFieldStorageDefinitions($this->entityTypeId);
       $result = $query->execute();
       foreach ($result as $row) {
         $id = $row[$record_key];
@@ -589,14 +594,19 @@ protected function loadFromSharedTables(array &$values, array &$translations, $l
         $translations[$id][$langcode] = TRUE;
 
         foreach ($all_fields as $field_name) {
+          $storage_definition = $storage_definitions[$field_name];
+          $definition_columns = $storage_definition->getColumns();
           $columns = $table_mapping->getColumnNames($field_name);
           // Do not key single-column fields by property name.
           if (count($columns) == 1) {
-            $values[$id][$field_name][$langcode] = $row[reset($columns)];
+            $column_name = reset($columns);
+            $column_attributes = $definition_columns[key($columns)];
+            $values[$id][$field_name][$langcode] = (!empty($column_attributes['serialize'])) ? unserialize($row[$column_name]) : $row[$column_name];
           }
           else {
             foreach ($columns as $property_name => $column_name) {
-              $values[$id][$field_name][$langcode][$property_name] = $row[$column_name];
+              $column_attributes = $definition_columns[$property_name];
+              $values[$id][$field_name][$langcode][$property_name] = (!empty($column_attributes['serialize'])) ? unserialize($row[$column_name]) : $row[$column_name];
             }
           }
         }

core/modules/field/tests/modules/field_test/src/Plugin/Field/FieldType/TestObjectItem.php

@@ -46,17 +46,4 @@ public static function schema(FieldStorageDefinitionInterface $field_definition)
     ];
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function setValue($values, $notify = TRUE) {
-    if (isset($values['value'])) {
-      // @todo Remove this in https://www.drupal.org/node/2788637.
-      if (is_string($values['value'])) {
-        $values['value'] = unserialize($values['value']);
-      }
-    }
-    parent::setValue($values, $notify);
-  }
-
 }

core/modules/link/src/Plugin/Field/FieldType/LinkItem.php

@@ -186,11 +186,10 @@ public function setValue($values, $notify = TRUE) {
         'options' => [],
       ];
     }
-    // Unserialize the values.
-    // @todo The storage controller should take care of this, see
-    //   SqlContentEntityStorage::loadFieldItems, see
-    //   https://www.drupal.org/node/2414835
+    // Unserialize the values, this is deprecated as the storage takes care of
+    // this, options must not be passed as a string anymore.
     if (is_string($values['options'])) {
+      @trigger_error('Support for passing options as a serialized string is deprecated in 8.7.0 and will be removed before Drupal 9.0.0. Pass them as an array instead. See https://www.drupal.org/node/2961643.', E_USER_DEPRECATED);
       if (version_compare(PHP_VERSION, '7.0.0', '>=')) {
         $values['options'] = unserialize($values['options'], ['allowed_classes' => FALSE]);
       }

core/modules/link/tests/src/Kernel/LinkItemTest.php

@@ -138,11 +138,11 @@ public function testLinkItem() {
     $this->assertNull($entity->field_test->title);
     $this->assertIdentical($entity->field_test->options, []);
 
-    // Check that if set uri and serialize options then the default values are
-    // properly initialized.
+    // Check that if we set uri and options then the default values are properly
+    // initialized.
     $entity->field_test = [
       'uri' => 'internal:/node/add',
-      'options' => serialize(['query' => NULL]),
+      'options' => ['query' => NULL],
     ];
     $this->assertEqual($entity->field_test->uri, 'internal:/node/add');
     $this->assertNull($entity->field_test->title);
@@ -174,4 +174,23 @@ public function testLinkItem() {
     $this->entityValidateAndSave($entity);
   }
 
+  /**
+   * Tests the deprecated behavior of LinkItem::setValue().
+   *
+   * @group legacy
+   * @expectedDeprecation Support for passing options as a serialized string is deprecated in 8.7.0 and will be removed before Drupal 9.0.0. Pass them as an array instead. See https://www.drupal.org/node/2961643.
+   */
+  public function testSerializedOptions() {
+    // Check that if we set uri and options then the default values are
+    // properly initialized.
+    $entity = EntityTest::create();
+    $entity->set('field_test', [
+      'uri' => 'internal:/node/add',
+      'options' => serialize(['query' => NULL]),
+    ]);
+    $this->assertEquals('internal:/node/add', $entity->get('field_test')->uri);
+    $this->assertNull($entity->get('field_test')->title);
+    $this->assertNull($entity->get('field_test')->options['query']);
+  }
+
 }