Skip to content

Commit 7ae5731

Browse files
committed
ensure model ID is URL-safe
1 parent d873381 commit 7ae5731

File tree

5 files changed

+25
-11
lines changed

5 files changed

+25
-11
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ phpstan.neon
99
testbench.yaml
1010
vendor
1111
node_modules
12+
.phpunit.cache/

src/Index/AbstractIndex.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Limenet\LaravelElasticaBridge\Index;
66

77
use Elastica\Document;
8+
use Elastica\Exception\InvalidException;
89
use Elastica\Exception\NotFoundException;
910
use Elastica\Index;
1011
use Elastica\Query;
@@ -89,14 +90,13 @@ public function documentResultToElements(ResultSet $result): array
8990

9091
public function getModelInstance(Document $document): Model
9192
{
92-
$id = $document->getId();
93-
94-
if (empty($id)) {
93+
try {
94+
$modelClass = $document->get(self::DOCUMENT_MODEL_CLASS);
95+
$modelId = $document->get(self::DOCUMENT_MODEL_ID);
96+
} catch(InvalidException) {
9597
throw new RuntimeException();
9698
}
9799

98-
[$modelClass,$modelId] = explode('|', $id);
99-
100100
return $modelClass::findOrFail($modelId);
101101
}
102102

src/Model/ElasticsearchableTrait.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ trait ElasticsearchableTrait
1111
{
1212
final public function getElasticsearchId(): string
1313
{
14-
return $this::class.'|'.$this->id;
14+
return str($this::class)
15+
->replace('\\', ' ')
16+
->snake()
17+
->append('-', $this->id)
18+
->toString();
1519
}
1620

1721
public function toElasticsearch(IndexInterface $indexConfig): array

tests/Unit/IndexTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Elastica\Response;
1010
use Elastica\ResultSet;
1111
use Limenet\LaravelElasticaBridge\Exception\Index\BlueGreenIndicesIncorrectlySetupException;
12+
use Limenet\LaravelElasticaBridge\Index\IndexInterface;
1213
use Limenet\LaravelElasticaBridge\Tests\App\Elasticsearch\CustomerIndex;
1314
use Limenet\LaravelElasticaBridge\Tests\App\Elasticsearch\ProductIndex;
1415
use Limenet\LaravelElasticaBridge\Tests\App\Models\Customer;
@@ -73,7 +74,7 @@ public function test_empty_document_to_model(): void
7374
/** @var Customer $customer */
7475
$customer = Customer::first();
7576
$document = $customer->toElasticaDocument($this->customerIndex);
76-
$document->setId(null);
77+
$document->remove(IndexInterface::DOCUMENT_MODEL_ID);
7778

7879
$this->expectException(RuntimeException::class);
7980
$this->customerIndex->getModelInstance($document);

tests/Unit/ModelTest.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,12 @@ public function test_convert_to_elastica_document_customized(): void
3737
$this->assertSame($customer->email, $document->get('email'));
3838
$this->assertSame($customer->type, $document->get('type'));
3939

40-
$this->assertStringContainsString('|'.$customer->id, $document->getId());
41-
$this->assertStringContainsString($customer::class.'|', $document->getId());
40+
$this->assertStringContainsString('-'.$customer->id, $document->getId());
41+
$this->assertStringContainsString(
42+
str($customer::class)->classBasename()->lower()->append('-')->toString(),
43+
$document->getId()
44+
);
45+
$this->assertMatchesRegularExpression('/[\w\d_-]/', $document->getId());
4246

4347
$this->assertSame($customer->id, $document->get(IndexInterface::DOCUMENT_MODEL_ID));
4448
$this->assertSame($customer::class, $document->get(IndexInterface::DOCUMENT_MODEL_CLASS));
@@ -55,8 +59,12 @@ public function test_convert_to_elastica_document_default(): void
5559

5660
$this->assertSame($product->name, $document->get('name'));
5761

58-
$this->assertStringContainsString('|'.$product->id, $document->getId());
59-
$this->assertStringContainsString($product::class.'|', $document->getId());
62+
$this->assertStringContainsString('-'.$product->id, $document->getId());
63+
$this->assertStringContainsString(
64+
str($product::class)->classBasename()->lower()->append('-')->toString(),
65+
$document->getId()
66+
);
67+
$this->assertMatchesRegularExpression('/[\w\d_-]/', $document->getId());
6068

6169
$this->assertSame($product->id, $document->get(IndexInterface::DOCUMENT_MODEL_ID));
6270
$this->assertSame($product::class, $document->get(IndexInterface::DOCUMENT_MODEL_CLASS));

0 commit comments

Comments
 (0)