Extract AssociationMapping in its own DTO #10613

greg0ire · 2023-04-03T21:07:57Z

No description provided.

greg0ire · 2023-04-03T21:12:13Z

I've tried splitting out JoinColumnMapping in a separate PR, but it was too hard. Sorry for the big PR 😓

lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php

lib/Doctrine/ORM/Mapping/AssociationMapping.php

derrabus · 2023-04-03T21:25:16Z

lib/Doctrine/ORM/Mapping/AssociationMapping.php

+            'isCascadeRefresh' => $this->isCascadeRefresh(),
+            'isCascadeDetach' => $this->isCascadeDetach(),
+            'isCascadeMerge' => $this->isCascadeMerge(),
+            default => $this->$offset,


We should make sure the property exists.

Should we? If we don't, we'll get the same behavior when using $mapping['offset'] as when using $mapping->offset, and IMO we want that, because it means one less difference when migrating to the object API.

>>> $mapping['foo']; <warning>PHP Warning: Undefined property: Doctrine\Tests\ORM\Mapping\MyAssociationMapping::$foo in /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php on line 213</warning> => null

I mean, this is just a warning and for setter part, it's even just a deprecation on PHP 8.2. Being more strict allows us to spot places where we forgot to add a property.

If accessing dynamic properties is what we want here, we should opt-in via #[AllowDynamicProperties].

I feel like we've had that discussion already. 🤔

Yes we very much did 😆 I can add an exception if you insist. Will \InvalidArgumentException suit you, since this is probably not meant to be caught? If not, please let me know what type you want exactly, to avoid more back and forth.

Marking as draft because adding an exception shows there are cases where I fetch properties that do not exist:

1) Doctrine\Tests\ORM\Functional\CompositePrimaryKeyWithAssociationsTest::testFindByAbleToGetCompositeEntitiesWithMixedTypeIdentifiers InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/CompositePrimaryKeyWithAssociationsTest.php:40 2) Doctrine\Tests\ORM\Functional\SecondLevelCacheCompositePrimaryKeyWithAssociationsTest::testFindByReturnsCachedEntity InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:277 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheCompositePrimaryKeyWithAssociationsTest.php:47 3) Doctrine\Tests\ORM\Functional\SecondLevelCacheManyToOneTest::testPutAndLoadNonCacheableManyToOne InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:277 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php:195 4) Doctrine\Tests\ORM\Functional\SecondLevelCacheManyToOneTest::testPutAndLoadNonCacheableCompositeManyToOne InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:277 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php:233 5) Doctrine\Tests\ORM\Functional\SecondLevelCacheOneToManyTest::testPutAndLoadNonCacheableOneToMany InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:277 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php:406 6) Doctrine\Tests\ORM\Functional\SecondLevelCacheOneToOneTest::testPutAndLoadNonCacheableOneToOne InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:277 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToOneTest.php:267 7) Doctrine\Tests\ORM\Functional\Ticket\DDC117Test::testReferencesToForeignKeyEntities InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:276 8) Doctrine\Tests\ORM\Functional\Ticket\DDC117Test::testLoadManyToManyCollectionOfForeignKeyEntities InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:391 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:301 9) Doctrine\Tests\ORM\Functional\Ticket\DDC117Test::testClearManyToManyCollectionOfForeignKeyEntities InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:391 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:318 10) Doctrine\Tests\ORM\Functional\Ticket\DDC117Test::testLoadInverseManyToManyCollection InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:391 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:332 11) Doctrine\Tests\ORM\Functional\Ticket\DDC117Test::testLoadOneToManyOfSourceEntityWithAssociationIdentifier InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:391 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php:357 12) Doctrine\Tests\ORM\Functional\Ticket\DDC2252Test::testIssue InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyOwningSideMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php:416 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php:57 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:401 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2252Test.php:75 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2252Test.php:50 13) Doctrine\Tests\ORM\Functional\Ticket\GH7062Test::testEntityWithAssociationKeyIdentityCanBeUpdated InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:661 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:686 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:236 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:1021 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:384 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7062Test.php:58 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7062Test.php:44 14) Doctrine\Tests\ORM\Persisters\BasicEntityPersisterCompositeTypeParametersTest::testExpandParametersWillExpandCompositeEntityKeys InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:1844 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:1777 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php:38 15) Doctrine\Tests\ORM\Persisters\BasicEntityPersisterCompositeTypeParametersTest::testExpandCriteriaParametersWillExpandCompositeEntityKeys InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\OneToManyAssociationMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:1844 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:858 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterCompositeTypeParametersTest.php:52 16) Doctrine\Tests\ORM\Persisters\ManyToManyPersisterTest::testDeleteManyToManyCollection InvalidArgumentException: Unknown property joinColumns on class Doctrine\ORM\Mapping\ManyToManyOwningSideMapping /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Mapping/AssociationMapping.php:220 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Utility/PersisterHelper.php:73 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php:416 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php:57 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/UnitOfWork.php:401 /home/gregoire/dev/doctrine-orm/lib/Doctrine/ORM/EntityManager.php:267 /home/gregoire/dev/doctrine-orm/tests/Doctrine/Tests/ORM/Persisters/ManyToManyPersisterTest.php:45

Fixed, as well as a similar condition below. The warnings/exceptions have disappear, please advise on

orm/lib/Doctrine/ORM/Utility/PersisterHelper.php

Lines 78 to 82 in e5fb1a4

// iterate over to-one association mappings

foreach ($class->associationMappings as $assoc) {

if (! isset($assoc['joinColumns'])) {

continue;

}

, I'm convinced having that exception, even if that's temporary may be useful now 😆

Will \InvalidArgumentException suit you, since this is probably not meant to be caught?

I tried hard to resist my impulse to nit-pick here, but… … …

OutOfBoundsException was made for this.

Sorry. 😓

Oh no, actually OutOfRangeException because the caller could have known in advance that the key is invalid.

Anyway, the exception is just temporary. Any of these three would do.

🏃🏻‍♂️💨

OutOfRangeException it is then… indeed, it looks perfectly suited to this 👍

lib/Doctrine/ORM/Mapping/JoinColumnMapping.php

lib/Doctrine/ORM/Mapping/ToOneAssociationMapping.php

SenseException

Those instanceof checks of mappings are reoccurring multiple times. I'd prefer to have them in their own mapping-methods for simplicity.

lib/Doctrine/ORM/Mapping/ManyToManyAssociationMapping.php

derrabus · 2023-04-04T23:42:40Z

lib/Doctrine/ORM/Mapping/AssociationMapping.php

+            $value = JoinTableMapping::fromMappingArray($value);
+        }
+
+        $this->$offset = $value;


OutOfRangeException if property does not exist.

greg0ire · 2023-04-05T07:01:07Z

Those instanceof checks of mappings are reoccurring multiple times. I'd prefer to have them in their own mapping-methods for simplicity.

I just pushed a new version where I introduce more of those methods. There are still instanceof checks that remain, some of which might disappear when phpstan/phpstan#8904 gets fixed. Other are checks inside tests, I assumed you didn't mean those ones.

lib/Doctrine/ORM/Mapping/AssociationMapping.php

lib/Doctrine/ORM/Mapping/JoinTableMapping.php

SenseException

👍 but there's an unaddressed comment #10613 (comment)

greg0ire · 2023-04-12T05:57:53Z

@SenseException @derrabus I actually addressed that comment by introducing a few lines of code above, which might explain why that diff is not marked as outdated.

requestd change has been addressed.

greg0ire requested review from SenseException, derrabus and beberlei April 3, 2023 21:12

This comment was marked as resolved.

Sign in to view

derrabus previously requested changes Apr 3, 2023

View reviewed changes

greg0ire force-pushed the association-mapping branch 2 times, most recently from 9614e46 to 5b8629c Compare April 4, 2023 07:28

greg0ire requested a review from derrabus April 4, 2023 07:34

greg0ire marked this pull request as draft April 4, 2023 08:33

greg0ire force-pushed the association-mapping branch 2 times, most recently from 3198a1a to 67fcf67 Compare April 4, 2023 09:12

greg0ire marked this pull request as ready for review April 4, 2023 09:28

SenseException requested changes Apr 4, 2023

View reviewed changes

lib/Doctrine/ORM/Mapping/ManyToManyAssociationMapping.php Show resolved Hide resolved

derrabus reviewed Apr 4, 2023

View reviewed changes

greg0ire force-pushed the association-mapping branch from 67fcf67 to 74d7ac7 Compare April 5, 2023 06:58

greg0ire requested review from derrabus and SenseException April 5, 2023 07:01

SenseException requested changes Apr 5, 2023

View reviewed changes

lib/Doctrine/ORM/Mapping/AssociationMapping.php Outdated Show resolved Hide resolved

lib/Doctrine/ORM/Mapping/AssociationMapping.php Outdated Show resolved Hide resolved

lib/Doctrine/ORM/Mapping/JoinTableMapping.php Outdated Show resolved Hide resolved

greg0ire force-pushed the association-mapping branch from 74d7ac7 to 4225b43 Compare April 7, 2023 06:36

Extract AssociationMapping in its own DTO

14d6152

greg0ire force-pushed the association-mapping branch from 4225b43 to 14d6152 Compare April 7, 2023 06:39

greg0ire requested a review from SenseException April 7, 2023 06:43

SenseException approved these changes Apr 11, 2023

View reviewed changes

greg0ire added the Improvement label Apr 12, 2023

greg0ire added this to the 3.0.0 milestone Apr 12, 2023

greg0ire merged commit eaac5cd into doctrine:3.0.x Apr 13, 2023

greg0ire deleted the association-mapping branch April 13, 2023 17:32

greg0ire mentioned this pull request Nov 19, 2023

Remove wrong assertion #11071

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract AssociationMapping in its own DTO #10613

Extract AssociationMapping in its own DTO #10613

greg0ire commented Apr 3, 2023

greg0ire commented Apr 3, 2023

This comment was marked as resolved.

derrabus Apr 3, 2023

greg0ire Apr 4, 2023

derrabus Apr 4, 2023

greg0ire Apr 4, 2023

greg0ire Apr 4, 2023

greg0ire Apr 4, 2023

derrabus Apr 4, 2023

derrabus Apr 4, 2023

derrabus Apr 4, 2023

greg0ire Apr 4, 2023

SenseException left a comment

derrabus Apr 4, 2023

greg0ire commented Apr 5, 2023

SenseException left a comment

greg0ire commented Apr 12, 2023

	// iterate over to-one association mappings
	foreach ($class->associationMappings as $assoc) {
	if (! isset($assoc['joinColumns'])) {
	continue;
	}

Extract AssociationMapping in its own DTO #10613

Extract AssociationMapping in its own DTO #10613

Conversation

greg0ire commented Apr 3, 2023

greg0ire commented Apr 3, 2023

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SenseException left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented Apr 5, 2023

SenseException left a comment

Choose a reason for hiding this comment

greg0ire commented Apr 12, 2023