Support HAL

1. +++ b/client/src/Plugin/collect_client/EntityItemHandler.php
   @@ -69,12 +69,20 @@ class EntityItemHandler extends PluginBase implements ItemHandlerInterface {
   +      if (in_array($field_definition->getType(), ['entity_reference', 'file', 'image']) && \Drupal::entityManager()->getDefinition($field_definition->getSetting('target_type'))->isSubclassOf('\Drupal\Core\Entity\FieldableEntityInterface')) {
   
   +++ b/src/CaptureEntity.php
   @@ -138,6 +150,13 @@ class CaptureEntity {
   +      if (in_array($field_definition->getType(), ['entity_reference', 'file', 'image']) && $this->entityManager->getDefinition($field_definition->getSetting('target_type'))->isSubclassOf('\Drupal\Core\Entity\FieldableEntityInterface')) {
   

   This huge line could use some splitting and/or comments to clarify what is happening. Also, could we extract it to a collect_common function? Because we use it on both sides.

2. +++ b/client/src/Plugin/collect_client/EntityItemHandler.php
   @@ -69,12 +69,20 @@ class EntityItemHandler extends PluginBase implements ItemHandlerInterface {
        $result->data = json_encode([
          'values' => $values,
          'fields' => $fields,
   -      'operation' => $item['operation']]
   +      'operation' => $item['operation'],
   +      ]
        );
   

   The ([ are on the same line, so the ]) should also be.

3. +++ b/src/Plugin/Field/FieldFormatter/CollectDataFormatter.php
   @@ -20,7 +21,10 @@ use Drupal\Core\Field\FormatterBase;
   +use Drupal\Core\TypedData\Plugin\DataType\Uri;
   +use Drupal\Core\TypedData\Type\UriInterface;
   

   Unused

4. +++ b/src/Plugin/Field/FieldFormatter/CollectDataFormatter.php
   @@ -170,13 +174,19 @@ class CollectDataFormatter extends FormatterBase implements ContainerFactoryPlug
   +    $uri = SafeMarkup::checkPlain($value->getString());
   +    if (UrlHelper::isValid($uri)) {
   +      $uri = $this->getContainerUrl($uri);
   +    }
   

   I think the checkPlain will encode some URLs, especially with & in them, so probably better to check UrlHelper::isValid() first, and if not a URL, checkPlain it. (As the comment says.)

5. +++ b/src/Plugin/collect/Schema/CollectJsonSchema.php
   @@ -127,13 +127,25 @@ class CollectJsonSchema extends SchemaBase implements DynamicSchemaTypedDataInte
   +    // Extract embedded values and create elements of data array with the field
   +    // values which are not explicitly displayed in values container data.
   

   This comment was a little bit confusing to me. I think this at least needs a description of what kind of data is in _embedded.

6. +++ b/src/Plugin/collect/Schema/CollectJsonSchema.php
   @@ -127,13 +127,25 @@ class CollectJsonSchema extends SchemaBase implements DynamicSchemaTypedDataInte
   +          $values[$field_name] = \Drupal::typedDataManager()->create($property_definition->getDataDefinition(), $data['values'][$field_name], $field_name, $container_entity_adapter->get('data')->first());
   

   Why the change of the create() params? If this causes problems, I think we can just remove the $container_entity_adapter and leave the 3rd and 4th parameters empty.

7. +++ b/src/Plugin/collect/Schema/FieldDefinitionSchema.php
   @@ -155,9 +163,15 @@ class FieldDefinitionSchema extends SchemaBase implements SpecializedDisplaySche
   +          if ($field_definition['type'] == 'field_item') {
   +            $field_definitions[$field_name] = $this->serializer->denormalize($field_definition, 'Drupal\Core\Field\FieldDefinitionInterface');
   +          }
   +          else {
   +            $field_definitions[$field_name] = $this->serializer->denormalize($field_definition, 'Drupal\Core\TypedData\DataDefinitionInterface');
   +          }
   

   The only difference between the cases is the class name, maybe we could move the denormalize call outside the if-else

8. As now, if we want to compare field definitions with the current system we will get different results (even if there are no missing fields/bundles) because we have different structure. The question is, do we want to convert current field definitions data to match the structure we are capturing or to leave like this so we can clearly see which fields are "different"?

   creation of missing fields is broken

   I'm also not really sure what we want. This is related to what @miro_dietiker said in #2: "We don't like that Hal removes values and we lose information through this." Let's discuss in a followup?

9. Recreating entity reference fields can probably be done by resolving the URL (as long as it is local). In a follwoup.

Looks great :D

Committed and pushed!

So, followups needed to make sure recreation of ER fields works, to decide on what to diff, and resolving URIs when recreating ER values. To tackle some of this, @miro_dietiker suggested changing the field definition serialization (for creating FD containers) to provide the URI-list definitions in parallel with all field definitions. Thus we could use the field definitions for diffing and creating fields, and the URIs with other standard Collect usage.

Created followups:

make sure recreation of ER fields works, to decide on what to diff

#2480109: Field definition container: include ER fields as well as URIs

and resolving URIs when recreating ER values

#2480103: Resolve URIs to recreate entity reference values