fix discriminator for classes in the middle of hierarchy

[scasei]

No description provided.

[goetas]

Something similar should be already on master.

What is this PR for?

[scasei]

In my application I have class hierarchy like this:

class A {}

abstract class B extends A {}

class BA extends B {}
class BB extends B {}

When I serialize an instance of class BA or BB there is a column 'type' in the json, describing which sub class is serialized.
When it comes to deserialization the serializer tries to instantiate class B instead of BA or BB.
The PR points the serializer to BA/BB so there is no error like: 'The provided class "B" is abstract, and can not be instantiated'

[goetas]

Isn't related to #382 ?

[scasei]

Today I updated serializer to tag 1.3.1 but the errors still occurs.

[scasei]

Should I try dev-master?

[goetas]

Yes please, was merged Friday

[scasei]

Ok, tested with 5ebae63 (dev-master), but problem still exists.

Can you please consider a merge?

[goetas]

Can you provide tests?

[scasei]

No test, no merge, right?

[goetas]

Yep

[goetas]

But for me is sospicous that master dev does not work. ..

[scasei]

Ok, I'll double check my setup

[goetas]

You can check the PR, I think is exactly the feature you need

[goetas]

(Remember to clear caches)

[scasei]

I'm with you, I check everything again

[scasei]

I saw, that I requested the same here: #309 :)

And found that my deserialization call is like the following:

$jms_serializer->deserialize($json_content,'WebCollection','json','AbstractMiddleClassName');

So the serializer has to find the right sub class from the 'type'' column in the json content.
The premise of the already merged fixes is, that you pass the right sub class name to deserialization.
Thats why it needs this addional code for my setup.

[goetas]

Ah, I see. Your pr is for the deserialization case.

Unfortunately without tests can't be merged

[scasei]

aight, will push the test here

[scasei]

Update: made a fresh clone of masteer and composer install, then run phpunit and got lots of errors, especially for my wanted feature (testNestedPolymorphicInterfaces).

Win10 / PHP 5.6.25 nts

Is there something I should investigate?

[goetas]

then run phpunit and got lots of errors,

what it means? can you share?

[scasei]

here is my console output:

MINGW64 /c/Program Files (x86)/wamp/www/serializer (master)
$ phpunit.phar
PHPUnit 5.5.4 by Sebastian Bergmann and contributors.

WWWWWWWW............................WWWWWWWWWWW...............  62 / 399 ( 15%)
...................................F..E..........WW......EEEWW 124 / 399 ( 31%)
WWWWWWWWWWW....WWWWWWWW....................................... 186 / 399 ( 46%)
..........WW............................................FF...F 248 / 399 ( 62%)
FFFFFF.SFFFFFFFFFFFFFFFFFFFFFFFSFFFFFFFFFFFFFFFFFFFFFFFFWFFFFF 310 / 399 ( 77%)
FFFF.FF.FFFFFF.F.FSSSSSFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF 372 / 399 ( 93%)
FFFFFFFFFFFF.FF.FFFFFF.F.FW                                    399 / 399 (100%)

[goetas]

no idea... travis says master it is green https://travis-ci.org/schmittjoh/serializer/builds/159323741

[scasei]

Should I attach the whole output log?

[goetas]

ok

[scasei]

output.txt

[goetas]

there are a bunch of errors in your environment... as example:

`Exception: DateTime::__construct(): It is not safe to rely on the system's timezone settings. You are *required* to use the date.timezone setting or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected the timezone 'UTC' for now, but please set date.timezone to select your timezone.

date time not configured well

Doctrine\ORM\Tools\ToolsException: Schema-Tool failed with Error 'An exception occured in driver: could not find driver'`

sqlite missing?

im not use your PHP is well configured

[scasei]

right, have fixed these errors already. the others could be line ending issues.

[goetas]

windows :(

[scasei]

So, after correcting the line endings, tests running well.

And thank you for pointing to coding a test, because my problem becomes clearly now:

My input is a collection of classes.
The difference to #382 is, that #382 uses a parent class 'Garage' to store the information, which type the input is of.
In my use case I have a deserilization context which get the information about the type of the classes at runtime.

class Vehicle {
    private $km;
}

class Car extends Vehicle {}
class Moped extends Vehicle {}


$json = '[{"km":3,"type":"car"},{"km":1,"type":"moped"}]';

$context = new \Own\Serializer\Context\DeserializationContext();
$context->setInnerObject('\Vehicle');

$ret = $this->jms_serializer->deserialize($json,'array','json',$context);

Questions:
Do you think I should work out the PR or should I trash everything and use static collection class as in #382?

[scasei]

here my use case classes, i try if 'array' will work

class DeserializationContext extends \JMS\Serializer\DeserializationContext
{

    /**
     * @var string
     */
    protected $inner_object;


    /**
     * @param string $inner_object
     */
    public function setInnerObject ($inner_object) {

        $this->inner_object = $inner_object;
    }

    /**
     * @return string
     */
    public function getInnerObject () {

        return $this->inner_object;
    }
}

namespace Prima\REST\ClientBundle\Serializer\Handler;

use JMS\Serializer\Handler\SubscribingHandlerInterface;
use JMS\Serializer\GraphNavigator;
use JMS\Serializer\VisitorInterface;
use Prima\REST\ClientBundle\Serializer\Context\DeserializationContext as Context;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping\ClassMetadata;

class WebCollectionHandler implements SubscribingHandlerInterface
{
    public static function getSubscribingMethods()
    {
        return array(
                array(
                        'direction' => GraphNavigator::DIRECTION_DESERIALIZATION,
                        'format' => 'json',
                        'type' => 'WebCollection',
                        'method' => 'deserializeWebCollection',
                ),
        );
    }

    public function deserializeWebCollection(VisitorInterface $visitor, $data, array $type, Context $context)
    {

        $ret = new ArrayCollection();
        $type['name'] = $context->getInnerObject();

        $key = key($data);
        if (is_numeric($key)) {

            foreach ($data as $object) {            
                $ret->add(
                        $visitor->getNavigator()->accept($object, $type, $context)
                );
            }           
        }
        else {

            $ret->add(
                    $visitor->getNavigator()->accept($data, $type, $context)
            );
        }

        $refl = new \ReflectionObject($visitor);        
        $property = $refl->getParentClass()->getProperty('result');
        $property->setAccessible(TRUE);
        $property->setValue($visitor, $ret);


        return $ret;
    }
}

[goetas]

doing this you are moving the "deserialization" logic into something else...

Do you use @discriminator in the serialization phase? why not use the same in "deserialization" ?

[scasei]

That is my doctrine config file for the classes:

Prima\Partner\AccountBundle\Entity\User:
    type: entity
    table: partner_account
    inheritanceType: SINGLE_TABLE
    discriminatorColumn:
        name: type
        type: string
    discriminatorMap:
        partner: UserChief
        sub: UserSub

[goetas]

looking at doctrine documentation.

If no discriminator map is provided, then the map is generated automatically. The automatically generated discriminator map contains the lowercase short name of each class as key.

This means that for hydration, doctrine does some magic ("lowercase short name"), that you have to do on your own later...
But as you can see http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/inheritance-mapping.html#single-table-inheritance doctrine in the example prefers to use explicitly @DiscriminatorMap, and I suggest you to do the same.

Doing it, also JMS will work better.

[scasei]

why not use the same in "deserialization" ?

My input has a type column, because of correct serialization with @discriminator. But in the deserialization phase the right subclass is not found,

[scasei]

I suggest you to do the same.

But I use a descriminator column :)

[goetas]

as you can see in the example, they uses both.

@DiscriminatorColumn(name="discr", type="string")
@DiscriminatorMap({"person" = "Person", "employee" = "Employee"})

[goetas]

missed

    discriminatorMap:
        partner: UserChief
        sub: UserSub

this means that you can copy & paste

    discriminatorMap:
        partner: UserChief
        sub: UserSub

into

# Vendor\MyBundle\Resources\config\serializer\Model.ClassName.yml
Vendor\MyBundle\Model\ClassName:
    discriminator:
        field_name: type
        map:
           partner: UserChief
           sub: UserSub

[scasei]

Ok, expect another abstract class 'FOS\UserBundle\Model\User' as parent of abstract 'Prima\Partner\AccountBundle\Entity\User' .

Then deserialization is definitely broken.
I can provide tests.

[scasei]

Pushed test case, would you please review?

[goetas]

it looks you have used tabs instead of spaces

[scasei]

You're right, I run cs fixer:

$ php-cs-fixer.phar fix ./ --fixers=linefeed,short_tag,indentation
   1) src\JMS\Serializer\Metadata\ClassMetadata.php
   2) tests\JMS\Serializer\Tests\Fixtures\DiscriminatorMiddle\Base.php
   3) tests\JMS\Serializer\Tests\Serializer\BaseSerializationTest.php
Fixed all files in 88.984 seconds, 7.750 MB memory used

[scasei]

Can I do something else to get merged?

[coyl]

+1. The error was not fixed in #382

[scasei]

@goetas Is something left unclear, why this patch is useful?

[coyl]

@goetas we really need this fix. I'm tired to live in forks =)

[coyl]

UP

[coyl]

=(

[scasei]

@goetas some news?

[goetas]

Replaced by #659 (the test classes were already in the repo, no need to add 120 lines of test classes)

[scasei]

thank you!