TypedData 'Any' can't be normalized to array if it stores an object

The any use TypedDataNormalizer to normalize data
If the getValue return object
add checking:wherher the object could be further normalized

symfony's Serializer's normalzer()'s design dont's will recursive normalize object and array, and don't allow object which can't be normalizable(didn't implement TypedDataInterface or it's child interface) to pass
So the patch Follow and take full advantage of the symfony's serializer's normalzer()'s design
Then when data which is unnormalizable object or has unnormalizable daughter object stores to a typed data,It must be normalizable
So,I then create a commerce issue about commerce order's adjustment as a demo:https://www.drupal.org/node/2916252

1. add test-only.patch
2. 2915705-10.patch :
   1. change comment of TypedDataNormalizer->normalize()
      

      object value of array's child object must be normalizable.

      to

      object must be typed data or instanceof Traversable.

   2. add test code

@tedbow Thanks for reviewing the code and so many improvable advice.

1. I corrected the non-standard code.
2. Rename the AnyDataNormalizeTest.php to AnyTypedDataNormalizeTest.php to be more significative.

Make one English sentence into one line

change comment instanceof Traversable to traversable

The serializer.php->normalize() has the ability to check whether the object is traversable,

       if (is_array($data) || $data instanceof \Traversable) {
            $normalized = array();
            foreach ($data as $key => $val) {
                $normalized[$key] = $this->normalize($val, $format, $context);
            }
            return $normalized;
        }

If the object is not traversable or typed data, It will give an error reminder to make module make corresponding modification.

If we add check "instanceof \Traversable" in TypedDataNormalizer.php, it will avoid the error reminder,
But it can't check one situation: when the object which isn't traversable appear in an array.
So check "instanceof \Traversable" in TypedDataNormalizer.php is inapposite.

@Wim Leers Thanks for the great advice. I make some change correspond #18

1. Create AnyNormalizer
2. add comment about how to make denormalize work
3. apply the newest drupal8.5.x code change

@Wim Leers Thanks for your patient guidance.

Change:

1. Optimize AnyNormalizer's comment
2. Add test testNormalizeNonNormalizableObject and testNormalizeEntity

Reply:

1. But the if-test only tests is_object(). Shouldn't that then test if ($value instanceof Traversable || $value instanceof TypedDataInterface)?
   Also, why can't we do only $value instanceof Traversable?

   AnyNomarlizer use Symfony\Component\Serializer

       public function normalize($data, $format = null, array $context = array())
       {
           // If a normalizer supports the given data, use it
           if ($normalizer = $this->getNormalizer($data, $format)) {
               return $normalizer->normalize($data, $format, $context);
           }
   
           if (null === $data || is_scalar($data)) {
               return $data;
           }
   
           if (is_array($data) || $data instanceof \Traversable) {
               $normalized = array();
               foreach ($data as $key => $val) {
                   $normalized[$key] = $this->normalize($val, $format, $context);
               }
   
               return $normalized;
           }
   
           if (is_object($data)) {
               if (!$this->normalizers) {
                   throw new LogicException('You must register at least one normalizer to be able to normalize objects.');
               }
   
               throw new UnexpectedValueException(sprintf('Could not normalize object of type %s, no supporting normalizer found.', get_class($data)));
           }
   
           throw new UnexpectedValueException(sprintf('An unexpected value could not be normalized: %s', var_export($data, true)));
       }
   

   The Symfony\Component\Serializer->normalize()
   will help to check if the value has corresponding normalizer, traversable or is an array.
   What AnyNormalzer should be down for object is to
   Continue normalizing object which can be normalized
   Of cause an non-normalizable object will come to an UnexpectedValueException.

   If we want to make it less dangerous. some possible resolution:

   1. Check if the object is normalizable in AnyNormalizer()
      

              if ($normalizer = $this->getNormalizer($data, $format)) {
                  return $normalizer->normalize($data, $format, $context);
              }
      

      This will add a repeated loop which has been checked in Symfony\Component\Serializer->normalize, low performance, deprecated.

   2. use
      ($value instanceof Traversable || $value instanceof TypedDataInterface || $value instanceof EntityInterface)
      These can't include all normalizable object, and these will prevent extension for some custom normalizer which want to nomralize special value, deprecated.

   After considering performance, full functional, expansibility, code quality, and risk
   I suggest don't change this part
   isset($value) && (is_object($value) || is_array($value))

2. I don't understand this "must convert the array value to object in setValue() to make it can be denormalized" at all. Can you point to an example of that?

   What's a real world use case for this?

   I change these comment to

   Field type with 'any' typed data property definition which stored traversable object, is advised to convert posted value to object in setValue() to make it can be denormalized easily without writing custom code to convert array to object when using api.
   

   And the real use is using RestApi or jsonapi to post a commerce order.
   AdjustmentIterm.php
   Comemrce order's commerce_adjustment field define an 'any' typed data with Adjustment object stored.
   When use jsonapi to post data to commerce order, If add the transaction code in setValue(),
   https://www.drupal.org/files/issues/2916252-51.patch

   +    // Used for denormalization.
   +    if (is_array($values)) {
   +      $values['amount'] = new Price($values['amount']['number'], $values['amount']['currency_code']);
   +      $values = new Adjustment($values);
   +    }
   

   the posted array value will be denormalized to Adjustment object, then be denormalized to commerce_adjustment field.
   without writing custom code in drupal.
   (Android app or Frontend can't construct an drupal object.)

Add drupal8.4 version patch to make the update risk fall to minimum with the corresponding module's update like #2916252

1. The 2915705-26-85x.patch is same with #23
2. 2915705-26-84x.patch without this line code
   $this->addCacheableDependency($context, $object);

@Wim Leers

1. I can't find existing/real example of store typedData or entity to 'any' typed data,
   So I change the comment to make it more appropriate

       // If the value is object or array, continue normalize it, object must be
       // normalizable. Traversable object is one of the object which can be
       // normalized, field type with typed data 'any' property definition which
       // stored the traversable object is advised to convert posted data to object
       // in setValue(), so that data can be denormalized automatically without
       // writing custom code to convert data to object when using machine readable
       // APIs.
   

2. #2916252-54: [PP-1] Order's Adjustment can't be normalized and serialized
   Must combine this patch to make Adjustment can be output successful in jsonapi(jsonapi/commerce_order/default) or restapi.
   so whether the test pass has nothing to do with whether we need this patch

@Wim Leers Thanks for the advice
#31

1. I can quite easily make it pass while making its normalizer a no-op.

   AnyNomalizer has the ability to normalize object, but not just object, so It meets expectation.

2. diff --git a/core/modules/serialization/src/Normalizer/AnyNormalizer.php b/core/modules/serialization/src/Normalizer/AnyNormalizer.php
   index 2c923c7..30105a8 100644
   --- a/core/modules/serialization/src/Normalizer/AnyNormalizer.php
   +++ b/core/modules/serialization/src/Normalizer/AnyNormalizer.php
   @@ -18,6 +18,8 @@ class AnyNormalizer extends NormalizerBase {
   * {@inheritdoc}
   */
   public function normalize($object, $format = NULL, array $context = []) {
   + return $this->serializer->normalize($object, $format, $context);
   +
   $this->addCacheableDependency($context, $object);
   $value = $object->getValue();
   // If the value is object or array, continue normalize it, object must be

   These code will make

     public function testNormalizeBase() {
       $typed_data = $this->buildDataBasicString();
       $this->assertSame('test', $this->serializer->normalize($typed_data, 'json'));
     }

   be an endless loop, So it can't pass this test.

#30

1. it's probably better to write another field type or entity type that does something more like Drupal Commerce's Adjustment.

   Good advice, I will do it later.

@Wim Leers

Once you've written that test, a change like the one I made in #31 should fail!

The change in #31

   public function normalize($object, $format = NULL, array $context = []) {
+    return $this->serializer->normalize($object, $format, $context);
+

can't make test: \Drupal\Tests\serialization\Kernel\AnyNormalizerTest pass.

Construct a field and then Add rest test for AnyNormalize, @Wim Leers thanks for the guidance on #30,
But it still has a failing test of post waiting to be solved:

Failed asserting that an array has the subset Array &0 (
    0 => Array &1 (
        'value' => Array &2 (
            'property1' => 'value1'
            'property2' => 'value2'
        )
    )
).
 /app/www/any86/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:918

Fix post and patch failing test of #36:

Failed asserting that an array has the subset Array &0 (
    0 => Array &1 (
        'value' => Array &2 (
            'property1' => 'value1'
            'property2' => 'value2'
        )
    )
).
 /app/www/any86/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:918

Oh, a failing test happen, this patch fix it.

I don't think the way i handle the denormalization is appropriate

1. Make the test green by making this change:
   

   --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -1524,7 +1524,7 @@ protected function assertStoredEntityMatchesSentNormalization(array $sent_normal
            // Subset, not same, because we can e.g. send just the target_id for the
            // bundle in a PATCH or POST request; the response will include more
            // properties.
   -        $this->assertArraySubset($expected_field_normalization, $modified_entity->get($field_name)->getValue(), TRUE);
   +        $this->assertArraySubset($expected_field_normalization, $this->serializer->normalize($modified_entity->get($field_name)), TRUE);
          }
        }
      }
   

2. At least the changes that this patch was making to EntityResourceTestBase are no longer necessary, #2895532: @DataType=map cannot be normalized, affects @FieldType=link, @FieldType=map improved that in a superior way 🙏

   Move to Drupal\Tests\entity_test and use Drupal\Tests\entity_test\Functional\Rest\EntityTestResourceTestBase directly.

1. Make the test green by making this change:
   --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -1524,7 +1524,7 @@ protected function assertStoredEntityMatchesSentNormalization(array $sent_normal
   // Subset, not same, because we can e.g. send just the target_id for the
   // bundle in a PATCH or POST request; the response will include more
   // properties.
   - $this->assertArraySubset($expected_field_normalization, $modified_entity->get($field_name)->getValue(), TRUE);
   + $this->assertArraySubset($expected_field_normalization, $this->serializer->normalize($modified_entity->get($field_name)), TRUE);
   }
   }
   }

   oh, this change make so many tests fail, delete it first

2. Use EntityResourceTestBase instead like ‘map' did.

Handle the failure test later:

Failed asserting that an array has the subset Array &0 (
    0 => Array &1 (
        'value' => Array &2 (
            'property1' => 'value1'
            'property2' => 'value2'
        )
    )
). <Click to see difference>

IN #45 I missed ->getValue() in

-        $this->assertArraySubset($expected_field_normalization, $modified_entity->get($field_name)->getValue(), TRUE);
+        $this->assertArraySubset($expected_field_normalization, $this->serializer->normalize($modified_entity->get($field_name)), TRUE);

So it caused so many failure test.

2915705-43.patch --> 2915705-47.patch
Add change log:

1. -        $this->assertArraySubset($expected_field_normalization, $modified_entity->get($field_name)->getValue(), TRUE);
   +        $this->assertArraySubset($expected_field_normalization, $this->serializer->normalize($modified_entity->get($field_name)->getValue()), TRUE);
   

   Add this change to make the test 2915705-43.patch green.

2. Because Drupal\Tests\rest\Functional\EntityResource\EntityTest\EntityTestResourceTestBase has been deprecated, so Changed to Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase just like map did.
   1. EntityTestAnyFieldResourceTestBase
   2. EntityTestAnyFieldJsonAnonTest
   3. EntityTestAnyFieldHalJsonAnonTest

   Above three tests was copied from 'Map' and made some minimized change.

When I write resource test for commerce's adjustment field.
I find EntityResourceTestBase didn't handle boolean value properly

      $expected = static::castToString($expected);
      static::recursiveKSort($expected);
      $actual = $this->serializer->decode((string) $response->getBody(), static::$format);
      static::recursiveKSort($actual);
      $this->assertSame($expected, $actual);

        $expected_field_normalization = ($field_type !== 'map')
          ? static::castToString($field_normalization)
          : $field_normalization;
        // Subset, not same, because we can e.g. send just the target_id for the
        // bundle in a PATCH or POST request; the response will include more
        // properties.
        $this->assertArraySubset($expected_field_normalization, $modified_entity->get($field_name)->getValue(), TRUE);

Boolean value just be cast to string on "expected", not "actual" on the above two places.

This patch fix it and add corresponding tests.

@Wim Leers
This will fix the failure test of #52 😊

😍 test green @Wim Leers

[Typed data layout_section can't be normalized if it stores object](https://www.drupal.org/project/drupal/issues/3012165#comment-12847218)

Typed data layout_section has the same problem with this issue, @Wim Leers what about merging the two issue with one and do sth in TypedDataNormalizer directly.

Two methods:

1. Merging the two issue with one and do sth in TypedDataNormalizer directly.
2. Two Normalizer: AnyNormalzier and LayoutSectionNormalizer