orm
orm copied to clipboard
[GH-4568] A new attempt at nullable embeddables based on Luis work.
This is a new attempt at solving #4568 which becomes a more present requirement to solve because of typed properties, see #8014
Its based of @lcobucci work on #1275, which was closed. The implementation of that PR put the burden on the UnitOfWork and introduced additional work in the hotpath. This new PR moves the logic into the ReflectionEmbeddedProperty.
TODO:
- [ ] Add tests for the mapping drivers
- [ ] Add more tests for the embeddable part, also nested embeddables.
Related https://github.com/doctrine/orm/pull/8015
I really hope this or something like it gets merged. I know people are calling this an edge case, but I deal with it all-the-time. It would clean up a lot of code for me. Fingers crossed, and thanks.
@beberlei is this still a state that can be continued and do you need help on this PR?
And do you think these changes can still be in Doctrine version 2? :-)
@michaeldnelson
I agree, this is definitely not an edge case.
I think every developer who works on a commercial app will encounter the problems around dealing with money. Fowlers money pattern is an idustry standard when it comes to implementing a good system. Money is best represented as a VO, so the natural thing to do is to handle it as an Embeddable in Doctrine. Money(0, 'EUR') and null are not the same.
I have to deal with this all the time, and each and every time I check back if the original PR is merged. This started in 2015.
Anyone currently waiting / in need of this feature, this is probably your best bet:
(This example is using brick/money Money value objects)
Create the nullable version of your VO.
/**
* This is a stop gap solution for a common doctrine problem; embedded objects cannot be nullable.
* See issues and PRs linked below.
*
* This VO acts as a proxy, which resolves into a \Brick\Money\Money object, or null
*
* @see https://github.com/doctrine/orm/pull/1275
* @see https://github.com/doctrine/orm/pull/8022
*
* @ORM\Embeddable()
*/
class NullableMoney
{
/**
* @ORM\Column(type="decimal_object", precision=28, scale=2, nullable=true)
*/
private ?BigDecimal $amount = null;
/**
* @ORM\Column(type="currency", nullable=true)
*/
private ?Currency $currency = null;
public function __construct(?Money $money = null)
{
$this->amount = $money ? $money->getAmount() : null;
$this->currency = $money ? $money->getCurrency() : null;
}
public function __invoke(): ?Money
{
if ($this->amount === null || $this->currency === null) {
return null;
}
return Money::of($this->amount, $this->currency);
}
}
NOTE: Brick\Money\Money is final, so I had to recreate it, you might not need to, just simply extend and implement __invoke (or any other method name you would like)
public function getMoney(): ?Money
{
return ($this->money)();
}
public function setMoney(?Money $money): self
{
$this->money= new NullableMoney($money);
return $this;
}
This way, doctrine always hydrates a NullableMoney object, which is esentially a proxy for the real VO.
The goal was keep encapsulation, so the entities wouldn't get over polluted with amount-currency pairs.
Hey, I just ran into the issue this PR is trying to solve, and this seems to be the only open PR/issue about nullable embeddables. Therefore I kindly want to ask what the state of this is. Is there any chance this will get merged (despite the fact that it is quite old)? Or is there an alternative PR I have missed, that is more recent? My workaround for now is that I don't use embeddables if they are nullable, which is not really nice...
Hope this will be merged at some point
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.
Hi, @beberlei ,
Thank you for all the effort made during doctrine orm development. This is indeed powerful tool used by millions, yet sometimes people do not fully understand the cost behind it.
Regarding the current PR, I see that you have tried to implement nullable embeddables - a very needed feature for ORM, yet something didn't work out as expected as it's never been merged.
From the point of community, I would like to earnestly ask to continue on this PR to complete this feature. It will indeed make our software better, as it communicates architectural design much better, and I really hope for this feature to be completed at some point.
Thank you for the time once again, and have a nice day :slightly_smiling_face:
Yours sincerely, Yevhen
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. If you want to continue working on it, please leave a comment.
@github-actions I think it would be a pity if this does not make it into the product 🙂
@greg0ire is there a new attempt in the pipe?
Sorry, it was closed because I deleted the branch. Feel free to reopen against another branch.