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

[Docs] Nullable Embeddables Hydration section #11586

Open
wants to merge 1 commit into
base: 3.2.x
Choose a base branch
from

Conversation

iku12phycho
Copy link

@iku12phycho iku12phycho commented Aug 27, 2024

Hi.

I faced to problem #4568. However, I could not find this issue quickly and I was confused...

The behavior should be documented in doctrine tutorial. It will be more kindness.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing this, here are a few fixes.

docs/en/tutorials/embeddables.rst Outdated Show resolved Hide resolved
docs/en/tutorials/embeddables.rst Outdated Show resolved Hide resolved
docs/en/tutorials/embeddables.rst Outdated Show resolved Hide resolved
Comment on lines 92 to 98
<?php

#[Entity]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please import the Attributes in this code examples.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 5b6bd6f

@SenseException
Is this right?
Would it be good to fix other examples in this file?

@greg0ire
Copy link
Member

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

  1. git rebase -i origin/3.2.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
  2. A window will show up with many lines, replace pick with fixup on every line but the first one
  3. Close your editor, git should do its magic, and you should end up with one commit
  4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

@greg0ire greg0ire requested a review from derrabus August 28, 2024 09:08
@derrabus
Copy link
Member

I'm trying to wrap my head around what's happening here. My understanding of embeddables is that we always hydrate them. Having this section elaborate on "nullable embeddables" feels weird because that concept does not exist in the ORM.

This section merely documents a weird workaround. I'm not sold that we should document that workaround, but if we do, we should first document the limitation that we attempt to work around before we document the actual workaround.

@iku12phycho
Copy link
Author

Thanks you for your comment.

In case all fields in the embeddable are nullable, you might want to initialize the embeddable, to avoid getting a null value instead of the embedded object.

There is the above description in initializing embeddables section.
If the step are not taken, "nullable embeddables" would effectively be possible.

We should document a difference between a instantiated entity object and a hydrated entity object.

@derrabus
Copy link
Member

derrabus commented Aug 28, 2024

If the step are not taken, "nullable embeddables" would effectively be possible.

If you manually construct the object, of course any structure is possible. It's your object, you've created it, you've populated all properties and the ORM is not even aware of its existence. But the ORM will never hydrate null instead of an actual embeddable object if it builds the entity for you.

This btw applies the same way to any mapped property. For instance, a one-to-many association is hydrated as a collection. We could add a section about "nullable collections" because it's in theory possible to construct an object where the property that's supposed to hold the collection is null. But since that only creates an unnecessary edge case, we rather recommend to initialize collections in the constructor. And that's what we also do for embeddables.

We should document a difference between a instantiated entity object and a hydrated entity object.

If you find a nice way to do that, please go ahead.

@iku12phycho
Copy link
Author

So, Is It like a bug that ORM accepts to flush a entity has a null value instead of the embedded object In Doctrine 3.2.x?

@derrabus
Copy link
Member

So, Is It like a bug that ORM accepts to flush a entity has a null value instead of the embedded object In Doctrine 3.2.x?

I wouldn't call it that. The ORM tries to handle this situation gracefully instead of raising an exception. Do you think we should be more strict here?

@SenseException
Copy link
Member

Sidenote: If this behavior isn't covered by tests yet, it would need one in case that the nullable embeddables concept will be officially supported and maintained.

@iku12phycho
Copy link
Author

I wouldn't call it that. The ORM tries to handle this situation gracefully instead of raising an exception. Do you think we should be more strict here?

Yes.
I feel odd that an object structure changes when to input into and output from the ORM.
But, I can understand a property value changes according to RDB table scheme like an autoincremented id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants