AbstractSqlExecutor::__sleep should return property names

[kerbert101]

From php.net:

If an object is converted to an array, the result is an array whose elements are the object's properties. The keys are the member variable names, with a few notable exceptions: integer properties are unaccessible; private variables have the class name prepended to the variable name; protected variables have a '*' prepended to the variable name. These prepended values have NUL bytes on either side. Uninitialized typed properties are silently discarded.

serialize supports the prepended property names, but this is undocumented. VarExporter does not support prepended property names. The PhpFilesAdapter uses VarExporter to create a php cache file. To prevent issues, we need to return the property names without the prefixes added by php when casting an object to an array.

Fixes #11063

[greg0ire]

I added a test, and now it seems the test breaks for PHP 7.1 and symfony/var-exporter v4.4.43

The failure says:

Failed asserting that null is an instance of class "Doctrine\ORM\Query\ResultSetMapping"

The exporter produces this:

\Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
        clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)['Doctrine\\ORM\\Query\\ParserResult'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('Doctrine\\ORM\\Query\\ParserResult')),
        clone ($p['Doctrine\\ORM\\Query\\Exec\\SingleSelectExecutor'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('Doctrine\\ORM\\Query\\Exec\\SingleSelectExecutor')),
        clone ($p['Doctrine\\ORM\\Query\\ResultSetMapping'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('Doctrine\\ORM\\Query\\ResultSetMapping')),
    ],
    null,
    [
        'Doctrine\\ORM\\Query\\Exec\\AbstractSqlExecutor' => [
            'sqlStatements' => [
                1 => 'SELECT c0_.id AS id_0, c0_.name AS name_1, c1_.salary AS salary_2, c1_.department AS department_3, c1_.startDate AS startDate_4, c2_.title AS title_5, c0_.discr AS discr_6, c0_.spouse_id AS spouse_id_7, c2_.car_id AS car_id_8 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id LEFT JOIN company_managers c2_ ON c1_.id = c2_.id WHERE c0_.name = ?',
            ],
        ],
        'stdClass' => [
            'aliasMap' => [
                2 => [
                    'u' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                ],
            ],
            'fieldMappings' => [
                2 => [
                    'id_0' => 'id',
                    'name_1' => 'name',
                    'salary_2' => 'salary',
                    'department_3' => 'department',
                    'startDate_4' => 'startDate',
                    'title_5' => 'title',
                ],
            ],
            'typeMappings' => [
                2 => [
                    'discr_6' => 'string',
                    'spouse_id_7' => 'integer',
                    'car_id_8' => 'integer',
                ],
            ],
            'entityMappings' => [
                2 => [
                    'u' => null,
                ],
            ],
            'metaMappings' => [
                2 => [
                    'discr_6' => 'discr',
                    'spouse_id_7' => 'spouse_id',
                    'car_id_8' => 'car_id',
                ],
            ],
            'columnOwnerMap' => [
                2 => [
                    'id_0' => 'u',
                    'name_1' => 'u',
                    'salary_2' => 'u',
                    'department_3' => 'u',
                    'startDate_4' => 'u',
                    'title_5' => 'u',
                    'discr_6' => 'u',
                    'spouse_id_7' => 'u',
                    'car_id_8' => 'u',
                ],
            ],
            'discriminatorColumns' => [
                2 => [
                    'u' => 'discr_6',
                ],
            ],
            'declaringClasses' => [
                2 => [
                    'id_0' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                    'name_1' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                    'salary_2' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                    'department_3' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                    'startDate_4' => 'Doctrine\\Tests\\Models\\Company\\CompanyEmployee',
                    'title_5' => 'Doctrine\\Tests\\Models\\Company\\CompanyManager',
                ],
            ],
        ],
    ],
    $o[0],
    [
        [
            'Doctrine\\ORM\\Query\\ParserResult' => [
                'sqlExecutor' => $o[1],
                'resultSetMapping' => $o[2],
                'parameterMappings' => [
                    'name' => [
                        0,
                    ],
                ],
            ],
        ],
        1,
    ]
)

I suspect this issue is orthogonal with the one we are dealing with here.

[greg0ire]

Would that work for MultiTableUpdateExecutor? It has private properties.

[kerbert101]

No, that would not work. Nvm 🙂

[greg0ire]

@nicolas-grekas I'd like your input on this Symfony 4.4 issue… I think this means serialization just does not work right now on Symfony 4 apps? It seems weird. Anyway, Symfony 4.4. is EOL so… not worth fixing IMO. Maybe we should drop it from our composer.json altogether, although not in a patch.

[nicolas-grekas]

I don't care about 4.4 anymore :)

[SenseException]

So what does this mean for this PR?

[greg0ire]

2 things:

1. It is safe to merge; nobody is entitled to complain that it does not work with sf 4.4.
2. That it would have been simpler to write if we had dropped support for sf 4.4. So we should do that on 2.18

[SenseException]

I've read the issue and the Symfony PR. I hope people can now sleep better without the prefixes. I will. Thanks to everyone who got involved in this fix.

[Seb33300]

If not compatible with Symfony 4.4, can't we add it in composer.json conflict?

[greg0ire]

I don't know for sure that it isn't. I mean, this is just a test, maybe there's something that makes the conflict not meaningful, like maybe PhpFilesCache isn't available on Symfony 4.4.

Are you by any chance using Symfony 4.4?

[Seb33300]

Are you by any chance using Symfony 4.4?

(Un)fortunately, no

[greg0ire]

Ok, cause I would be surprised that something this big and unrelated to what I did recently in 2.17 would have gone unnoticed for so long.

[greg0ire]

@kerbert101 congrats on your first contribution BTW 🎉