Skip to content

Commit 9d436ca

Browse files
committed
Changed (Async)PorterRecords semantics to always run generators to first suspension point.
This was always a side-effect of the (previous, coroutine) async behaviour, but since fibers, is no longer a necessary side-effect. Yet, some libraries may depend on this behaviour, such as those expecting to resolve a deferred before the first suspension point. We now adopt these semantics intentionally and replicate them to the sync flow.
1 parent b168eaf commit 9d436ca

File tree

9 files changed

+52
-23
lines changed

9 files changed

+52
-23
lines changed

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
},
2222
"require-dev": {
2323
"infection/infection": ">=0.26,<0.27",
24-
"mockery/mockery": "^1.4.2",
24+
"mockery/mockery": "^1.5",
2525
"phpunit/phpunit": "^9.5.23"
2626
},
2727
"suggest" : {

src/Collection/AsyncPorterRecords.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@
77

88
class AsyncPorterRecords extends AsyncRecordCollection
99
{
10-
private AsyncImportSpecification $specification;
11-
12-
public function __construct(AsyncRecordCollection $records, AsyncImportSpecification $specification)
13-
{
10+
public function __construct(
11+
AsyncRecordCollection $records,
12+
private readonly AsyncImportSpecification $specification
13+
) {
1414
parent::__construct($records, $records);
1515

16-
$this->specification = $specification;
16+
// Force generators to run to first suspension point.
17+
$records->valid();
1718
}
1819

1920
public function getSpecification(): AsyncImportSpecification

src/Collection/PorterRecords.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@
77

88
class PorterRecords extends RecordCollection
99
{
10-
private ImportSpecification $specification;
11-
12-
public function __construct(RecordCollection $records, ImportSpecification $specification)
10+
public function __construct(RecordCollection $records, private readonly ImportSpecification $specification)
1311
{
1412
parent::__construct($records, $records);
1513

16-
$this->specification = $specification;
14+
// Force generators to run to first suspension point.
15+
$records->valid();
1716
}
1817

1918
public function getSpecification(): ImportSpecification

test/Integration/Collection/AsyncRecordCollectionTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ final class AsyncRecordCollectionTest extends TestCase
1919
public function testGetPreviousCollection(): void
2020
{
2121
$records = new AsyncPorterRecords(
22-
$previous = \Mockery::mock(AsyncRecordCollection::class),
22+
$previous = \Mockery::spy(AsyncRecordCollection::class),
2323
\Mockery::mock(AsyncImportSpecification::class)
2424
);
2525

@@ -33,7 +33,7 @@ public function testGetPreviousCollection(): void
3333
public function testFindFirstCollection(): void
3434
{
3535
$collection3 = new AsyncFilteredRecords(
36-
$iterator = \Mockery::mock(\Iterator::class),
36+
$iterator = \Mockery::spy(\Iterator::class),
3737
$collection2 = new AsyncPorterRecords(
3838
$collection1 =
3939
new AsyncProviderRecords($iterator, \Mockery::mock(AsyncResource::class)),

test/Integration/PorterAsyncTest.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,23 @@ public function testImportSingle(): void
6363
$this->porter->importAsync($this->singleSpecification);
6464
}
6565

66+
/**
67+
* Tests that when importing records implemented using deferred execution with generators, the generator runs up
68+
* to the first suspension point instead of being paused at the start.
69+
*/
70+
public function testImportGenerator(): void
71+
{
72+
$this->resource->expects('fetchAsync')->andReturnUsing(function () use (&$init): \Generator {
73+
$init = true;
74+
75+
yield [];
76+
});
77+
78+
$this->porter->importAsync($this->specification);
79+
80+
self::assertTrue($init);
81+
}
82+
6683
/**
6784
* Tests that the full async import path, via connector, resource and provider, fetches one record correctly.
6885
*/
@@ -186,7 +203,7 @@ public function testPorterAwareAsyncTransformer(): void
186203
->with($this->porter)
187204
->once()
188205
->shouldReceive('transformAsync')
189-
->andReturn(\Mockery::mock(AsyncRecordCollection::class))
206+
->andReturn(\Mockery::spy(AsyncRecordCollection::class))
190207
->getMock()
191208
)
192209
);

test/Integration/PorterSyncTest.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ public function testImportAndFilterCountableRecords(): void
7373
{
7474
$records = $this->porter->import(
7575
(new StaticDataImportSpecification(
76-
new \ArrayIterator(range(1, 10))
77-
))->addTransformer(new FilterTransformer((__METHOD__)(...)))
76+
new \ArrayIterator(array_map(fn ($i) => [$i], range(1, 10)))
77+
))->addTransformer(new FilterTransformer(fn () => true))
7878
);
7979

8080
// Innermost collection.
@@ -102,6 +102,23 @@ public function testRewind(): void
102102
self::assertSame($i1, $records->current());
103103
}
104104

105+
/**
106+
* Tests that when importing records implemented using deferred execution with generators, the generator runs up
107+
* to the first suspension point instead of being paused at the start.
108+
*/
109+
public function testImportGenerator(): void
110+
{
111+
$this->resource->expects('fetch')->andReturnUsing(function () use (&$init): \Generator {
112+
$init = true;
113+
114+
yield [];
115+
});
116+
117+
$this->porter->import($this->specification);
118+
119+
self::assertTrue($init);
120+
}
121+
105122
/**
106123
* Tests that when a Transformer is PorterAware it receives the Porter instance that invoked it.
107124
*/
@@ -114,7 +131,7 @@ public function testPorterAwareTransformer(): void
114131
->with($this->porter)
115132
->once()
116133
->shouldReceive('transform')
117-
->andReturn(\Mockery::mock(RecordCollection::class))
134+
->andReturn(\Mockery::spy(RecordCollection::class))
118135
->getMock()
119136
)
120137
);

test/Unit/Collection/AsyncPorterRecordsTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ final class AsyncPorterRecordsTest extends TestCase
1919
public function testGetSpecification(): void
2020
{
2121
$records = new AsyncPorterRecords(
22-
\Mockery::mock(AsyncRecordCollection::class),
22+
\Mockery::spy(AsyncRecordCollection::class),
2323
$specification = \Mockery::mock(AsyncImportSpecification::class)
2424
);
2525

test/Unit/Collection/PorterRecordsTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ final class PorterRecordsTest extends TestCase
1919
public function testGetSpecification(): void
2020
{
2121
$records = new PorterRecords(
22-
\Mockery::mock(RecordCollection::class),
22+
\Mockery::spy(RecordCollection::class),
2323
$specification = \Mockery::mock(ImportSpecification::class)
2424
);
2525

test/Unit/Collection/RecordCollectionTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ final class RecordCollectionTest extends TestCase
2020
*/
2121
public function testFindFirstCollection(): void
2222
{
23-
/**
24-
* @var RecordCollection $collection1
25-
* @var RecordCollection $collection2
26-
* @var RecordCollection $collection3
27-
*/
2823
$collection3 = \Mockery::mock(
2924
RecordCollection::class,
3025
[

0 commit comments

Comments
 (0)