8000 [Serializer] When using DiscriminatorMap, order of the mapping matters · Issue #37742 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Serializer] When using DiscriminatorMap, order of the mapping matters #37742

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.

Sign up for GitHub

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

Jump to bottom
Closed
stephane-lou opened this issue Aug 5, 2020 · 2 comments
Closed

[Serializer] When using DiscriminatorMap, order of the mapping matters #37742

stephane-lou opened this issue Aug 5, 2020 · 2 comments
Labels
Bug Serializer Status: Reviewed

Comments

@stephane-lou
Copy link

Symfony version(s) affected: 4.4

Description
When using inheritance, if you have multi-level inheritance, you have to set the DiscriminatorMap in the correct order for the Serializer to find the right class because of the way it tests the class type.

How to reproduce
Create 2 classes to serialize with inheritance:
Foo
Bar extends Foo

Set the DiscriminatorMap like this:

@DiscriminatorMap(typeProperty="type", mapping={
 *   "foo"    = "App\Foo",
 *   "bar"   = "App\Bar"
 * })

If you serialize this, your Bar class will have to "type" set to Foo.

Now, invert the map:

@DiscriminatorMap(typeProperty="type", mapping={
 *   "bar"   = "App\Bar",
 *   "foo"    = "App\Foo"
 * })

The Bar class will have the correct type.

This is because the function that checks the type uses is_a() and then exits the mapping loop if one returns true.

public function getMappedObjectType($object): ?string
    {
        foreach ($this->typesMapping as $type => $typeClass) {
            if (is_a($object, $typeClass)) {
                return $type;
            }
        }

        return null;
    }

Possible Solution
Improve the loop detecting the type and check precedence
@fancyweb fancyweb mentioned this issue Aug 17, 2020
Merged
@fancyweb
Copy link
Contributor

Status: reviewed

Thanks for reporting and for explaining correctly the problem.

@fabpot fabpot closed this as completed Aug 18, 2020
fabpot added a commit that referenced this issue Aug 18, 2020
@fabpot
bug #37860 [Serializer][ClassDiscriminatorMapping] Fix getMappedObjec…
7f7b447 
…tType() when a discriminator child extends another one (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[Serializer][ClassDiscriminatorMapping] Fix getMappedObjectType() when a discriminator child extends another one

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #37742
| License       | MIT
| Doc PR        | -

The strategy is to sort the passed classes from the "bottom" one in the hierarchy to the "top" one so that the first `is_a` in `getMappedObjectType()` is right.

Commits
-------

c16a192 [Serializer][ClassDiscriminatorMapping] Fix getMappedObjectType() when a discriminator child extends another one
@yarovskiy
Copy link
yarovskiy commented Dec 14, 2020 •
edited
Loading

Mapping problems were found in the new ordering logic. Example:

/**
 * @DiscriminatorMap(typeProperty="type", mapping={
 *   'ign' => 'App\Type\Ignore',
 *   'attr' => 'App\Type\Attribute',
 *   'sf' => 'App\Type\Search',
 *   'plb' => 'App\Type\Link',
 *   'pb' => 'App\Type\Block',
 *   'nf' => 'App\Type\NotFound',
 *   'und' => 'App\Type\Undefined',
 *   'smp' => 'App\Type\Simple',
 *   'foto' => 'App\Type\Media\Sub\Foto',
 *   'img' => 'App\Type\Media\Image',
 *   'pdf' => 'App\Type\Media\Pdf',
 *   'vid' => 'App\Type\Media\Video',
 *   'snd' => 'App\Type\Media\Sound',
 *   'feat' => 'App\Type\List\Features',
 *   'spec' => 'App\Type\List\Specifications',
 *   'table' => 'App\Type\Table\Table',
 *   'iframe' => 'App\Type\Else\Iframe',
 * });
 */

where

App\Type\Ignore extends App\Type\Simple
App\Type\NotFound extends App\Type\Simple

The mapping result order:

array (
    'ign' => 'App\Type\Ignore',
    'img' => 'App\Type\Media\Image',
    'table' => 'App\Type\Table\Table',
    'spec' => 'App\Type\List\Specifications',
    'feat' => 'App\Type\List\Features',
    'snd' => 'App\Type\Media\Sound',
    'vid' => 'App\Type\Media\Video',
    'pdf' => 'App\Type\Media\Pdf',
    'foto' => 'App\Type\Media\Sub\Foto',
    'attr' => 'App\Type\Attribute',
    'und' => 'App\Type\Undefined',
    'smp' => 'App\Type\Simple',
    'nf' => 'App\Type\NotFound',
    'pb' => 'App\Type\Block',
    'plb' => 'App\Type\Link',
    'sf' => 'App\Type\Search',
    'iframe' => 'App\Type\Else\Iframe',
);

Classes 'smp' and 'nf' in the wrong order!
Although they were initially placed correctly for the old mapping logic.

Symfony 4.4.17, PHP 7.3.22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Serializer Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @xabbuh @fancyweb @carsonbot @yarovskiy @stephane-lou
0