Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Union Types Complex #1

Closed
wants to merge 14 commits into from

Conversation

idbentley
Copy link
Owner

Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT

This pull request is part two, building on schmittjoh#1546.

I made some assumptions while making this change, and am very open to feedback. I've tested the Union functionality pretty thoroughly by running my external project's test suite against it, so I'm fairly confident that things work in general, but because of my inexperience with jms/serializer there may be many interactions that I've missed.

This adds Union support for objects in 2 ways:

  1. Discriminated Unions

This is the easy case. For objects that have a field which specifies their type, i.e.:

class DiscriminatedAuthor
{
    /**
     * @Type("string")
     * @SerializedName("full_name")
     */
    #[Type(name: 'string')]
    #[SerializedName(name: 'full_name')]
    private $name;

    /**
     * @Type("string")
     */
    #[Type(name: 'string')]
    private $type = 'JMS\Serializer\Tests\Fixtures\DiscriminatedAuthor';
    ..snip...
}

I added a new Annotation: UnionDiscriminator that allows you to use this field to determine which object to deserialize into.

    #[UnionDiscriminator(field: 'type')]
    private DiscriminatedAuthor|DiscriminatedComment $data;

For more complex cases, the UnionDiscriminator Annotation can also receive a map attribute, which will map from the values contained in the discriminator field, to the fully qualified class.

i.e.

    #[UnionDiscriminator(field: 'objectType', map: ['author' => 'JMS\Serializer\Tests\Fixtures\DiscriminatedAuthor', 'comment' => 'JMS\Serializer\Tests\Fixtures\DiscriminatedComment'])]
    private DiscriminatedAuthor|DiscriminatedComment $data;

Known issues with Discriminated Unions:

  • @Type annotations don't support unions (I looked into modifying the lexer/parser, but it's pretty non trivial to add).
  • There's no support for "top level" deserialization for discriminated unions, because there's no way to annotate the call to ->deserialize. i.e. they are only supported when they are Properties on objects.
  1. Non-Discriminated unions

There is no "correct way" to deserialize into Union Types without discriminated fields, so I borrowed an approach I've seen in other languages (I recently added a similar implementation to the Python dataclasses_json package, but this approach is similar to TypeScript's zod package).

Essentially, in order to deserialize a Union, I "try" to deserialize into the possible types one by one until I find a type that the data matches well, and I assume that is the correct type.

I first sort the objects based on the number of required properties on the object. If the Union is a mix of objects and primitives, I try the primitives first.

If while deserializing the JSON contains data of a type that doesn't match the Object, then we give up on that type.

If while deserializing, the JSON does not contain a required property from the Object, then we give up on that type.

If while deserializing, the JSON contains a field that the Object doesn't have a matching field for, then we ignore that field, continuing to deserialize the current type.

The bulk of the code supporting this change can be seen in the UnionHandler and the DerserializationGraphNavigator.

The enumeration of possible exceptions in UnionHandler seems like a point of fragility.

The introduction of requireAllRequiredProperties field on the JsonDeserializationVisitor is a bit peculiar - any feedback on a more idiomatic way to add this functionality is welcomed.

@scyzoryck @goetas - sorry that this PR is so big! I hope you'll agree it's worth the time to add.

@idbentley idbentley closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant