From 08bbb3fc996f01792a0a7de7be216f7852c29bb3 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Fri, 15 Nov 2024 10:21:04 +0200 Subject: [PATCH] orm: soft-deleted deferred load handling --- .../Espo/ORM/Relation/RDBRelations.php | 35 +++++++++++++---- schema/metadata/entityDefs.json | 2 +- tests/integration/Espo/ORM/RelationsTest.php | 26 +++++++++---- .../testClasses/Entities/Opportunity.php | 39 ------------------- 4 files changed, 47 insertions(+), 55 deletions(-) delete mode 100644 tests/integration/testClasses/Entities/Opportunity.php diff --git a/application/Espo/ORM/Relation/RDBRelations.php b/application/Espo/ORM/Relation/RDBRelations.php index 48c0730064..b4c22c7226 100644 --- a/application/Espo/ORM/Relation/RDBRelations.php +++ b/application/Espo/ORM/Relation/RDBRelations.php @@ -30,6 +30,7 @@ namespace Espo\ORM\Relation; use Espo\ORM\BaseEntity; +use Espo\ORM\Defs\Defs; use Espo\ORM\Entity; use Espo\ORM\EntityCollection; use Espo\ORM\EntityManager; @@ -57,9 +58,13 @@ class RDBRelations implements Relations RelationType::HAS_CHILDREN, ]; + private Defs $defs; + public function __construct( private EntityManager $entityManager, - ) {} + ) { + $this->defs = $this->entityManager->getDefs(); + } public function setEntity(Entity $entity): void { @@ -253,8 +258,7 @@ class RDBRelations implements Relations return new EntityCollection(); } - $relationDefs = $this->entityManager - ->getDefs() + $relationDefs = $this->defs ->getEntity($this->entity->getEntityType()) ->getRelation($relation); @@ -291,8 +295,7 @@ class RDBRelations implements Relations throw new LogicException(); } - return $this->entityManager - ->getDefs() + return $this->defs ->getEntity($this->entity->getEntityType()) ->tryGetRelation($relation) ?->getType(); @@ -304,8 +307,7 @@ class RDBRelations implements Relations throw new LogicException(); } - $defs = $this->entityManager - ->getDefs() + $defs = $this->defs ->getEntity($this->entity->getEntityType()) ->getRelation($relation); @@ -320,7 +322,26 @@ class RDBRelations implements Relations if ($relationType === RelationType::BELONGS_TO) { $foreignEntityType = $defs->getForeignEntityType(); + $nameAttribute = $relation . 'Name'; $id = $this->entity->get($relation . 'Id'); + $name = $this->entity->get($nameAttribute); + + if ( + $id && + $name === null && + $this->entity->hasAttribute($nameAttribute) && + $this->entity->has($nameAttribute) + ) { + $hasDeleted = $this->defs + ->getEntity($foreignEntityType) + ->hasAttribute(Attribute::DELETED); + + if ($hasDeleted) { + // Could be either soft-deleted or have name set to null. + // We resort to not using a partially loaded entity. + return null; + } + } } else if ($relationType === RelationType::BELONGS_TO_PARENT) { $foreignEntityType = $this->entity->get($relation . 'Type'); $id = $this->entity->get($relation . 'Id'); diff --git a/schema/metadata/entityDefs.json b/schema/metadata/entityDefs.json index 5ff8c132f4..71bf02649b 100644 --- a/schema/metadata/entityDefs.json +++ b/schema/metadata/entityDefs.json @@ -1346,7 +1346,7 @@ "properties": { "deferredLoad": { "type": "boolean", - "description": "When getting a related entity from an entity, it will be returned without loaded values. Values will be loaded on accessing. The downside is that it can return an entity even if it's deleted." + "description": "When getting a related entity from an entity, it will be returned without loaded values. Values will be loaded on accessing. For belongsToParent there's the downside that it can return an entity even if it's deleted." } } } diff --git a/tests/integration/Espo/ORM/RelationsTest.php b/tests/integration/Espo/ORM/RelationsTest.php index 4b6ca8a48f..9ee4797604 100644 --- a/tests/integration/Espo/ORM/RelationsTest.php +++ b/tests/integration/Espo/ORM/RelationsTest.php @@ -37,7 +37,6 @@ use Espo\Modules\Crm\Entities\Opportunity; use Espo\ORM\Repository\Option\SaveOption; use tests\integration\Core\BaseTestCase; use tests\integration\testClasses\Entities\Account as AccountExtended; -use tests\integration\testClasses\Entities\Opportunity as OpportunityExtended; class RelationsTest extends BaseTestCase { @@ -45,7 +44,11 @@ class RelationsTest extends BaseTestCase { $metadata = $this->getMetadata(); $metadata->set('entityDefs', Opportunity::ENTITY_TYPE, [ - 'entityClassName' => OpportunityExtended::class, + 'links' => [ + 'account' => [ + 'deferredLoad' => true, + ], + ], ]); $metadata->save(); @@ -54,8 +57,8 @@ class RelationsTest extends BaseTestCase $em = $this->getEntityManager(); $opp = $em->getNewEntity(Opportunity::ENTITY_TYPE); - $this->assertInstanceOf(OpportunityExtended::class, $opp); - $this->assertNull($opp->getRelatedAccount()); + $this->assertInstanceOf(Opportunity::class, $opp); + $this->assertNull($opp->getAccount()); /** @var Account $account */ $account = $em->createEntity(Account::ENTITY_TYPE, ['name' => 'Account 1']); @@ -64,16 +67,16 @@ class RelationsTest extends BaseTestCase 'accountId' => $account->getId(), ]); - $this->assertInstanceOf(OpportunityExtended::class, $opp); + $this->assertInstanceOf(Opportunity::class, $opp); - $account1 = $opp->getRelatedAccount(); + $account1 = $opp->getAccount(); $this->assertInstanceOf(Account::class, $account1); $this->assertEquals($account->getId(), $account1->getId()); $em->refreshEntity($opp); - $account2 = $opp->getRelatedAccount(); + $account2 = $opp->getAccount(); $this->assertInstanceOf(Account::class, $account2); $this->assertNotSame($account1, $account2); @@ -82,11 +85,18 @@ class RelationsTest extends BaseTestCase $em->saveEntity($opp); - $account3 = $opp->getRelatedAccount(); + $account3 = $opp->getAccount(); $this->assertInstanceOf(Account::class, $account3); $this->assertNotSame($account1, $account3); $this->assertEquals($account->getId(), $account3->getId()); + + // Soft-deleted. + + $em->removeEntity($account3); + $em->refreshEntity($opp); + + $this->assertNull($opp->getAccount()); } public function testGetMany(): void diff --git a/tests/integration/testClasses/Entities/Opportunity.php b/tests/integration/testClasses/Entities/Opportunity.php deleted file mode 100644 index e38ad35c8e..0000000000 --- a/tests/integration/testClasses/Entities/Opportunity.php +++ /dev/null @@ -1,39 +0,0 @@ -. - * - * The interactive user interfaces in modified source and object code versions - * of this program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU Affero General Public License version 3. - * - * In accordance with Section 7(b) of the GNU Affero General Public License version 3, - * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. - ************************************************************************/ - -namespace tests\integration\testClasses\Entities; - -class Opportunity extends \Espo\Modules\Crm\Entities\Opportunity -{ - public function getRelatedAccount(): ?\Espo\Modules\Crm\Entities\Account - { - /** @var ?\Espo\Modules\Crm\Entities\Account */ - return $this->relations->getOne('account'); - } -}