diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 88bc299..0e239b7 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -49,6 +49,17 @@ function file_help($route_name, RouteMatchInterface $route_match) { } /** + * Implements hook_field_widget_info_alter(). + */ +function file_field_widget_info_alter(array &$info) { + // This allows setting a valid default widget in the FileUriItem annotation. + // It's not strictly needed, but it's a required value on the annotation so + // should be a valid value. + // @see \Drupal\file\Plugin\Field\FieldType\FileUriItem + $info['uri']['field_types'][] = 'file_uri'; +} + +/** * Loads file entities from the database. * * @param array|null $fids diff --git a/core/modules/file/src/ComputedFileUrl.php b/core/modules/file/src/ComputedFileUrl.php new file mode 100644 index 0000000..2eb012e --- /dev/null +++ b/core/modules/file/src/ComputedFileUrl.php @@ -0,0 +1,47 @@ +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); + } + } + +} diff --git a/core/modules/file/src/Entity/File.php b/core/modules/file/src/Entity/File.php index a9ade9e..4060b77 100644 --- a/core/modules/file/src/Entity/File.php +++ b/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) diff --git a/core/modules/file/src/Plugin/Field/FieldFormatter/FileUriFormatter.php b/core/modules/file/src/Plugin/Field/FieldFormatter/FileUriFormatter.php index 055e32d..0facb7e 100644 --- a/core/modules/file/src/Plugin/Field/FieldFormatter/FileUriFormatter.php +++ b/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", * } * ) */ diff --git a/core/modules/file/src/Plugin/Field/FieldType/FileUriItem.php b/core/modules/file/src/Plugin/Field/FieldType/FileUriItem.php new file mode 100644 index 0000000..84f0940 --- /dev/null +++ b/core/modules/file/src/Plugin/Field/FieldType/FileUriItem.php @@ -0,0 +1,40 @@ +setLabel(t('Root-relative file URL')) + ->setSetting('case_sensitive', $field_definition->getSetting('case_sensitive')) + ->setComputed(TRUE) + ->setInternal(FALSE) + ->setClass(ComputedFileUrl::class); + + return $properties; + } + +} diff --git a/core/modules/file/tests/src/Kernel/FileUriItemTest.php b/core/modules/file/tests/src/Kernel/FileUriItemTest.php new file mode 100644 index 0000000..d67eba0 --- /dev/null +++ b/core/modules/file/tests/src/Kernel/FileUriItemTest.php @@ -0,0 +1,40 @@ + 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); + } + +} diff --git a/core/modules/file/tests/src/Unit/ComputedFileUrlTest.php b/core/modules/file/tests/src/Unit/ComputedFileUrlTest.php new file mode 100644 index 0000000..8e2a4a4 --- /dev/null +++ b/core/modules/file/tests/src/Unit/ComputedFileUrlTest.php @@ -0,0 +1,91 @@ +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()); + } + +} diff --git a/core/modules/hal/config/install/hal.settings.yml b/core/modules/hal/config/install/hal.settings.yml index 67107af..1c895c1 100644 --- a/core/modules/hal/config/install/hal.settings.yml +++ b/core/modules/hal/config/install/hal.settings.yml @@ -1,3 +1,4 @@ # Set the domain for HAL type and relation links. # If left blank, the site's domain will be used. link_domain: ~ +bc_file_uri_as_url_normalizer: false diff --git a/core/modules/hal/config/schema/hal.schema.yml b/core/modules/hal/config/schema/hal.schema.yml index 3192d67..b70f47e 100644 --- a/core/modules/hal/config/schema/hal.schema.yml +++ b/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 file URI values as a full URL.' diff --git a/core/modules/hal/hal.install b/core/modules/hal/hal.install index 78810e3..0dc3fd5 100644 --- a/core/modules/hal/hal.install +++ b/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('The REST API will no longer return the full URL for file entity URI values for HAL+JSON responses. It will return the URI value itself, with an additional URL property providing a root relative file path. If your site depends on these value being strings, read the change record to learn how to enable the BC mode.'); +} diff --git a/core/modules/hal/hal.services.yml b/core/modules/hal/hal.services.yml index b2c898f..a877163 100644 --- a/core/modules/hal/hal.services.yml +++ b/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: diff --git a/core/modules/hal/src/Normalizer/FileEntityNormalizer.php b/core/modules/hal/src/Normalizer/FileEntityNormalizer.php index ec870e9..d3b1797 100644 --- a/core/modules/hal/src/Normalizer/FileEntityNormalizer.php +++ b/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.x, to be removed before Drupal 9.0.x. */ class FileEntityNormalizer extends ContentEntityNormalizer { @@ -27,6 +30,13 @@ class FileEntityNormalizer extends ContentEntityNormalizer { protected $httpClient; /** + * The hal settings config. + * + * @var \Drupal\Core\Config\Config + */ + protected $halSettings; + + /** * Constructs a FileEntityNormalizer object. * * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager @@ -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; } diff --git a/core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonAnonTest.php b/core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonAnonTest.php index ff89f74..2e605a0 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/File/FileHalJsonAnonTest.php +++ b/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,10 @@ 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') === TRUE) { + $normalization['uri'][0]['value'] = $url; + } $uid = $this->author->id(); return $normalization + [ @@ -93,6 +97,13 @@ protected function getNormalizedPostEntity() { /** * {@inheritdoc} */ + protected function getExpectedCacheTags() { + return Cache::mergeTags(parent::getExpectedCacheTags(), ['config:hal.settings']); + } + + /** + * {@inheritdoc} + */ protected function getExpectedCacheContexts() { return [ 'url.site', @@ -101,6 +112,40 @@ 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(); + $expected += [ + 'field_rest_test_multivalue' => [ + 0 => [ + 'value' => 'One', + ], + 1 => [ + 'value' => 'Two', + ], + ] + ]; + 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} */ public function testPatch() { diff --git a/core/modules/hal/tests/src/Functional/EntityResource/Media/MediaHalJsonAnonTest.php b/core/modules/hal/tests/src/Functional/EntityResource/Media/MediaHalJsonAnonTest.php index c71b54e..c9ee773 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/Media/MediaHalJsonAnonTest.php +++ b/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']); + } + } diff --git a/core/modules/hal/tests/src/Functional/FileDenormalizeTest.php b/core/modules/hal/tests/src/Functional/FileDenormalizeTest.php index 05ee234..dbb708f 100644 --- a/core/modules/hal/tests/src/Functional/FileDenormalizeTest.php +++ b/core/modules/hal/tests/src/Functional/FileDenormalizeTest.php @@ -20,6 +20,17 @@ class FileDenormalizeTest extends BrowserTestBase { */ public static $modules = ['hal', 'file', 'node']; + protected function setUp() { + parent::setUp(); + + // Override the default configuration to the hal BC setting is enabled, to + // return the full URL value as the file URI 'value'. + \Drupal::configFactory() + ->getEditable('hal.settings') + ->set('bc_file_uri_as_url_normalizer', TRUE) + ->save(TRUE); + } + /** * Tests file entity denormalization. */ diff --git a/core/modules/hal/tests/src/Kernel/FileNormalizeTest.php b/core/modules/hal/tests/src/Kernel/FileNormalizeTest.php index c4b67f5..355c09d 100644 --- a/core/modules/hal/tests/src/Kernel/FileNormalizeTest.php +++ b/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())), + ], ], ]; diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 5416d30..2e8d46e 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -473,8 +473,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()) { diff --git a/core/modules/rest/tests/src/Functional/EntityResource/File/FileResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/File/FileResourceTestBase.php index c63853e..1fa5a38 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/File/FileResourceTestBase.php +++ b/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', ], ], diff --git a/core/tests/Drupal/Tests/Listeners/DeprecationListener.php b/core/tests/Drupal/Tests/Listeners/DeprecationListener.php index 80b3d31..35fa2f0 100644 --- a/core/tests/Drupal/Tests/Listeners/DeprecationListener.php +++ b/core/tests/Drupal/Tests/Listeners/DeprecationListener.php @@ -113,6 +113,7 @@ public static function getSkippedDeprecations() { 'Automatically creating the first item for computed fields is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. Use \Drupal\Core\TypedData\ComputedItemListTrait instead.', '"\Drupal\Core\Entity\ContentEntityStorageBase::doLoadRevisionFieldItems()" is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. "\Drupal\Core\Entity\ContentEntityStorageBase::doLoadMultipleRevisionsFieldItems()" should be implemented instead. See https://www.drupal.org/node/2924915.', 'Passing a single revision ID to "\Drupal\Core\Entity\Sql\SqlContentEntityStorage::buildQuery()" is deprecated in Drupal 8.5.x and will be removed before Drupal 9.0.0. An array of revision IDs should be given instead. See https://www.drupal.org/node/2924915.', + 'The "serializer.normalizer.file_entity.hal" normalizer service is deprecated: it is obsolete, it only remains available for backwards compatibility.', ]; }