Problem/Motivation

Normalizers that provide JSON Schema disagree on how to describe optional data. See the change record for the JSON Schema support added to the serializer.

  • PrimitiveDataNormalizer::getNormalizationSchema() checks isRequired() and adds a null branch for optional data.
  • DateTimeNormalizer, TimestampNormalizer, and DateTimeIso8601Normalizer use the default getNormalizationSchema() from SchematicNormalizerTrait or SchematicNormalizerHelperTrait. It reflects the JsonSchema attribute and never checks the definition. DateTimeNormalizer::doNormalize() returns NULL when there is no date, so the schema rejects valid output.
  • TypedDataNormalizer::getNormalizationSchema() checks isRequired(), but returns invalid JSON Schema for stringable values. oneOf takes an array of schemas, not type names.
// TypedDataNormalizer::getNormalizationSchema()
return $nullable
  ? ['oneOf' => ['string', 'null']]
  : ['type' => 'string'];

Steps to reproduce

$serializer = \Drupal::service('serializer');
$typed_data_manager = \Drupal::typedDataManager();

$string = $typed_data_manager->create(DataDefinition::create('string'));
$serializer->getJsonSchema($string, []);
// ['oneOf' => [['type' => 'string'], ['type' => 'null']]]

$timestamp = $typed_data_manager->create(DataDefinition::create('timestamp'));
$serializer->getJsonSchema($timestamp, []);
// ['type' => 'string', 'format' => 'date-time']

$serializer->normalize($timestamp);
// NULL

Both definitions are optional. Only the string schema allows null.

Proposed resolution

Decide nullability in one place and pass it to getJsonSchemaForMethod() from both default getNormalizationSchema() implementations. Use the rule PrimitiveDataNormalizer already follows: optional typed data is nullable, and so is a class name, because there is no definition to check.

protected function isNullable(mixed $object): bool {
  return is_string($object)
    || ($object instanceof TypedDataInterface && !$object->getDataDefinition()->isRequired());
}

// SchematicNormalizerTrait
protected function getNormalizationSchema(mixed $object, array $context = []): array {
  return $this->getJsonSchemaForMethod(
    $this,
    'doNormalize',
    ['$comment' => static::generateNoSchemaAvailableMessage($object)],
    $this->isNullable($object),
  );
}

PrimitiveDataNormalizer and TypedDataNormalizer switch to isNullable(). TypedDataNormalizer also fixes its stringable branch:

return $nullable
  ? ['oneOf' => [['type' => 'string'], ['type' => 'null']]]
  : ['type' => 'string'];

Remaining tasks

  • Merge request with test coverage for optional and required data in each normalizer.

User interface changes

None.

Introduced terminology

None.

API changes

JSON Schema for optional dates, timestamps, and stringable typed data allows null. Normalization output does not change.

Data model changes

None.

Release notes snippet

AI-Generated: Yes. I used Claude Code to turn my notes into this issue summary and to check the proposed resolution against the Drupal 11.4.6 source. I reviewed and edited the result. The proposed code is a sketch and has not been run.

Issue fork drupal-3627331

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue tags: +rosetta

dries made their first commit to this issue’s fork.

dries’s picture

dries commented

I ran into this bug: my custom photo album module describes its data with Tool API. See https://dri.es/one-description-inside-and-outside-drupal for background. An image's date is optional, but it gets a schema that doesn't allow NULL.

As a first step, I looked into whether core's existing tests could reproduce this bug report. The good news: the date normalizers already have tests that check each example value against its schema. The not so good news: every example has a date, but none empty/optional.

The merge request adds an empty optional date to each of the three date normalizer tests, plus a test for the invalid schema of an optional Markup value, the issue's other bug. The new test cases should fail, confirming the bug is in core.

PrimitiveDataNormalizer already allows NULL and has tests for it. PrimitiveDataNormalizerTest has three null cases, string-null, integer-null and boolean-null.

dries opened merge request !17620

dries’s picture

dries commented

The test job correctly failed on the four new test cases:

DateTimeIso8601NormalizerTest::testNormalizedValuesAgainstJsonSchema with data set "null"
DateTimeNormalizerTest::testNormalizedValuesAgainstJsonSchema with data set "null"
TimestampNormalizerTest::testNormalizedValuesAgainstJsonSchema with data set "null"
TypedDataNormalizerTest::testStringableSchema with data set "optional"
dries’s picture

dries commented

I've now pushed a fix to the same MR. Disclaimer: I worked on the fix with the help of Claude Code.

The idea behind the fix: PrimitiveDataNormalizer and TypedDataNormalizer each figured out on their own whether a schema should allow null. I moved that logic into a small method, isNullable(), in SchematicNormalizerHelperTrait. The date normalizers now get it through the default getNormalizationSchema().

I also added a few more tests. Taking a closer look at the fix showed that my first tests were incomplete (thank you, Claude). The fix basically has to get two things right: an optional date's schema should allow null, and a required date's schema should not allow null. My tests only checked the first. Each date normalizer test now also has testJsonSchemaNullable(), which checks both, the same way testStringableSchema() does.

All tests should pass with this fix, but it's a good idea to get eyes on it from someone more experienced with the serialization system.

dries’s picture

dries commented
Related issues: +#3627328: ListNormalizer does not provide a JSON Schema for lists

One gotcha I forgot to mention. I found a related problem that I didn't fix here, because it's part of JSON:API, not serialization. Plus, it's sort of an existing problem.

JSON:API adds its own null option to optional fields. When a field type has a single optional property, that property's schema already allows null, so null ends up in the field's schema twice. You get something weird like this:

{"oneOf": [{"oneOf": [{"type": "string"}, {"type": "null"}]}, {"type": "null"}]}

Now that a date property's schema supports null, dates have the same problem. Prior to this fix, dates were not a problem, but it was sort of "by accident only".

I believe the fix for that belongs in a separate issue, but I wanted to mention it here. There are no tests for this in core so it won't fail any tests. I created a new issue for it at #3634179: JSON:API's field schema rejects its own output when a field item has a single optional property.

dries’s picture

dries commented
Status: Active » Needs review

Tests pass so setting this to 'Needs review'.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work
  • Does the isNullable() rule differ from the proposed resolution on purpose? The MR treats any object that isn't typed data as nullable, while the issue summary only treats class names and optional typed data as nullable. I'd go with the issue summary's rule and have normalizers declare #[JsonSchema(['type' => 'null'])] when they can return NULL. If the broader rule stays, the issue summary needs to match it.
  • TypedDataNormalizerTest::testStringableSchema() should validate the schema with JsonSchemaTestTrait instead of only comparing against a hardcoded array.
  • The MR description still says "Tests only, for now."

Generated with the help of an LLM

dries’s picture

dries commented
Status: Needs work » Needs review

Thank you for the review, Matt. I updated the code to use the summary's proposal. I also updated the MR. More work might be needed, but take a look.

f0ns’s picture

f0ns commented

I had a closer look at this one this morning.

Ran some tests and Matt's points in #10 all look addressed to me.

I did find one case that breaks:

#10 suggests normalizers add #[JsonSchema(['type' => 'null'])] when they can return NULL. If a normalizer does that and uses the default getNormalizationSchema(), optional data now gets null added twice:

#[JsonSchema(['type' => 'string'])]
#[JsonSchema(['type' => 'null'])]
public function doNormalize($object, $format = NULL, array $context = []): ?string 

{"oneOf": [{"type": "string"}, {"type": "null"}, {"type": "null"}]}

oneOf means "exactly one of these", and NULL now matches two of them, so it gets rejected.

A small fix in getJsonSchemaForMethod() works for me locally: only add the null branch when it isn't already there.

$schemas = array_map(fn ($schema) => $schema->newInstance()->getJsonSchema(), $schemas);
if ($nullable && !in_array(['type' => 'null'], $schemas, TRUE)) {
  $schemas[] = ['type' => 'null'];
}

The serialization and JSON:API unit tests still pass with it. A test for this case would be a good thing to add.

This only catches the exact ['type' => 'null'] form, so it might need a broader check (e.g. ['type' => ['string', 'null']]).

We are very close!

f0ns’s picture

f0ns commented
Status: Needs review » Needs work