Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions src/EntityManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}


Expand Down
15 changes: 15 additions & 0 deletions src/Model/Get.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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());
}
Expand Down
4 changes: 2 additions & 2 deletions tests/SpameriTests/Elastic/EdgeCases/ConcurrencyTest.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/SpameriTests/Elastic/Model/DeleteMultipleTest.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/SpameriTests/Elastic/Model/DeleteTest.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

Expand Down Expand Up @@ -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
Expand Down
16 changes: 12 additions & 4 deletions tests/SpameriTests/Elastic/Model/GetTest.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
);
}

Expand Down Expand Up @@ -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
Expand Down
Loading