Normalizers violate NormalizerInterface

Closing as duplicate of #2860350: Document why JSON API only supports @DataType-level normalizers.

You can read there why external normalizers that are not JSON API specific should not be supported by JSON API. The exception is the normalizers at the typed data level, which don't have the conditions described in this issue.

The current behavior is to explicitly ignore normalizers that would engage in the behavior described in the IS.

But @phenaproxima was pointing out not that JSON API normalizers cannot be reused, he was pointing out that JSON API normalizers returning value objects violates

namespace Symfony\Component\Serializer\Normalizer;

/**
 * Defines the interface of normalizers.
 *
 * @author Jordi Boggiano <j.boggiano@seld.be>
 */
interface NormalizerInterface
{
    /**
     * Normalizes an object into a set of arrays/scalars.
     *
…
     *
     * @return array|scalar
     */
    public function normalize($object, $format = null, array $context = array());

normalizers cannot be reused

Can you provide a specific example of what you mean? I don't see a practical example where that would be a problem. You have to explicitly use api_json format. When you do, you need to deal with the format of the API calls, as with every other API.

@return array|scalar

AFAIK docblocks are not part of an interface. In any case, returning ['the_thing' => $an_object] instead of $an_object would comply. However that will add only churn.

A viable solution could be to change the internal normalizers to implement a custom interface instead of NormalizerInterface. I think the best course of action is to detach JSON API more from Serialization, since it only adds confusion and false expectations.

People tend to think cross-format re-use, which is not possible. See #2860350: Document why JSON API only supports @DataType-level normalizers for a longer explanation of why that is not possible.

Can you provide a specific example

I was merely replying to you in #4. I wrote But @phenaproxima was pointing out not that JSON API normalizers cannot be reused. i.e. that's precisely not the point.

AFAIK docblocks are not part of an interface.

They are, because PHP 5 doesn't support return type hints.

I think the best course of action is to detach JSON API more from Serialization, since it only adds confusion and false expectations.

Interesting idea!

@e0ipso at #2931785-15: The path for JSON API to core:

I don't think we need #2923779: Normalizers violate NormalizerInterface to be added here. Any strong feelings about it?

@Wim Leers at #2931785-16: The path for JSON API to core:

#15: I do. Core committers will never allow a new module to be added if it violates existing interfaces. We either need to stop violating that interface or we need to thoroughly document/justify A) why we're doing it, B) why it's safe to do so.

Core committers will never allow a new module to be added if it violates existing interfaces.

Hmm. That's one of the reasons I'd like to get a first assessment of the core committers.

On the particular subject I don't think that there is an interface violation. PHP fails on interface violations. I am not convinced by the comment in #7:

They are, because PHP 5 doesn't support return type hints.

More importantly, if we were to address this we can either:

1. Stop leveraging the serialization component (with its normalizers). You can imagine the drawbacks here. Also, this breaks BC.
2. Start returning scalars from the current normalizers instead of smarter objects. Then start tracking state and execute logic in parallel. This approach will be messy, error prone, require a deep refactory and break BC.

… all this because of a @return array code comment.

Very interesting comment, @e0ipso, I'll need to think some more about that :) 👍

Do others already have thoughts about #9? @phenaproxima, @gabesullice?

Hmm. That's one of the reasons I'd like to get a first assessment of the core committers.

I agree that we should not base our decision on speculation. Better to get a final answer.

On the particular subject I don't think that there is an interface violation. PHP fails on interface violations.

This is a "letter vs spirit of the law" problem. @e0ipso is right, the letter of the law (PHP) doesn't care about return types. Such is PHP. @Wim Leers is right that the docblocks extend the interface beyond the limitation of of old PHP versions.

My opinion is that we are certainly in violation the spirit of the law here. I think we have all written code would be considered unsafe if not for a docblock's return type. Consider:

return $storage->loadMultiple($query->execute());

As @phenaproxima already pointed out, we're definitely violating a social contract, that's why we "broke Metatag".

So, in my eyes, this is a bug which needs to be fixed.

Since I see it as a bug, then we have a few choices about what to do about this bug:

1. Ship (into core) with a bug that we consider to be acceptable for now (and determine at which level of stability it will no longer be acceptable)
2. Refactor the normalizers before we ship.
3. "Stop leveraging the serialization component (with its normalizers)."

So...

1 or 2 means we need to refactor. So, do we do that refactoring in contrib or do we do it in core? If a maintainer says this needs to happen before we go into core, then our decision is made for us. If not, we need to weigh a 6 month delay against the burden of refactoring this in core vs. contrib. My preference would be to do this in contrib.

3 seems pretty unacceptable. I could only see doing this as an option if we provided some kind of shim between our custom stuff and the serialization API.

Also, this breaks BC.

Please forgive my ignorance here. I sincerely am not clear where the BC break is.

As @phenaproxima already pointed out, we're definitely violating a social contract, that's why we "broke Metatag".

That's not correct. Metatag was wrongly assuming they can provide normalizers for fields for a "generic format" and expect that to be valid in all formats. As we have now clarified (thanks to you) that is not possible. This problem is different.

This is a "letter vs spirit of the law" problem.

I tend to agree. However, there is an important fact here: PHP does not support union of types. "Scalar or array" is not a thing in programmable PHP interfaces. Just like you cannot typehint that into a return value, hence the upstream lib will never add them in PHP7. Even if it was possible, altering that symfony interface is a BC break (and that is because a PHPdoc does not enforce & therefore does not guarantee).

Let's finish this discussion later in video :-)

#13:

1. That doesn't work because there's lots of metadata that will still be exposed when it shouldn't be. And indeed, it'd still fail is_array() checks.
2. That definitely won't work because JSON API's normalizers need to explicitly be listed BEFORE serialization's, because serialization's are generic and will override anything else. Yes, the more specific normalizers are ignored if a generic normalizer is found. Yes, the Symfony Serializer component is that naïve. See #2575761: Discuss a better system for discovering and selecting normalizers.

@e0ipso, @gabe.sullice and I just had a call, we'll summarize it soon.

Summary of videochat discussion:

@e0ipso started off with some history.

In short: he originally tried to use the normalizers “as intended”, but ended up with complex logic tracking state in parallel to the normalization process: state to track both cacheability metadata associated with each normalized item in the tree (entity -> fields -> properties) *and* JSON API-specific metadata to ensure the right data ends up in the right places in the right place of the document structure: http://jsonapi.org/format/#document-structure.

This is why he ended up returning value objects from each normalizer instead, and ending up with a tree of value objects, each containing their metadata (cacheability + JSON API), which can then be “rasterized”). See:

\Drupal\jsonapi\Normalizer\Value\JsonApiDocumentTopLevelNormalizerValue::rasterizeValue(), \Drupal\jsonapi\Normalizer\Value\JsonApiDocumentTopLevelNormalizerValue::rasterizeIncludes(), etc.)

@e0ipso started the discussion with 3 key observations:

1. If we go back to the @return scalar|array limitation imposed by Symfony’s NormalizerInterface typehint, then we will again have to deal with the complex state tracking mentioned above, and attempted before.
2. Return types aren’t enforced in PHP. This is the “letter vs spirit of the law” aspect mentioned in the comments above. And to the question What if Symfony adds this to the interface in PHP 7?, the answer is simple: it’s impossible, because union types (e.g. scalar|array)are unsupported. See https://stackoverflow.com/questions/36873567/php-return-type-hinting-obj... and https://wiki.php.net/rfc/union_types.^{Observation by @wim.leers Leers not mentioned in the call: "If Symfony really wanted to enforce this, they could’ve added an if-test or assert() call that would be guaranteeing a scalar|array return value."}
3. The need for point 1 implies the need for extra infrastructure (for tracking that metadata). We could achieve this by building a JSON API-specific serializer service. Then it wouldn’t violate the interface anymore.

@wim.leers Leers explained that his key concern was not per se the strict compliance with an interface, but rather A) the impact on the API-First ecosystem in Drupal, B) whether core committers would accept this into core (extremely unlikely). We either need:

1. change in code to stop violating the interface, or;
2. justify the violation with clear documentation

@wim.leers Leers piggy-backed off @e0ipso’s third observation, and proposed to essentially stop using the Symfony serializer service for JSON API’s “document top”-level, entity-level and field-level normalizers, and hopefully simplify that code thanks to no longer needing to conform to that structure. Only the field normalizer would then still call out to the “normal” serializer, and would hence pick up the @DataType-level normalizers. This would then better comply with the existing docs in jsonapi.api.php too!

@e0ipso observed, and we all agreed with him, that a good first step would be to at least “cordon off” the existing JSON API normalizers, so that they’re not available to contrib/custom code, by making them use a separate service tag. All of the code is already @internal. If that tag is then also clearly for internal use only, then we can:

1. make minimal changes
2. yet make it even more clear that these are JSON API implementation details that can change at any time
3. and stop violating the NormalizerInterface!

Therefore, the plan, which I'll be implementing:

1. jsonapi_normalizer_do_not_use_removal_imminent
2. entity and field -> jsonapi_normalizer_do_not_use_removal_imminent
3. field level calls ‘normalizer’, other serializer
4. remove “implements NormalizerInterface” from jsonapi_normalizer_do_not_use_removal_imminent services

jsonapi_normalizer_do_not_use_removal_imminent now that's a declaration of intentions! hahaha

This summary is a perfect recollection of the call. Many thanks @gabesullice!

jsonapi_normalizer_do_not_use_removal_imminent now that's a declaration of intentions! hahaha

😀

I can't take credit for the excellent summary, that was written by @Wim Leers. BUT, my sole addition was the tag name :) so there's that!

I tried to do this with as tiny a scalpel as I could. All of our normalizers are now completely segregated from the rest of Drupal's serialization system.

I was not able to remove NormalizerInterface from our normalizers or else I would have to rewrite a lot more of Symfony's code (which does type assertions on their interface). But I think we're sufficiently "cordoned off" that I'm okay with the interface violation for the time being, @Wim Leers, would you agree?

Not sure how much my opinion counts here

Very. For years there was only my opinion and it was fine, then Wim got more involved and things were better, then Gabe jumped back in and things are even better.

The more opinions the better solutions.

Not sure how much my opinion counts here

It absolutely does! I should have called you out specifically. I called @Wim Leers out because it was his suggestion to "4. remove “implements NormalizerInterface” from jsonapi_normalizer_do_not_use_removal_imminent services"

I didn't realize how the format thing worked. This should fix the test failure and make the patch smaller :)

Fixing the missing newline...

1. +++ b/jsonapi.services.yml
   @@ -1,79 +1,84 @@
   +  jsonapi.serializer_do_not_use_removal_imminent:
   

   Let's make this a private service.

2. +++ b/jsonapi.services.yml
   @@ -1,79 +1,84 @@
   +      - { name: jsonapi_normalizer_do_not_use_removal_imminent, priority: 1 }
   ...
   +      - { name: jsonapi_normalizer_do_not_use_removal_imminent, priority: 1 }
   ...
   +      - { name: jsonapi_normalizer_do_not_use_removal_imminent, priority: 1 }
   ...
   +      - { name: jsonapi_normalizer_do_not_use_removal_imminent, priority: 1 }
   

   Let's document why we need to specify these priorities. Will make future refactoring easier.

3. +++ b/src/Controller/RequestHandler.php
   @@ -51,7 +51,7 @@ class RequestHandler implements ContainerAwareInterface, ContainerInjectionInter
   -    $serializer = $this->container->get('serializer');
   +    $serializer = $this->container->get('jsonapi.serializer_do_not_use_removal_imminent');
   

   This will stop working when this service is marked private. It'll need to be injected.

4. +++ b/src/Serializer/Serializer.php
   @@ -0,0 +1,114 @@
   + * @link https://www.drupal.org/project/jsonapi/issues/2923779#comment-12407443
   

   <3

5. +++ b/src/Serializer/Serializer.php
   @@ -0,0 +1,114 @@
   +  /**
   +   * Adds a secondary normalizer.
   +   *
   +   * This normalizer will be attempted when JSON API has no applicable
   +   * normalizer.
   +   *
   +   * @param \Symfony\Component\Serializer\Normalizer\NormalizerInterface $normalizer
   +   *   The secondary normalizer.
   +   */
   +  public function setFallbackNormalizer(NormalizerInterface $normalizer) {
   +    $this->fallbackNormalizer = $normalizer;
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function normalize($data, $format = NULL, array $context = []) {
   +    if ($this->selfSupportsNormalization($data, $format)) {
   +      return parent::normalize($data, $format, $context);
   +    }
   +    return $this->fallbackNormalizer->normalize($data, $format, $context);
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function denormalize($data, $type, $format = NULL, array $context = []) {
   +    if ($this->selfSupportsDenormalization($data, $type, $format)) {
   +      return parent::denormalize($data, $type, $format, $context);
   +    }
   +    return $this->fallbackNormalizer->denormalize($data, $type, $format, $context);
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function supportsNormalization($data, $format = NULL) {
   +    return $this->selfSupportsNormalization($data, $format) || $this->fallbackNormalizer->supportsNormalization($data, $format);
   +  }
   +
   

   Oh, CLEVER! This means even fewer changes. Otherwise, we'd need to update JSON API's field normalizers to specifically use core's serializer service rather than jsonapi's.

   This is a great first step :)

6. +++ b/src/Serializer/Serializer.php
   @@ -0,0 +1,114 @@
   \ No newline at end of file
   

   Supernit: Let's add a trailing \n :)

NW for #25 points 1+2.

Now that we have an agreement and we have the ball rolling I'm increasing the priority on this.

I really like where we're headed.

Me too! :)

#25.2: I commented on all the priorities and found one that was not necessary.

#25.1/3: I was not able to mark the service as private. I don't think this is possible because controllers are instantiated at runtime using the
static create(ContainerInterface $container) method. Therefore, one must call $container->get('private_service');, which is not allowed.

Let's make a follow-up to mark it as private once we have #2883086: [PP-1] Port RequestHandler + ResourceResponseSubscriber improvements from REST module to JSON API in, which might help find a cleaner way to do injections.

Follow-up created: #2935370: Mark the JSON API serializer, normalizer and encoder services as private

RE #25.1/3: d'oh you're right :(

I can only find one potential problem:

+++ b/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
@@ -430,7 +430,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
-      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
+      ->get('jsonapi.serializer_do_not_use_removal_imminent')

@@ -458,8 +458,8 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
-      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
...
+      ->get('jsonapi.serializer_do_not_use_removal_imminent')

Instead of getting a specific normalizer service, this is changing that to use the JSON API serializer service. Maybe we should do that, but isn't that an out-of-scope change?

Instead of getting a specific normalizer service, this is changing that to use the JSON API serializer service. Maybe we should do that, but isn't that an out-of-scope change?

Not really. That's in a test class and it ensures that the normalizer is correctly implementing supports(De)Normalization.

Ah, that makes sense!

I'm in the process of reviewing the code. However I'm missing a clear release change note for this issue since it may cause BC breaks on people writing their own custom JSON API-specific normalizers. We agree it's an edge case that's easy to solve (change your service tag) but we need to document that in the release notes.

FWIW whenever this is released it will go as the only commit in the release. This changes architecture and it should be easy to roll back / spot.

Needs work not for the code, but for the missing release notes.

This release removes JSON API's normalizers from the serializer service. Therefore, custom normalizers with a higher priority than JSON API's normalizers will no longer have any effect.Therefore, it is no longer possible to call \Drupal::service('serializer')->serialize($object, 'api_json');.

This change was necessary to indicate that the current JSON API serialization process is incompatible with the rest of Drupal's serialization system (see #2923779 for additional background).

The only level at which the JSON API module supports the standard normalization process is at the TypedData data type level. If you need to alter the serialization of attributes, this is the recommended approach.

We would like to restate that the JSON API normalization process is not a public API and there remains no backwards compatibility guarantee. However, we understand that a minority of existing applications may have overridden JSON APIs normalizers. Those projects may update their service tags from normalizer to jsonapi_normalizer_do_not_use_removal_imminent in order to stay up to date with latest feature and security releases while they remove their applications' dependencies on this internal API.

Therefore, it is no longer possible to call \Drupal::service('serializer')->serialize($object, 'api_json');

Shouldn't one already have been using \Drupal::service('jsonapi.entity.to_jsonapi')->serialize($entity) for that anyway?

Everything else looks fine.

@Wim Leers, updated my original comment.

Putting it back to RTBC as per #33. I'll add my review and commit soon.

CR created: https://www.drupal.org/node/2935900

🎉

Sorry it took longer than expected, but the little one was sick this week.

1. +++ b/src/DependencyInjection/Compiler/RegisterSerializationClassesCompilerPass.php
   @@ -0,0 +1,92 @@
   +    foreach ($container->findTaggedServiceIds('encoder') as $id => $attributes) {
   +      $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
   +      $encoders[$priority][] = new Reference($id);
   +    }
   ...
   +    if (!empty($encoders)) {
   +      $definition->replaceArgument(1, $this->sort($encoders));
   +    }
   +
   ...
   +    $formats = [];
   +    $format_providers = [];
   +    foreach ($container->findTaggedServiceIds('encoder') as $service_id => $attributes) {
   +      $format = $attributes[0]['format'];
   +      $formats[] = $format;
   +
   +      if ($provider_tag = $container->getDefinition($service_id)->getTag('_provider')) {
   +        $format_providers[$format] = $provider_tag[0]['provider'];
   +      }
   +    }
   +    $container->setParameter(static::OVERRIDDEN_SERVICE_ID . '.formats', $formats);
   +    $container->setParameter(static::OVERRIDDEN_SERVICE_ID . '.format_providers', $format_providers);
   

   Do we really need this? I get this will go away soon, but it sounds like we could simplify.

2. +++ b/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
   @@ -214,7 +214,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
   -      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
   +      ->get('jsonapi.serializer_do_not_use_removal_imminent')
          ->normalize(
   
   @@ -313,7 +313,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
   -      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
   +      ->get('jsonapi.serializer_do_not_use_removal_imminent')
          ->normalize(
   
   @@ -352,7 +352,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
   -      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
   +      ->get('jsonapi.serializer_do_not_use_removal_imminent')
          ->normalize(
   
   @@ -430,7 +430,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
   -      ->get('serializer.normalizer.jsonapi_document_toplevel.jsonapi')
   +      ->get('jsonapi.serializer_do_not_use_removal_imminent')
          ->normalize($document_wrapper->reveal(), 'api_json', [
   

   These (and others) seem like a regression. Why introduce a level of indirection to find serializer.normalizer.jsonapi_document_toplevel.jsonapi instead of calling it direcly?

   It also feels out of scope of the current change.

I didn't run tests locally, so this will surely fail.

Re-roll.

I didn't run tests locally, so this will surely fail.

Ha! They passed after all.

I'm going to merge this. Since we'll have additional next steps for this, we can address any code deficiencies there.

Thanks all!

#41.2: I raised the same at #31. @gabesullice answered at #32.

Looks like Gabe's comment was inaccurate, since tests did pass?

Looks like Gabe's comment was inaccurate, since tests did pass?

The comment wasn't inaccurate. The tests pass because that change was just an expansion of coverage—ensuring that the normalizer was properly registering itself with the serializer. We were doing it right before, there just wasn't coverage proving it before.

+++ b/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
@@ -593,7 +593,7 @@ class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {
-        ->denormalize(Json::decode($payload), JsonApiDocumentTopLevelNormalizer::class, 'api_json', [
+        ->denormalize(Json::decode($payload), NULL, 'api_json', [

It also revealed a little "bug" in the test... notice that JsonApiDocumentTopLevelNormalizer::class became NULL in the patch. This previous line was a "bug" because it was telling the normalizer to normalize the data into itself (!? lol :P ).

While it might be technically out of scope, I think it's a little unfortunate that we lost that quick win.

#49: aha! I definitely read the patch too fast :) Could you open a new issue for that?

#49: aha! I definitely read the patch too fast :) Could you open a new issue for that?

Do we need an issue for that? We're moving away from this pattern anyways. Besides the coverage expansion wasn't such. The JsonApiDocumentTopLevelNormalizer is polymorphic and hence we decide the class to denormalize to at runtime. It's one of the abuses of the serialization that we had to make.

Hi Guys,

Just found this thread while trying to work out why a custom Normalizer class I'd made was no longer being used after updating to the latest jsonapi dev release.

The class extended FieldItemNormalizer and was declared in it's module's services.yml file with a name: normalizer tag.

Now the only way I can seem to get this to work again is to change the tag to name: jsonapi_normalizer_do_not_use_removal_imminent, which obviously seems horribly wrong, but I can't seem to work out what the correct way of doing this is now that this change has been made.

Any advice would be greatly appreciated.

@PQ can you explain why you need a new normalizer? Maybe a computed field will work just as fine.

Hi @e0ipso, Thanks for getting back to me.

The normaliser is currently modifying the output of a custom field type. The field contains YAML encoded data (because reasons), and the normaliser decodes the YAML, sanitizes the data and outputs it as nested arrays (so it will appear in the output as nested JSON if that is the requested format).

If this isn't the preferred way to do this then I'm open to suggestions.

@PQ, I think you should be able to do this at the TypedData data type level. We still respect/use normalizers at that very low level (IOW, the properties of your custom field).

@PQ,

I realized my last comment was probably not very helpful. So I'll try again.

You probably have a propertyDefinitions() method on a class extending FieldItemBase that returns something like $properties['your_yaml'] = DataDefinition::create('string') (if not, you can stop reading the rest of this and ping me in IRC/Slack lol cause I'd be interested to hear it and to put your unique case into this queue for others who might have the same problem).

If my guess is right, then what you'll need to do is create a new custom data type for your YAML. So that in the end,, you can write DataDefinition::create('custom_yaml') in that method instead.

Then, what you'll do is modify your existing normalizer to apply to instances of that data type rather than at the level of the field item which contains your custom data.

FWIW, check out the implementation of the 'email' data type in core:

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21TypedData...

All it does is extend from StringData, inheriting all its methods without overriding anything. The only difference is the annotation and a custom constraint.

Hi @gabesullice,

Many thanks for the info, I'll be having a go at your suggestion today if everything goes to plan.

My field class extends Drupal\Core\Field\Plugin\Field\FieldType\StringLongItem, which does has the propertyDefinitions() method. If I understand you correctly, I'll want to override to use a custom data type (that I also need to define).

I'll post back here to let you know how it goes.

The class extended FieldItemNormalizer

Note that this class is marked @internal, meaning its implementation can change at any time.

If I understand you correctly, I'll want to override to use a custom data type (that I also need to define).

That's right! And then you'll be able to create a @DataType-level normalizer. Few examples of those exist in core: \Drupal\serialization\Normalizer\PrimitiveDataNormalizer (for \Drupal\Core\TypedData\PrimitiveInterface, i.e. for several @DataType plugins). #2926508: Add DateTimeNormalizer+TimestampNormalizer, deprecate TimestampItemNormalizer: @DataType-level normalizers are reusable by JSON:API is adding one for @DataType=timestamp (\Drupal\Core\TypedData\Plugin\DataType\Timestamp).
The added benefit is that this normalizer will work for both core's REST module and the JSON API module! Both will use the same (de)normalization logic :)

In fact, if you're willing to publish the code of your custom field type, I'm willing to provide the code for you, because I think it can serve as a valuable example in the change record.

Hi @Wim Leers,

Thanks for that, I'll probably be looking at making the suggested changes over the next few days as balancing tasks at the mo. I'll have a look at the possibility of publishing the code, although it will have to be so heavily redacted due to NDA's etc that the use case might no longer be apparent.

Moving to Drupal core's issue queue.

I'm working on https://www.drupal.org/project/drupal/issues/3122113