Skip to content

Commit 02ab4ba

Browse files
SpamerczSpamercz
authored
Merge pull request #17 from Spameri/fix/value-collection-hydration
Hydrate value collections instead of returning them empty
2 parents c6984f4 + e8a3126 commit 02ab4ba

4 files changed

Lines changed: 226 additions & 0 deletions

File tree

‎src/Factory/EntityFactory.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,34 @@ class: $entity[\Spameri\Elastic\Model\Insert\PrepareEntityArray::ENTITY_CLASS],
183183
);
184184
}
185185

186+
} elseif (
187+
$attribute->getName() === \Spameri\Elastic\Mapping\ValueCollection::class
188+
) {
189+
/** @var array{class: class-string} $arguments */
190+
$arguments = $attribute->getArguments();
191+
192+
// The mirror of PrepareEntityArray's ValueCollectionInterface
193+
// branch, which writes value() for each member and so leaves a
194+
// flat list of scalars in the document. Rebuilding one member
195+
// per scalar is the whole of it; there are no nested properties
196+
// to resolve, and looking for them under `field.*` is what this
197+
// used to do by falling through to the tail below - producing an
198+
// empty collection, silently, on every single read.
199+
$propertyValue = new $propertyTypeName();
200+
201+
if (\is_array($value)) {
202+
foreach ($value as $item) {
203+
if ($item === null || $item === '') {
204+
continue;
205+
}
206+
207+
$collectionValue = new $arguments['class']($item);
208+
$propertyValue->add($collectionValue);
209+
210+
$this->changeSet->markExisting($collectionValue);
211+
}
212+
}
213+
186214
} elseif (
187215
$attribute->getName() === \Spameri\Elastic\Mapping\STIEntity::class
188216
) {

‎src/Mapping/ValueCollection.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Spameri\Elastic\Mapping;
4+
5+
/**
6+
* A collection of value objects, stored as a flat list of scalars.
7+
*
8+
* The class is named for the same reason ElasticCollection names one: the
9+
* document holds ["Action", "Drama"] and nothing else, so the value type
10+
* cannot be recovered from what was written. Without it the collection can be
11+
* written but never read back.
12+
*/
13+
#[\Attribute(\Attribute::TARGET_PROPERTY|\Attribute::TARGET_PARAMETER)]
14+
class ValueCollection
15+
{
16+
17+
18+
public function __construct(
19+
public string $class,
20+
)
21+
{
22+
}
23+
24+
}

‎tests/SpameriTests/Elastic/Data/Entity/EntityWithValueCollection.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace SpameriTests\Elastic\Data\Entity;
4+
5+
/**
6+
* Entity with a collection of value objects, stored as a flat list of scalars.
7+
*
8+
* Distinct from EntityWithCollection, whose members are nested objects with
9+
* properties of their own: a value collection round-trips through
10+
* ["Action", "Drama"], not through a list of documents.
11+
*/
12+
class EntityWithValueCollection extends \Spameri\Elastic\Entity\AbstractElasticEntity
13+
{
14+
15+
/**
16+
* @param \SpameriTests\Elastic\Data\Entity\Video\Details\GenreCollection<\SpameriTests\Elastic\Data\Entity\Video\Details\Genre> $genres
17+
*/
18+
public function __construct(
19+
#[\Spameri\Elastic\Mapping\Entity(class: \Spameri\Elastic\Entity\Property\ElasticId::class)]
20+
public \Spameri\Elastic\Entity\Property\ElasticIdInterface $id,
21+
#[\Spameri\Elastic\Mapping\ValueCollection(class: \SpameriTests\Elastic\Data\Entity\Video\Details\Genre::class)]
22+
public \SpameriTests\Elastic\Data\Entity\Video\Details\GenreCollection $genres,
23+
)
24+
{
25+
parent::__construct($id);
26+
}
27+
28+
}

‎tests/SpameriTests/Elastic/Factory/EntityFactory/ValueCollectionTest.phpt‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace SpameriTests\Elastic\Factory\EntityFactory;
4+
5+
require_once __DIR__ . '/../../../../bootstrap.php';
6+
7+
/**
8+
* A value collection is written as a flat list of scalars and has to come back
9+
* as the same list of value objects.
10+
*
11+
* PrepareEntityArray has always known how: it walks a ValueCollectionInterface
12+
* and writes value() for each member. EntityFactory did not, so the property
13+
* fell through to the generic "instantiate from nested properties" tail, where
14+
* the collection's own properties are looked for under `genres.*` — nothing is
15+
* stored there, because the document holds `genres: ["Action"]` — and every
16+
* read produced an empty collection.
17+
*
18+
* Nothing reported an error. The data was in Elasticsearch, indexed and
19+
* searchable, and simply never reached anything that read an entity.
20+
*
21+
* @testCase
22+
*/
23+
class ValueCollectionTest extends \SpameriTests\Elastic\AbstractTestCase
24+
{
25+
26+
public function testValueCollectionSurvivesTheRoundTrip(): void
27+
{
28+
/** @var \Spameri\Elastic\EntityManager $entityManager */
29+
$entityManager = $this->container->getByType(\Spameri\Elastic\EntityManager::class);
30+
/** @var \Spameri\Elastic\Factory\EntityFactory $entityFactory */
31+
$entityFactory = $this->container->getByType(\Spameri\Elastic\Factory\EntityFactory::class);
32+
/** @var \Spameri\Elastic\Model\Insert\PrepareEntityArray $prepareEntityArray */
33+
$prepareEntityArray = $this->container->getByType(\Spameri\Elastic\Model\Insert\PrepareEntityArray::class);
34+
35+
$entity = new \SpameriTests\Elastic\Data\Entity\EntityWithValueCollection(
36+
new \Spameri\Elastic\Entity\Property\ElasticId('vc-1'),
37+
new \SpameriTests\Elastic\Data\Entity\Video\Details\GenreCollection(
38+
new \SpameriTests\Elastic\Data\Entity\Video\Details\Genre('Action'),
39+
new \SpameriTests\Elastic\Data\Entity\Video\Details\Genre('Science Fiction'),
40+
),
41+
);
42+
43+
$source = $prepareEntityArray->prepare($entity);
44+
45+
// What the write side puts in the document: a flat list of scalars.
46+
\Tester\Assert::same(['Action', 'Science Fiction'], $source['genres']);
47+
48+
$hit = new \Spameri\ElasticQuery\Response\Result\Hit(
49+
source: $source,
50+
position: 0, index: '', type: '', id: 'vc-1', score: 0.0, version: 0,
51+
);
52+
53+
/** @var \SpameriTests\Elastic\Data\Entity\EntityWithValueCollection $hydrated */
54+
$hydrated = $entityFactory->create(
55+
$hit,
56+
\SpameriTests\Elastic\Data\Entity\EntityWithValueCollection::class,
57+
$entityManager,
58+
);
59+
60+
$genres = [];
61+
foreach ($hydrated->genres as $genre) {
62+
$genres[] = $genre->value();
63+
}
64+
65+
\Tester\Assert::same(['Action', 'Science Fiction'], $genres);
66+
}
67+
68+
69+
public function testAnEmptyValueCollectionStaysEmpty(): void
70+
{
71+
/** @var \Spameri\Elastic\EntityManager $entityManager */
72+
$entityManager = $this->container->getByType(\Spameri\Elastic\EntityManager::class);
73+
/** @var \Spameri\Elastic\Factory\EntityFactory $entityFactory */
74+
$entityFactory = $this->container->getByType(\Spameri\Elastic\Factory\EntityFactory::class);
75+
76+
$hit = new \Spameri\ElasticQuery\Response\Result\Hit(
77+
source: ['genres' => []],
78+
position: 0, index: '', type: '', id: 'vc-2', score: 0.0, version: 0,
79+
);
80+
81+
/** @var \SpameriTests\Elastic\Data\Entity\EntityWithValueCollection $hydrated */
82+
$hydrated = $entityFactory->create(
83+
$hit,
84+
\SpameriTests\Elastic\Data\Entity\EntityWithValueCollection::class,
85+
$entityManager,
86+
);
87+
88+
\Tester\Assert::same(0, \iterator_count($hydrated->genres->getIterator()));
89+
}
90+
91+
92+
public function testAnAbsentValueCollectionIsNotAnError(): void
93+
{
94+
/** @var \Spameri\Elastic\EntityManager $entityManager */
95+
$entityManager = $this->container->getByType(\Spameri\Elastic\EntityManager::class);
96+
/** @var \Spameri\Elastic\Factory\EntityFactory $entityFactory */
97+
$entityFactory = $this->container->getByType(\Spameri\Elastic\Factory\EntityFactory::class);
98+
99+
// A document written before the field existed. It has to read as empty
100+
// rather than throw, or one old document takes down a whole index.
101+
$hit = new \Spameri\ElasticQuery\Response\Result\Hit(
102+
source: [],
103+
position: 0, index: '', type: '', id: 'vc-3', score: 0.0, version: 0,
104+
);
105+
106+
/** @var \SpameriTests\Elastic\Data\Entity\EntityWithValueCollection $hydrated */
107+
$hydrated = $entityFactory->create(
108+
$hit,
109+
\SpameriTests\Elastic\Data\Entity\EntityWithValueCollection::class,
110+
$entityManager,
111+
);
112+
113+
\Tester\Assert::same(0, \iterator_count($hydrated->genres->getIterator()));
114+
}
115+
116+
117+
public function testNullMembersAreNotTurnedIntoValues(): void
118+
{
119+
/** @var \Spameri\Elastic\EntityManager $entityManager */
120+
$entityManager = $this->container->getByType(\Spameri\Elastic\EntityManager::class);
121+
/** @var \Spameri\Elastic\Factory\EntityFactory $entityFactory */
122+
$entityFactory = $this->container->getByType(\Spameri\Elastic\Factory\EntityFactory::class);
123+
124+
$hit = new \Spameri\ElasticQuery\Response\Result\Hit(
125+
source: ['genres' => ['Action', NULL, '', 'Drama']],
126+
position: 0, index: '', type: '', id: 'vc-4', score: 0.0, version: 0,
127+
);
128+
129+
/** @var \SpameriTests\Elastic\Data\Entity\EntityWithValueCollection $hydrated */
130+
$hydrated = $entityFactory->create(
131+
$hit,
132+
\SpameriTests\Elastic\Data\Entity\EntityWithValueCollection::class,
133+
$entityManager,
134+
);
135+
136+
$genres = [];
137+
foreach ($hydrated->genres as $genre) {
138+
$genres[] = $genre->value();
139+
}
140+
141+
\Tester\Assert::same(['Action', 'Drama'], $genres);
142+
}
143+
144+
}
145+
146+
(new ValueCollectionTest())->run();

0 commit comments

Comments
 (0)