-
Notifications
You must be signed in to change notification settings - Fork 11.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[8.x] Added sole() and soleWhere() methods for Collections #37034
Changes from 7 commits
1aef429
af6270d
d9c21db
034bad1
5465ded
04a1df1
d6a023e
534bb57
d8cd628
483e43b
4e76b91
c0a82d8
3c4f386
2d3b63c
9c42ab1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,6 +4,8 @@ | |||||
|
||||||
use ArrayAccess; | ||||||
use ArrayIterator; | ||||||
use Illuminate\Collections\ItemNotFoundException; | ||||||
use Illuminate\Collections\MultipleItemsFoundException; | ||||||
use Illuminate\Support\Traits\EnumeratesValues; | ||||||
use Illuminate\Support\Traits\Macroable; | ||||||
use stdClass; | ||||||
|
@@ -1050,6 +1052,31 @@ public function splitIn($numberOfGroups) | |||||
return $this->chunk(ceil($this->count() / $numberOfGroups)); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Get the first item in the collection, but only if exactly | ||||||
* item exists. Otherwise, throw an exception. | ||||||
* | ||||||
* @param callable|null $callback | ||||||
* @return mixed | ||||||
* | ||||||
* @throws ItemNotFoundException | ||||||
* @throws MultipleItemsFoundException | ||||||
*/ | ||||||
public function sole(callable $callback = null) | ||||||
{ | ||||||
$items = $this->filter($callback); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code as is will filter out
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh I must have missed that, cheers |
||||||
|
||||||
if ($items->isEmpty()) { | ||||||
throw new ItemNotFoundException; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice if the query builder etc would throw the exact same exceptions when calling sole() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
} | ||||||
|
||||||
if ($items->count() > 1) { | ||||||
throw new MultipleItemsFoundException; | ||||||
} | ||||||
|
||||||
return $items->first(); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Chunk the collection into chunks of the given size. | ||||||
* | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ | |
namespace Illuminate\Support; | ||
|
||
use Countable; | ||
use Illuminate\Collections\ItemNotFoundException; | ||
use Illuminate\Collections\MultipleItemsFoundException; | ||
use Illuminate\Contracts\Support\Arrayable; | ||
use Illuminate\Contracts\Support\Jsonable; | ||
use IteratorAggregate; | ||
|
@@ -808,6 +810,30 @@ public function slice($offset, $length = null); | |
*/ | ||
public function split($numberOfGroups); | ||
|
||
/** | ||
* Get the first item in the collection, but only if exactly | ||
* item exists. Otherwise, throw an exception. | ||
* | ||
* @param callable|null $callback | ||
* @return mixed | ||
* | ||
* @throws ItemNotFoundException | ||
* @throws MultipleItemsFoundException | ||
*/ | ||
public function sole(callable $callback = null); | ||
|
||
/** | ||
* Get the first item by the given key value pair, but only if | ||
* exactly one item matches the criteria. Otherwise, throw | ||
* an exception. | ||
* | ||
* @param string $key | ||
* @param mixed $operator | ||
* @param mixed $value | ||
* @return mixed | ||
*/ | ||
public function soleWhere($key, $operator = null, $value = null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't add methods to an interface mid-cycle. Remove these from the interface. After it's merged, submit a separate PR to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense thinking about it. I'll take this out and make a separate PR if this one gets merged |
||
|
||
/** | ||
* Chunk the collection into chunks of the given size. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
|
||
namespace Illuminate\Collections; | ||
|
||
use RuntimeException; | ||
|
||
class ItemNotFoundException extends RuntimeException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
use ArrayIterator; | ||
use Closure; | ||
use DateTimeInterface; | ||
use Illuminate\Collections\ItemNotFoundException; | ||
use Illuminate\Collections\MultipleItemsFoundException; | ||
use Illuminate\Support\Traits\EnumeratesValues; | ||
use Illuminate\Support\Traits\Macroable; | ||
use IteratorAggregate; | ||
|
@@ -1010,6 +1012,51 @@ public function split($numberOfGroups) | |
return $this->passthru('split', func_get_args()); | ||
} | ||
|
||
/** | ||
* Get the first item in the collection, but only if exactly | ||
* item exists. Otherwise, throw an exception. | ||
* | ||
* @param callable|null $callback | ||
* @return mixed | ||
* | ||
* @throws ItemNotFoundException | ||
* @throws MultipleItemsFoundException | ||
*/ | ||
public function sole(callable $callback = null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implementation is way more complex than it needs to be. Something like this (not tested) should suffice: public function sole(callable $callback = null)
{
return $this
->when($callback)
->filter($callback)
->take(2)
->collect()
->sole();
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do agree that it's more complex. I was looking at some of the existing code for things like the |
||
{ | ||
$iterator = $this->getIterator(); | ||
|
||
if (is_null($callback)) { | ||
if (! $iterator->valid()) { | ||
throw new ItemNotFoundException; | ||
} | ||
|
||
if ($this->take(2)->count() > 1) { | ||
throw new MultipleItemsFoundException; | ||
} | ||
|
||
return $iterator->current(); | ||
} | ||
|
||
$items = []; | ||
|
||
foreach ($iterator as $key => $value) { | ||
if ($callback($value, $key)) { | ||
$items[] = $value; | ||
} | ||
} | ||
|
||
if (! count($items)) { | ||
throw new ItemNotFoundException; | ||
} | ||
|
||
if (count($items) > 1) { | ||
throw new MultipleItemsFoundException; | ||
} | ||
|
||
return $items[0]; | ||
} | ||
|
||
/** | ||
* Chunk the collection into chunks of the given size. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
|
||
namespace Illuminate\Collections; | ||
|
||
use RuntimeException; | ||
|
||
class MultipleItemsFoundException extends RuntimeException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -453,6 +453,21 @@ public function sum($callback = null) | |
}, 0); | ||
} | ||
|
||
/** | ||
* Get the first item by the given key value pair, but only if | ||
* exactly one item matches the criteria. Otherwise, throw | ||
* an exception. | ||
* | ||
* @param string $key | ||
* @param mixed $operator | ||
* @param mixed $value | ||
* @return mixed | ||
*/ | ||
public function soleWhere($key, $operator = null, $value = null) | ||
{ | ||
return $this->sole($this->operatorForWhere(...func_get_args())); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since, unlike the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense. So do you suggest removed the |
||
|
||
/** | ||
* Apply the callback if the value is truthy. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,8 @@ | |
use ArrayObject; | ||
use CachingIterator; | ||
use Exception; | ||
use Illuminate\Collections\ItemNotFoundException; | ||
use Illuminate\Collections\MultipleItemsFoundException; | ||
use Illuminate\Contracts\Support\Arrayable; | ||
use Illuminate\Contracts\Support\Jsonable; | ||
use Illuminate\Support\Collection; | ||
|
@@ -66,6 +68,90 @@ public function testFirstWithDefaultAndWithoutCallback($collection) | |
$this->assertSame('default', $result); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleReturnsFirstItemInCollectionIfOnlyOneExists($collection) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also add tests in SupportLazyCollectionIsLazyTest.php ensuring this method is lazy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah sure, I didn't realise those tests were there, so I'll add some to it |
||
{ | ||
$collection = new $collection([ | ||
['name' => 'foo'], | ||
['name' => 'bar'], | ||
]); | ||
|
||
$this->assertSame(['name' => 'foo'], $collection->where('name', 'foo')->sole()); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleThrowsExceptionIfNoItemsExists($collection) | ||
{ | ||
$this->expectException(ItemNotFoundException::class); | ||
|
||
$collection = new $collection([ | ||
['name' => 'foo'], | ||
['name' => 'bar'], | ||
]); | ||
|
||
$collection->where('name', 'INVALID')->sole(); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleThrowsExceptionIfMoreThanOneItemExists($collection) | ||
{ | ||
$this->expectException(MultipleItemsFoundException::class); | ||
|
||
$collection = new $collection([ | ||
['name' => 'foo'], | ||
['name' => 'foo'], | ||
['name' => 'bar'], | ||
]); | ||
|
||
$collection->where('name', 'foo')->sole(); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleReturnsFirstItemInCollectionIfOnlyOneExistsWithCallback($collection) | ||
{ | ||
$data = new $collection(['foo', 'bar', 'baz']); | ||
$result = $data->sole(function ($value) { | ||
return $value === 'bar'; | ||
}); | ||
$this->assertSame('bar', $result); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleThrowsExceptionIfNoItemsExistsWithCallback($collection) | ||
{ | ||
$this->expectException(ItemNotFoundException::class); | ||
|
||
$data = new $collection(['foo', 'bar', 'baz']); | ||
|
||
$data->sole(function ($value) { | ||
return $value === 'invalid'; | ||
}); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleThrowsExceptionIfMoreThanOneItemExistsWithCallback($collection) | ||
{ | ||
$this->expectException(MultipleItemsFoundException::class); | ||
|
||
$data = new $collection(['foo', 'bar', 'bar']); | ||
|
||
$data->sole(function ($value) { | ||
return $value === 'bar'; | ||
}); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
|
@@ -82,6 +168,50 @@ public function testFirstWhere($collection) | |
$this->assertNull($data->firstWhere('nonexistent', 'key')); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleWhere($collection) | ||
{ | ||
$data = new $collection([ | ||
['material' => 'paper', 'type' => 'book'], | ||
['material' => 'rubber', 'type' => 'gasket'], | ||
]); | ||
|
||
$this->assertSame('book', $data->soleWhere('material', 'paper')['type']); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleWhereThrowsExceptionIfNoItemExists($collection) | ||
{ | ||
$this->expectException(ItemNotFoundException::class); | ||
|
||
$data = new $collection([ | ||
['material' => 'paper', 'type' => 'book'], | ||
['material' => 'rubber', 'type' => 'gasket'], | ||
]); | ||
|
||
$data->soleWhere('material', 'invalid'); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
public function testSoleWhereThrowsExceptionIfMultipleItemsExists($collection) | ||
{ | ||
$this->expectException(MultipleItemsFoundException::class); | ||
|
||
$data = new $collection([ | ||
['material' => 'paper', 'type' => 'book'], | ||
['material' => 'paper', 'type' => 'letter'], | ||
['material' => 'rubber', 'type' => 'gasket'], | ||
]); | ||
|
||
$data->soleWhere('material', 'paper'); | ||
} | ||
|
||
/** | ||
* @dataProvider collectionClassProvider | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always use fully qualified class names in DocBlocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh okay, sorry I wasn't aware of that. I'll get them changed to be fully qualified