-
Notifications
You must be signed in to change notification settings - Fork 2
Add Items and in-memory/null implementations #5
Conversation
| use function tests\Libero\ContentApiBundle\capture_output; | ||
|
|
||
| abstract class FunctionalTestCase extends TestCase | ||
| abstract class FunctionalTestCase extends KernelTestCase |
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.
State (such as InMemoryItems) wasn't be reset. This makes it slower; will see if there's a way to reset the kernel/container and extract the change.
| $items->add($item2); | ||
|
|
||
| $this->assertCount(2, $items); | ||
| $this->assertEquals([$item1, $item2], iterator_to_array($items)); |
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.
While looking at other implementations (filesystem and DB), this doesn't work. Turns out it only works here as the resource is the same instance as the one provided on input. https://github.com/sebastianbergmann/comparator/blob/71bdaddd52ddb753a144c78289f21abb719aebc4/src/ResourceComparator.php#L43 actually does a strict comparison (resources are converted to a unique identifier, so aren't being compared on their values).
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.
Opened sebastianbergmann/comparator#65.
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.
Explicitly reading the streams and compare the resulting strings would work here? Which would also cover un-seekable streams like fopen('https://s3.amazonaws.com/bucket/file.xml')
src/Model/ItemList.php
Outdated
| use Traversable; | ||
| use function count; | ||
|
|
||
| final class ItemList implements Countable, IteratorAggregate |
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.
generic name, could it be Item(s)Page given it contains a pointer to the next element?
src/Model/Items.php
Outdated
| */ | ||
| public function get(ItemId $id, ?ItemVersionNumber $version = null) : ItemVersion; | ||
|
|
||
| public function list(int $limit = 10, ?ItemId $fromId = null) : ItemList; |
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.
$fromId can be interpreted as filtering based on that id, could start or similar pagination-related terminology be used?
| "symfony/dependency-injection": "^3.4 || ^4.0", | ||
| "symfony/framework-bundle": "^3.4 || ^4.0", | ||
| "symfony/http-foundation": "^3.4 || ^4.0", | ||
| "symfony/http-foundation": "^3.4.13 || ^4.0", |
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.
The downside of --prefer-lowest (the problem applies to 4.0.0 and 4.1.0 too).
No description provided.