Support JSON API, REST, GraphQL and custom normalizations via new computed field

Your IDE creates unusual patches, so I've recreated it.

Would love to have someone with knowledge of JSON API test this.

Unfortunately, it yelled at me when I tried to install Drupal or rebuild the container on an already-installed site.

I'm not sure why the hell this is happening, because everything seems in order:

Error: Class 'Drupal\metatag\Normalizer\MetatagJsonApiNormalizer' not found in /path/to/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php

Reroll seems needed.

Patch in #9 had the MetatagJsonApiNormalizer wrongly located (in "Normalizer" instead of "src/Normalizer").

1. +++ b/src/Normalizer/MetatagJsonApiNormalizer.php
   @@ -0,0 +1,28 @@
   +    $field_item_value = new FieldItemNormalizerValue($normalized);
   

   The interface for FieldItemNormalizerValue doesn't match what's here.

2. +++ b/src/Normalizer/MetatagJsonApiNormalizer.php
   @@ -0,0 +1,28 @@
   +    return new FieldNormalizerValue([$field_item_value], 1);
   

   The interface for FieldNormalizerValue doesn't match what's here.

Needs some changes per #15.

What is the "jsonapi_normalizer_do_not_use_removal_imminent" bit for?

Updated arguments passed to `FieldItemNormalizerValue` and `FieldNormalizerValue`.

I've followed the approach implemented here \Drupal\jsonapi\Normalizer\FieldNormalizer::normalize

@DamienMcKenna I only briefly looked at `jsonapi_normalizer_do_not_use_removal_imminent` and couldn't see an example of how to implement it.

#18 Patch applied successfully and works fine

Path reviewed - works as per the requirement.

Based on my test of the patch, the endpoint is displaying internal data that look like machine names and don't correlate to valid markup. I think this data needs some more normalisation.

What is "canonical_url"? Should it be a "meta" or a "link" tag. What is the the correct attribute to apply the value to? Now ask these questions for each of 100+ meta tags.

Sorry to do this, but I would also argue that we should not be using the jsonapi_normalizer_do_not_use_removal_imminent service tag, because "do not use" is right there in the name of the tag. That tag is not an API; it's a temporary hack in JSON API, and it's going to be removed (at some point in the near future). Although the most recent patch does indeed stop the bleeding, we should try to find a better way to fix this in Metatag. I suggest we contact the JSON API maintainers and ask them what the best course of action here might be.

Some code to ponder - this is how we solve it in absence of a better way. The last patch doesn't allow us to do this.

Our backend code already overrides `JsonApiDocumentTopLevelNormalizer` for other purposes, bear i mind this this class is deprecated, override at your own risk.

  /**
   * {@inheritdoc}
   */
  public function normalize($object, $format = NULL, array $context = []) {

    .... snip ...

    // Metatags
`    // The following is based on metatag_get_tags_from_route().
    $metatag_manager = \Drupal::service('metatag.manager');
    $renderer = \Drupal::service('renderer');
    $entity = $object->getData();
    if ($entity instanceof Node) {
      $metatags = metatag_get_default_tags($entity);
      foreach ($metatag_manager->tagsFromEntity($entity) as $tag => $data) {
        $metatags[$tag] = $data;
      }
      $context = [
        'entity' => $entity,
      ];
      \Drupal::service('module_handler')->alter('metatags', $metatags, $context);
      $pre_rendered_tags = $metatag_manager->generateRawElements($metatags, $entity);
      $normalized['data']['attributes']['metatag'] = $pre_rendered_tags;
    }

    return $normalized;
  }

Frontend code is a simple handling of a drupal render array. I think it is input to the vue-meta plugin, but you get the idea. Thanks to Dan Laush.

export const buildHeadTags = (data) => {
  const tags = {}
  
  Object.keys(data).forEach(meta => {
    const tag = data[meta]['#tag']
    if(!tags[tag]) tags[tag] = []
    tags[tag].push(data[meta]['#attributes'])
  })
  
  return tags
}

I have tried both the patches with no success with the following setup:
Drupal 8.5.3
Metatag 8.x-1.5
JSONAPI 8.x-1.18

After updating JSON API module to latest 8.x-1.22, which supposedly fixes the issues outlined in this issue:
https://www.drupal.org/project/jsonapi/issues/2888157

When I attempt to view the JSON API endpoint, I get this error:

TypeError: Argument 2 passed to Drupal\jsonapi\Normalizer\Value\FieldItemNormalizerValue::__construct() must implement interface Drupal\Core\Cache\CacheableDependencyInterface, null given, called in /home/vagrant/docroot/web/modules/contrib/metatag/src/Normalizer/MetatagJsonApiNormalizer.php on line 29 in Drupal\jsonapi\Normalizer\Value\FieldItemNormalizerValue->__construct() (line 38 of modules/contrib/jsonapi/src/Normalizer/Value/FieldItemNormalizerValue.php). 

If i remove the patch from this issue that adds the metatag support, I no longer get the error, but of course, I also no longer have the metatags output into the JSON API endpoint.

Things I've tried:
* Downgrading JSON API to earlier versions before 1.22 but after 1.18
* Using latest dev release of Metatag
* Seeing if any combo of the above works to both 1) work with no error 2) adds metatags to JSON API
* Tried both #14 and #18 patches

And this is why we add tests.

@joshua.boltz: thanks for the details.

@joshua.boltz and I did some troubleshooting. We determined that this patch works with jsonapi 8.x-1.21 and not with 8.x-1.22. The issue is that the context array in MetatagJsonApiNormalizer::normalize() does not have the key 'cacheable_metadata'. In JsonAPI .22, 'cacheable_metadata' was removed:

Issue #2948666 by Wim Leers, gabesullice: Remove JSON API's use of $context['cacheable_metadata']

#2948666 is the ticket in which the key was removed and it has a lot of useful information around why that decision was made.

We determined that there was a change in jsonapi/src/Normalizer/JsonApiDocumentTopLevelNormalizer.php that no longer populated the context with the needed key.

Below are two screenshots from PHP storm show the stack trace and values in the context array.
JsonApiDocumentTopLevelNormalizer_22

JsonApiDocumentTopLevelNormalizer_21

I think the next step is determining how to get the required CacheableDependencyInterface for jsonapi/src/Normalizer/Value/FieldItemNormalizerValue.php.

Alright, I am going to push this back to needs work. After reviewing jsonapi, I think I found the right way to instantiate FieldItemNormalizerValue().

We still need a solution for #23, if someone could explain what we are doing in the service provider that would be helpful.

Needs work for #23

Hi all! I'm incredibly glad to see y'all trying to operate w/ JSON API :)

I think you're heading down the wrong rabbit hole (but you should be forgiven for it, the "right" way is not at all obvious).

Your normalizers should be operating on the TypedData level, not the field level. If they work that way, they will work for JSON API, GraphQL and REST without customization for any of them.

What does that mean though?

An entity like a tree, you have an entity object at the top, fields on that entity, every field is a list of field item, and a field item is composed of field properties (which must be TypedData properties). Finally, the value is stored on that property level.

node                                <- Entity level
  fields
    field_one_item_list             <- Field level
      field_one_item_one
        field_property_foo          <- TypedData level
           "I'm an actual value"
        field_property_bar
           42
      field_one_item_two
        property_foo
        property_bar
    field_two_item_list
      ...
      ...

Right now, your normalizers are working on the field item list level because that's the level they "support":

use Drupal\node\Entity\Node;
use Drupal\metatag\Plugin\Field\MetatagEntityFieldItemList;
use Drupal\metatag\Plugin\DataType\Metatag;

Entity level:
$supportedInterfaceOrClass = Node::class;

Field list level:
$supportedInterfaceOrClass = MetatagEntityFieldItemList::class;

TypedData/Field property level:
$supportedInterfaceOrClass = Metatag::class;

Drupal, JSON API and GraphQL already have standard normalizers for the entity and field list and field item levels. You don't need to override those. What all those existing normalizers do is just call $serializer->normalize() the the next layer down. You want to do your normalizers at the lowest possible level to have the most possible compatibility with any kind of normalization.

Does that help?

@gabesullice is exactly right in #32 about working within the Typed Data API instead. This will offer the most flexibility across the entire Drupal ecosystem since that API is a much lower level driver for many others built on top of it.

Another red flag I spotted worth noting was the return of a non-primitive value in the normalize() method. That breaks the interface definition for the normalizers which assume a simple value will be returned that can be passed directly into the encoder for writing out. I vaguely recall seeing an issue related to this same issue in other normalizers within jsonapi as well, so that may be a separate effort to address if it was something you were following by example. Either way though, @gabesullice's recommendation to use $serializer->normalize() should resolve that issue since it allows any non-primitive values that still need to be handled to pass through the whole serialization system again and receive their own specific treatment.

One excellent article to review that might help wrap your head around the overall serialization process and what is happing is this one: Drupal Serialization Step-by-Step.

Nice work so far everybody! There's a lot of movement in this space, so it's hard to hit the moving target.

Another red flag I spotted worth noting was the return of a non-primitive value in the normalize() method. That breaks the interface definition for the normalizers which assume a simple value will be returned that can be passed directly into the encoder for writing out. I vaguely recall seeing an issue related to this same issue in other normalizers within jsonapi as well, so that may be a separate effort to address if it was something you were following by example.

This is exactly why we introduced the jsonapi_normalizer_do_not_use_removal_imminent service tag and made all normalizers internal.

We cordoned everything off above the TypeData level so that we could isolate our own wrong implementation and then refactor and do it "right". That process is not complete.

+++ b/src/MetatagServiceProvider.php
@@ -31,6 +32,10 @@ class MetatagServiceProvider extends ServiceProviderBase {
+      $metatag_json_api = new Definition(MetatagJsonApiNormalizer::class);
+      $metatag_json_api->addTag('jsonapi_normalizer_do_not_use_removal_imminent', ['priority' => 2]);
+      $container->setDefinition('metatag.normalizer.metatag.json_api', $metatag_json_api);

As of JSON API 2.x. It is impossible to add normalizers via this method.

You'll need to implement your normalizers on the TypedData level as I explained above and use the normalizer service tag, not jsonapi_normalizer_do_not_use_removal_imminent tag.

I talked with Gabe and think I follow what he's laying down. I had to get this "fixed" since it completely breaks default_content's exports so I tossed together this quick patch. No tests and I only really tested it didn't break _other_ imports so not even really manually tested but it but I think gets things going in the right direction.

FWIW, I haven't looked at the new patch.

Just setting this so we get a test run.

After a video session with @gabesullice, I did some changes to try to enable the normalizer with JSON 2.x

1. Eliminating MetatagJsonApiNormalizer, already eliminated in branch 8.x-1.x
2. Removing service metatag.normalizer.metatag.json_api using tag jsonapi_normalizer_do_not_use_removal_imminent, already eliminated in branch 8.x-1.x
3. Update class MetatagNormalizer to work are TypedData level

The problem I see is that the meta tags itself are not processes, and are not rendered in JSON API response.

Please if someone could test and let me know if the problem is related to Metatag logic or JSON API implementation logic., would be appreciated.

@-enzo- and @gabesullice i think the problem here is that metatag does not actually save the data in this field but only the overwrites.

So the actual output is calculated out of all defaults and the overwrites on rendering. And this does not happen here.

I think we will need to change the meta tag field to something like a computed field which does this and not render the metatags only in metatag_get_tags_from_route.

Does this make sense?

I guess there are several issues here and they overlap in ways people don't quite understand:

• The Metatag field is completely optional, it's possible to have meta tags output on an entity without there being a field.
• The Metatag field only stores values which override the relevant defaults, and even then they can store tokens. This means that the field could be completely empty for a given entity, while the rendered entity could display a ton of meta tags.
• The final output meta tags are generated at runtime, they aren't stored anywhere.

The question that must be asked before additional work is done is: what's the actual goal of this functionality? Is the intention to show the overridden values when there is a Metatag field present on the entity, or is the intention to show all of the rendered meta tags for the entity? These are two very different goals.

The question that must be asked before additional work is done is: what's the actual goal of this functionality? Is the intention to show the overridden values when there is a Metatag field present on the entity, or is the intention to show all of the rendered meta tags for the entity? These are two very different goals.

Ahhh, thanks @DamienMcKenna! I did not really look into what was actually going on in Metatag module. I was just trying to show how one has to implement normalizers for JSON API to use them.

To answer your question, I think the answer is a mix of both.

The current field should be a computed field which accepts new values so that metatags can be altered via an API. However, this should be a low priority done in a different issue. For now, I think the field needs to be marked internal so that it is not normalized by any web services.

In its place, I think the most important step is to add another computed field which shows all of the rendered meta tags for the entity so that decoupled applications can make use of them in a read-only capacity. This should be the highest priority of this issue.

Do others agree?

#39:

I think we will need to change the meta tag field to something like a computed field which does this

+1000

But based on what @DamienMcKenna wrote in #40, it sounds like we need a separate computed field to store not the override, but always computes the end result, or the existing field should be made to not only allow storing values, but also compute the value in case it has no stored value (which @gabesullice suggested in #41).

The architecture described in #40 makes sense given Drupal's history (and Metatag's), but it makes it impossible to use in decoupled use cases. In other words: Metatag is not yet API-First.

I think we will need to change the meta tag field to something like a computed field which does this

No, because the field is not required in order to output meta tags for an entity, and I don't want to make it required.

The architecture described in #40 makes sense given Drupal's history (and Metatag's), but it makes it impossible to use in decoupled use cases. In other words: Metatag is not yet API-First.

True.

I'd be happy to hear ideas on how to make this work.

No, because the field is not required in order to output meta tags for an entity, and I don't want to make it required.

Right, the Metatag module is "entity-centric" if you will, and not "field-centric". But all entity data is modeled as fields. So the only way to add data to entities consistently, and in a way that is API-First, is to add a field.

I completely understand you don't want to require Metatag users to go and add a particular field to every entity type though! It also wouldn't make sense in the Field UI.

I'd be happy to hear ideas on how to make this work.

What if we add a computed field automatically through hook_entity_(base|bundle)_field_info(), for those entity types/bundles that have Metatag enabled?

What if we add a computed field automatically through hook_entity_(base|bundle)_field_info(), for those entity types/bundles that have Metatag enabled?

That would be just fine!

One question is how we would control which entities would support this? Are there reasonable ways to identify which entities should be supported, to avoid unnecessary processing on entities where it makes no sense, e.g. blocks, paragraphs, etc?

One question is how we would control which entities would support this?

Any fieldable entity type could support this.

Are there reasonable ways to identify which entities should be supported, to avoid unnecessary processing on entities where it makes no sense, e.g. blocks, paragraphs, etc?

Blocks aren't fieldable (BlockContent is, Block isn't). Paragraph is an interesting one. I guess I'd blacklist Paragraph for now, because it's such a special snowflake. I don't know the architectural details of Paragraph well enough to make a recommendation for that.

One question is how we would control which entities would support this?

Any fieldable entity type could support this.

I guess the question should have been asked in reverse - how would we make sure the extra field is only added to entities where it's wanted so it doesn't show up in places that isn't necessary? In D7 there was a settings page to control the list of entities which were supported, unless you have other ideas we could just do that again.

no_ui = TRUE just like ChangedItem and other field types have. Or did you mean something else?

no_ui = TRUE just like ChangedItem and other field types have. Or did you mean something else?

I wasn't aware of that. Neato.

😃

@DamienMcKenna is currently in this week's API-First Initiative call. He brought up this issue, and asked for examples that do something like #44–#50.

I found that the "path" field is super close to what Metatag needs! 🎉 See path_entity_base_field_info() and \Drupal\path\Plugin\Field\FieldType\PathItem. If you have the need for bundle-specific field definitions, you may want to look at https://www.drupal.org/node/2982512 too (8.7 only, so you may want to backport some of that).

@e0ipso warned that there were some problems with JSON:API+path fields. I responded that I'm 90% certain this is due to language-specific path aliases and a core bug: #2511968: Path field should fall back to language neutral aliases (also makes this happen for the form widget!). I doubt (hope!) that Metatag is not like path aliases in this respect.

This is definately more complicated then it looks on the first sight but i think this is the correct direction.

I got stuck at the following this:

There is no way to "just get the current entity overwrites" without running into circular references because of the computed field and the way metatag "renders" the metatags.

For the output i added JSON::encode to get "some" output which currently is the default entity definitions which is "something".

For a clean json output i think the field definitions are not working here or we need to add an additional field with own propertyDefinitions because for the jsonapi output we definately want this to be a multi value field and the keys need to differ from the current render elements.

The html output rendering stopped working like this with the following error:

Error: Unsupported operand types in Drupal\metatag\MetatagManager->getFieldTags() (line 342 of modules/contrib/metatag/src/MetatagManager.php).

The jsonapi output works so ;)

Leaving this on "needs work"

Okay here the mentioned different approach. It works like this but i think it's not really nice.

This will output the meta tags like the following:

"metatag_normalized": [
	{
		"tag": "meta",
		"name": "title",
		"content": "test abcd"
	},
	{
		"tag": "meta",
		"name": "description",
		"content": "test 12341234"
	},
	{
		"tag": "meta",
		"name": "abstract",
		"content": "test 1234"
	}
],

Will leave for review if this approach is whats wanted here.

This should hopefully resolve the coding standards and notices.

@yobottehg: I really appreciate you taking the time to work on this, hopefully someone will get to review it soon.

At a cursory glance I feel like this will only work for certain meta tags, ones which specifically use the "name" and "content" attributes, but it seems like a reasonable starting point.

Patch in #55 works for basic tags that have a name and a content attribute (tested in a decoupled project using jsonapi).

Adding a patch that supports all metatags, regardless of their attribute keys. Wasn't exactly sure about the datatype for the property definition, so I used "any" for now.

The patch in #58 works nicely. Thanks @s_leu!

I'm also unsure about the datatype but perhaps someone else can review this.

I realized that this is seen as a base field which will be computed also for entity references which are not included in the response. That means if you query a node which has 5 terms attached which are not included you always get all metatags for them. I'm asking myself how expansive this is in regards to computing power.

I think we should limit the rendering of the metatags to the main entity which is queried and perhaps @Wim Leers knows a mechanism for that inside jsonapi?

Also i would like to get some feedback on the naming of the field which is not the final one i guess?

Leaving this on NR for other people to chime in.

Thanks for working on this, @yobottehg! 👏

perhaps @Wim Leers knows a mechanism for that inside jsonapi?

JSON:API doesn't do anything special; it only maps Entity/Field/Typed Data to https://jsonapi.org/format/'s data structures. If you say it doesn't work quite as expected, then the same problem will be true in core REST and GraphQL.

I realized that this is seen as a base field which will be computed also for entity references which are not included in the response. That means if you query a node which has 5 terms attached which are not included you always get all metatags for them.

Why would the referenced entity objects be instantiated? Only when the referenced entities are instantiated would their base fields be constructed, and hence this computed base field's computations get executed.

1. +++ b/metatag.module
   @@ -530,6 +530,13 @@ function metatag_entity_base_field_info(EntityTypeInterface $entity_type) {
   +      ->setCardinality(-1)
   

   Should use the CARDINALITY_UNLIMITED constant. Fixed.

2. +++ b/src/Plugin/Field/FieldType/MetatagNormalizedFieldItem.php
   @@ -0,0 +1,50 @@
   +    $properties['attributes'] = DataDefinition::create('any')
   

   Why any?

3. +++ b/src/Plugin/Field/FieldType/MetatagNormalizedFieldItem.php
   @@ -0,0 +1,50 @@
   +    return $value === NULL || $value === serialize([]);
   

   Why can this both be NULL and the PHP-serialized empty array?

4. +++ b/src/Plugin/Field/FieldType/MetatagNormalizedFieldItemList.php
   @@ -0,0 +1,33 @@
   +      $this->list[] = $this->createItem(0, $item);
   

   This 0 needs to match the index in $this->list.

Test coverage for the various edge cases would be super valuable here.

I'm using this patch in a D8 "factory" site feeding content to a huge amount of Nuxt/Vuejs SSR websites.

It saved my bacon, until I configure Metatag to use tokens related to the node URL: MetatagNormalizedFieldItemList doesn't check if the entity exists (has an ID), which breaks the add node form (I guess it'll be the same for any entity as in https://www.drupal.org/project/metatag/issues/2947493).

This patch just adds the check for the ID, so when the node is added, Metatag doesn't try to generate this URL related stuff.

Patch #61 works with both Metatag 8.x-1.7 and 8.x-1.x dev on JSON:API 8.x-2.0.

I can also confirm patch #61 works perfectly on one site in production.

Yep I can also confirm that it is working for me (same versions as #62).

Just thought I'd add a quick comment in case anybody else gets a bit confused initially... For JSONAPI the meta tag data is not output as part of the field data. Instead there is a new 'metatag_normalized' key within 'attributes' (which is within 'data') that has all of the relevant information.

This is why I think we should have test coverage to document what the output should look like.

Agreed. Needs tests still exists.

See \Drupal\Tests\datetime\Functional\EntityResource\EntityTest\EntityTestDateonlyTest for an example of a generic REST test for a particular field type (the date field in this case).

Thanks @Wim-Leers, I will try to write an initial test for this issue this week.

I made an initial attempt at the test, however, I was getting some failures in my local dev environment. I'm posting here to get a little feedback as well as to see what the testbot reports.

FYI the branch tests aren't working because the Devel module needs a new release due to API changes in core: #3042739: Fix tests on Metatag 8.x-1.x branch

I changed the default tests to work against 8.6.x (#3042739: Fix tests on Metatag 8.x-1.x branch), so lets see what the testbot says.

Thanks for writing that test @axle_foley00, good start!

1. +++ b/tests/src/Functional/EntityTestMetatagTest.php
   @@ -0,0 +1,89 @@
   +/**
   +   * Datetime test field name.
   +   *
   +   * @var string
   +   */
   +  protected static $fieldName = 'metatag'; // or should this be 'field_meta_tags' or 'metatag_normalized'?
   

   I think we can just drop this static, TBH.

2. +++ b/tests/src/Functional/EntityTestMetatagTest.php
   @@ -0,0 +1,89 @@
   +    return parent::getExpectedNormalizedEntity() + [
   +        'metatag_normalized' => [
   

   We'll also need to add the uncomputed metatag field here too since they're both gonna be on the normalized entity.

Running the tests locally, it looks like we need to override the expected cache contexts and tags to take into consideration the metatag field's cacheability.

The attached interdiff does that and also addresses #60.4 plus some other minor things.

Setting to NR for tests.

Just wanted to +1 to the patch in #61 as it solves the gap in using jsonapi and metatag. Can't help out with the tests unfortunately.

And one OT documentation notice for people using Metatag and Jsonapi, and might face an issue why their metatags work when using the node detail view but not in jsonapi request:
If you have configured Metatags in Drupal to use the Global settings and using the current_page options in tokens, then they will not work for entities being requested via jsonapi, as current_page then is actually the jsonapi endpoint.

So for example when you have for the title tag something like this: [current_page:title] under the Global config, this needs to be moved under Content and the token should be entity aware, so something like this: [node:title].

But other than that, thanks for the patch and the module! Looking forward to the patch being committed and removing another rock from making the API first Drupal truly great! :)

Out of interest, does #2901039: Add a TypedData plugin to support Search API help with this effort in any way?

Apologies in advance if this is not the right issue to bring this up in :)

We are using the patch #61, which works great for the normal title / meta description fields. I noticed however that when used in conjunction with the Schema.org Metatag module, we end up with some type errors when we build in gatsby.

Looking at the JSON API output, my best guess at what is going on is that most "content" values are strings, but some of the Schema.org "content" values are objects. For example here is the output for the datePublished Schema.org metatag:

{
  tag: 'meta'
  attributes: {
    name: "datePublished",
    content: "2019-06-18T15:58:57-0600",
    group: "schema_article",
    schema_metatag: true
  }
}

And here is the output for the schema.org author tag:

{
  "tag": "meta",
  "attributes": {
      "name":"author",
      "content": { 
         "@type": "Person",
         "@id": "http://example.com/taxonomy/term/11",
         "name":"First Name Last Name"
       }
   }
}

I don't have a proposed solution for this, but wanted to raise it here in case someone comes across a similar issue.

Running on Drupal core 8.7.4. The patch `2945817-73.patch` works for me. Fixes issue with `metatag_normalized` in the JSON API and fixes the error which keeps happening on node save or term save.

First, thanks to all the contributors working on this issue.

For the record, inspired from patch in comment 73. I have made a new field enhancer for JSON:API in comment #3060702-18: Compatibility with Metatag Fields to be able to deploy metatags using Entity share module.

Does this field enhancer should be in Entity share or in JSON:API Extras or in Metatag?

@Grimreaper,

My thoughts :

If this enhancer is of any use when not using Entity share (which is what I understant), I think that it should NOT leave in Entity share.

Then chosing between putting that in JSON:API Extras or Metatag is just a matter of where it allows to more properly handle the code dependencies of the enhancer (which surely references classes from JSON:API Extras and maybe some code from Metatag).

Thanks @garphy for the feedback.

I will ping back when it will be ok for at least Entity Share usage.

Hello,

As said in my previous comment, I will ping back when ok for Entity share.

Here is the commit: https://git.drupalcode.org/project/entity_share/commit/63f7282

And the following issue for tests #3089844: Tests: Metatag field.

Hi guys, I applied the patch #61 in my project and works very well, thanks @superbiche.

Anyone want to take a try at finishing the tests?

Hi guys, I applied the patch #61 in my project but throwing the following error.

LogicException: The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\jsonapi\ResourceResponse. in Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (line 154 of /var/www/html/drupal/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php).

Re-Rolling patch #73, as it does not apply to latest dev.

1. +++ b/src/Plugin/Field/FieldType/MetatagNormalizedFieldItemList.php
   @@ -0,0 +1,37 @@
   +use Drupal\Core\TypedData\ComputedItemListTrait;
   ...
   +  protected function computeValue() {
   +    $entity = $this->getEntity();
   +    if (!$entity->id()) {
   +      return;
   +    }
   

   Drupal\Core\TypedData\ComputedItemListTrait caches the first computed value, so this means if the value's computed before the entity's saved (eg. on validate) it will be cached as empty. I don't think we can use this trait as-is because there are different metatags for a new vs saved entity (eg. the entity's canonical URL is available).

2. +++ b/tests/src/Functional/EntityTestMetatagTest.php
   @@ -0,0 +1,98 @@
   +    $entity_test = EntityTest::create([
   +      'name' => 'Some Cool Content',
   +      'type' => static::$entityTypeId,
   +      'metatag' => [
   +        'value' => [
   +          'title' => 'Some Cool Content',
   +          'canonical_url' => '',
   +          'description' => 'This is a description for use in Search Engines',
   +          'keywords' => 'drupal8, testing, jsonapi, metatag'
   +        ]
   +      ],
   +    ]);
   

   As I understand it this is trying to write to the old/existing computed metatag field, which I don't think is supported. Use metatag configuration entities and/or a field of type metatag.

I've had a go at getting the tests to run green: I've updated the new computed field to recompute once the entity is saved and added some metatag config at the global and entity type level. (I did also try to add a field of type metatag, but I got a failure in \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testGet() connected with bc_primitives_as_strings. I'm posting this simpler patch first to get testbot onside.) Feedback welcome, thanks!

Btw it occurred to me that there's not much documentation around the difference between the two computed fields (metatag and metatag_normalized). AFAICT at the moment at least they both only exist for serialization?

The test was assuming no base path: I'm not sure if it would be better to return absolute URLs, but I've modified it to add the base path. Also codesniffer fixes. Thanks!

The new test passes, but my changes have introduced a raft of new failures in other tests (: They seem to be connected with trying to generate metatags from an unsaved entity; looks like that's not a safe thing to allow in general, so the new patch doesn't generate the metatags in computeValue until the entity's saved.

Here's an update which adds a field of type metatag to the entity. I've had to dynamically alter the expected normalization based on the value of serialization.settings.bc_primitives_as_strings: I don't really know anything about that setting so would welcome review/feedback. Thanks!

I was looking to expose metatags per node on my JSON:API endpoint.

Endpoint uri: jsonapi/node/landing/[id]

Patch #93 worked for Drupal core 8.8.7.

First, thanks to all that are working on this issue. I have applied patch #93 for Drupal core 8.8.7 and looks like we aren't able to retrieve node fields using tokens, for example, [node:field_name], is there a way we can add this option as well? Thanks in advanced.

I can't replicate #93. Tokens on node fields are working fine. However, it appears that this patch does not offer the opportunity to alter the metatags once they are generated. Shouldn't one of the hooks in metatag.api.php be called after the tags have been generated?

I'm seeing #96 as well. Token output seems to be solid when I request _format=json versions of the content in question, but not set when I request it through JSON:API. I'm not sure why I'm seeing #96 and @achap isn't. I'm pressed for time but I might be able to look into this and find the root cause.

EDIT: After setting the value of the 'abstract' field to a non-token value and saving, the fields started showing up in the JSON:API response.

Patch #93 works well:

{
    "metatag_normalized": [
        {
            "tag": "meta",
            "attributes": {
                "name": "title",
                "content": "metatag test | Site-Install"
            }
        },
        {
            "tag": "link",
            "attributes": {
                "rel": "canonical",
                "href": "http:\/\/drupal.local:8080\/articles\/tue-09222020-1304"
            }
        },
        {
            "tag": "meta",
            "attributes": {
                "name": "description",
                "content": "summary"
            }
        },
        {
            "tag": "meta",
            "attributes": {
                "name": "abstract",
                "content": "test"
            }
        }
    ]
}

@dmitry: Did you clear the site's caches after applying the patch? What version of core are you using? What sort of page request resulted in that error?

@damienmckenna,
Updated my comment.
Seems that something is wrong with applying patch not through composer patches. PhpStorm + "Apply Patch from Clipboard" did not apply patch correctly. Probably it can be related to git tree of metatag module.
Version used: Drupal Core 8.9.5 + Metatag 1.14.

So, I know there is the two computed field approach. What I was wondering:

Why can't MetatagEntityFieldItemList return a value in ::computeValue, and the field property be a new DataType plugin that can have a lower-level normalized attached? The problem with \Drupal\metatag\Normalizer\MetatagNormalizer is that it targets the field level and not the data property level. If we adjusted things a bit, this could "just work" if we computed the value and allowed the normalized to target the field property.

I've done this in Commerce API several times with custom dynamic computed fields.

For instance:

Address

• https://git.drupalcode.org/project/commerce_api/-/blob/8.x-1.x/src/Plugi...
• https://git.drupalcode.org/project/commerce_api/-/blob/8.x-1.x/src/Typed...

I'll read over again. It was hard to grok since this started with JSON:API 1.x, then 2.x then JSON:API in Drupal core.

Here is my take on #93, refactored to modify the existing computed field instead of adding another one.

The interdiff is here: https://git.drupalcode.org/project/metatag/-/merge_requests/4/diffs?comm...

I guess my changes introduce backwards compatibility problems for anyone relying on REST.module whereas #93 provided a more compatible fix. But I think we can be honest that most folks are not using HAL or the default JSON encoding. And, the BC is just that the shape of the JSON changed, to something more usable.

Pushed a fix to my changes: https://git.drupalcode.org/project/metatag/-/merge_requests/4/diffs?comm.... DrupalCI is running against the MR

After using this patch I realized our entities were throwing leaked metadata on `update`.

Workaround to execute in a render context pushed.

One test failure left.

Failure seems to come from https://git.drupalcode.org/project/metatag/-/merge_requests/4/diffs?comm... ?

1) Drupal\Tests\metatag\Functional\EntityTestMetatagTest::testGet
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
     0 => 'config:rest.resource.entity.entity_test'
     1 => 'config:rest.settings'
-    2 => 'config:system.site'
-    3 => 'config:user.role.anonymous'
-    4 => 'entity_test:1'
-    5 => 'http_response'
+    2 => 'config:user.role.anonymous'
+    3 => 'entity_test:1'
+    4 => 'http_response'
 )

Re-roll of @mglaman's patch in #104 to capture updates to the base branch (8.x-1.x)

To avoid pushing the merge and potentially breaking anyone's builds I decided not to request push access to the MR branch but can update once properly tested.

Patch #93 also works well for Drupal 9.1.5.

Fixing the test errors.

I think #114 lost some changes..

Am looking into this.

This fixes some of the tests.

The last test failure comes from EntityResourceTestBase::testGet() where $this->getExpectedCacheTags() and $response->getHeader('X-Drupal-Cache-Tags')[0] are different, the former having "config:system.site" which the latter does not.

Hi everybody,

I use Drupal decoupled as backend CMS and I'm trying to change the canonical URL. In the metatag settings (/admin/config/search/metatag) I set the canonical for content (articles) to be [node:url:path] since the frontend is on another domain. While doing this, if I open an article in drupal, the canonical URL is ok, but the metatag normalized returns the full absolute URL and it seems that it doesn't take in account the Tokens used in the Metatag settings page.
Any ideas how I can fix this or any workarounds available? (I'm using the #93 patch, but I also tested #118 with the same results)

Best Regards,
Adrian

@adrienco88: Please open a separate issue for us to help you with that support request. Thank you.

The latest patch (#118) doesn't work for me, but #93 works flawlessly.

+1 for patch on #93

Using Drupal 9.2.8, and a combination of few JSON:API contrib modules, including the jsonapi_include which alters the output of JSON to include referenced entities.

metatag_normalized includes all meta tags for the entity; the ones defined on the entity directly, and also the inherited tags from the defaults as well.

One tiny issue I noticed, and didn't find a reference to it in the comments above is the TITLE tag rendering:

      {
        "tag": "meta",
        "attributes": {
          "name": "title",
          "content": "My page title"
        }
      }

IMHO, considering the <title> tag is the only exception among the rest of meta tags (<meta />, or <link /> tags), defining an exception in the consumer app might be a much simpler solution than "fixing" the output of it here.

Reroll.

Let's see if this fixes MetatagSerializationTest.

So #126 fixes one of the tests (yay!), but someone else broke. :facepalm:

I reran the tests on #126 and now it shows just the one failure related to cache contexts, which I was expecting.

So how do we get the cache contexts to have the "config:system.site" item?

My mistake, it's the cache tags :-\

re-roll @GRO's patch in #122 to capture updates to the base branch (8.x-1.x) (1.7) , works with Drupal9.2.10

The patch in #132 works for me in Drupal 9.3

Rerolled #126.

This reverts the original Normalizer classes and marks them deprecated; we don't want to break the APIs.

Back to this problem again:

1) Drupal\Tests\metatag\Functional\EntityTestMetatagTest::testGet
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => 'config:rest.resource.entity.e...y_test'
-    1 => 'config:system.site'
-    2 => 'config:user.role.anonymous'
-    3 => 'entity_test:1'
-    4 => 'http_response'
+    1 => 'config:user.role.anonymous'
+    2 => 'entity_test:1'
+    3 => 'http_response'
 )

From a technical perspective are there any problems keeping the older normalizers? While they might be redundant with the new formatters, sites are using them already and I don't want to break them. Thank you.

If getExpectedCacheTags() is not overridden then it fails with this:

1) Drupal\Tests\metatag\Functional\EntityTestMetatagTest::testGet
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     0 => Array ()
     1 => Array (
         0 => Array (
-            0 => 'Llama'
+            0 => '5defb102-f37b-4668-8d2b-bf5af730f1b2'
         )
     )
     2 => Array (
         0 => Array (
-            0 => 'en'
+            0 => 'Llama'
         )
     )
     3 => Array (...)
     4 => Array (
         0 => Array (
-            0 => 1
+            0 => '/entity_test/1'
+            1 => 'Llama | Drupal'
+            2 => 'This is a description for use...ngines'
+            3 => 'drupal8, testing, jsonapi, metatag'
         )
     )
     5 => Array (
         0 => Array (
-            0 => '5defb102-f37b-4668-8d2b-bf5af730f1b2'
+            0 => 'en'
         )
     )
     6 => Array (
         0 => Array (
+            0 => 1
+        )
+    )
+    7 => Array (
+        0 => Array (
             0 => Array (...)
         )
     )
-    7 => Array (
+    8 => Array (
         0 => Array (
             0 => '2022-02-21T17:55:33+00:00'
             1 => 'Y-m-d\TH:i:sP'
         )
     )
-    8 => Array (
+    9 => Array (
         0 => Array (
             0 => 0
             1 => '/user/0'
@@ @@
             2 => '9cd85bd1-ed45-49a5-b246-2a09aa5a7ffd'
             3 => 'user'
         )
-    )
-    9 => Array (
-        0 => Array (
-            0 => 'link'
-            1 => Array (...)
-        )
-        1 => Array (...)
-        2 => Array (...)
-        3 => Array (...)
     )
 )

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/IsEqual.php:100
/var/www/html/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:568
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:703

The getExpectedCacheTags() override comes from #73 where gabesullice was trying to trying resolves problems with meta data in the tests. Does the fact that it's failing mean that there's a problem elsewhere in Metatag, that something else needs to be changed to add the cache tag elsewhere, or that the changes Gabe added need to be handled a different way?

Some minor coding standards improvements.

Updating #143 to ensure that it invoked hook_metatags_alter(). The interdiff is messy for some reason so I'm not including it, but it's just adding the following to the computed field:

      // Trigger hook_metatags_alter().
      // Allow modules to override tags or the entity used for token replacements.
      $context = [
        'entity' => &$entity,
      ];
      Drupal::service('module_handler')->alter('metatags', $metatags_for_entity, $context);

Didn't upload the patch, whoops.

EDIT: Hmm, something isn't right. Will need to upload a new one.

Alright this one should work.

@mrweiner stange but patch 147 does not invoke Drupal::service('module_handler')->alter('metatags and does not expose hreflang at all via drupal/jsonapi_extras module (metatag field not empty).

with patch 73 it exposes hreflang via drupal/jsonapi_extras module but not invokes alter() too

Re-rolled for 8.x-1.20

Re-roll for (8.x-1.20)

Re-roll for (8.x-1.20)

Patch doesn't seem to work with v 8.x-1.22 anymore

Re-rolling this for latest dev (applies for 8.x-1.22 as well).

No changes, just resolved conflict since src/Normalizer/MetatagNormalizer.php was updated in https://git.drupalcode.org/project/metatag/-/commit/b54dffd?page=2 as part of API updates for Drupal 10, and is removed in this patch.

Patch from 154 works for me on 8.x-1.22

Patch from 154 works for me as well.

Patch 154 works well, but the part for altering metatags introduced in patch 147 is missing.

Attached the modified patch and the interdiff

@robertom invokation of metatags_alter(array &$metatags, array &$context) still not happening when called from JSON API endpoint. MetatagEntityFieldItemList ::valueNeedsRecomputing() and MetatagEntityFieldItemList ::computeValue() are not invoked too.

UPD: found there was base_field_override on metatag field + patch was not applied correctly. Seem now all working, checking. Dunno how to delete comment
So #157 seems works, I like it.

rebased.

(hiding the old patches)

Fixing one of the test failures.

Back to one test failure, the same issue reported in #73 - the cache context problems.

Working on the last test failure.

Does this help?