Skip to content

Commit

Permalink
Conditional load fixes (#23369)
Browse files Browse the repository at this point in the history 
* fix conditional loading logic for numeric keyed arrays

* Apply fixes from StyleCI (#23368)

* more tests

* Apply fixes from StyleCI (#23370)

* another test

* Apply fixes from StyleCI (#23371)

* complicate test more
  • Loading branch information
@taylorotwell
taylorotwell authored Mar 2, 2018
1 parent a35c52d commit 83dd8cd
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 18 deletions.
51 changes: 33 additions & 18 deletions src/Illuminate/Http/Resources/ConditionallyLoadsAttributes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ protected function filter($data)
{
$index = -1;

$numericKeys = array_values($data) === $data;

foreach ($data as $key => $value) {
$index++;

Expand All @@ -26,24 +28,15 @@ protected function filter($data)
}

if (is_numeric($key) && $value instanceof MergeValue) {
return $this->merge($data, $index, $this->filter($value->data));
}

if (($value instanceof PotentiallyMissing && $value->isMissing()) ||
($value instanceof self &&
$value->resource instanceof PotentiallyMissing &&
$value->isMissing())) {
unset($data[$key]);

$index--;
return $this->merge($data, $index, $this->filter($value->data), $numericKeys);
}

if ($value instanceof self && is_null($value->resource)) {
$data[$key] = null;
}
}

return $data;
return $this->removeMissingValues($data, $numericKeys);
}

/**
Expand All @@ -52,20 +45,42 @@ protected function filter($data)
* @param array $data
* @param int $index
* @param array $merge
* @param bool $numericKeys
* @return array
*/
protected function merge($data, $index, $merge)
protected function merge($data, $index, $merge, $numericKeys)
{
if (array_values($data) === $data) {
return array_merge(
if ($numericKeys) {
return $this->removeMissingValues(array_merge(
array_merge(array_slice($data, 0, $index, true), $merge),
$this->filter(array_slice($data, $index + 1, null, true))
);
$this->filter(array_values(array_slice($data, $index + 1, null, true)))
), $numericKeys);
}

return array_slice($data, 0, $index, true) +
return $this->removeMissingValues(array_slice($data, 0, $index, true) +
$merge +
$this->filter(array_slice($data, $index + 1, null, true));
$this->filter(array_slice($data, $index + 1, null, true)));
}

/**
* Remove the missing values from the filtered data.
*
* @param array $data
* @param bool $numericKeys
* @return array
*/
protected function removeMissingValues($data, $numericKeys = false)
{
foreach ($data as $key => $value) {
if (($value instanceof PotentiallyMissing && $value->isMissing()) ||
($value instanceof self &&
$value->resource instanceof PotentiallyMissing &&
$value->isMissing())) {
unset($data[$key]);
}
}

return $numericKeys ? array_values($data) : $data;
}

/**
Expand Down
137 changes: 137 additions & 0 deletions tests/Integration/Http/ResourceTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

use Orchestra\Testbench\TestCase;
use Illuminate\Support\Facades\Route;
use Illuminate\Http\Resources\MergeValue;
use Illuminate\Pagination\LengthAwarePaginator;
use Illuminate\Tests\Integration\Http\Fixtures\Post;
use Illuminate\Tests\Integration\Http\Fixtures\Author;
use Illuminate\Http\Resources\ConditionallyLoadsAttributes;
use Illuminate\Tests\Integration\Http\Fixtures\PostResource;
use Illuminate\Tests\Integration\Http\Fixtures\Subscription;
use Illuminate\Tests\Integration\Http\Fixtures\PostCollectionResource;
Expand Down Expand Up @@ -521,4 +523,139 @@ public function test_original_on_response_is_collection_of_model_when_collection
$this->assertTrue($response->getOriginalContent()->contains($post));
});
}

public function test_leading_merge_value_is_merged_correctly()
{
$filter = new class {
use ConditionallyLoadsAttributes;

public function work()
{
return $this->filter([
new MergeValue(['First', 'Second']),
'Taylor',
'Mohamed',
new MergeValue(['Adam', 'Matt']),
'Jeffrey',
new MergeValue(['Abigail', 'Lydia']),
]);
}
};

$results = $filter->work();

$this->assertEquals([
'First', 'Second', 'Taylor', 'Mohamed', 'Adam', 'Matt', 'Jeffrey', 'Abigail', 'Lydia',
], $results);
}

public function test_merge_values_may_be_missing()
{
$filter = new class {
use ConditionallyLoadsAttributes;

public function work()
{
return $this->filter([
new MergeValue(['First', 'Second']),
'Taylor',
'Mohamed',
$this->mergeWhen(false, ['Adam', 'Matt']),
'Jeffrey',
new MergeValue(['Abigail', 'Lydia']),
]);
}
};

$results = $filter->work();

$this->assertEquals([
'First', 'Second', 'Taylor', 'Mohamed', 'Jeffrey', 'Abigail', 'Lydia',
], $results);
}

public function test_initial_merge_values_may_be_missing()
{
$filter = new class {
use ConditionallyLoadsAttributes;

public function work()
{
return $this->filter([
$this->mergeWhen(false, ['First', 'Second']),
'Taylor',
'Mohamed',
$this->mergeWhen(true, ['Adam', 'Matt']),
'Jeffrey',
new MergeValue(['Abigail', 'Lydia']),
]);
}
};

$results = $filter->work();

$this->assertEquals([
'Taylor', 'Mohamed', 'Adam', 'Matt', 'Jeffrey', 'Abigail', 'Lydia',
], $results);
}

public function test_all_merge_values_may_be_missing()
{
$filter = new class {
use ConditionallyLoadsAttributes;

public function work()
{
return $this->filter([
$this->mergeWhen(false, ['First', 'Second']),
'Taylor',
'Mohamed',
$this->mergeWhen(false, ['Adam', 'Matt']),
'Jeffrey',
$this->mergeWhen(false, (['Abigail', 'Lydia'])),
]);
}
};

$results = $filter->work();

$this->assertEquals([
'Taylor', 'Mohamed', 'Jeffrey',
], $results);
}

public function test_nested_merges()
{
$filter = new class {
use ConditionallyLoadsAttributes;

public function work()
{
return $this->filter([
$this->mergeWhen(true, [['Something']]),
[
$this->mergeWhen(true, ['First', $this->mergeWhen(true, ['Second'])]),
'Third',
],
[
'Fourth',
],
]);
}
};

$results = $filter->work();

$this->assertEquals([
[
'Something',
],
[
'First', 'Second', 'Third',
],
[
'Fourth',
],
], $results);
}
}

0 comments on commit 83dd8cd

Please sign in to comment.