Skip to content
New issue

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

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

Already on GitHub? # to your account

Nullable embedded documents not initialized #2301

Closed
webmozart opened this issue Apr 14, 2021 · 3 comments · Fixed by #2302
Closed

Nullable embedded documents not initialized #2301

webmozart opened this issue Apr 14, 2021 · 3 comments · Fixed by #2302
Assignees
Labels
Milestone

Comments

@webmozart
Copy link
Contributor

Bug Report

Q A
BC Break no
Version 2.2.x (4984784)

Summary

When using <embed-one ... nullable="true"/>, hydration fails due to a missing nullable check in the generated hydrator.

Current behavior

When using <embed-one ... nullable="true"/> in combination with typed, uninitialized properties, then these properties are not correctly initialized to null. This works for regular fields due to the following generated condition in the hydrator:

if (isset($data['theField']) || (! empty($this->class->fieldMappings['theField']['nullable']) && array_key_exists('theField', $data))) {

For embedded fields, however, the following condition is generated:

if (isset($data['theField'])) {

As you can see, the check for the nullable property is missing, hence the field is not initialized.

How to reproduce

class Container {
    private int $id;
    private ?Embedded $embedded;

    public function __construct(int $id, ?Embedded $embedded)
    {
        $this->id = $id;
        $this->embedded = $embedded;
    }

    public function getId(): int
    {
        return $this->id;
    }

    public function getEmbedded(): ?Embedded
    {
        return $this->embedded;
    }
}

class Embedded {
    public int $value;

    public function __construct(int $value)
    {
        $this->value = $value;
    }

    public function getValue(): int
    {
        return $this->value;
    }
}
<?xml version="1.0"?>
<doctrine-mongo-mapping xmlns="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping"
                        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                        xsi:schemaLocation="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping
                    https://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd">
    <document name="Container">
        <id type="integer" strategy="NONE"/>
        <embed-one field="embedded" target-document="Embedded" nullable="true"/>
    </document>
    <embedded-document name="Embedded">
        <field name="value" type="integer"/>
    </embedded-document>
</doctrine-mongo-mapping>
$container = new Container(10, null);

$documentManager->persist($container);
$documentManager->flush();
$documentManager->clear();

$container = $documentManager->find(Container::class, 10);

var_dump($container->getEmbedded());

// => Fatal error: Typed property Container::$embedded must not be accessed before initialization

Expected behavior

var_dump($container->getEmbedded());

// => null
@alcaeus alcaeus self-assigned this Apr 15, 2021
@alcaeus alcaeus added the Bug label Apr 15, 2021
@alcaeus alcaeus added this to the 2.2.1 milestone Apr 15, 2021
@alcaeus
Copy link
Member

alcaeus commented Apr 15, 2021

Thanks for the report. I've taken a crack at fixing this in #2302. As shown in the tests, you can set a default value on the property as a workaround until we have a release:

class Container {
    private int $id;
    private ?Embedded $embedded = null;

    // ...
}

@webmozart
Copy link
Contributor Author

@alcaeus Wow, that's quick! Thanks! :)

@alcaeus
Copy link
Member

alcaeus commented Apr 15, 2021

Wow, that's quick!

I have my moments 😊

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants