The current JSON-LD Normalizer directly processes the properties, accessing them with the getValue method and adding them to the array. However, there are some fields which should have custom processing of values, such as EntityReference fields.

Adding Normalizers at the field level in addition to the entity level will allow us to control the normalization on a type by type basis.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Status: Active » Needs review
FileSize
25.17 KB

This patch changes the JsonldNormalizer to JsonldEntityNormalizer. It also adds:

  • JsonldFieldNormalizer
  • JsonldFieldItemNormalizer
  • JsonldEntityReferenceNormalizer (which is used instead of FieldItem Normalizer for EntityReference field items)

It removes the DrupalJsonld variant of the Normalizer, and makes it so that all Normalizers support both formats. For the cases where the two differ (for example, FieldItem), we will simply add a second Normalizer that only responds to drupal_jsonld and give it higher priority.

I removed the test for the regular jsonld format, since we don't yet support the difference between the two. It will be added back once there is a difference.

Anonymous’s picture

Title: Add Normalizers for all levels of Typed Data API » Enable fieldtype-specific JSON-LD normalization
Anonymous’s picture

We're trying to prioritize POST and PUT handling in the REST development, which depends on #1838596: Add deserialize for JSON-LD, which depends on this issue... so hopefully we can start working through the review process.

fago’s picture

+1 for running each complex data item through the serializer again.

+    $fieldValues = array();
+    $list = $object->getIterator();
+    foreach ($list as $delta => $item) {
+      $fieldValues[$delta] = $this->serializer->normalize($item, $format);
+    }
+    return $fieldValues;

What about doing a general typed data normalizer? I.e. iterate over the object and if the property/item is implementing the list or complexdatainterface invoke the normalizer on it. Or maybe better have 2 implementations, one for normalizing lists and one for complex data. The nice thing is that any typed data structure will have at least some reasonable default serialization.

Then, e.g. ER fields could chime in and override the normalizer with their own implementation.

webchick’s picture

Priority: Normal » Major

I believe this is pretty important to web services in core. Escalating to a "major" feature request.

Anonymous’s picture

@fago Thanks, that pointed out a flaw in the initial patch.

Or maybe better have 2 implementations, one for normalizing lists and one for complex data.

List: Traversables should be handled natively by Serializer.... so it turns out that Field (being a list) is already supported without the FieldNormalizer. I've removed that from the patch.

Complex Data: As far as I can tell, ComplexDataInterface is too broad to be useful when normalizing for JSON-LD. Both EntityInterface and FieldItemInterface extend it. For the EntityInterface, the bulk of normalizing is about placing the id, context, type, etc. For FieldItems (except EntityReference) these attributes don't apply. It might be useful, though, I could be missing something.

moshe weitzman’s picture

Status: Needs review » Needs work

This looks solid to me.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -1,14 +1,14 @@
 <?php
-
+ ¶
 /**
  * @file
  * Definition of Drupal\jsonld\JsonldEntityWrapper.
  */
-
+ ¶
 namespace Drupal\jsonld;
-
-use Drupal\Core\Entity\EntityNG;
-
+ ¶
+use Drupal\Core\Entity\Entity;
+ ¶

4 instances of a single whitespace on a line.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -22,17 +22,36 @@ class JsonldEntityWrapper {
+  /**
    * Constructor.
    *
    * @param string $entity
    *   The Entity API entity
    */
-  public function __construct(EntityNG $entity) {
+  public function __construct(Entity $entity, $format, $serializer) {

Let's document the $format and $serializer params

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -42,11 +61,26 @@ public function getId() {
+        $langKey = empty($definition['translatable']) ? 'und' : $langcode;

LANGUAGE_NOT_SPECIFIED instead of 'und'

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -42,11 +61,26 @@ public function getId() {
+    // Only return properties which are not in the $skip array.
+    return array_diff_key($properties, array_fill_keys($skip, ''));

Would array_intersect() be simpler here?

fubhy’s picture

Status: Needs work » Needs review
+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldBundle.phpundefined
@@ -22,9 +22,39 @@ class JsonldBundle extends Bundle {
+        $container->register("serializer.normalizer.{$supported_class}.{$format}", $normalizer_class)->addTag('normalizer', array('priority' => $priority));
...
+      $container->register("serializer.encoder.{$format}", $encoder_class)->addTag('encoder', array('priority' => $priority));

Can you move the ->addTag to a new line so it's more readable?

Also, I am not sure if the loop is more readable now. I think I preffered the 'old' version :).

In this simple case I wouldn't even put the $priority in a variable. But yeah, these are all just nitpicks.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEncoder.phpundefined
@@ -18,11 +18,11 @@
+   * The formats that this Normalizer supports.

Normalizer? This is in the encoder, no?

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityNormalizer.phpundefined
@@ -0,0 +1,37 @@
+    return parent::supportsNormalization($data, $format) && ($data instanceof EntityNG);

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityReferenceNormalizer.phpundefined
@@ -0,0 +1,41 @@
+    return parent::supportsNormalization($data, $format) && ($data instanceof EntityReferenceItem);

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldFieldItemNormalizer.phpundefined
@@ -0,0 +1,33 @@
+    return parent::supportsNormalization($data, $format) && ($data instanceof FieldItemBase);

If you flip that statement around it would skip the parent::supportsNormalization if $data is not an instance of EntityNG. Just a minor improvement though I guess.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.phpundefined
@@ -42,11 +61,26 @@ public function getId() {
+    $skip = array('id');
...
+    return array_diff_key($properties, array_fill_keys($skip, ''));

Why so complicated? $skip is just a single item, so why not just use unset($properties['skip']);

Anonymous’s picture

Thanks for the reviews.

Made all the changes in #7, except the last. We want to exclude the keys in the skip array, whereas intersect would give us only those keys.

From #8, moved ->addTag to a new line and fixed the comment.

RE: parent::supportsNormalization, order matters for that because the parent checks whether it is an object.

RE: the $skip array, the idea is that we will probably have other items that we want to ignore, like langcode. I'm waiting until we have a full representation before starting that discussion.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

I think feedback has been fully addressed. Please wait for green before commit.

fubhy’s picture

from IRC:

moshe_work, linclark 2 more things then: I would use array_flip then instead of array_fill_keys. Don't have any data to back that up but I would guess that's more efficient... Also, why did you choose to go with a foreach() loop to set up the services? I preferred the old pattern (for readability)
not pushing back from RTBC for that though :)

fago’s picture

Status: Reviewed & tested by the community » Needs work

List: Traversables should be handled natively by Serializer.... so it turns out that Field (being a list) is already supported without the FieldNormalizer. I've removed that from the patch.

Sounds great.

Complex Data: As far as I can tell, ComplexDataInterface is too broad to be useful when normalizing for JSON-LD. Both EntityInterface and FieldItemInterface extend it. For the EntityInterface, the bulk of normalizing is about placing the id, context, type, etc. For FieldItems (except EntityReference) these attributes don't apply. It might be useful, though, I could be missing something.

I think there could be a general complex-data serializer, which then the entity-serializer extends in order to customize it. It can work like the field-item normalizer, but would require one addition: For each property check whether its a list or complex data itself and invoke normalizing on it if so. Howsoever, that's just a nice-to-have point, so if we want to do so, this can happen in a follow-up.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityNormalizer.php
@@ -0,0 +1,37 @@
+  public function supportsNormalization($data, $format = NULL) {
+    return parent::supportsNormalization($data, $format) && ($data instanceof EntityNG);

Maybe add a TODO to replace the check for EntityNG by EntityInterface once all entity types are converted.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldFieldItemNormalizer.php
@@ -0,0 +1,33 @@
+  public function normalize($object, $format = NULL) {
+    return $object->getValue();
+  }

I think this should use ->getPropertyValues() not getValue(). You could do your own field item having whatever plain value, while getPropertyValues() always returns the wanted array structure.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldFieldItemNormalizer.php
@@ -0,0 +1,33 @@
+  public function supportsNormalization($data, $format = NULL) {
+    return parent::supportsNormalization($data, $format) && ($data instanceof FieldItemBase);

Should check for FieldItemInterface.

Setting to needs work for the last two points.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
2.12 KB
23.72 KB

Thanks fago, I've made those changes.

fago’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, looks good! Back to RTBC then.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

This patch is a nice clean-up and very straight-forward. Committed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.