Problem/Motivation

We want to be able to respond to GET requests on entity pages like node/1 with a JSON-LD representation of the node.

Proposed resolution

We can leverage Symfony's Serializer component to do this. We simply need to provide a Normalizer and Encoder for JSON-LD.

The Normalizer receives an entity and processes it into the correct object/array structure for JSON-LD. The encoder then takes that normalized object and spits it out as JSON. Both Normalizers and Encoders can specify what they are able to process. The JSON-LD N/E should only be used when the object is a subclass of Entity (or EntityNG for now) and the requested format is 'jsonld'.

This activity diagram demonstrates how it could work. The section in purple is addressed in this issue. The section in green is the duty of the Symfony Serializer, to be added with #1810472: Add Symfony's Serializer component to core despite Symfony potentially releasing BC-breaking updates after 2.3.. Sections in white are still in discussion and outside the scope of this issue.

CommentFileSizeAuthor
#60 1811510_60_jsonld_mimetype.patch1.43 KBscor
#57 1811510-57-jsonld-entity.patch18.67 KBlinclark
#57 interdiff.txt1.35 KBlinclark
#54 1811510-54-jsonld-entity.patch17.32 KBlinclark
#54 interdiff.txt2.91 KBlinclark
#51 1811510-51-jsonld-entity.patch18.08 KBlinclark
#49 1811510-49-jsonld-entity.patch18.11 KBlinclark
#49 interdiff.txt1.55 KBlinclark
#47 1811510-47-jsonld-entity__for-review.patch17.94 KBlinclark
#47 interdiff.txt5.06 KBlinclark
#39 1811510-39-jsonld-entity__for-review.patch25.51 KBlinclark
#39 interdiff.txt17.28 KBlinclark
#29 1811510-29-jsonld-entity__for-review.patch16.62 KBlinclark
#29 interdiff.txt8.73 KBlinclark
#19 1811510-19-jsonld-entity__for-review.patch17.01 KBlinclark
#17 1811510-17-jsonld-entity__for-review.patch17.04 KBlinclark
#17 interdiff.txt8.54 KBlinclark
#12 1811510-12-jsonld-entity__for-review.patch8.67 KBlinclark
#12 interdiff.txt2.82 KBlinclark
#8 interdiff.txt5.47 KBlinclark
#8 1811510-jsonld-entity__temp.txt3.19 KBlinclark
#8 1811510-08-jsonld-entity__for-review.patch7.8 KBlinclark
#7 1811510-07-jsonld-entity__for-review.txt4.81 KBlinclark
#3 1811510-3-jsonld-entity__for-testing.patch110 KBlinclark
#3 1811510-3-jsonld-entity__for-review.txt4.48 KBlinclark
#3 1811510-jsonld-entity__temp.txt3.13 KBlinclark
#1 1811510-1-jsonld-entities__for-testing.patch110.45 KBlinclark
#1 1811510-1-jsonld-entities__for-review.patch4.93 KBlinclark
#1 1811510-jsonld-entities__temp.patch3.27 KBlinclark
add-jsonld-big.png256.92 KBlinclark
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

This patch covers the purple parts of the diagram above, enabling basic serialization using a Normalizer and Encoder. It will only respond if the format requested is jsonld and the object for serialization is an Entity.

To enable testing of this patch, I have added a route and controller, and have also added a URI callback to entity_test. These should not be evaluated since routing/controllers should not be a part of serialization. I've broken out the code that should be evaluated in the __for-review patch. Just to show what stopgap code is necessary for testing, I've broken that out into a __temp patch. The __for-testing patch includes both, along with the Serializer patch from #1810472.

Because core entities have not been converted to EntityNG yet, this patch uses the entity_test entity.

To test this patch:

  1. Apply the __for-testing patch & enable JSON-LD module
  2. Add an entity_test at entity-test/add
  3. Go to entity/entity-test/1

Status: Needs review » Needs work

The last submitted patch, 1811510-jsonld-entities__temp.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI, +json-ld
FileSize
3.13 KB
4.48 KB
110 KB

I removed the @context handling, which will be tackled in #1794958: Generate the @context for JSON-LD.

tstoeckler’s picture

The patch was named "for-review" so I took a stab at it :-)
Disclaimer: I know absolutely nothing about JSON-LD and about the new property API, so it might be that this makes absolutely no sense.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -0,0 +1,42 @@
+  /**
+   * Get the Entity's URI for the @id attribute.
+   */
+  public function getId() {
+    $uri_info = $this->entity->uri();
+    return url($uri_info['path'], array('absolute' => TRUE));
+  }

Don't really know what the use-case would be, so maybe the answer to this question is "No", but shouldn't this also keep the 'options' from $uri_info and just merge the 'absolute' => TRUE in?

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEncoder.php
@@ -0,0 +1,33 @@
+    return self::FORMAT === $format;

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldNormalizer.php
@@ -0,0 +1,55 @@
+    return is_object($data) && is_subclass_of($data, 'Drupal\Core\Entity\EntityNG') && self::FORMAT === $format;

I guess this should use static:: instead of self::

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php
@@ -0,0 +1,42 @@
+    $properties = $this->entity->values;

First of all, values is protected, so no idea how that can work :-)

Secondly, how does this relate to translations? I've yet to grasp the new entity property API, but I see a method called getTranslatedField() in EntityNG and then also the following @todo on EntityNG::$values: @todo: Add methods for getting original fields and for determining changes.
Is that why you are using $values?

Anonymous’s picture

Don't really know what the use-case would be, so maybe the answer to this question is "No", but shouldn't this also keep the 'options' from $uri_info and just merge the 'absolute' => TRUE in?

We want a canonical URI for the entity here, so I don't think we'd want to use any options that are passed in. For example, we don't want the language to be included in the URI.

I guess this should use static:: instead of self::

I used self:: because Symfony's JsonNormalizer and Encoder use self::, but I can switch to static::. It would actually be better if Symfony used static:: there, because then the method could be removed on the Encoder. I might post an issue on Symfony.

First of all, values is protected, so no idea how that can work :-)

I didn't check the visibility, but it definitely works. RE: translations... they are included in the field structure in the same way for both Drupal and JSON-LD.

Crell’s picture

I used self:: because Symfony's JsonNormalizer and Encoder use self::, but I can switch to static::. It would actually be better if Symfony used static:: there, because then the method could be removed on the Encoder. I might post an issue on Symfony.

Please do. :-) In practice there's little reason to use self:: anymore as of PHP 5.3; usually static:: is what you mean.

Anonymous’s picture

Status: Needs review » Needs work
FileSize
4.81 KB

This just has some small changes, self changed to static and code style.

I will be posting a patch with actual changes soon, so going to switch this to needs work for now.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
7.8 KB
3.19 KB
5.47 KB

Based on the discussion in #1797210: Decide how to negotiate between the 2 JSON-LD serializations, we will need to have two different JSON-LD serializations; one for content staging, and another that provides data in a way that external consumers expect it. We will negotiate between these two by using different mime types:

  • application/ld+json
  • application/vnd.drupal.ld+json

This patch provides Normalizers, Encoders, and EntityWrappers for both of those serializations. The Drupal-specific Jsonld classes extend from the default Jsonld classes.

JsonldEntityWrapper should probably be extracted out into an interface. However, I'm going to let the patch mature more before doing that.

To test this:

  1. Apply the __for-review patch & the __temp.txt patch
  2. Enable JSON-LD module
  3. Go to entity-test/add and add an entity_test
  4. Go to entity/entity-test/1

If you want to toggle between vnd.drupal.ld+json and the (incomplete) ld+json, go to JsonldController and change 'drupal_jsonld' to 'jsonld'. Eventually, this string will be a variable provided by the content negotiation framework.

moshe weitzman’s picture

I finally got this patch applied and enabled the json_ld and entity_test modules. The patch works, and is nice and small.

  1. In the JSON, some properties like uuid are nested under UND even though they clearly are not translatable. Not sure if that is by design.
  2. jsonld.info file needs package = core
  3. jsonld.module needs hook_help, readme, api docs, ...
Crell’s picture

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEncoder.php
@@ -0,0 +1,27 @@
+class DrupalJsonldEncoder extends JsonldEncoder implements EncoderInterface {

"Drupal" is not needed in the class name.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEncoder.php
@@ -0,0 +1,27 @@
+class DrupalJsonldEncoder extends JsonldEncoder implements EncoderInterface {

"Drupal" is not needed in the class name.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldController.php
@@ -0,0 +1,40 @@
+  public function entity_get($entity_type, $eid) {

camelCase, please.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldController.php
@@ -0,0 +1,40 @@
+    $serializer = new Serializer($normalizers, $encoders);
+
+    $jsonld = $serializer->serialize($entity, 'drupal_jsonld');

This is the sexy part. :-)

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldController.php
@@ -0,0 +1,40 @@
+    $response = new Response($jsonld, 200, array('Content-type' => 'application/json'));
+    return $response;

We should be able to use a JsonResponse here, no?

I like how straightforward this looks so far. That means we must be doing something right.

Anonymous’s picture

Status: Needs review » Needs work

Thanks for the reviews :)

In the JSON, some properties like uuid are nested under UND even though they clearly are not translatable. Not sure if that is by design.

This is because that's how the Entity Field API handles those properties. The options I can see are either 1) fixing that in Entity Field API (which I think is preferable), or 2) we could handle it in this module by having a defined list of properties which should be extracted from their language wrappers.

"Drupal" is not needed in the class name.

I was expecting this would be brought up. Using "Drupal" in a class name is against coding standards. However, in this case DrupalJsonldEncoder corresponds to the media type vnd.drupal.ld+json. I can't think of a more suitable name (either for the media type or the class). However, I'm happy to change this if someone has a suggestion. Would VndDrupalJsonld be better for the class name?

We should be able to use a JsonResponse here, no?

I tried that earlier and it actually double encodes the json, so I don't think Serializer was meant to work with JsonResponse... it seems like they are two ways of doing the same thing, basically.

Also, the JsonldController will go away once the REST module adds its controllers. Because we want the controller to be format agnostic, it will not have any knowledge of which response class it should use. However, that does touch on the issue of how we are going to figure out which Content-type header to use. The JsonldController is hard coded to pass the json Content-type header into the Response, but that's going to need to be handled differently once REST module is handling the controller.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
2.82 KB
8.67 KB

This patch adds some of the things requested in #9.

jsonld.module needs hook_help, readme, api docs, ...

RE: README... I might be missing something. I checked and it doesn't look like other core modules have readme files.

RE: api docs... There isn't yet anything that can be documented in jsonld.api.php, since there are no hooks. I believe I followed all the doc block standards. Let me know if I'm missing something.

moshe weitzman’s picture

Issue tags: -WSCCI

Oh right. No README for core modules ... How about a unit test which exercises the JSON-LD Normalizer+Serializer? Hopefully we could use UnitTestBase and not WebTestBase

effulgentsia’s picture

Status: Needs review » Needs work
Issue tags: +WSCCI
+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEntityWrapper.php
@@ -0,0 +1,28 @@
+    $properties = $this->entity->values;

This line feels wrong and doesn't enable distinction between translatable and non-translatable fields (per #1346214-36: [meta] Unified Entity Field API, we now call everything fields, not properties). I don't know what the right API is yet. Maybe fago can shed some light.

Otherwise, I think this patch is great, and have nothing to add to what Moshe and Crell already pointed out.

effulgentsia’s picture

Status: Needs work » Needs review

Oops. xpost. I reviewed #8 rather than #12.

Status: Needs review » Needs work

The last submitted patch, 1811510-12-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
8.54 KB
17.04 KB

I added tests for the normalizer. I don't think we need tests for the encoder at this point, since it is simply using Symfony's JsonEncoder behaviors, but could add those tests if necessary.

I haven't addressed the issue in #14 about using the values array. I thought that the values array would include any language handling we would need.

Status: Needs review » Needs work

The last submitted patch, 1811510-17-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
17.01 KB

The patch had an unnecessary dependency on entity_test, which I removed.

I also removed the whitespace issues there were in the last chunk.

Status: Needs review » Needs work
Issue tags: -WSCCI, -json-ld

The last submitted patch, 1811510-19-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +WSCCI, +json-ld

The last submitted patch, 1811510-19-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

The same exception has been thrown twice:

Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "plugin.manager.layout" does not exist. in Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition()

I don't understand how this could be related to the changes made in this patch. Does anyone else know?

effulgentsia’s picture

Status: Needs work » Needs review
effulgentsia’s picture

Locally, I'm getting a different exception thrown in the same test: Base table or view not found: 1146 Table 'd8.simpletest360676system' doesn't exist". But I'm getting that same exception on HEAD. So I queued a bot retest to see what comes back.

Anonymous’s picture

There is a bug in Entity Field API which will prevent us from getting the appropriate language handled values as inquired about in #14.

fago said "that method [Entity::getTranslation] does not apply strict mode, so that's a bug". He agreed to file an issue about it and will hopefully post here (or ping me) when the issue is created.

Status: Needs review » Needs work

The last submitted patch, 1811510-19-jsonld-entity__for-review.patch, failed testing.

effulgentsia’s picture

The test failure is very likely not due to any direct problem with this patch, but rather with simpletest and the DIC. #1708692: Bundle services aren't available in the request that enables the module may solve it, though it would be nice to figure out why exactly the introduction of this patch is triggering it. My guess is that jsonld.module is the disabled module directly preceding layout.module alphabetically (language.module not being disabled by default), but that's just a guess right now, it could be something else.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
8.73 KB
16.62 KB

The first set of tests I did were too abstract for tests. After comparing with both core tests and Symfony's Normalizer tests, I decided to go more procedural. The attached interdiff is against the patch in #12. It is not a diff against the first set of tests.

Status: Needs review » Needs work

The last submitted patch, 1811510-29-jsonld-entity__for-review.patch, failed testing.

fago’s picture

ad ##2: I've created #1822706: EntityTranslation fails to consistenly apply strict mode.

Still, you should be able to work around that rather easily. E.g. this (untested) code shold do it:

foreach ($entity as $name => $field) {
  $data[$name][LANGUAGE_DEFAULT] = $field->value();
}
// Add in translation values.
foreach ($this->getTranslationLanguages(FALSE) as $langcode => $language) {
  foreach ($this->getTranslation($langcode) as $name => $field) {
    $data[$name][$langcode] = $field->value();
  }
}

Anonymous’s picture

The problem with that is that non-translatable values such as UUID are still language tagged. It gives me the same value as the ->values property.

{
    "@id": "http://d8.l/",
    "id": {
        "und": [
            {
                "value": 1
            }
        ]
    },
    "uuid": {
        "und": [
            {
                "value": "a4f4925f-481b-4f33-8800-a6e00eda1b81"
            }
        ]
    },
    "langcode": {
        "und": [
            {
                "value": "und"
            }
        ]
    },
    "name": {
        "und": [
            {
                "value": "Test this out"
            }
        ]
    },
    "user_id": {
        "und": [
            {
                "value": 1
            }
        ]
    },
    "field_test_text": {
        "und": [
            {
                "value": "This is test",
                "format": null
            }
        ]
    }
}
effulgentsia’s picture

On a phone call, Crell had suggested that since the JSON output is intended for machines, not humans, then language tagging everything for consistency might not be a bad thing.

Alternatively, I could see the JSON output maybe not language tagging the fields that are 'entity keys'. Not sure if the responsibility for that should be the entity API, or if the JSON normalizer should call entity_get_info() and compare each field against the 'entity keys' list.

Anonymous’s picture

I'm less knowledgable about standards concerning multilingual, so this could very well be wrong...

Using 'und' in general will most likely be against the spirit, if not the letter, of language maps in JSON-LD. Language map keys are expected to follow the IETF's best current practice doc BCP 47. AFAIK, there is no defined 'und'. While you can use private use subtags, the BCP doc says:

They... SHOULD NOT be used in content or protocols intended for general use.

Private use subtags are simply useless for information exchange without prior arrangement. The value and semantic meaning of private use tags and of the subtags used within such a language tag are not defined by this document.

If we DID go ahead and mint a private use subtag, it would be something like x-und.

If we don't use und, I believe the JSON above would look as follows:

{
    "@id": "http://d8.l/",
    "uuid": [
        {
            "value": "a4f4925f-481b-4f33-8800-a6e00eda1b81"
        }
    ],
    "langcode": [
        {
            "value": "en"
        }
    ],
    "name": {
        "en": [
            {
                "value": "Test this out"
            }
        ]
    },
    "user_id": {
        "en": [
            {
                "value": 1
            }
        ]
    },
    "field_test_text": {
        "en": [
            {
                "value": "This is test",
                "format": null
            }
        ]
    }
}
fago’s picture

Maybe it would make sense to follow the structure the new entity API uses? E.g. like that:

{
    "@id": "http://d8.l/",
    "uuid": [
        {
            "value": "a4f4925f-481b-4f33-8800-a6e00eda1b81"
        }
    ],
    "langcode": [
        {
            "value": "en"
        }
    ],
    "name": [
            {
                "value": "Test this out"
            }
      ],
    "field_test_text": [
            {
                "value": "This is test",
                "format": null
            }
        ],
    // If the entity has some translations, the translation values are added below a separate key

    "lang-de":  {
      "name": [
            {
                "value": "Test this out"
            }
        ],
      "field_test_text": [
            {
                "value": "This is test",
                "format": null
            }
        ]
    }
}

This way we've a the same structure for translatable and untranslatable fields and you don't have to care about language by default, as you just go with the default language. I'm not sure whether this would map well to json-ld though?

Anonymous’s picture

That would be a very unusual way to model the data. If we were going with something so unusual, I'm not sure that we get any advantage from using JSON-LD.

Rather than using 'und' in the JSON-LD to mean that it is in the default language, could we use the language code for the default language? When a production site is consuming the data from staging, wouldn't it know what the default language is? Then it could access the values using the language code.

effulgentsia’s picture

That would be a very unusual way to model the data. If we were going with something so unusual, I'm not sure that we get any advantage from using JSON-LD.

I don't necessarily think we want to do #35, but if we did, why is it so unusual? The idea is, here's "an entity in its default language" as a resource, and here's "a translation of the entity to language X" as a resource, and then ideally, having a way to properly connect the translation to the entity so that the entity and all translations could be inlined in the same JSON response. I'm sure there's systems out there that model translations of documents as separate documents. Again, I don't know what the pros and cons of #35 are, but I'd be surprised if it's flat out wrong.

Rather than using 'und' in the JSON-LD to mean that it is in the default language, could we use the language code for the default language?

Yes, we can and should. Not sure where the responsibility for this belongs (inside the Entity API or inside the normalizer), but it should be easy.

When a production site is consuming the data from staging, wouldn't it know what the default language is?

Not automatically it wouldn't (it could be a new node, and each node can have its own default language), but we can make it part of the JSON. Is there any already existing standard for where to put that info?

Anonymous’s picture

Again, I don't know what the pros and cons of #35 are, but I'd be surprised if it's flat out wrong.

It's not that it is wrong, it's that it is unusual. I could explain this more, but I will be posting a patch that shows how we could do it in a more common way, so I'd prefer to see whether that suits us. After reviewing the patch, if people are still interested in the less common approach I can explain what makes it idiosyncratic.

Is there any already existing standard for where to put that info?

The language that is set for an entity is a property of the entity, and is currently exposed. As far as exposing the site's default language, I don't believe that is necessary for our use case.

I need to work on the tests a bit more, but should be posting a patch later tonight.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
17.28 KB
25.51 KB

I added translation handling to DrupalJsonldEntityWrapper::getProperties(). I changed the tests to account for this.

I also created a jsonld_test entity type. I assumed this was necessary to get translation handling. It is not, but I figured it was worth keeping. It is probably better not to depend on an external module's test entity anyways.

Assuming you have an entity with the following:

  • German as the default language
  • A German value for translatable field user_id
  • German and English values for translatable field name
  • An untranslatable field field_test_text

The JSON-LD would look like the following:

      '@id' => $this->getEntityId($entity),
      'uuid' => array(
        array(
          'value' => $entity->uuid(),
        ),
      ),
      'user_id' => array(
        'de' => array(
          array(
            'value' => 1,
          ),
        ),
      ),
      'name' => array(
        'de' => array(
          array(
            'value' => $values['name'],
          ),
        ),
        'en' => array(
          array(
            'value' => $langSpecificValues['name'],
          ),
        ),
      ),
      'field_test_text' => array(
        array(
          'value' => $values['field_test_text']['value'],
          'format' => $values['field_test_text']['format'],
        ),
      ),

To Crell's point about ease of processing, I believe it will be pretty easy to transform this back to our internal structure using a simple processing routine on Denormalization. The @context keeps track of which properties contain language maps and which do not. For any that are not language maps, we can simply nest the value inside an array keyed by 'und'. That should return us to the array structure we would expect.

Currently, two of the new tests should fail.

  1. "Translatable field with multiple language values is nested correctly.", based on #1823584: EntityNG::language() returns FALSE
  2. "Translatable field with single language value is nested correctly.", which I assume fails because of #1822706: EntityTranslation fails to consistenly apply strict mode

There are also a number of exceptions, "Trying to get property of non-object" and "Undefined index: translatable". It was odd, because the code triggering those exceptions is producing the expected results.

EDIT: It turns out that one of those fails is because the entity_test module (which I based jsonld_test on) has a langcode field which needs special property handling. I will add this back into jsonld_test (or switch back to entity_test) to fix this.

Status: Needs review » Needs work

The last submitted patch, 1811510-39-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

So after switching back to entity_test, I'm still getting FALSE for $this->entity->language().

I think I'm going to need help from someone who knows the entity translation system better than I do. The parts that need work are DrupalJsonldEntityWrapper and the testNormalize function in DrupalJsonldNormalizerTest.

fago’s picture

It's not that it is wrong, it's that it is unusual. I could explain this more, but I will be posting a patch that shows how we could do it in a more common way, so I'd prefer to see whether that suits us.

I'm not sure it's really so unusual, but anyway I'd be interested in the explanation as it it might at least partly apply to the entity system as well.

@ease-of-processing:
Doesn't that apply to the client side the very same way?

I also created a jsonld_test entity type. I assumed this was necessary to get translation handling. It is not, but I figured it was worth keeping. It is probably better not to depend on an external module's test entity anyways.

We already use the 'entity_test' entity type for a lots of different entity related tests, so I think it would be fine to continue this for json-ld / entity serialization. Entity translation does also.
Having a single entity type saves us from maintaining lots of different test entities also and ensures that a single implementation can serve all the different components at the same time.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEntityWrapper.php
@@ -0,0 +1,57 @@
+    // Get default language.
+    if ($this->entity->language()->langcode !== 'und') {
+      $defaultLangcode = $this->entity->language();
+    }
+    else {
+      $language = variable_get('language_default');
+      $defaultLangcode = $language['langcode'];
+    }

So should this default to request content language maybe? But then it sounds like being another use-case as it could potentially add-in a translation then.

Thus, if the entity is marked as language neutral it should be probably output that way also. That's something that might be interesting to the client.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEntityWrapper.php
@@ -0,0 +1,57 @@
+      if ($definition['translatable'] == FALSE) {

translatable is optional, thus use empty() to check it.

I think I'm going to need help from someone who knows the entity translation system better than I do.

I'm not sure where the problem lies. Is the structure you've read out from the entity not as intended?

So after switching back to entity_test, I'm still getting FALSE for $this->entity->language().

That should not happen. Maybe that requires language module or something to work? entity translation tests seem to enable 'locale' module, so maybe we've got an unwanted dependency there.

Anonymous’s picture

I'm at a one day conference, so I cant respond fully ATM regarding the data model.

@ease-of-processing:
Doesn't that apply to the client side the very same way?

Many clients will request a single language version of the entity. I suggest that we serve that without language maps.

For those that are processing the full multilingual version, they can choose to use the same simple processing algorithm.

/* Make array structure uniform. */

Get @context
Iterate through terms in context
    If term has "@container": "@language", it is tranlatable
        Do nothing
    Else, it is not translatable
        Wrap the value's properties in an array, key with 'und'
    End if
End loop

The nice thing about this is that all of the information you need to figure out the relationship the properties have to the entity is directly in the data. With the other way, you need to have out-of-band knowledge that the lang_de property is a special property that contains other values of its sibling properties.

effulgentsia’s picture

Thus, if the entity is marked as language neutral it should be probably output that way also. That's something that might be interesting to the client.

Why would we have any entity with a default language of 'und' (LANGUAGE_NOT_SPECIFIED)? Language neutral entities should have a default language of 'zxx' (LANGUAGE_NOT_APPLICABLE), which is a standard langcode that means that, so doesn't run into the problem of #34. Perhaps the only entities with 'und' are ones that were updated from D7? It's an interesting question what to do with those, since we don't yet know that they are language neutral, all we know is that we don't know their language.

Anonymous’s picture

As I mentioned, multilingual is not something that I've familiarized myself with before. So I found IANA's language subtag registry and it turns out that there is actually an 'und' in the registry. It is one of the 4 tags marked as "special" (as is zxx), but that shouldn't be a problem.

For this issue, I'm going to go back to using 'und' for non-translatable fields. I plan to use the langcode of the default language (entity default if set, or site default if not) when I find 'und' in translatable fields.

We may want to consider further how we want to use 'und' for non-translatable fields. There are some values, such as UUID, which simply are not linguistic content, and could use the zxx tag. There would also be fields on single language sites that aren't technically translatable (because the site isn't multilingual) but are clearly translatable content, and could use the default langcode of the site. AFAIK, we don't currently have a way of distinguishing these, but could consider introducing one in a follow up issue.

Anonymous’s picture

That should not happen. Maybe that requires language module or something to work? entity translation tests seem to enable 'locale' module, so maybe we've got an unwanted dependency there.

So it turns out I have to manually add German as a language before using it as the default language for an entity.

    $language = new Language(array(
      'langcode' => 'de',
      'name' => $this->randomString(),
    ));
    language_save($language);

Setting an entity's language to an invalid language (as I was doing) should probably result in an exception, I have filed an issue for this, #1824964: Setting entity language to an invalid language should throw exception.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
5.06 KB
17.94 KB

The attached patch:

  • Switches the test entity back to entity_test
  • Fixes the test error as described in #46
  • Changes the language map structure as described in #45 and shown below
      '@id' => $this->getEntityId($entity),
      'uuid' => array(
        'und' => array(
          array(
            'value' => $entity->uuid()
          ),
        ),
      ),
      'user_id' => array(
        'de' => array(
          array(
            'value' => 1,
          ),
        ),
      ),
      'name' => array(
        'de' => array(
          array(
            'value' => $values['name'],
          ),
        ),
        'en' => array(
          array(
            'value' => $translationValues['name'],
          ),
        ),
      ),
      'field_test_text' => array(
        'und' => array(
          array(
            'value' => $values['field_test_text']['value'],
            'format' => $values['field_test_text']['format'],
          ),
        ),
      ),

The interdiff is from #39, except that for ease of reviewing, it does not show the removal of the jsonld_test entity.

There will still be one test failure, as described in #39. "Translatable field with single language value is nested correctly.", which I assume fails because of #1822706: EntityTranslation fails to consistenly apply strict mode.

Status: Needs review » Needs work

The last submitted patch, 1811510-47-jsonld-entity__for-review.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
1.55 KB
18.11 KB

It seems that the behavior I thought was part of the bug in #1822706: EntityTranslation fails to consistenly apply strict mode is instead the expected behavior. Even if a field hasn't been given a value in a particular translation, it will still be a part of the translation's property set.

This now checks whether $translation['field_name']->getValue() returns an empty array. If there is no value for the field in the language, the language is not shown in the JSON-LD.

fago’s picture

$translation['field_name']->getValue() returns an empty array.

You can use isEmpty() for that, which will also work with field items containg an empty value, e.g. array('value' => '').

Anonymous’s picture

Thanks, I've updated the patch to use Field::isEmpty().

Crell’s picture

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEncoder.php
@@ -0,0 +1,27 @@
+class DrupalJsonldEncoder extends JsonldEncoder implements EncoderInterface {

Convention elsewhere has been to alias the parent class to SymfonyJsonEncoder or BaseJsonEncoder or something, then just name ours JsonldEncoder. Don't repeat the "Drupal" string in the class name.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/DrupalJsonldEntityWrapper.php
@@ -0,0 +1,62 @@
+    // Get default language.
+    if ($this->entity->language()->langcode !== 'und') {
+      $defaultLangcode = $this->entity->language()->langcode;
+    }
+    else {
+      $language = variable_get('language_default');
+      $defaultLangcode = $language['langcode'];
+    }

Is there somewhere else we can get language_default from other than variable_get? Please? :-( If this is coming from the Container than we can set something up in the container config, probably...

+++ b/core/modules/jsonld/lib/Drupal/jsonld/JsonldNormalizer.php
@@ -0,0 +1,68 @@
+    // If this is an Entity object and the request is for JSON-LD.
+    return is_object($data) && is_subclass_of($data, 'Drupal\Core\Entity\EntityNG') && static::$format === $format;

Why is_subclass_of? instanceOf is preferred, and works for base classes and interfaces, too.

+++ b/core/modules/jsonld/lib/Drupal/jsonld/Tests/JsonldNormalizerTestBase.php
@@ -0,0 +1,39 @@
+abstract class JsonldNormalizerTestBase extends WebTestBase {

:-( Why can't this be a unit test?

effulgentsia’s picture

Don't repeat the "Drupal" string in the class name.

Per #11, why is it bad for Drupal to be in the class name if it corresponds to Drupal being in the mime type?

Anonymous’s picture

Convention elsewhere has been to alias the parent class to SymfonyJsonEncoder or BaseJsonEncoder or something, then just name ours JsonldEncoder. Don't repeat the "Drupal" string in the class name.

As @effulgentsia pointed out, I discussed this in #11. We are creating two classes here, the JsonldEncoder and the DrupalJsonldEncoder, one per mime type.

Since there is currently no difference in the behaviors between the two, we probably could just collapse them into one and check against the array of supported formats in supportsEncoding. However, there definitely need to be two normalizers, so if the issue is having "Drupal" in the class name, the comments from #11 still stand. I'm open to changing it if there is a suggestion for a better descriptor.

Is there somewhere else we can get language_default

It turns out there is, language_default()... multilingual is not my jam :-/

But instead of using that, I was able to rewrite the language handling code so that default language is no longer needed, plus it is more concise. I'm not sure whether this new code will break once #1822706: EntityTranslation fails to consistenly apply strict mode is fixed... I don't understand how that is supposed to work and/or how it is breaking.

instanceOf is preferred

Fixed in patch.

:-( Why can't this be a unit test?

I'd be happy to be shown wrong, but I don't think we can use the entity system fully enough in a UnitTest. We definitely need to use the entity system for the normalizer tests... the main function of the normalizer is to convert from the entity object structure to the array. Thus, we want these tests to detect whether changes in the entity system affect the JSON-LD output.

We could use UnitTest for encoder tests, but I haven't included those since all behaviors are simply inherited from JsonEncoder.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed the code and the tests and they look good. This is slowing down rest module and web services in general. Lets commit and iterate as we need to.

Anonymous’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for the review, Moshe.

For REST to use this, #1814864: Provide way to register serialization classes will need to be committed and this module will need to register its Normalizers and Encoders. I'd like to add that support before this patch is committed.

I work on that now and post a patch.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
1.35 KB
18.67 KB

This adds integration with the serializer service. It depends on #1814864: Provide way to register serialization classes being committed.

I would feel comfortable committing this and iterating in follow up issues.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

This one is ready too.

We discussed that sometimes caller wants rendered text fields and sometimes not. At BADCamp sprint, we thought that application/ld_json would send rendered and application/vnd.drupal.ld+json would send raw text plus format. Alex and Lin discussed the mechanism for rendering text values and I heard a lot of TypedData and Normalizer words but can't clearly recount them here. Anyway, thats a followup.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome work! This is SO EXCITING!!

We are over thresholds by a couple of major bugs right now, but I feel that it's strategically important to commit this while there are people sprinting at BADCamp, so...

Committed and pushed to 8.x. Thanks!

scor’s picture

Status: Fixed » Needs review
FileSize
1.43 KB

minor fix in the help text: JSON-LD MIME type is application/ld+json.

klausi’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed that the latest patch only changes one character with git diff --color-words. Good to go.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed the follow-up to 8.x as well.

Anonymous’s picture

I've added a followup at #1832840: Enable fieldtype-specific JSON-LD normalization, which will allow us to properly support entity references, and also to do things like sanitize the values of a text field when application/ld+json is requested.

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