diff --git a/src/EntityManager.php b/src/EntityManager.php index 13a63299..13d4a05d 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -8,6 +8,7 @@ public function __construct( private \Spameri\Elastic\Model\Insert $insert, private \Spameri\Elastic\Model\GetAllBy $getAllBy, + private \Spameri\Elastic\Model\Get $get, private \Spameri\Elastic\Model\Delete $delete, private \Spameri\Elastic\Model\EntitySettingsLocator $entitySettingsLocator, private \Spameri\Elastic\EventManager $eventManager, @@ -39,15 +40,21 @@ class: $class, return $entity; } - $elasticQuery = new \Spameri\ElasticQuery\ElasticQuery(); - $elasticQuery->addMustQuery( - new \Spameri\ElasticQuery\Query\Term( - '_id', - $id, - ), - ); + $indexConfig = $this->entitySettingsLocator->locateByEntityClass($class); + + try { + $singleResult = $this->get->execute( + new \Spameri\Elastic\Entity\Property\ElasticId($id), + $indexConfig->indexName(), + ); + + } catch (\Spameri\Elastic\Exception\ElasticSearch $exception) { + \Tracy\Debugger::log($exception->getMessage(), \Tracy\ILogger::CRITICAL); + + throw $exception; + } - return $this->findOneBy($elasticQuery, $class); + return $this->entityFactory->create($singleResult->hit(), $class, $this); } diff --git a/src/Model/Get.php b/src/Model/Get.php index 1154cb2b..f66f56f8 100644 --- a/src/Model/Get.php +++ b/src/Model/Get.php @@ -14,6 +14,7 @@ public function __construct( /** + * @throws \Spameri\Elastic\Exception\DocumentNotFound when the document is not there * @throws \Spameri\Elastic\Exception\ElasticSearch */ public function execute( @@ -34,6 +35,20 @@ public function execute( ) ; + } catch (\Elastic\Elasticsearch\Exception\ClientResponseException $exception) { + if ($exception->getCode() !== 404) { + throw new \Spameri\Elastic\Exception\ElasticSearch($exception->getMessage()); + } + + // The client reports a missing document as a 404 and throws. Saying so with the + // exception the library already has for it makes this path agree with every other one: + // findOneBy() raises DocumentNotFound for the same situation, and AbstractBaseService + // means to as well - its found() check cannot be reached today, because the blanket + // wrap below turns a 404 into ElasticSearch and its catch rethrows that first. + throw new \Spameri\Elastic\Exception\DocumentNotFound( + $index . ' with id ' . $id->value(), + ); + } catch (\Elastic\Elasticsearch\Exception\ElasticsearchException $exception) { throw new \Spameri\Elastic\Exception\ElasticSearch($exception->getMessage()); } diff --git a/tests/SpameriTests/Elastic/EdgeCases/ConcurrencyTest.phpt b/tests/SpameriTests/Elastic/EdgeCases/ConcurrencyTest.phpt index b873bdb5..61d97d77 100644 --- a/tests/SpameriTests/Elastic/EdgeCases/ConcurrencyTest.phpt +++ b/tests/SpameriTests/Elastic/EdgeCases/ConcurrencyTest.phpt @@ -223,7 +223,7 @@ class ConcurrencyTest extends \SpameriTests\Elastic\AbstractTestCase \usleep(300000); // Verify deletions - use a fresh Get model to bypass identity map cache - // Get model throws ElasticSearch exception for 404 + // Get model reports a document that is not there as DocumentNotFound /** @var \Spameri\Elastic\Model\Get $get */ $get = $this->container->getByType(\Spameri\Elastic\Model\Get::class); @@ -233,7 +233,7 @@ class ConcurrencyTest extends \SpameriTests\Elastic\AbstractTestCase new \Spameri\Elastic\Entity\Property\ElasticId($id), \SpameriTests\Elastic\Config::INDEX_EDGE_CASE, ), - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); } diff --git a/tests/SpameriTests/Elastic/Model/DeleteMultipleTest.phpt b/tests/SpameriTests/Elastic/Model/DeleteMultipleTest.phpt index f642d4a0..a9ee55d7 100644 --- a/tests/SpameriTests/Elastic/Model/DeleteMultipleTest.phpt +++ b/tests/SpameriTests/Elastic/Model/DeleteMultipleTest.phpt @@ -219,7 +219,7 @@ class DeleteMultipleTest extends \SpameriTests\Elastic\AbstractTestCase static function () use ($get, $id): void { $get->execute(new \Spameri\Elastic\Entity\Property\ElasticId($id), self::INDEX); }, - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); } diff --git a/tests/SpameriTests/Elastic/Model/DeleteTest.phpt b/tests/SpameriTests/Elastic/Model/DeleteTest.phpt index 6552a9bb..493d2ee3 100644 --- a/tests/SpameriTests/Elastic/Model/DeleteTest.phpt +++ b/tests/SpameriTests/Elastic/Model/DeleteTest.phpt @@ -67,7 +67,7 @@ class DeleteTest extends \SpameriTests\Elastic\AbstractTestCase // Verify it's gone \Tester\Assert::exception( static fn () => $get->execute(new \Spameri\Elastic\Entity\Property\ElasticId($id), self::INDEX), - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); } @@ -132,7 +132,7 @@ class DeleteTest extends \SpameriTests\Elastic\AbstractTestCase // Entity A should be gone \Tester\Assert::exception( static fn () => $get->execute(new \Spameri\Elastic\Entity\Property\ElasticId($idA), $indexA), - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); // Cleanup diff --git a/tests/SpameriTests/Elastic/Model/GetTest.phpt b/tests/SpameriTests/Elastic/Model/GetTest.phpt index 542b9700..72636ce7 100644 --- a/tests/SpameriTests/Elastic/Model/GetTest.phpt +++ b/tests/SpameriTests/Elastic/Model/GetTest.phpt @@ -65,7 +65,14 @@ class GetTest extends \SpameriTests\Elastic\AbstractTestCase } - public function testGetNonExistentThrowsException(): void + /** + * The exception says which situation it is now. + * + * It used to be ElasticSearch, not by design but because every client exception was wrapped the + * same way - so a missing document and an unreachable cluster arrived as the same type, while + * findOneBy() raised DocumentNotFound for that very situation. + */ + public function testGetNonExistentThrowsDocumentNotFound(): void { /** @var \Spameri\Elastic\Model\Get $get */ $get = $this->container->getByType(\Spameri\Elastic\Model\Get::class); @@ -75,7 +82,7 @@ class GetTest extends \SpameriTests\Elastic\AbstractTestCase new \Spameri\Elastic\Entity\Property\ElasticId('nonexistent-id-12345'), self::INDEX, ), - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); } @@ -126,13 +133,14 @@ class GetTest extends \SpameriTests\Elastic\AbstractTestCase ); \Tester\Assert::same($idB, $resultB->hit()->id()); - // Trying to get A's ID from B should fail + // Trying to get A's ID from B should fail - as a document that is not there, which is what + // it is, rather than as an unspecified Elasticsearch failure. \Tester\Assert::exception( static fn () => $get->execute( new \Spameri\Elastic\Entity\Property\ElasticId($idA), $indexB, ), - \Spameri\Elastic\Exception\ElasticSearch::class, + \Spameri\Elastic\Exception\DocumentNotFound::class, ); // Cleanup extra indexes