diff --git a/src/Model/IdentityMap.php b/src/Model/IdentityMap.php index 1136b98..5a7cca9 100644 --- a/src/Model/IdentityMap.php +++ b/src/Model/IdentityMap.php @@ -27,6 +27,13 @@ class IdentityMap */ public array $uninitializedEntityList = []; + /** + * entities currently being persisted (write side) - used to break circular references + * + * @var array> + */ + public array $persistingList = []; + public function add( \Spameri\Elastic\Entity\AbstractElasticEntity $entity, @@ -90,6 +97,42 @@ public function markInserted( } + public function markPersisting( + \Spameri\Elastic\Entity\AbstractElasticEntity $entity, + ): void + { + if ($entity->id instanceof \Spameri\Elastic\Entity\Property\EmptyElasticId) { + return; + } + + $this->persistingList[$entity::class][$entity->id()->value()] = true; + } + + + public function unmarkPersisting( + \Spameri\Elastic\Entity\AbstractElasticEntity $entity, + ): void + { + if ($entity->id instanceof \Spameri\Elastic\Entity\Property\EmptyElasticId) { + return; + } + + unset($this->persistingList[$entity::class][$entity->id()->value()]); + } + + + public function isPersisting( + \Spameri\Elastic\Entity\AbstractElasticEntity $entity, + ): bool + { + if ($entity->id instanceof \Spameri\Elastic\Entity\Property\EmptyElasticId) { + return false; + } + + return isset($this->persistingList[$entity::class][$entity->id()->value()]); + } + + public function isChanged( \Spameri\Elastic\Entity\AbstractElasticEntity $entity, ): bool @@ -128,6 +171,7 @@ public function clear(): void $this->persisted = []; $this->creatingEntityList = []; $this->uninitializedEntityList = []; + $this->persistingList = []; } } diff --git a/src/Model/Insert.php b/src/Model/Insert.php index f207add..c4c21c8 100644 --- a/src/Model/Insert.php +++ b/src/Model/Insert.php @@ -15,6 +15,17 @@ public function __construct( /** + * New entities are indexed in two phases so that nested entities can reference + * the parent (and circular references do not loop): + * + * 1. the parent is indexed first (empty body, no refresh) to obtain its id and + * is marked as "persisting" in the identity map, + * 2. its nested entities are persisted - a back-reference to the parent now + * resolves to the parent's id instead of re-persisting it, + * 3. the parent is re-indexed with the full body. + * + * Entities that already have an id are indexed once, as before. + * * @throws \Spameri\Elastic\Exception\ElasticSearch * @throws \Spameri\Elastic\Exception\DocumentInsertFailed */ @@ -31,14 +42,42 @@ public function execute( return $entity->id()->value(); } - // Only mark inserted for entities with real IDs (prevents collision on empty string key) - if ($hasRealId) { + // New entity: index it first (shallow body) to obtain its id before its + // nested entities are persisted, so circular references can resolve to it. + if ($hasRealId === false) { + $this->index($entity, $index, $this->shallowBody($entity), false); + } + + $this->identityMap->markInserted($entity); + $this->identityMap->markPersisting($entity); + + try { + $entityArray = $this->prepareEntityArray->prepare($entity, $hasSti); + unset($entityArray['id']); + + $id = $this->index($entity, $index, $entityArray, true); $this->identityMap->markInserted($entity); + + return $id; + + } finally { + $this->identityMap->unmarkPersisting($entity); } + } - $entityArray = $this->prepareEntityArray->prepare($entity, $hasSti); - unset($entityArray['id']); + /** + * @param array $entityArray + * @throws \Spameri\Elastic\Exception\ElasticSearch + * @throws \Spameri\Elastic\Exception\DocumentInsertFailed + */ + private function index( + \Spameri\Elastic\Entity\AbstractElasticEntity $entity, + string $index, + array $entityArray, + bool $refresh, + ): string + { try { $response = $this->clientProvider->client()->index( ( @@ -55,22 +94,23 @@ public function execute( throw new \Spameri\Elastic\Exception\ElasticSearch($exception->getMessage()); } - try { - $this->clientProvider->client()->indices()->refresh( - ( - new \Spameri\ElasticQuery\Document($index) + if ($refresh === true) { + try { + $this->clientProvider->client()->indices()->refresh( + ( + new \Spameri\ElasticQuery\Document($index) + ) + ->toArray(), ) - ->toArray(), - ) - ; + ; - } catch (\Elastic\Elasticsearch\Exception\ElasticsearchException $exception) { - throw new \Spameri\Elastic\Exception\ElasticSearch($exception->getMessage()); + } catch (\Elastic\Elasticsearch\Exception\ElasticsearchException $exception) { + throw new \Spameri\Elastic\Exception\ElasticSearch($exception->getMessage()); + } } if (isset($response['result']) && ($response['result'] === 'created' || $response['result'] === 'updated')) { $entity->id = new \Spameri\Elastic\Entity\Property\ElasticId($response['_id']); - $this->identityMap->markInserted($entity); return $response['_id']; } @@ -78,4 +118,28 @@ public function execute( throw new \Spameri\Elastic\Exception\DocumentInsertFailed(); } + + /** + * Minimal non-empty body for the first index of a new entity: every field + * (except the id) nulled. The real values are written by the second index; + * this only reserves the document so it gets a server generated id. + * + * @return array + */ + private function shallowBody( + \Spameri\Elastic\Entity\AbstractElasticEntity $entity, + ): array + { + $body = []; + foreach (\array_keys($entity->entityVariables()) as $key) { + if ($key === 'id') { + continue; + } + + $body[$key] = null; + } + + return $body; + } + } diff --git a/src/Model/Insert/PrepareEntityArray.php b/src/Model/Insert/PrepareEntityArray.php index f69811e..fad7d6a 100644 --- a/src/Model/Insert/PrepareEntityArray.php +++ b/src/Model/Insert/PrepareEntityArray.php @@ -111,7 +111,10 @@ public function iterateVariables( $preparedArray[$key][self::ENTITY_CLASS] = $property::class; - if ($this->identityMap->isChanged($property) === false) { + if ( + $this->identityMap->isPersisting($property) === true + || $this->identityMap->isChanged($property) === false + ) { $preparedArray[$key][self::ENTITY_ID] = $property->id()->value(); } else { @@ -127,7 +130,10 @@ public function iterateVariables( } if ($property instanceof \Spameri\Elastic\Entity\AbstractElasticEntity) { - if ($this->identityMap->isChanged($property) === false) { + if ( + $this->identityMap->isPersisting($property) === true + || $this->identityMap->isChanged($property) === false + ) { $preparedArray[$key] = $property->id()->value(); } else { @@ -178,7 +184,10 @@ public function iterateVariables( } else { /** @var \Spameri\Elastic\Entity\AbstractElasticEntity $item */ foreach ($property as $item) { - if ($this->identityMap->isChanged($item) === false) { + if ( + $this->identityMap->isPersisting($item) === true + || $this->identityMap->isChanged($item) === false + ) { $preparedArray[$key][] = $item->id()->value(); } else { diff --git a/tests/SpameriTests/Elastic/EntityManager/PersistCircularTest.phpt b/tests/SpameriTests/Elastic/EntityManager/PersistCircularTest.phpt new file mode 100644 index 0000000..4615dc1 --- /dev/null +++ b/tests/SpameriTests/Elastic/EntityManager/PersistCircularTest.phpt @@ -0,0 +1,68 @@ +container->getByType(\Spameri\Elastic\EntityManager::class); + + $title = new \SpameriTests\Elastic\Data\Entity\Title( + new \Spameri\Elastic\Entity\Property\EmptyElasticId(), + null, + ); + $image = new \SpameriTests\Elastic\Data\Entity\Image( + new \Spameri\Elastic\Entity\Property\EmptyElasticId(), + $title, + true, + ); + // circular reference: title -> image -> title + $title->backdrop = $image; + + // must finish (no infinite loop) and assign both ids + \Tester\Assert::noError( + static function () use ($entityManager, $title): void { + $entityManager->persist($title); + }, + ); + + \Tester\Assert::false($title->id() instanceof \Spameri\Elastic\Entity\Property\EmptyElasticId); + \Tester\Assert::false($image->id() instanceof \Spameri\Elastic\Entity\Property\EmptyElasticId); + \Tester\Assert::notSame('', $title->id()->value()); + \Tester\Assert::notSame('', $image->id()->value()); + + // each side recorded the other's resolved id on the in-memory graph + \Tester\Assert::same($image->id()->value(), $title->backdrop->id()->value()); + \Tester\Assert::same($title->id()->value(), $image->title->id()->value()); + + // read the stored documents and verify the references were persisted + $client = $this->container->getByType(\Spameri\Elastic\ClientProvider::class)->client(); + + $titleSource = $client->get([ + 'index' => \SpameriTests\Elastic\Config::INDEX_TITLE, + 'id' => $title->id()->value(), + ])->asArray()['_source']; + $imageSource = $client->get([ + 'index' => \SpameriTests\Elastic\Config::INDEX_IMAGE, + 'id' => $image->id()->value(), + ])->asArray()['_source']; + + \Tester\Assert::same($image->id()->value(), $titleSource['backdrop']); + \Tester\Assert::same($title->id()->value(), $imageSource['title']); + } + +} + +(new PersistCircularTest())->run();