From 2d801f4e34a5a6dd10b7e81ea9e400c9c58e2463 Mon Sep 17 00:00:00 2001 From: Spamer Date: Thu, 18 Jun 2026 23:16:29 +0200 Subject: [PATCH] feat(persist): index new entity first so circular references resolve Persisting a new entity (EmptyElasticId) with a nested elastic entity that references it back looped: Insert prepared the parent first, which persisted the child, whose prepare persisted the parent again, and so on. New entities are not tracked in the IdentityMap (empty-string id), so isChanged() always reported them as changed and they were re-persisted. Insert::execute now saves a new entity in two phases: 1. index it once with a shallow (all-null) body to obtain its id and register it as "persisting" in the IdentityMap, 2. prepare/persist its nested entities - a back-reference to the parent now resolves to the parent's id instead of re-persisting it, 3. re-index it with the full body. Entities that already have an id are still indexed once. IdentityMap gains a persistingList with markPersisting/unmarkPersisting/ isPersisting. PrepareEntityArray short-circuits to the id (instead of recursing) whenever a referenced entity is currently being persisted, for the nested AbstractElasticEntity, ElasticEntityCollection item and STIElasticEntity cases. The "persisting" check is needed in addition to isChanged() because the parent's hash changes as its children receive ids. Adds PersistCircularTest, which persists a Title whose backdrop is an Image whose title points back at the Title, and asserts it terminates, both ids are assigned and each stored document references the other. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Model/IdentityMap.php | 44 +++++++++ src/Model/Insert.php | 92 ++++++++++++++++--- src/Model/Insert/PrepareEntityArray.php | 15 ++- .../EntityManager/PersistCircularTest.phpt | 68 ++++++++++++++ 4 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 tests/SpameriTests/Elastic/EntityManager/PersistCircularTest.phpt 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();