-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
fix: can create inversed one to one #659
Conversation
}; | ||
|
||
// creation delegated to afterPersist hook - return empty array here | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it is OK to return null here.
Can a "OneToOne" relationship be "not nullable" on the both sides of the relationship?
EDIT: I think it can:
#[Entity]
class Contact
{
#[ORM\OneToOne(targetEntity: Address::class, inversedBy: 'contact')]
#[ORM\JoinColumn(nullable: false)]
public Address $address;
public function __construct()
{
$this->address = new Address($this);
}
}
#[Entity]
class Address
{
public function __construct(
#[ORM\OneToOne(targetEntity: Contact::class, mappedBy: 'address')]
public Contact $contact;
) {}
}
and then, it would not be valid to return null
here 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this very specific case, where both side of OneToOne
are mandatory, could not be fixed : it is a circular dependency. Maybe we could find a way to emit a meaningful error for the user?
Also, maybe we could find a way to return something else than null
here: in Foundry2, we used to return a PostPersistCallback
which was not added to the normalized parameters. Maybe we could mimic this (but it would complexify the whole normalization phase)
BTW, I just figured out that you did notice this case was error prone: https://github.com/zenstruck/foundry/blob/2.x/tests/Integration/ORM/EntityFactoryRelationshipTestCase.php#L183
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I don't remember the exact context of what happened but I think you're right, it's the circular dependency issue.
src/ORM/OrmV2PersistenceStrategy.php
Outdated
@@ -52,6 +52,7 @@ public function relationshipMetadata(string $parent, string $child, string $fiel | |||
return new RelationshipMetadata( | |||
isCascadePersist: $association['isCascadePersist'], | |||
inverseField: $metadata->isSingleValuedAssociation($association['fieldName']) ? $association['fieldName'] : null, | |||
isCollection: $metadata->isCollectionValuedAssociation($association['fieldName']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to fix this, to mimic how it is done with orm v3
915d679
to
a26a159
Compare
a26a159
to
d49feb4
Compare
fixes #655