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

Throw a full-fledged exception on invalid call #11308

Merged
merged 1 commit into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Upgrade to 3.0

## BC BREAK: Calling `ClassMetadata::getAssociationMappedByTargetField()` with the owning side of an association now throws an exception

Previously, calling
`Doctrine\ORM\Mapping\ClassMetadata::getAssociationMappedByTargetField()` with
the owning side of an association returned `null`, which was undocumented, and
wrong according to the phpdoc of the parent method.

If you do not know whether you are on the owning or inverse side of an association,
you can use `Doctrine\ORM\Mapping\ClassMetadata::isAssociationInverseSide()`
to find out.

## BC BREAK: `Doctrine\ORM\Proxy\Autoloader` no longer extends `Doctrine\Common\Proxy\Autoloader`

Make sure to use the former when writing a type declaration or an `instanceof` check.
Expand Down
16 changes: 14 additions & 2 deletions src/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use function ltrim;
use function method_exists;
use function spl_object_id;
use function sprintf;
use function str_contains;
use function str_replace;
use function strtolower;
Expand Down Expand Up @@ -2457,9 +2458,20 @@ public function isAssociationInverseSide(string $assocName): bool

public function getAssociationMappedByTargetField(string $assocName): string
{
$assoc = $this->associationMappings[$assocName];
$assoc = $this->getAssociationMapping($assocName);

assert($assoc instanceof InverseSideMapping);
if (! $assoc instanceof InverseSideMapping) {
throw new LogicException(sprintf(
<<<'EXCEPTION'
Context: Calling %s() with "%s", which is the owning side of an association.
Problem: The owning side of an association has no "mappedBy" field.
Solution: Call %s::isAssociationInverseSide() to check first.
EXCEPTION,
__METHOD__,
$assocName,
self::class,
));
}

return $assoc->mappedBy;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/Tests/ORM/Mapping/ClassMetadataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
use Doctrine\Tests\ORM\Mapping\TypedFieldMapper\CustomIntAsStringTypedFieldMapper;
use Doctrine\Tests\OrmTestCase;
use DoctrineGlobalArticle;
use LogicException;
use PHPUnit\Framework\Attributes\Group as TestGroup;
use ReflectionClass;
use stdClass;
Expand Down Expand Up @@ -1054,6 +1055,21 @@ public function testItAddingLifecycleCallbackOnEmbeddedClassIsIllegal(): void

$metadata->addLifecycleCallback('foo', 'bar');
}

public function testItThrowsOnInvalidCallToGetAssociationMappedByTargetField(): void
{
$metadata = new ClassMetadata(self::class);
$metadata->mapOneToOne(['fieldName' => 'foo', 'targetEntity' => 'bar']);

$this->expectException(LogicException::class);
$this->expectExceptionMessage(<<<'EXCEPTION'
Context: Calling Doctrine\ORM\Mapping\ClassMetadata::getAssociationMappedByTargetField() with "foo", 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.
EXCEPTION);

$metadata->getAssociationMappedByTargetField('foo');
}
}

#[MappedSuperclass]
Expand Down