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

getAssociationMappedByTargetField on the owning side of a relation fails in 3.0 #11250

Closed
stof opened this issue Feb 12, 2024 · 6 comments · Fixed by #11308
Closed

getAssociationMappedByTargetField on the owning side of a relation fails in 3.0 #11250

stof opened this issue Feb 12, 2024 · 6 comments · Fixed by #11308
Assignees

Comments

@stof
Copy link
Member

stof commented Feb 12, 2024

BC Break Report

Q A
BC Break yes
Version 3.0.0

Summary

Calling Doctrine\ORM\Mapping\ClassMetadata::getAssociationMappedByTargetField on a relation which is not an inverse side of a relation does not work anymore in 3.0

Previous behavior

In 2.x, the method returned null for such cases.

Current behavior

this assertion fails:

assert($assoc instanceof InverseSideMapping);

How to reproduce

I found this when investigating a support question in the Symfony Slack channel. This BC break affects ApiPlatform: https://github.com/api-platform/core/blob/8a35ee20c7aaf03b5a3d96baf920e2c99c5ae0fd/src/Doctrine/Orm/Metadata/Resource/DoctrineOrmLinkFactory.php#L70

To reproduce it, call getAssociationMappedByTargetField for a field being the owning side of a relation.

@derrabus
Copy link
Member

cc @greg0ire

@Fl0-dev
Copy link

Fl0-dev commented Feb 25, 2024

To avoid the problem, downgrade doctrine/orm while waiting for a correction

@greg0ire greg0ire self-assigned this Feb 25, 2024
@greg0ire
Copy link
Member

greg0ire commented Feb 25, 2024

In 2.x, the method returned null for such cases.

That's a violation of the contract in persistence. The ORM CM class should have a signature compatible with that of the persistence CM class. It is documented to always return a string: https://github.com/doctrine/persistence/blob/e7cf418cafe83a75f7a7096cc9fa890b201d28f1/src/Persistence/Mapping/ClassMetadata.php#L126-L131

I think we should replace the assertion with a LogicException , advising callers to check with isAssociationInverseSide before attempting to call getAssociationMappedByTargetField, and add a deprecation about that on 2.19.x

greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Feb 25, 2024
In 2.x, getAssociationMappedByTargetField() used to return null when
called with the owning side of an association.
That was undocumented and wrong because the phpdoc advertises a string
as a return type.

In 6ce0cf4, I wrongly assumed that
nobody would be calling this method with the owning side of an
association.

Let us throw a full fledged exception and advertise the proper way of
avoiding this situation.

Closes doctrine#11250
greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Feb 25, 2024
`getAssociationMappedByTargetField()` returns `null` when called with
the owning side of an association.
This is undocumented and wrong because the phpdoc advertises a string as
a return type.

Instead, callers should ensure they are calling that method with an
inverse side.

Closes doctrine#11250
greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Feb 25, 2024
`getAssociationMappedByTargetField()` returns `null` when called with
the owning side of an association.
This is undocumented and wrong because the phpdoc advertises a string as
a return type.

Instead, callers should ensure they are calling that method with an
inverse side.

Closes doctrine#11250
greg0ire added a commit to greg0ire/doctrine-orm that referenced this issue Feb 25, 2024
`getAssociationMappedByTargetField()` returns `null` when called with
the owning side of an association.
This is undocumented and wrong because the phpdoc advertises a string as
a return type.

Instead, callers should ensure they are calling that method with an
inverse side.

Closes doctrine#11250
@greg0ire
Copy link
Member

@ZeinMansor
Copy link

ZeinMansor commented Feb 27, 2024

The solution resolved the association error but still getting the following error

php version: 8.2
symfony version: 6.4
symfony cli version: 5.8.11
doctrine/orm version: 3.1.x-dev (latest one)
api-platform/core version: ^3.2

Error message

Context: Calling Doctrine\ORM\Mapping\ClassMetadata::getAssociationMappedByTargetField() with "ro
!!    le", which is the owning side of an association.

!!    Problem: The owning side of an association has no "mappedBy" field.

!!    Solution: Call Doctrine\ORM\Mapping\ClassMetadata::isAssociationInverseSide() to check first in .
!!     (which is being imported from "C:\zein_workspace\FAST\fast-api-v2\config/routes/api_platform.yam
!!    l"). Make sure there is a loader supporting the "api_platform" type.

Role class

#[ORM\Entity(repositoryClass: UserRepository::class)]
#[ApiResource]
class Role
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\Column(length: 64)]
    private ?string $name = null;

    #[ORM\OneToMany(targetEntity: User::class, mappedBy: 'role')]
    private Collection $users;



    public function __construct()
    {
        $this->users = new ArrayCollection();
    }
    
/// etc

}

User Class

#[ORM\Entity(repositoryClass: UserRepository::class)]
#[ApiResource]
class User
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\Column(length: 255)]
    private ?string $firstName = null;


    #[ORM\ManyToOne(targetEntity: Role::class, inversedBy: 'users')]
    #[ORM\JoinColumn(nullable: false, name: "role_id", referencedColumnName: "id")]
    private ?Role $role = null;


    public function __construct()
    {
    }

   // etc
}

i am following the example provided by doctrine project documentation page here

is there anything i am missing?

@greg0ire
Copy link
Member

greg0ire commented Feb 27, 2024

@ZeinMansor you need to address this inside API platform. If you can't, then report this to API platform.

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

Successfully merging a pull request may close this issue.

5 participants