Skip to content

Commit 0149221

Browse files
authored
Merge pull request #2595 from flow-php/type-collection-casting-bug
fix(flow-php/types): structure/list/map casting no longer fabricates missing data
2 parents 45626bb + bb10992 commit 0149221

19 files changed

Lines changed: 478 additions & 30 deletions

File tree

documentation/components/libs/types.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,15 @@ $variable = $input->get('some-input');
9696
$string = type_string()->cast($variable);
9797
```
9898

99+
Casting structures, lists and maps:
100+
101+
- a structure element that is absent, or present with `null`, throws `CastingException` when the element's type does
102+
not accept `null` - `getPrevious()` returns a `MissingElementCastingException` whose `element` property names the
103+
failing element
104+
- elements whose type accepts `null` (`type_optional(...)`, `type_union(..., type_null())`) cast `null` to `null`;
105+
an absent optional element stays absent from the output
106+
- `type_list(...)->cast(null)` and `type_map(...)->cast(null)` throw `CastingException`
107+
- a JSON string payload is decoded and cast element-wise, exactly like an array payload
99108

100109
### Complex Types
101110

documentation/upgrading.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ Columns that may only become null in later batches, declare the schema explicitl
4545
| `detectType([[1.2], [4.0, 5]])``list<list<float>>` | `list<array<mixed>>` |
4646
| `detectType([[], [1, 2]])``list<array<mixed>>` | `list<list<integer>>` |
4747

48+
### 5) `flow-php/types` - structure/list/map casting no longer fabricates missing data
49+
50+
| Before | After |
51+
|----------------------------------------------------------------------------------------------------------------------------------------|---------------------------|
52+
| `type_structure(['id' => type_integer(), 'name' => type_string()])->cast(['id' => 1])``['id' => 1, 'name' => '']` | throws `CastingException` |
53+
| same type, `->cast(['id' => 1, 'name' => null])``['id' => 1, 'name' => '']` | throws `CastingException` |
54+
| same type, `->cast([])`, `->cast(null)``['id' => 0, 'name' => '']` | throws `CastingException` |
55+
| `type_structure(['id' => type_integer()], ['name' => type_string()])->cast(['id' => 1, 'name' => null])``['id' => 1, 'name' => '']` | throws `CastingException` |
56+
| `type_list(type_string())->cast(null)``['']` | throws `CastingException` |
57+
| `type_structure(['id' => type_integer()])->cast('{"id":"1"}')` → throws | `['id' => 1]` |
58+
| `type_list(type_integer())->cast('["1","2"]')` → throws | `[1, 2]` |
59+
4860
---
4961

5062
## Upgrading from 0.42.x to 0.43.x
@@ -2458,7 +2470,7 @@ After:
24582470
->run();
24592471
```
24602472

2461-
### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _ (underscore)
2473+
### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _ (underscore)
24622474

24632475
In order to avoid collisions with datasets columns, additional columns created after using putInputIntoRows ()
24642476
would now be prefixed with `_` (underscore) symbol.

src/adapter/etl-adapter-http/tests/Flow/ETL/Adapter/HTTP/Tests/Integration/PsrHttpClientPaginatedExtractorTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Flow\ETL\Exception\RuntimeException;
99
use Flow\ETL\Row\Entry\StructureEntry;
1010
use Flow\ETL\Tests\FlowTestCase;
11+
use Flow\Types\Exception\CastingException;
1112
use Http\Mock\Client;
1213
use Nyholm\Psr7\Factory\Psr17Factory;
1314
use Nyholm\Psr7\Response;
@@ -341,6 +342,24 @@ public function test_schema_typed_response_body(): void
341342
static::assertInstanceOf(StructureEntry::class, $rows[0]->first()->get('response_body'));
342343
}
343344

345+
public function test_schema_typed_response_body_with_missing_field(): void
346+
{
347+
$client = new Client(new Psr17Factory());
348+
$client->addResponse(PaginationMother::jsonResponse(['login' => 'flow-php']));
349+
350+
$this->expectException(CastingException::class);
351+
352+
iterator_to_array(from_http_paginated(
353+
$client,
354+
PaginationMother::request('GET', 'https://api.example.com/orgs/flow-php'),
355+
http_pagination_cursor('next', http_request_option_query('cursor')),
356+
schema(structure_schema('response_body', type_structure([
357+
'login' => type_string(),
358+
'id' => type_integer(),
359+
]))),
360+
)->extract(flow_context(config())));
361+
}
362+
344363
public function test_schema_typed_response_body_via_with_schema(): void
345364
{
346365
$client = new Client(new Psr17Factory());

src/adapter/etl-adapter-http/tests/Flow/ETL/Adapter/HTTP/Tests/Integration/PsrHttpClientStaticExtractorTest.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Flow\ETL\Row\Entry\StructureEntry;
88
use Flow\ETL\Rows;
99
use Flow\ETL\Tests\FlowTestCase;
10+
use Flow\Types\Exception\CastingException;
1011
use Http\Mock\Client;
1112
use Nyholm\Psr7\Factory\Psr17Factory;
1213
use Nyholm\Psr7\Response;
@@ -93,6 +94,48 @@ public function test_http_extractor(): void
9394
static::assertSame('tomaszhanc', $tomekResponseBody['login']);
9495
}
9596

97+
public function test_schema_typed_response_body_with_empty_body(): void
98+
{
99+
$factory = new Psr17Factory();
100+
$client = new Client($factory);
101+
$client->addResponse(new Response(200, ['Content-Type' => 'application/json'], '{}'));
102+
103+
$this->expectException(CastingException::class);
104+
105+
from_static_http_requests(
106+
$client,
107+
[$factory->createRequest('GET', 'https://api.github.com/users/norberttech')],
108+
schema(structure_schema('response_body', type_structure([
109+
'login' => type_string(),
110+
'id' => type_integer(),
111+
]))),
112+
)
113+
->extract(flow_context(config()))
114+
->current();
115+
}
116+
117+
public function test_schema_typed_response_body_with_missing_field(): void
118+
{
119+
$factory = new Psr17Factory();
120+
$client = new Client($factory);
121+
$client->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode([
122+
'login' => 'norberttech',
123+
], JSON_THROW_ON_ERROR)));
124+
125+
$this->expectException(CastingException::class);
126+
127+
from_static_http_requests(
128+
$client,
129+
[$factory->createRequest('GET', 'https://api.github.com/users/norberttech')],
130+
schema(structure_schema('response_body', type_structure([
131+
'login' => type_string(),
132+
'id' => type_integer(),
133+
]))),
134+
)
135+
->extract(flow_context(config()))
136+
->current();
137+
}
138+
96139
public function test_schema_typed_response_body(): void
97140
{
98141
$factory = new Psr17Factory();

src/core/etl/tests/Flow/ETL/Tests/Unit/Row/AdaptiveRowHydratorTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Flow\ETL\Row\AdaptiveRowHydrator;
88
use Flow\ETL\Row\RawRowValues;
99
use Flow\ETL\Tests\FlowTestCase;
10+
use Flow\Types\Exception\CastingException;
1011

1112
use function Flow\ETL\DSL\int_entry;
1213
use function Flow\ETL\DSL\int_schema;
@@ -15,6 +16,10 @@
1516
use function Flow\ETL\DSL\schema;
1617
use function Flow\ETL\DSL\str_entry;
1718
use function Flow\ETL\DSL\str_schema;
19+
use function Flow\ETL\DSL\structure_schema;
20+
use function Flow\Types\DSL\type_integer;
21+
use function Flow\Types\DSL\type_string;
22+
use function Flow\Types\DSL\type_structure;
1823

1924
final class AdaptiveRowHydratorTest extends FlowTestCase
2025
{
@@ -45,4 +50,13 @@ public function test_cast_infers_rows_without_a_schema(): void
4550
static::assertSame(1, $rows->first()->valueOf('id'));
4651
static::assertSame('x', $rows->first()->valueOf('name'));
4752
}
53+
54+
public function test_cast_throws_on_missing_required_structure_element(): void
55+
{
56+
$this->expectException(CastingException::class);
57+
58+
(new AdaptiveRowHydrator())->cast([new RawRowValues(['data' => [
59+
'id' => 1,
60+
]])], schema(structure_schema('data', type_structure(['id' => type_integer(), 'name' => type_string()]))));
61+
}
4862
}

src/core/etl/tests/Flow/ETL/Tests/Unit/Row/NativeRowHydratorTest.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ public static function castable_datasets(): Generator
319319
];
320320

321321
yield 'empty cast batch' => [schema(int_schema('id')), []];
322+
323+
yield 'all-optional structure with no matching keys' => [
324+
schema(structure_schema('st', type_structure([], ['b' => type_string()]))),
325+
[new RawRowValues(['st' => ['other' => 1]])],
326+
];
322327
}
323328

324329
/**
@@ -394,9 +399,19 @@ public static function throwing_cast_datasets(): Generator
394399
[new RawRowValues(['l' => [-3]])],
395400
];
396401

397-
yield 'all-optional structure with no matching keys' => [
398-
schema(structure_schema('st', type_structure([], ['b' => type_string()]))),
399-
[new RawRowValues(['st' => ['other' => 1]])],
402+
yield 'structure missing required element' => [
403+
schema(structure_schema('data', type_structure(['id' => type_integer(), 'name' => type_string()]))),
404+
[new RawRowValues(['data' => ['id' => 1]])],
405+
];
406+
407+
yield 'structure present-null required element' => [
408+
schema(structure_schema('data', type_structure(['id' => type_integer(), 'name' => type_string()]))),
409+
[new RawRowValues(['data' => ['id' => 1, 'name' => null]])],
410+
];
411+
412+
yield 'structure present-null optional element' => [
413+
schema(structure_schema('data', type_structure(['id' => type_integer()], ['name' => type_string()]))),
414+
[new RawRowValues(['data' => ['id' => 1, 'name' => null]])],
400415
];
401416
}
402417

src/core/etl/tests/Flow/ETL/Tests/Unit/Row/PhpRowHydratorTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Flow\ETL\Row\RawRowValues;
1717
use Flow\ETL\Row\TypedRowValues;
1818
use Flow\ETL\Tests\FlowTestCase;
19+
use Flow\Types\Exception\CastingException;
1920
use Flow\Types\Value\Uuid;
2021

2122
use function Flow\ETL\DSL\bool_schema;
@@ -51,6 +52,15 @@ public function test_absent_schema_column_is_filled_with_typed_null(): void
5152
static::assertSame(['id' => 1, 'name' => null], $rows->first()->toArray());
5253
}
5354

55+
public function test_cast_throws_on_missing_required_structure_element(): void
56+
{
57+
$this->expectException(CastingException::class);
58+
59+
(new PhpRowHydrator())->cast([new RawRowValues(['data' => [
60+
'id' => 1,
61+
]])], schema(structure_schema('data', type_structure(['id' => type_integer(), 'name' => type_string()]))));
62+
}
63+
5464
public function test_casts_datetime_and_uuid_strings(): void
5565
{
5666
$rows = (new PhpRowHydrator())->cast(

src/extension/flow-php-ext/src/cast.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,13 +540,19 @@ fn cast_value(kind: &CastKind, value: &Zval, ctx: &mut Ctx) -> Result<Option<Zva
540540
for element in elements {
541541
let Some(item) = values.get(element.name.as_str()) else {
542542
if element.required {
543-
// PHP feeds cast(null) to absent required elements
543+
// PHP throws MissingElementCastingException for absent required elements
544544
return Ok(None);
545545
}
546546

547547
continue;
548548
};
549549

550+
if item.is_null() && !matches!(element.kind, CastKind::Optional(_)) {
551+
// PHP throws MissingElementCastingException for present-null elements
552+
// whose type rejects null - required and structure-level optional alike
553+
return Ok(None);
554+
}
555+
550556
let Some(casted) = cast_value(&element.kind, item, ctx)? else {
551557
return Ok(None);
552558
};

src/extension/flow-php-ext/tests/phpt/026_row_hydrator_cast_parity.phpt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ $datasets = [
9696
new RawRowValues(['id' => null], ['id' => Metadata::fromArray(['k' => 'v2'])]),
9797
],
9898
],
99+
'all_optional_st' => [
100+
schema(structure_schema('st', type_structure([], ['b' => type_string()]))),
101+
[new RawRowValues(['st' => ['other' => 1]])],
102+
],
99103
'empty' => [schema(int_schema('id')), []],
100104
];
101105

@@ -135,6 +139,7 @@ uuid_json cast:yes
135139
containers cast:yes
136140
exotic_fallback cast:yes
137141
fill_and_metadata cast:yes
142+
all_optional_st cast:yes
138143
empty cast:yes
139144
schema_mutation cast:yes
140145
null_schema cast:yes

src/extension/flow-php-ext/tests/phpt/027_cast_fallback_exceptions.phpt

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use Flow\ETL\Row\NativeRowHydrator;
1010
use Flow\ETL\Row\PhpRowHydrator;
1111
use Flow\ETL\Row\RawRowValues;
1212

13-
use function Flow\ETL\DSL\{schema, datetime_schema, date_schema, uuid_schema, json_schema, list_schema, map_schema, structure_schema};
14-
use function Flow\Types\DSL\{type_list, type_map, type_structure, type_integer, type_string, type_positive_integer};
13+
use function Flow\ETL\DSL\{schema, datetime_schema, date_schema, uuid_schema, json_schema, list_schema, map_schema};
14+
use function Flow\Types\DSL\{type_list, type_map, type_integer, type_string, type_positive_integer};
1515

1616
$throwing = [
1717
'uuid invalid' => [schema(uuid_schema('u')), [new RawRowValues(['u' => 'not-a-uuid'])]],
@@ -38,10 +38,6 @@ $throwing = [
3838
schema(list_schema('l', type_list(type_positive_integer()))),
3939
[new RawRowValues(['l' => [-3]])],
4040
],
41-
'all-optional structure' => [
42-
schema(structure_schema('st', type_structure([], ['b' => type_string()]))),
43-
[new RawRowValues(['st' => ['other' => 1]])],
44-
],
4541
];
4642

4743
$php = new PhpRowHydrator();
@@ -86,4 +82,3 @@ string map int keys exception:match aborted:yes
8682
list bad keys exception:match aborted:yes
8783
positive int list string exception:match aborted:yes
8884
positive int list negative exception:match aborted:yes
89-
all-optional structure exception:match aborted:yes

0 commit comments

Comments
 (0)