Issue #2825487 by damiankloip, Wim Leers, garphy, cburschka, tedbow, dpovshed,...

Issue #2825487 by damiankloip, Wim Leers, garphy, cburschka, tedbow, dpovshed, tstoeckler, Munavijayalakshmi, Berdir, dawehner, e0ipso: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field

core/modules/file/file.module

@@ -48,6 +48,16 @@ function file_help($route_name, RouteMatchInterface $route_match) {
   }
 }
 
+/**
+ * Implements hook_field_widget_info_alter().
+ */
+function file_field_widget_info_alter(array &$info) {
+  // Allows using the 'uri' widget for the 'file_uri' field type, which uses it
+  // as the default widget.
+  // @see \Drupal\file\Plugin\Field\FieldType\FileUriItem
+  $info['uri']['field_types'][] = 'file_uri';
+}
+
 /**
  * Loads file entities from the database.
  *

core/modules/file/src/ComputedFileUrl.php

+<?php
+
+namespace Drupal\file;
+
+use Drupal\Core\TypedData\TypedData;
+
+/**
+ * Computed file URL property class.
+ */
+class ComputedFileUrl extends TypedData {
+
+  /**
+   * Computed root-relative file URL.
+   *
+   * @var string
+   */
+  protected $url = NULL;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getValue() {
+    if ($this->url !== NULL) {
+      return $this->url;
+    }
+
+    assert($this->getParent()->getEntity() instanceof FileInterface);
+
+    $uri = $this->getParent()->getEntity()->getFileUri();
+    $this->url = file_url_transform_relative(file_create_url($uri));
+
+    return $this->url;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setValue($value, $notify = TRUE) {
+    $this->url = $value;
+
+    // Notify the parent of any changes.
+    if ($notify && isset($this->parent)) {
+      $this->parent->onChange($this->name);
+    }
+  }
+
+}

core/modules/file/src/Entity/File.php

@@ -243,7 +243,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
       ->setLabel(t('Filename'))
       ->setDescription(t('Name of the file with no path components.'));
 
-    $fields['uri'] = BaseFieldDefinition::create('uri')
+    $fields['uri'] = BaseFieldDefinition::create('file_uri')
       ->setLabel(t('URI'))
       ->setDescription(t('The URI to access the file (either local or remote).'))
       ->setSetting('max_length', 255)

core/modules/file/src/Plugin/Field/FieldFormatter/FileUriFormatter.php

@@ -13,7 +13,8 @@
  *   id = "file_uri",
  *   label = @Translation("File URI"),
  *   field_types = {
- *     "uri"
+ *     "uri",
+ *     "file_uri",
  *   }
  * )
  */

core/modules/file/src/Plugin/Field/FieldType/FileUriItem.php

+<?php
+
+namespace Drupal\file\Plugin\Field\FieldType;
+
+use Drupal\Core\Field\FieldStorageDefinitionInterface;
+use Drupal\Core\Field\Plugin\Field\FieldType\UriItem;
+use Drupal\Core\TypedData\DataDefinition;
+use Drupal\file\ComputedFileUrl;
+
+/**
+ * File-specific plugin implementation of a URI item to provide a full URL.
+ *
+ * @FieldType(
+ *   id = "file_uri",
+ *   label = @Translation("File URI"),
+ *   description = @Translation("An entity field containing a file URI, and a computed root-relative file URL."),
+ *   no_ui = TRUE,
+ *   default_formatter = "file_uri",
+ *   default_widget = "uri",
+ * )
+ */
+class FileUriItem extends UriItem {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
+    $properties = parent::propertyDefinitions($field_definition);
+
+    $properties['url'] = DataDefinition::create('string')
+      ->setLabel(t('Root-relative file URL'))
+      ->setComputed(TRUE)
+      ->setInternal(FALSE)
+      ->setClass(ComputedFileUrl::class);
+
+    return $properties;
+  }
+
+}

core/modules/file/tests/src/Kernel/ComputedFileUrlTest.php

+<?php
+
+namespace Drupal\Tests\file\Kernel;
+
+use Drupal\Core\Field\FieldItemInterface;
+use Drupal\Core\TypedData\DataDefinitionInterface;
+use Drupal\file\FileInterface;
+use Drupal\file\ComputedFileUrl;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * @coversDefaultClass \Drupal\file\ComputedFileUrl
+ *
+ * @group file
+ */
+class ComputedFileUrlTest extends KernelTestBase {
+
+  /**
+   * The test URL to use.
+   *
+   * @var string
+   */
+  protected $testUrl = 'public://druplicon.txt';
+
+  /**
+   * @covers ::getValue
+   */
+  public function testGetValue() {
+    $entity = $this->prophesize(FileInterface::class);
+    $entity->getFileUri()
+      ->willReturn($this->testUrl);
+
+    $parent = $this->prophesize(FieldItemInterface::class);
+    $parent->getEntity()
+      ->shouldBeCalledTimes(2)
+      ->willReturn($entity->reveal());
+
+    $definition = $this->prophesize(DataDefinitionInterface::class);
+
+    $typed_data = new ComputedFileUrl($definition->reveal(), $this->randomMachineName(), $parent->reveal());
+
+    $expected = base_path() . $this->siteDirectory . '/files/druplicon.txt';
+
+    $this->assertSame($expected, $typed_data->getValue());
+    // Do this a second time to confirm the same value is returned but the value
+    // isn't retrieved from the parent entity again.
+    $this->assertSame($expected, $typed_data->getValue());
+  }
+
+  /**
+   * @covers ::setValue
+   */
+  public function testSetValue() {
+    $name = $this->randomMachineName();
+    $parent = $this->prophesize(FieldItemInterface::class);
+    $parent->onChange($name)
+      ->shouldBeCalled();
+
+    $definition = $this->prophesize(DataDefinitionInterface::class);
+    $typed_data = new ComputedFileUrl($definition->reveal(), $name, $parent->reveal());
+
+    // Setting the value explicitly should mean the parent entity is never
+    // called into.
+    $typed_data->setValue($this->testUrl);
+
+    $this->assertSame($this->testUrl, $typed_data->getValue());
+    // Do this a second time to confirm the same value is returned but the value
+    // isn't retrieved from the parent entity again.
+    $this->assertSame($this->testUrl, $typed_data->getValue());
+  }
+
+  /**
+   * @covers ::setValue
+   */
+  public function testSetValueNoNotify() {
+    $name = $this->randomMachineName();
+    $parent = $this->prophesize(FieldItemInterface::class);
+    $parent->onChange($name)
+      ->shouldNotBeCalled();
+
+    $definition = $this->prophesize(DataDefinitionInterface::class);
+    $typed_data = new ComputedFileUrl($definition->reveal(), $name, $parent->reveal());
+
+    // Setting the value should explicitly should mean the parent entity is
+    // never called into.
+    $typed_data->setValue($this->testUrl, FALSE);
+
+    $this->assertSame($this->testUrl, $typed_data->getValue());
+  }
+
+}

core/modules/file/tests/src/Kernel/FileUriItemTest.php

+<?php
+
+namespace Drupal\Tests\file\Kernel;
+
+use Drupal\file\Entity\File;
+
+/**
+ * File URI field item test.
+ *
+ * @group file
+ *
+ * @see \Drupal\file\Plugin\Field\FieldType\FileUriItem
+ * @see \Drupal\file\FileUrl
+ */
+class FileUriItemTest extends FileManagedUnitTestBase {
+
+  /**
+   * Tests the file entity override of the URI field.
+   */
+  public function testCustomFileUriField() {
+    $uri = 'public://druplicon.txt';
+
+    // Create a new file entity.
+    $file = File::create([
+      'uid' => 1,
+      'filename' => 'druplicon.txt',
+      'uri' => $uri,
+      'filemime' => 'text/plain',
+      'status' => FILE_STATUS_PERMANENT,
+    ]);
+    file_put_contents($file->getFileUri(), 'hello world');
+
+    $file->save();
+
+    $this->assertSame($uri, $file->uri->value);
+    $expected_url = base_path() . $this->siteDirectory . '/files/druplicon.txt';
+    $this->assertSame($expected_url, $file->uri->url);
+  }
+
+}

core/modules/hal/config/install/hal.settings.yml

 # Set the domain for HAL type and relation links.
 # If left blank, the site's domain will be used.
 link_domain: ~
+# Before Drupal 8.5, the File entity 'uri' field value was overridden to return
+# the absolute file URL instead of the actual (stream wrapper) URI. The default
+# for new sites is now to return the actual URI as well as a root-relative file
+# URL. Enable this setting to use the previous behavior. For existing sites,
+# the previous behavior is kept by default.
+# @see hal_update_8501()
+# @see https://www.drupal.org/node/2925783
+bc_file_uri_as_url_normalizer: false

core/modules/hal/config/schema/hal.schema.yml

@@ -6,3 +6,6 @@ hal.settings:
     link_domain:
       type: string
       label: 'Domain of the relation'
+    bc_file_uri_as_url_normalizer:
+      type: boolean
+      label: 'Whether to retain pre Drupal 8.5 behavior of normalizing the File entity "uri" field value to an absolute URL.'

core/modules/hal/hal.install

@@ -31,3 +31,15 @@ function hal_update_8301() {
   $hal_settings->set('link_domain', $link_domain);
   $hal_settings->save(TRUE);
 }
+
+/**
+ * Add hal.settings::bc_file_uri_as_url_normalizer configuration.
+ */
+function hal_update_8501() {
+  $config_factory = \Drupal::configFactory();
+  $config_factory->getEditable('hal.settings')
+    ->set('bc_file_uri_as_url_normalizer', TRUE)
+    ->save(TRUE);
+
+  return t('Backwards compatibility mode has been enabled for File entities\' HAL normalization of the "uri" field. Like before, it will continue to return only the absolute file URL. If you want the new behavior, which returns both the stored URI and a root-relative file URL, <a href="https://www.drupal.org/node/2925783">read the change record to learn how to opt in.</a>');
+}

core/modules/hal/hal.services.yml

@@ -14,9 +14,10 @@ services:
       - { name: normalizer, priority: 10 }
   serializer.normalizer.file_entity.hal:
     class: Drupal\hal\Normalizer\FileEntityNormalizer
+    deprecated: 'The "%service_id%" normalizer service is deprecated: it is obsolete, it only remains available for backwards compatibility.'
+    arguments: ['@entity.manager', '@http_client', '@hal.link_manager', '@module_handler', '@config.factory']
     tags:
       - { name: normalizer, priority: 20 }
-    arguments: ['@entity.manager', '@http_client', '@hal.link_manager', '@module_handler']
   serializer.normalizer.timestamp_item.hal:
    class: Drupal\hal\Normalizer\TimestampItemNormalizer
    tags:

core/modules/hal/src/Normalizer/FileEntityNormalizer.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\hal\Normalizer;
 
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\hal\LinkManager\LinkManagerInterface;
@@ -9,6 +10,8 @@
 
 /**
  * Converts the Drupal entity object structure to a HAL array structure.
+ *
+ * @deprecated in Drupal 8.5.0, to be removed before Drupal 9.0.0.
  */
 class FileEntityNormalizer extends ContentEntityNormalizer {
 
@@ -26,6 +29,13 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
    */
   protected $httpClient;
 
+  /**
+   * The HAL settings config.
+   *
+   * @var \Drupal\Core\Config\ImmutableConfig
+   */
+  protected $halSettings;
+
   /**
    * Constructs a FileEntityNormalizer object.
    *
@@ -37,11 +47,14 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
    *   The hypermedia link manager.
    * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
    *   The module handler.
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory.
    */
-  public function __construct(EntityManagerInterface $entity_manager, ClientInterface $http_client, LinkManagerInterface $link_manager, ModuleHandlerInterface $module_handler) {
+  public function __construct(EntityManagerInterface $entity_manager, ClientInterface $http_client, LinkManagerInterface $link_manager, ModuleHandlerInterface $module_handler, ConfigFactoryInterface $config_factory) {
     parent::__construct($link_manager, $entity_manager, $module_handler);
 
     $this->httpClient = $http_client;
+    $this->halSettings = $config_factory->get('hal.settings');
   }
 
   /**
@@ -49,8 +62,13 @@ public function __construct(EntityManagerInterface $entity_manager, ClientInterf
    */
   public function normalize($entity, $format = NULL, array $context = []) {
     $data = parent::normalize($entity, $format, $context);
-    // Replace the file url with a full url for the file.
-    $data['uri'][0]['value'] = $this->getEntityUri($entity);
+
+    $this->addCacheableDependency($context, $this->halSettings);
+
+    if ($this->halSettings->get('bc_file_uri_as_url_normalizer')) {
+      // Replace the file url with a full url for the file.
+      $data['uri'][0]['value'] = $this->getEntityUri($entity);
+    }
 
     return $data;
   }

core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonAnonTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\hal\Functional\EntityResource\File;
 
+use Drupal\Core\Cache\Cache;
 use Drupal\Tests\hal\Functional\EntityResource\HalEntityNormalizationTrait;
 use Drupal\Tests\rest\Functional\AnonResourceTestTrait;
 use Drupal\Tests\rest\Functional\EntityResource\File\FileResourceTestBase;
@@ -38,7 +39,11 @@ protected function getExpectedNormalizedEntity() {
     $normalization = $this->applyHalFieldNormalization($default_normalization);
 
     $url = file_create_url($this->entity->getFileUri());
-    $normalization['uri'][0]['value'] = $url;
+    // @see \Drupal\Tests\hal\Functional\EntityResource\File\FileHalJsonAnonTest::testGetBcUriField()
+    if ($this->config('hal.settings')->get('bc_file_uri_as_url_normalizer')) {
+      $normalization['uri'][0]['value'] = $url;
+    }
+
     $uid = $this->author->id();
 
     return $normalization + [
@@ -90,6 +95,13 @@ protected function getNormalizedPostEntity() {
     ];
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedCacheTags() {
+    return Cache::mergeTags(parent::getExpectedCacheTags(), ['config:hal.settings']);
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -100,6 +112,30 @@ protected function getExpectedCacheContexts() {
     ];
   }
 
+  /**
+   * @see hal_update_8501()
+   */
+  public function testGetBcUriField() {
+    $this->config('hal.settings')->set('bc_file_uri_as_url_normalizer', TRUE)->save(TRUE);
+
+    $this->initAuthentication();
+    $url = $this->getEntityResourceUrl();
+    $url->setOption('query', ['_format' => static::$format]);
+    $request_options = $this->getAuthenticationRequestOptions('GET');
+    $this->provisionEntityResource();
+    $this->setUpAuthorization('GET');
+    $response = $this->request('GET', $url, $request_options);
+    $expected = $this->getExpectedNormalizedEntity();
+    static::recursiveKSort($expected);
+    $actual = $this->serializer->decode((string) $response->getBody(), static::$format);
+    static::recursiveKSort($actual);
+    $this->assertSame($expected, $actual);
+
+    // Explicitly assert that $file->uri->value is an absolute file URL, unlike
+    // the default normalization.
+    $this->assertSame($this->baseUrl . '/' . $this->siteDirectory . '/files/drupal.txt', $actual['uri'][0]['value']);
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/hal/tests/src/Functional/EntityResource/Media/MediaHalJsonAnonTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\hal\Functional\EntityResource\Media;
 
+use Drupal\Core\Cache\Cache;
 use Drupal\file\Entity\File;
 use Drupal\Tests\hal\Functional\EntityResource\HalEntityNormalizationTrait;
 use Drupal\Tests\rest\Functional\AnonResourceTestTrait;
@@ -86,11 +87,6 @@ protected function getExpectedNormalizedEntity() {
               ],
             ],
             'lang' => 'en',
-            'uri' => [
-              [
-                'value' => $file->url(),
-              ],
-            ],
             'uuid' => [
               [
                 'value' => $file->uuid(),
@@ -126,11 +122,6 @@ protected function getExpectedNormalizedEntity() {
               ],
             ],
             'lang' => 'en',
-            'uri' => [
-              [
-                'value' => $thumbnail->url(),
-              ],
-            ],
             'uuid' => [
               [
                 'value' => $thumbnail->uuid(),
@@ -173,4 +164,11 @@ protected function getNormalizedPostEntity() {
     ];
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getExpectedCacheTags() {
+    return Cache::mergeTags(parent::getExpectedCacheTags(), ['config:hal.settings']);
+  }
+
 }

core/modules/hal/tests/src/Functional/FileDenormalizeTest.php

@@ -20,6 +20,20 @@ class FileDenormalizeTest extends BrowserTestBase {
    */
   public static $modules = ['hal', 'file', 'node'];
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    // @todo Remove this work-around in https://www.drupal.org/node/1927648.
+    // @see hal_update_8501()
+    \Drupal::configFactory()
+      ->getEditable('hal.settings')
+      ->set('bc_file_uri_as_url_normalizer', TRUE)
+      ->save(TRUE);
+  }
+
   /**
    * Tests file entity denormalization.
    */

core/modules/hal/tests/src/Kernel/FileNormalizeTest.php

@@ -44,7 +44,10 @@ public function testNormalize() {
 
     $expected_array = [
       'uri' => [
-        ['value' => file_create_url($file->getFileUri())],
+        [
+          'value' => $file->getFileUri(),
+          'url' => file_url_transform_relative(file_create_url($file->getFileUri())),
+        ],
       ],
     ];
 

core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php

@@ -461,8 +461,13 @@ public function testGet() {
     // Note: deserialization of the XML format is not supported, so only test
     // this for other formats.
     if (static::$format !== 'xml') {
-      $unserialized = $this->serializer->deserialize((string) $response->getBody(), get_class($this->entity), static::$format);
-      $this->assertSame($unserialized->uuid(), $this->entity->uuid());
+      // @todo Work-around for HAL's FileEntityNormalizer::denormalize() being
+      // broken, being fixed in https://www.drupal.org/node/1927648, where this
+      // if-test should be removed.
+      if (!(static::$entityTypeId === 'file' && static::$format === 'hal_json')) {
+        $unserialized = $this->serializer->deserialize((string) $response->getBody(), get_class($this->entity), static::$format);
+        $this->assertSame($unserialized->uuid(), $this->entity->uuid());
+      }
     }
     // Finally, assert that the expected 'Link' headers are present.
     if ($this->entity->getEntityType()->getLinkTemplates()) {

core/modules/rest/tests/src/Functional/EntityResource/File/FileResourceTestBase.php

@@ -154,6 +154,7 @@ protected function getExpectedNormalizedEntity() {
       ],
       'uri' => [
         [
+          'url' => base_path() . $this->siteDirectory . '/files/drupal.txt',
           'value' => 'public://drupal.txt',
         ],
       ],

core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

@@ -122,6 +122,7 @@ public static function getSkippedDeprecations() {
       'drupal_set_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead. See https://www.drupal.org/node/2774931',
       'drupal_get_message() is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Messenger\MessengerInterface::all() or \Drupal\Core\Messenger\MessengerInterface::messagesByType() instead. See https://www.drupal.org/node/2774931',
       'Adding or retrieving messages prior to the container being initialized was deprecated in Drupal 8.5.0 and this functionality will be removed before Drupal 9.0.0. Please report this usage at https://www.drupal.org/node/2928994.',
+      'The "serializer.normalizer.file_entity.hal" normalizer service is deprecated: it is obsolete, it only remains available for backwards compatibility.',
     ];
   }