Skip to content

Commit 0e068c0

Browse files
tjwiebelltjwiebell
authored
Merge pull request #40 from magento-research/bug/resolve-parent-definition
Attempt to Resolve Parent Definitions in Iterator
2 parents 1eb8e61 + a304ea3 commit 0e068c0

File tree

4 files changed

+156
-14
lines changed

4 files changed

+156
-14
lines changed

‎src/AbstractKeyValueStore.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,31 @@ public function get($lookup)
6060
return \is_array($value) ? new static($value) : $value;
6161
}
6262

63+
/**
64+
* Find the longest portion of $lookup that exists.
65+
*/
66+
public function getExistingParentLookup(string $lookup): string
67+
{
68+
$subArray = $this->data;
69+
70+
$parentSegments = [];
71+
72+
foreach (explode('.', $lookup) as $segment) {
73+
if (\is_array($subArray)) {
74+
if (array_key_exists($segment, $subArray)) {
75+
$subArray = $subArray[$segment];
76+
$parentSegments[] = $segment;
77+
} else {
78+
break;
79+
}
80+
} else {
81+
break;
82+
}
83+
}
84+
85+
return implode('.', $parentSegments);
86+
}
87+
6388
/**
6489
* List keys.
6590
*/

‎src/DefinitionIterator.php

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
namespace Magento\Upward;
1010

11+
use Zend\Http\Response;
12+
1113
class DefinitionIterator
1214
{
1315
/**
@@ -51,37 +53,47 @@ public function __clone()
5153
*/
5254
public function get($lookup, $definition = null)
5355
{
54-
$updateContext = false;
56+
$updateContext = false;
57+
$originalLookup = '';
5558

5659
if ($definition === null && $this->isContextFullyPopulated($lookup)) {
5760
$value = $this->context->get($lookup);
5861

5962
return ($value instanceof Context) ? $value->toArray() : $value;
6063
}
6164

62-
if (\in_array($lookup, $this->lookupStack)) {
63-
throw new \RuntimeException('Definition appears to contain a loop: ' . json_encode($this->lookupStack));
64-
}
65-
6665
if ($definition === null) {
67-
if (!$this->getRootDefinition()->has($lookup)) {
68-
throw new \RuntimeException(sprintf(
69-
'No definition for %s',
70-
\is_string($lookup) || is_numeric($lookup) ? $lookup : \gettype($lookup)
71-
));
72-
}
73-
7466
$definition = $this->getRootDefinition();
7567
$updateContext = true;
7668
}
7769

78-
$definedValue = ($definition instanceof Definition) ? $definition->get($lookup) : $definition;
70+
$definedValue = $definition;
71+
72+
if ($definition instanceof Definition) {
73+
if (!$definition->has($lookup)) {
74+
if ($parentLookup = $definition->getExistingParentLookup($lookup)) {
75+
$originalLookup = $lookup;
76+
$lookup = $parentLookup;
77+
} else {
78+
throw new \RuntimeException(sprintf(
79+
'No definition for %s',
80+
\is_string($lookup) || is_numeric($lookup) ? $lookup : \gettype($lookup)
81+
));
82+
}
83+
}
84+
85+
$definedValue = $definedValue->get($lookup);
86+
}
7987

8088
// Expand $lookup to full tree address so we can safely detect loops across different parts of the tree
8189
if ($definedValue instanceof Definition) {
8290
$lookup = $definedValue->getTreeAddress();
8391
}
8492

93+
if (\in_array($lookup, $this->lookupStack)) {
94+
throw new \RuntimeException('Definition appears to contain a loop: ' . json_encode($this->lookupStack));
95+
}
96+
8597
$this->lookupStack[] = $lookup;
8698

8799
try {
@@ -98,6 +110,18 @@ public function get($lookup, $definition = null)
98110

99111
array_pop($this->lookupStack);
100112

113+
if (!empty($originalLookup)) {
114+
if (\is_array($value)) {
115+
$value = $this->get($originalLookup);
116+
} elseif (!$value instanceof Response) {
117+
throw new \RuntimeException(sprintf(
118+
'Could not get nested value %s from value of type %s',
119+
$originalLookup,
120+
\gettype($value)
121+
));
122+
}
123+
}
124+
101125
return $value;
102126
}
103127

@@ -151,7 +175,7 @@ private function getFromDefinedValue(string $lookup, $definedValue)
151175
}
152176

153177
if ($definedValue instanceof Definition && $definedValue->isList()) {
154-
return array_map(function ($key) use ($lookup) {
178+
return array_map(function ($key) {
155179
return $this->get($key);
156180
}, $definedValue->toArray());
157181
}
@@ -173,6 +197,7 @@ private function getFromDefinedValue(string $lookup, $definedValue)
173197
private function getFromResolver(string $lookup, $definedValue, Resolver\ResolverInterface $resolver)
174198
{
175199
$resolver->setIterator($this);
200+
176201
if ($definedValue instanceof Definition && !$resolver->isValid($definedValue)) {
177202
throw new \RuntimeException(sprintf(
178203
'Definition %s is not valid for %s.',

‎test/AbstractKeyValueStoreTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,20 @@ public function testGet(): void
7070
verify($subject->get(''))->is()->null();
7171
}
7272

73+
public function testGetExistingParentLookup(): void
74+
{
75+
$subject = $this->getSubject([
76+
'key0' => [
77+
'key00' => 'value',
78+
],
79+
]);
80+
81+
verify($subject->getExistingParentLookup('key0.key00'))->is()->sameAs('key0.key00');
82+
verify($subject->getExistingParentLookup('key0.key00.key000'))->is()->sameAs('key0.key00');
83+
verify($subject->getExistingParentLookup('key0.key01'))->is()->sameAs('key0');
84+
verify($subject->getExistingParentLookup('key1'))->is()->sameAs('');
85+
}
86+
7387
public function testGetKeys(): void
7488
{
7589
$subject = $this->getSubject();

‎test/DefinitionIteratorTest.php

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Mockery;
1717
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
1818
use PHPUnit\Framework\TestCase;
19+
use Zend\Http\Response;
1920
use function BeBat\Verify\verify;
2021

2122
class DefinitionIteratorTest extends TestCase
@@ -174,6 +175,83 @@ public function testLookupInContext(): void
174175
verify($iterator->get('key', $definitionWithSameKey))->is()->sameAs(true);
175176
}
176177

178+
public function testParentResolver(): void
179+
{
180+
$context = new Context([]);
181+
$definition = new Definition(['key' => 'resolver-definition']);
182+
$childDefinition = new Definition(['child-key' => '200']);
183+
$iterator = new DefinitionIterator($definition, $context);
184+
$resolverFactory = Mockery::mock('alias:' . ResolverFactory::class);
185+
$mockResolver = Mockery::mock(ResolverInterface::class);
186+
187+
$resolverFactory->shouldReceive('get')
188+
->with('resolver-definition')
189+
->andReturn($mockResolver);
190+
191+
$mockResolver->shouldReceive('setIterator')
192+
->with($iterator);
193+
$mockResolver->shouldReceive('isValid')
194+
->with($definition)
195+
->andReturn(true);
196+
$mockResolver->shouldReceive('resolve')
197+
->with('resolver-definition')
198+
->andReturn($childDefinition);
199+
200+
verify($iterator->get('key.child-key'))->is()->sameAs('200');
201+
}
202+
203+
public function testParentResolverIsNotArray(): void
204+
{
205+
$context = new Context([]);
206+
$definition = new Definition(['key' => 'resolver-definition']);
207+
$iterator = new DefinitionIterator($definition, $context);
208+
$resolverFactory = Mockery::mock('alias:' . ResolverFactory::class);
209+
$mockResolver = Mockery::mock(ResolverInterface::class);
210+
211+
$resolverFactory->shouldReceive('get')
212+
->with('resolver-definition')
213+
->andReturn($mockResolver);
214+
215+
$mockResolver->shouldReceive('setIterator')
216+
->with($iterator);
217+
$mockResolver->shouldReceive('isValid')
218+
->with($definition)
219+
->andReturn(true);
220+
$mockResolver->shouldReceive('resolve')
221+
->with('resolver-definition')
222+
->andReturn('200');
223+
224+
$this->expectException(\RuntimeException::class);
225+
$this->expectExceptionMessage('Could not get nested value key.child-key from value of type string');
226+
227+
$iterator->get('key.child-key');
228+
}
229+
230+
public function testParentResolverIsResponse(): void
231+
{
232+
$context = new Context([]);
233+
$definition = new Definition(['key' => 'resolver-definition']);
234+
$response = new Response();
235+
$iterator = new DefinitionIterator($definition, $context);
236+
$resolverFactory = Mockery::mock('alias:' . ResolverFactory::class);
237+
$mockResolver = Mockery::mock(ResolverInterface::class);
238+
239+
$resolverFactory->shouldReceive('get')
240+
->with('resolver-definition')
241+
->andReturn($mockResolver);
242+
243+
$mockResolver->shouldReceive('setIterator')
244+
->with($iterator);
245+
$mockResolver->shouldReceive('isValid')
246+
->with($definition)
247+
->andReturn(true);
248+
$mockResolver->shouldReceive('resolve')
249+
->with('resolver-definition')
250+
->andReturn($response);
251+
252+
verify($iterator->get('key.child-key'))->is()->sameAs($response);
253+
}
254+
177255
public function testResolverValueDefinition(): void
178256
{
179257
$context = new Context([]);

0 commit comments

Comments
 (0)