Commit da74f223 authored by alexpott's avatar alexpott

Issue #2768651 by damiankloip, Wim Leers, mpdonadio, Munavijayalakshmi, Jo...

Issue #2768651 by damiankloip, Wim Leers, mpdonadio, Munavijayalakshmi, Jo Fitzgerald, jhedstrom: Let TimestampItem (de)normalize to/from RFC3339 timestamps, not UNIX timestamps, for better DX
parent f3b477f0
......@@ -17,6 +17,11 @@ services:
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:
# Priority must be higher than serializer.normalizer.field_item.hal.
- { name: normalizer, priority: 20, bc: bc_timestamp_normalizer_unix, bc_config_name: 'serialization.settings' }
serializer.normalizer.entity.hal:
class: Drupal\hal\Normalizer\ContentEntityNormalizer
arguments: ['@hal.link_manager', '@entity.manager', '@module_handler']
......
......@@ -21,25 +21,13 @@ class FieldItemNormalizer extends NormalizerBase {
* {@inheritdoc}
*/
public function normalize($field_item, $format = NULL, array $context = []) {
$values = [];
// We normalize each individual property, so each can do their own casting,
// if needed.
/** @var \Drupal\Core\TypedData\TypedDataInterface $property */
foreach ($field_item as $property_name => $property) {
$values[$property_name] = $this->serializer->normalize($property, $format, $context);
}
if (isset($context['langcode'])) {
$values['lang'] = $context['langcode'];
}
// The values are wrapped in an array, and then wrapped in another array
// keyed by field name so that field items can be merged by the
// FieldNormalizer. This is necessary for the EntityReferenceItemNormalizer
// to be able to place values in the '_links' array.
$field = $field_item->getParent();
return [
$field->getName() => [$values],
$field->getName() => [$this->normalizedFieldValues($field_item, $format, $context)],
];
}
......@@ -85,6 +73,35 @@ protected function constructValue($data, $context) {
return $data;
}
/**
* Normalizes field values for an item.
*
* @param \Drupal\Core\Field\FieldItemInterface $field_item
* The field item instance.
* @param string|null $format
* The normalization format.
* @param array $context
* The context passed into the normalizer.
*
* @return array
* An array of field item values, keyed by property name.
*/
protected function normalizedFieldValues(FieldItemInterface $field_item, $format, array $context) {
$denormalized = [];
// We normalize each individual property, so each can do their own casting,
// if needed.
/** @var \Drupal\Core\TypedData\TypedDataInterface $property */
foreach ($field_item as $property_name => $property) {
$denormalized[$property_name] = $this->serializer->normalize($property, $format, $context);
}
if (isset($context['langcode'])) {
$denormalized['lang'] = $context['langcode'];
}
return $denormalized;
}
/**
* Get a translated version of the field item instance.
*
......
<?php
namespace Drupal\hal\Normalizer;
use Drupal\Core\Field\FieldItemInterface;
use Drupal\Core\Field\Plugin\Field\FieldType\TimestampItem;
use Drupal\serialization\Normalizer\TimeStampItemNormalizerTrait;
/**
* Converts values for TimestampItem to and from common formats for hal.
*/
class TimestampItemNormalizer extends FieldItemNormalizer {
use TimeStampItemNormalizerTrait;
/**
* The interface or class that this Normalizer supports.
*
* @var string
*/
protected $supportedInterfaceOrClass = TimestampItem::class;
/**
* {@inheritdoc}
*/
protected function normalizedFieldValues(FieldItemInterface $field_item, $format, array $context) {
$normalized = parent::normalizedFieldValues($field_item, $format, $context);
return $this->processNormalizedValues($normalized);
}
}
......@@ -34,11 +34,11 @@ class NodeHalJsonAnonTest extends NodeResourceTestBase {
* {@inheritdoc}
*/
protected static $patchProtectedFieldNames = [
'revision_timestamp',
'created',
'changed',
'promote',
'sticky',
'revision_timestamp',
'revision_uid',
];
......
......@@ -43,7 +43,7 @@ public function testNodeTranslation() {
$node = Node::create([
'title' => $this->randomMachineName(),
'uid' => $user->id(),
'uid' => (int) $user->id(),
'type' => $node_type->id(),
'status' => NodeInterface::PUBLISHED,
'langcode' => 'en',
......
<?php
namespace Drupal\Tests\rest\Functional;
/**
* Trait for ResourceTestBase subclasses formatting expected timestamp data.
*/
trait BcTimestampNormalizerUnixTestTrait {
/**
* Formats a UNIX timestamp.
*
* Depending on the 'bc_timestamp_normalizer_unix' setting. The return will be
* an RFC3339 date string or the same timestamp that was passed in.
*
* @param int $timestamp
* The timestamp value to format.
*
* @return string|int
* The formatted RFC3339 date string or UNIX timestamp.
*
* @see \Drupal\serialization\Normalizer\TimestampItemNormalizer
*/
protected function formatExpectedTimestampItemValues($timestamp) {
// If the setting is enabled, just return the timestamp as-is now.
if ($this->config('serialization.settings')->get('bc_timestamp_normalizer_unix')) {
return ['value' => $timestamp];
}
// Otherwise, format the date string to the same that
// \Drupal\serialization\Normalizer\TimestampItemNormalizer will produce.
$date = new \DateTime();
$date->setTimestamp($timestamp);
$date->setTimezone(new \DateTimeZone('UTC'));
// Format is also added to the expected return values.
return [
'value' => $date->format(\DateTime::RFC3339),
'format' => \DateTime::RFC3339,
];
}
}
......@@ -6,13 +6,14 @@
use Drupal\comment\Entity\CommentType;
use Drupal\comment\Tests\CommentTestTrait;
use Drupal\entity_test\Entity\EntityTest;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\user\Entity\User;
use GuzzleHttp\RequestOptions;
abstract class CommentResourceTestBase extends EntityResourceTestBase {
use CommentTestTrait;
use CommentTestTrait, BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
......@@ -148,14 +149,10 @@ protected function getExpectedNormalizedEntity() {
],
],
'created' => [
[
'value' => 123456789,
],
$this->formatExpectedTimestampItemValues(123456789),
],
'changed' => [
[
'value' => $this->entity->getChangedTime(),
],
$this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
],
'default_langcode' => [
[
......
......@@ -465,9 +465,9 @@ public function testGet() {
// for the keys with the array order the same (it needs to match with
// identical comparison).
$expected = $this->getExpectedNormalizedEntity();
ksort($expected);
static::recursiveKSort($expected);
$actual = $this->serializer->decode((string) $response->getBody(), static::$format);
ksort($actual);
static::recursiveKSort($actual);
$this->assertSame($expected, $actual);
// Not only assert the normalization, also assert deserialization of the
......@@ -507,12 +507,14 @@ public function testGet() {
}
$this->assertSame($get_headers, $head_headers);
// BC: serialization_update_8302().
// Only run this for fieldable entities. It doesn't make sense for config
// entities as config values are already casted. They also run through the
// ConfigEntityNormalizer, which doesn't deal with fields individually.
if ($this->entity instanceof FieldableEntityInterface) {
// Test primitive data casting BC (strings).
$this->config('serialization.settings')->set('bc_primitives_as_strings', TRUE)->save(TRUE);
// Rebuild the container so new config is reflected in the removal of the
// Rebuild the container so new config is reflected in the addition of the
// PrimitiveDataNormalizer.
$this->rebuildAll();
......@@ -528,10 +530,48 @@ public function testGet() {
// Config entities are not affected.
// @see \Drupal\serialization\Normalizer\ConfigEntityNormalizer::normalize()
$expected = static::castToString($expected);
ksort($expected);
static::recursiveKSort($expected);
$actual = $this->serializer->decode((string) $response->getBody(), static::$format);
ksort($actual);
static::recursiveKSort($actual);
$this->assertSame($expected, $actual);
// Reset the config value and rebuild.
$this->config('serialization.settings')->set('bc_primitives_as_strings', FALSE)->save(TRUE);
$this->rebuildAll();
}
// BC: serialization_update_8401().
// Only run this for fieldable entities. It doesn't make sense for config
// entities as config values always use the raw values (as per the config
// schema), returned directly from the ConfigEntityNormalizer, which
// doesn't deal with fields individually.
if ($this->entity instanceof FieldableEntityInterface) {
// Test the BC settings for timestamp values.
$this->config('serialization.settings')->set('bc_timestamp_normalizer_unix', TRUE)->save(TRUE);
// Rebuild the container so new config is reflected in the addition of the
// TimestampItemNormalizer.
$this->rebuildAll();
$response = $this->request('GET', $url, $request_options);
$this->assertResourceResponse(200, FALSE, $response);
// This ensures the BC layer for bc_timestamp_normalizer_unix works as
// expected. This method should be using
// ::formatExpectedTimestampValue() to generate the timestamp value. This
// will take into account the above config setting.
$expected = $this->getExpectedNormalizedEntity();
// Config entities are not affected.
// @see \Drupal\serialization\Normalizer\ConfigEntityNormalizer::normalize()
static::recursiveKSort($expected);
$actual = $this->serializer->decode((string) $response->getBody(), static::$format);
static::recursiveKSort($actual);
$this->assertSame($expected, $actual);
// Reset the config value and rebuild.
$this->config('serialization.settings')->set('bc_timestamp_normalizer_unix', FALSE)->save(TRUE);
$this->rebuildAll();
}
......@@ -634,6 +674,27 @@ protected static function castToString(array $normalization) {
return $normalization;
}
/**
* Recursively sorts an array by key.
*
* @param array $array
* An array to sort.
*
* @return array
* The sorted array.
*/
protected static function recursiveKSort(array &$array) {
// First, sort the main array.
ksort($array);
// Then check for child arrays.
foreach ($array as $key => &$value) {
if (is_array($value)) {
static::recursiveKSort($value);
}
}
}
/**
* Tests a POST request for an entity, plus edge cases to ensure good DX.
*/
......@@ -968,15 +1029,19 @@ public function testPatch() {
// DX: 403 when sending PATCH request with read-only fields.
foreach (static::$patchProtectedFieldNames as $field_name) {
$normalization = $this->getNormalizedPatchEntity() + [$field_name => [['value' => $this->randomString()]]];
$request_options[RequestOptions::BODY] = $this->serializer->serialize($normalization, static::$format);
// First send all fields (the "maximum normalization"). Assert the expected
// error message for the first PATCH-protected field. Remove that field from
// the normalization, send another request, assert the next PATCH-protected
// field error message. And so on.
$max_normalization = $this->getNormalizedPatchEntity() + $this->serializer->normalize($this->entity, static::$format);
for ($i = 0; $i < count(static::$patchProtectedFieldNames); $i++) {
$max_normalization = $this->removeFieldsFromNormalization($max_normalization, array_slice(static::$patchProtectedFieldNames, 0, $i));
$request_options[RequestOptions::BODY] = $this->serializer->serialize($max_normalization, static::$format);
$response = $this->request('PATCH', $url, $request_options);
$this->assertResourceErrorResponse(403, "Access denied on updating field '$field_name'.", $response);
$this->assertResourceErrorResponse(403, "Access denied on updating field '" . static::$patchProtectedFieldNames[$i] . "'.", $response);
}
// 200 for well-formed request that sends the maximum number of fields.
$max_normalization = $this->getNormalizedPatchEntity() + $this->serializer->normalize($this->entity, static::$format);
$max_normalization = $this->removeFieldsFromNormalization($max_normalization, static::$patchProtectedFieldNames);
$request_options[RequestOptions::BODY] = $this->serializer->serialize($max_normalization, static::$format);
$response = $this->request('PATCH', $url, $request_options);
......
......@@ -3,11 +3,14 @@
namespace Drupal\Tests\rest\Functional\EntityResource\EntityTest;
use Drupal\entity_test\Entity\EntityTest;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\user\Entity\User;
abstract class EntityTestResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -92,9 +95,7 @@ protected function getExpectedNormalizedEntity() {
]
],
'created' => [
[
'value' => (int) $this->entity->get('created')->value,
]
$this->formatExpectedTimestampItemValues((int) $this->entity->get('created')->value)
],
'user_id' => [
[
......
......@@ -3,11 +3,14 @@
namespace Drupal\Tests\rest\Functional\EntityResource\EntityTestLabel;
use Drupal\entity_test\Entity\EntityTestLabel;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\user\Entity\User;
abstract class EntityTestLabelResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -94,9 +97,7 @@ protected function getExpectedNormalizedEntity() {
],
],
'created' => [
[
'value' => (int) $this->entity->get('created')->value,
],
$this->formatExpectedTimestampItemValues((int) $this->entity->get('created')->value),
],
'user_id' => [
[
......
......@@ -2,11 +2,14 @@
namespace Drupal\Tests\rest\Functional\EntityResource\Feed;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityTest\EntityTestResourceTestBase;
use Drupal\aggregator\Entity\Feed;
abstract class FeedResourceTestBase extends EntityTestResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -92,14 +95,10 @@ protected function getExpectedNormalizedEntity() {
]
],
'checked' => [
[
'value' => 123456789
]
$this->formatExpectedTimestampItemValues(123456789),
],
'queued' => [
[
'value' => 123456789
]
$this->formatExpectedTimestampItemValues(123456789),
],
'link' => [
[
......@@ -127,9 +126,7 @@ protected function getExpectedNormalizedEntity() {
]
],
'modified' => [
[
'value' => 123456789
]
$this->formatExpectedTimestampItemValues(123456789),
],
];
}
......
......@@ -4,6 +4,7 @@
use Drupal\aggregator\Entity\Feed;
use Drupal\aggregator\Entity\Item;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
/**
......@@ -11,6 +12,8 @@
*/
abstract class ItemResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -113,9 +116,7 @@ protected function getExpectedNormalizedEntity() {
'author' => [],
'description' => [],
'timestamp' => [
[
'value' => 123456789,
],
$this->formatExpectedTimestampItemValues(123456789),
],
'guid' => [],
];
......
......@@ -3,6 +3,7 @@
namespace Drupal\Tests\rest\Functional\EntityResource\MenuLinkContent;
use Drupal\menu_link_content\Entity\MenuLinkContent;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
/**
......@@ -10,6 +11,8 @@
*/
abstract class MenuLinkContentResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -161,9 +164,7 @@ protected function getExpectedNormalizedEntity() {
],
],
'changed' => [
[
'value' => $this->entity->getChangedTime(),
],
$this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
],
'default_langcode' => [
[
......
......@@ -4,11 +4,14 @@
use Drupal\node\Entity\Node;
use Drupal\node\Entity\NodeType;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\user\Entity\User;
abstract class NodeResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -23,12 +26,12 @@ abstract class NodeResourceTestBase extends EntityResourceTestBase {
* {@inheritdoc}
*/
protected static $patchProtectedFieldNames = [
'revision_timestamp',
'revision_uid',
'created',
'changed',
'promote',
'sticky',
'revision_timestamp',
'revision_uid',
];
/**
......@@ -119,14 +122,10 @@ protected function getExpectedNormalizedEntity() {
],
],
'created' => [
[
'value' => 123456789,
],
$this->formatExpectedTimestampItemValues(123456789),
],
'changed' => [
[
'value' => $this->entity->getChangedTime(),
],
$this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
],
'promote' => [
[
......@@ -139,9 +138,7 @@ protected function getExpectedNormalizedEntity() {
],
],
'revision_timestamp' => [
[
'value' => 123456789,
],
$this->formatExpectedTimestampItemValues(123456789),
],
'revision_translation_affected' => [
[
......
......@@ -4,10 +4,13 @@
use Drupal\taxonomy\Entity\Term;
use Drupal\taxonomy\Entity\Vocabulary;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
abstract class TermResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -107,9 +110,7 @@ protected function getExpectedNormalizedEntity() {
],
],
'changed' => [
[
'value' => $this->entity->getChangedTime(),
],
$this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
],
'default_langcode' => [
[
......
......@@ -3,12 +3,15 @@
namespace Drupal\Tests\rest\Functional\EntityResource\User;
use Drupal\Core\Url;
use Drupal\Tests\rest\Functional\BcTimestampNormalizerUnixTestTrait;
use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\user\Entity\User;
use GuzzleHttp\RequestOptions;
abstract class UserResourceTestBase extends EntityResourceTestBase {
use BcTimestampNormalizerUnixTestTrait;
/**
* {@inheritdoc}
*/
......@@ -98,14 +101,10 @@ protected function getExpectedNormalizedEntity() {
],
],
'created' => [
[
'value' => 123456789,
],
$this->formatExpectedTimestampItemValues(123456789),
],
'changed' => [
[
'value' => $this->entity->getChangedTime(),
],
$this->formatExpectedTimestampItemValues($this->entity->getChangedTime()),
],
'default_langcode' => [
[
......
......@@ -2,3 +2,10 @@
# this was usually returned from database storage. A primitive data normalizer
# has been introduced to get the casted value instead.
bc_primitives_as_strings: false
# Before Drupal 8.3, timestamps were always returned as Unix timestamps, which
# are not a universal format for interchange. Now, RFC3339 timestamps are
# returned. New Drupal installations opt out from this by default (hence this
# is set to false), existing installations opt in to it.
# @see serialization_update_8301()
# @see https://www.drupal.org/node/2768651
bc_timestamp_normalizer_unix: false
......@@ -5,3 +5,6 @@ serialization.settings:
bc_primitives_as_strings:
type: boolean
label: 'Whether to retain pre Drupal 8.3 behavior of serializing all primitive items as strings.'
bc_timestamp_normalizer_unix:
type: boolean
label: 'Whether the pre Drupal 8.4 behavior of returning Unix timestamps instead of RFC3339 timestamps for TimestampItem fields is enabled or not.'
......@@ -46,3 +46,12 @@ function serialization_update_8302() {
return t('The REST API will no longer output all values as strings. Integers/booleans will be used where appropriate. If your site depends on these value being strings, <a href="https://www.drupal.org/node/2837696">read the change record to learn how to enable the BC mode.</a>');
}
/**
* Enable BC for timestamp formatting: continue to return UNIX timestamps.
*/
function serialization_update_8401() {
$config_factory = \Drupal::configFactory();
$serialization_settings = $config_factory->getEditable('serialization.settings');
$serialization_settings->set('bc_timestamp_normalizer_unix', TRUE)->save(TRUE);
}
......@@ -52,6 +52,12 @@ services:
# Priority must be higher than serialization.normalizer.field but less
# than hal field normalizer.
- { name: normalizer, priority: 9 }
serializer.normalizer.timestamp_item:
class: Drupal\serialization\Normalizer\TimestampItemNormalizer
tags:
# Priority must be higher than serializer.normalizer.field_item and lower
# than hal normalizers.
- { name: normalizer, priority: 8, bc: bc_timestamp_normalizer_unix, bc_config_name: 'serialization.settings' }
serializer.normalizer.password_field_item:
class: Drupal\serialization\Normalizer\NullNormalizer
arguments: ['Drupal\Core\Field\Plugin\Field\FieldType\PasswordItem']
......
......@@ -46,7 +46,7 @@ public function onConfigSave(ConfigCrudEvent $event) {
$saved_config = $event->getConfig();
if ($saved_config->getName() === 'serialization.settings') {
if ($event->isChanged('bc_primitives_as_strings')) {
if ($event->isChanged('bc_primitives_as_strings') || $event->isChanged('bc_timestamp_normalizer_unix')) {
$this->kernel->invalidateContainer();
}
}
......
......@@ -40,13 +40,20 @@ public function denormalize($data, $class, $format = NULL, array $context = [])
// The bundle property will be required to denormalize a bundleable
// fieldable entity.
if ($entity_type_definition->hasKey('bundle') && $entity_type_definition->isSubclassOf(FieldableEntityInterface::class)) {
// Get an array containing the bundle only. This also remove the bundle
// key from the $data array.
$bundle_data = $this->extractBundleData($data, $entity_type_definition);
if ($entity_type_definition->isSubclassOf(FieldableEntityInterface::class)) {
// Extract bundle data to pass into entity creation if the entity type uses
// bundles.
if ($entity_type_definition->hasKey('bundle')) {
// Get an array containing the bundle only. This also remove the bundle
// key from the $data array.
$create_params = $this->extractBundleData($data, $entity_type_definition);
}
else {
$create_params = [];
}
// Create the entity from bundle data only, then apply field values after.
$entity = $this->entityManager->getStorage($entity_type_id)->create($bundle_data);
$entity = $this->entityManager->getStorage($entity_type_id)->create($create_params);
$this->denormalizeFieldData($data, $entity, $format, $context);
}
......