diff --git a/documentation/adrs.md b/documentation/adrs.md index 9a95eb2758..7828c155bf 100644 --- a/documentation/adrs.md +++ b/documentation/adrs.md @@ -38,6 +38,7 @@ consistency in decision-making across the project. - [2025-01-07: Static Analysis Baseline](/documentation/adrs/static-analysis-baseline.md) - [2025-01-09: Extension Points](/documentation/adrs/extension-points.md) - [2026-03-02: Variadic Arguments Pattern for Required Parameters](/documentation/adrs/variadic-arguments-pattern.md) +- [2026-07-27: Schema Immutability](/documentation/adrs/schema-immutability.md) ### [Proposed AD](https://github.com/flow-php/flow/pulls?q=is%3Apr+is%3Aopen+label%3AAD+) diff --git a/documentation/adrs/schema-immutability.md b/documentation/adrs/schema-immutability.md new file mode 100644 index 0000000000..be0414144a --- /dev/null +++ b/documentation/adrs/schema-immutability.md @@ -0,0 +1,116 @@ +# Schema Immutability + +[TOC] + +Proposed by: @norberttech +Date: 2026-07-27 + +## Context +--- + +`Schema` was mutable — `add()`, `remove()`, `rename()`, `merge()` and every other mutator rewrote +`$this->definitions` and returned `$this`. `Definition::addMetadata()` and `Definition::setMetadata()` did the same +with `$this->metadata`. + +File extractors extend the schema they were given so the hydrator can cast auto-added columns +(`_input_file_uri`, partition columns, sheet metadata): `Hydrator::cast()` iterates schema definitions and drops +undeclared row keys, so a column that should materialize in rows must be present in the schema. An extractor +storing the caller's `Schema` therefore wrote every internal extension into the caller's object, producing three +observable defects (#2536 regression): + +1. **Caller schema pollution** — a `Schema` the user holds for other purposes gains non-nullable columns it never + declared. +2. **Extractor-lifetime pollution** — columns added during one `extract()` run persist into subsequent runs. +3. **Cross-stream pollution** — partition columns of one stream leak into the next, and + `Hydrator::cast(fillMissing: true)` injects `null` into a non-nullable definition. + +The first fix cloned: `withSchema()` stored `clone $schema`, and extractors cloned again per run and per stream. +That fix was incomplete. `clone` is shallow and `Schema`'s only state is `array`, so a cloned +`Schema` **shares its `Definition` instances**. `Schema::addMetadata()` / `setMetadata()` reached into a shared +`Definition` and mutated it in place, so metadata writes aliased through every copy — including the caller's. + +The same mutable contract left latent aliasing traps elsewhere: `Rows::schema()` seeded its merge loop with row 0's +*memoized* `Schema` and corrupted it, `FloeStreamWriter` retained a caller-owned `Schema` for the lifetime of a +write session, and `merge()`'s fast paths returned `$this` or the argument. + +## Decision +--- + +**`Schema` and its whole state chain are immutable. Every mutator returns a new instance; nothing is ever written +in place.** + +- `Schema` — a `final readonly class`. All 19 mutators return `new self(...)`; `setDefinitions()` is the + constructor's validation helper and is called from the constructor only. +- `Definition` (19 implementations) — each a `final readonly class`. `addMetadata()` and `setMetadata()` return a + per-class `new self(...)`, matching the idiom `makeNullable()` and `rename()` already used. +- `Metadata` — already a `final readonly class`. + +Immutability is declared at the class level, not per property: a `readonly class` cannot gain a writable property +later, so the guarantee survives future edits instead of depending on whoever adds property number 20 remembering +the rule. It is compiler-enforced, not convention. Extractors and the DSL hold caller-provided `Schema` instances +directly: there is nothing to clone because there is nothing to mutate. Sharing an instance — `merge()`'s fast +paths, a retained base `Definition` in the hydrator, `Rows`' memoized schema — is safe by construction. + +DSL `from_*()` functions stay pure delegation. + +### Out of scope + +`EntryReference` remains mutable — `as()`, `asc()` and `desc()` write `$alias` / `$sort` on `$this`. It is shared +with the entire expression DSL, so making it immutable is a separate project and is not attempted here. + +### Breaking change + +Calling a mutator and discarding the result is now a **silent no-op**. There is no `#[\NoDiscard]`; the change is +communicated through [upgrading.md](/documentation/upgrading.md). + +```php +$schema->add(str_schema('x')); // before: mutates $schema. now: no-op. +$schema = $schema->add(str_schema('x')); // correct +``` + +## Pros & Cons +--- + +**Advantages:** + +- **The bug class is gone**, not patched — aliasing is impossible because there is no writable state to alias. +- **Compiler-enforced**: a `readonly` violation is a fatal error, not a convention a new extractor can forget. +- **Sharing becomes free**: no defensive clones in extractors, `PhpRowHydrator`, or the native hydrator. +- Fixes the `Rows::schema()` and `FloeStreamWriter` aliasing traps without touching either. + +**Disadvantages:** + +- Breaking change for downstream code that discards a mutator's return value, and it breaks **silently**. +- A long mutator chain allocates one `Schema` per link. Schemas are small and built once per pipeline, not per + row, so this does not show up in profiles. + +## Alternatives Considered +--- + +### 1. Clone at reception, clone before extending + +Every `withSchema()` stores `clone $schema`; extractors clone again per run and per stream. + +**Rejected because:** the clone is shallow, so `Definition` instances stay shared and metadata mutations alias +through every copy anyway. It is also convention rather than a compiler-enforced rule — a new extractor can forget +to clone — and it leaves unobservable dead clones in extractors that never extend the schema. + +### 2. Deep `Schema::__clone()` + `Definition::__clone()` + +Give `Schema` and every `Definition` a `__clone()` that copies the definition array and its objects. + +**Rejected because:** it patches the symptom while keeping the mutable contract, so every future aliasing trap +(`Rows::schema()`, `FloeStreamWriter`, `merge()`'s fast paths) still has to be found and cloned around by hand. It +also makes every clone more expensive without removing the need to remember to clone. + +### 3. Materialize auto-added columns post-hydration via `Row::add()` (Floe style) + +**Rejected because:** the extended schema *is* the hydrator's instruction set — `Hydrator::cast()` drops undeclared +row keys, and the `findDefinition()` guard lets a user-declared partition column keep its user-defined type. +Post-hydration adds would bypass both. + +## Links and References +--- + +- [PR #2536](https://github.com/flow-php/flow/pull/2536) - encoder/hydrator row contract that introduced the + schema extension in extractors diff --git a/documentation/upgrading.md b/documentation/upgrading.md index b2f4fccb90..122ee384a6 100644 --- a/documentation/upgrading.md +++ b/documentation/upgrading.md @@ -478,6 +478,28 @@ Custom aggregators must implement `references()` - return the references the agg | `->groupBy(...)->aggregate(...)->pivot(ref('x'))` | `->groupBy(...)->pivot(ref('x'))->aggregate(...)` | | `DataFrame::pivot()` | removed; `GroupedDataFrame::pivot()` only | +### 32) `flow-php/etl-adapter-csv`, `-excel`, `-json`, +`-xml` - explicit schema no longer projects partition columns away + +| Before | After | +|------------------------------------------------------------------|------------------------------------------------------| +| partition columns undeclared in the schema are dropped from rows | force-added to rows as non-nullable `string` columns | + +Declare the partition column in the schema to control its type. + +### 33) `flow-php/etl` - `Schema` and `Schema\Definition` mutators return a new instance + +| Before | After | +|-----------------------------------------|------------------------------------------------------| +| `$schema->add(str_schema('x'));` | `$schema = $schema->add(str_schema('x'));` | +| `$schema->addMetadata('id', 'k', 'v');` | `$schema = $schema->addMetadata('id', 'k', 'v');` | +| `$definition->setMetadata($metadata);` | `$definition = $definition->setMetadata($metadata);` | + +Assign the result of every `Schema` mutator - `add`, `addAfter`, `addBefore`, `addMetadata`, `gracefulRemove`, +`insertAt`, `keep`, `makeNullable`, `merge`, `moveAfter`, `moveBefore`, `moveTo`, `prepend`, `remove`, `rename`, +`reorder`, `replace`, `setMetadata`, `sort` - and of `Definition::addMetadata()` / `Definition::setMetadata()`. +Discarding it is a silent no-op. + --- ## Upgrading from 0.40.x to 0.41.x @@ -2233,7 +2255,7 @@ After: ->run(); ``` -### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _ (underscore) +### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _ (underscore) In order to avoid collisions with datasets columns, additional columns created after using putInputIntoRows () would now be prefixed with `_` (underscore) symbol. diff --git a/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/date=2026-01-01/data.csv b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/date=2026-01-01/data.csv new file mode 100644 index 0000000000..9e1228cfbb --- /dev/null +++ b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/date=2026-01-01/data.csv @@ -0,0 +1,2 @@ +id,value +1,a diff --git a/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/nopart/data.csv b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/nopart/data.csv new file mode 100644 index 0000000000..033ede2622 --- /dev/null +++ b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/nopart/data.csv @@ -0,0 +1,2 @@ +id,value +2,b diff --git a/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Integration/CSVExtractorTest.php b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Integration/CSVExtractorTest.php index 0b551db412..6ce11f01ac 100644 --- a/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Integration/CSVExtractorTest.php +++ b/src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Integration/CSVExtractorTest.php @@ -18,8 +18,12 @@ use function Flow\ETL\DSL\config; use function Flow\ETL\DSL\df; use function Flow\ETL\DSL\flow_context; +use function Flow\ETL\DSL\int_schema; use function Flow\ETL\DSL\ref; +use function Flow\ETL\DSL\schema; +use function Flow\ETL\DSL\schema_metadata; use function Flow\ETL\DSL\schema_to_ascii; +use function Flow\ETL\DSL\str_schema; use function Flow\Filesystem\DSL\path_real; use function iterator_to_array; @@ -145,6 +149,33 @@ public function test_bom_removal_utf8(): void ); } + public function test_extract_does_not_mutate_metadata_of_user_provided_schema(): void + { + $schema = schema(int_schema('id', metadata: schema_metadata(['primary_key' => true])), str_schema('value')); + + $before = $schema->normalize(); + + $extractor = from_csv(__DIR__ . '/../Fixtures/cross_stream/*/data.csv', schema: $schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertSame($before, $schema->normalize()); + } + + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $schema = schema(int_schema('id'), str_schema('value')); + + $extractor = from_csv(__DIR__ . '/../Fixtures/cross_stream/*/data.csv', schema: $schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertNull($schema->findDefinition('date')); + static::assertNull($schema->findDefinition('_input_file_uri')); + } + public function test_extracting_csv_empty_columns_as_empty_strings(): void { $extractor = from_csv( @@ -438,6 +469,23 @@ public function test_loading_data_from_all_partitions(): void ); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + static::assertSame( + [ + ['id' => 1, 'value' => 'a', 'date' => '2026-01-01'], + ['id' => 2, 'value' => 'b'], + ], + df() + ->read(from_csv(__DIR__ . '/../Fixtures/cross_stream/*/data.csv', schema: schema( + int_schema('id'), + str_schema('value'), + ))) + ->fetch() + ->toArray(), + ); + } + public function test_signal_stop(): void { $extractor = from_csv(path_real(__DIR__ . '/../Fixtures/orders_flow.csv')); diff --git a/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/date=2026-01-01/data.xlsx b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/date=2026-01-01/data.xlsx new file mode 100644 index 0000000000..658d23787a Binary files /dev/null and b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/date=2026-01-01/data.xlsx differ diff --git a/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/nopart/data.xlsx b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/nopart/data.xlsx new file mode 100644 index 0000000000..4f123c2a01 Binary files /dev/null and b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/nopart/data.xlsx differ diff --git a/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelExtractorTest.php b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelExtractorTest.php index cb08862f31..7c82e87b30 100644 --- a/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelExtractorTest.php +++ b/src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelExtractorTest.php @@ -5,6 +5,7 @@ namespace Flow\ETL\Adapter\Excel\Tests\Integration; use Flow\ETL\Adapter\Excel\ExcelReader; +use Flow\ETL\Config; use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\Extractor\Signal; use Flow\ETL\Rows; @@ -250,6 +251,19 @@ public function test_is_valid_excel_sheet_name_function(): void static::assertTrue($result[4]['is_valid']); } + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $schema = schema(string_schema('group'), int_schema('id'), string_schema('value')); + + $extractor = from_excel(__DIR__ . '/../Fixtures/cross_stream/*/*.xlsx')->withSchema($schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertNull($schema->findDefinition('date')); + static::assertNull($schema->findDefinition('_input_file_uri')); + } + public function test_loading_data_from_all_partitions(): void { df()->read(from_excel(__DIR__ . '/../Fixtures/partitioned/group=*/*.xlsx'))->run(function (Rows $rows): void { @@ -260,6 +274,26 @@ public function test_loading_data_from_all_partitions(): void }); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + static::assertSame( + [ + ['group' => '1', 'id' => 1, 'value' => 'a', 'date' => '2026-01-01'], + ['group' => '1', 'id' => 2, 'value' => 'b', 'date' => '2026-01-01'], + ['group' => '2', 'id' => 5, 'value' => 'e'], + ['group' => '2', 'id' => 6, 'value' => 'f'], + ], + df() + ->read(from_excel(__DIR__ . '/../Fixtures/cross_stream/*/*.xlsx')->withSchema(schema( + string_schema('group'), + int_schema('id'), + string_schema('value'), + ))) + ->fetch() + ->toArray(), + ); + } + public function test_signal_stop(): void { $generator = from_excel(path_real(__DIR__ . '/../Fixtures/fixture.xlsx'))->extract(flow_context(config())); diff --git a/src/adapter/etl-adapter-google-sheet/tests/Flow/ETL/Adapter/GoogleSheet/Tests/Unit/GoogleSheetExtractorTest.php b/src/adapter/etl-adapter-google-sheet/tests/Flow/ETL/Adapter/GoogleSheet/Tests/Unit/GoogleSheetExtractorTest.php index 6287438ffd..1f1f972201 100644 --- a/src/adapter/etl-adapter-google-sheet/tests/Flow/ETL/Adapter/GoogleSheet/Tests/Unit/GoogleSheetExtractorTest.php +++ b/src/adapter/etl-adapter-google-sheet/tests/Flow/ETL/Adapter/GoogleSheet/Tests/Unit/GoogleSheetExtractorTest.php @@ -15,12 +15,66 @@ use function Flow\ETL\Adapter\GoogleSheet\from_google_sheet_columns; use function Flow\ETL\DSL\flow_context; use function Flow\ETL\DSL\row; +use function Flow\ETL\DSL\schema; use function Flow\ETL\DSL\str_entry; +use function Flow\ETL\DSL\str_schema; use function Flow\ETL\DSL\string_entry; use function iterator_to_array; final class GoogleSheetExtractorTest extends FlowTestCase { + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $sheetName = 'sheet'; + + $gridProperties = new Sheets\GridProperties(); + $gridProperties->setRowCount(100); + + $properties = new Sheets\SheetProperties(); + $properties->title = $sheetName; + $properties->setGridProperties($gridProperties); + + $sheet = new Sheets\Sheet(); + $sheet->setProperties($properties); + + $spreadsheet = $this->createMock(Sheets\Spreadsheet::class); + $spreadsheet->expects(self::exactly(2))->method('getSheets')->willReturn([$sheet]); + + $resource = $this->createMock(Sheets\Resource\Spreadsheets::class); + $resource + ->expects(self::exactly(2)) + ->method('get') + ->with('spread-id', ['ranges' => [], 'includeGridData' => false]) + ->willReturn($spreadsheet); + + $service = new Sheets(); + $service->spreadsheets = $resource; + + $valueRange = new ValueRange(); + $valueRange->setValues([['header'], ['row1']]); + + $response = new Sheets\BatchGetValuesResponse(); + $response->setValueRanges([$valueRange]); + + $spreadsheetsValues = $this->createMock(SpreadsheetsValues::class); + $spreadsheetsValues->expects(self::exactly(2))->method('batchGet')->willReturn($response); + + $service->spreadsheets_values = $spreadsheetsValues; + + $schema = schema(str_schema('header')); + + $extractor = from_google_sheet_columns($service, 'spread-id', $sheetName, 'A', 'B') + ->withHeader(true) + ->withRowsPerPage(2) + ->withSchema($schema); + + iterator_to_array($extractor->extract(flow_context((new ConfigBuilder())->putInputIntoRows()->build()))); + iterator_to_array($extractor->extract(flow_context((new ConfigBuilder())->putInputIntoRows()->build()))); + + static::assertNull($schema->findDefinition('_spread_sheet_id')); + static::assertNull($schema->findDefinition('_sheet_name')); + } + public function test_its_fails_if_sheet_not_found(): void { $spreadsheet = $this->createMock(Sheets\Spreadsheet::class); diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.json b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.json new file mode 100644 index 0000000000..e140fc876e --- /dev/null +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.json @@ -0,0 +1 @@ +[{"id":1,"value":"a"}] \ No newline at end of file diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.jsonl b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.jsonl new file mode 100644 index 0000000000..f658685b69 --- /dev/null +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.jsonl @@ -0,0 +1 @@ +{"id":1,"value":"a"} diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.json b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.json new file mode 100644 index 0000000000..3cbc521379 --- /dev/null +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.json @@ -0,0 +1 @@ +[{"id":2,"value":"b"}] \ No newline at end of file diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.jsonl b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.jsonl new file mode 100644 index 0000000000..4488c7e021 --- /dev/null +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.jsonl @@ -0,0 +1 @@ +{"id":2,"value":"b"} diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonExtractorTest.php b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonExtractorTest.php index 13d480068b..52ecc36df4 100644 --- a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonExtractorTest.php +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonExtractorTest.php @@ -15,7 +15,10 @@ use function Flow\ETL\DSL\data_frame; use function Flow\ETL\DSL\df; use function Flow\ETL\DSL\flow_context; +use function Flow\ETL\DSL\int_schema; +use function Flow\ETL\DSL\schema; use function Flow\ETL\DSL\schema_to_ascii; +use function Flow\ETL\DSL\str_schema; use function Flow\Filesystem\DSL\path; use function Flow\Filesystem\DSL\path_real; use function Flow\Types\DSL\type_array; @@ -23,6 +26,19 @@ final class JsonExtractorTest extends FlowTestCase { + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $schema = schema(int_schema('id'), str_schema('value')); + + $extractor = from_json(__DIR__ . '/../../Fixtures/cross_stream/*/data.json', schema: $schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertNull($schema->findDefinition('date')); + static::assertNull($schema->findDefinition('_input_file_uri')); + } + public function test_extracting_json_from_local_file_stream(): void { $rows = data_frame(Config::builder()->putInputIntoRows()) @@ -131,6 +147,23 @@ public function test_extracting_json_from_local_file_string_uri(): void static::assertSame(247, $total); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + static::assertSame( + [ + ['id' => 1, 'value' => 'a', 'date' => '2026-01-01'], + ['id' => 2, 'value' => 'b'], + ], + df() + ->read(from_json(__DIR__ . '/../../Fixtures/cross_stream/*/data.json', schema: schema( + int_schema('id'), + str_schema('value'), + ))) + ->fetch() + ->toArray(), + ); + } + public function test_limit(): void { $extractor = from_json(path(__DIR__ . '/../../Fixtures/timezones.json')); diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonLinesExtractorTest.php b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonLinesExtractorTest.php index 3e348e43a9..34fde0f185 100644 --- a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonLinesExtractorTest.php +++ b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JSONMachine/JsonLinesExtractorTest.php @@ -15,7 +15,10 @@ use function Flow\ETL\DSL\data_frame; use function Flow\ETL\DSL\df; use function Flow\ETL\DSL\flow_context; +use function Flow\ETL\DSL\int_schema; +use function Flow\ETL\DSL\schema; use function Flow\ETL\DSL\schema_to_ascii; +use function Flow\ETL\DSL\str_schema; use function Flow\Filesystem\DSL\path; use function Flow\Filesystem\DSL\path_real; use function Flow\Types\DSL\type_array; @@ -46,6 +49,19 @@ public function test_broken(): void static::assertSame(247, $rows->count()); } + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $schema = schema(int_schema('id'), str_schema('value')); + + $extractor = from_json_lines(__DIR__ . '/../../Fixtures/cross_stream/*/data.jsonl')->withSchema($schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertNull($schema->findDefinition('date')); + static::assertNull($schema->findDefinition('_input_file_uri')); + } + public function test_extracting_jsonl_from_local_file_stream_using_pointer(): void { $rows = data_frame() @@ -128,6 +144,23 @@ public function test_extracting_jsonl_from_local_file_string_uri(): void static::assertSame(247, $total); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + static::assertSame( + [ + ['id' => 1, 'value' => 'a', 'date' => '2026-01-01'], + ['id' => 2, 'value' => 'b'], + ], + df() + ->read(from_json_lines(__DIR__ . '/../../Fixtures/cross_stream/*/data.jsonl')->withSchema(schema( + int_schema('id'), + str_schema('value'), + ))) + ->fetch() + ->toArray(), + ); + } + public function test_limit(): void { $extractor = from_json_lines(path(__DIR__ . '/../../Fixtures/timezones.jsonl')); diff --git a/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/date=2026-01-01/data.txt b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/date=2026-01-01/data.txt new file mode 100644 index 0000000000..d7a060d766 --- /dev/null +++ b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/date=2026-01-01/data.txt @@ -0,0 +1 @@ +line a diff --git a/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/nopart/data.txt b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/nopart/data.txt new file mode 100644 index 0000000000..7675ab6390 --- /dev/null +++ b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/nopart/data.txt @@ -0,0 +1 @@ +line b diff --git a/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Integration/TextExtractorTest.php b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Integration/TextExtractorTest.php index 3789cde8e7..2ca34ef759 100644 --- a/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Integration/TextExtractorTest.php +++ b/src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Integration/TextExtractorTest.php @@ -38,6 +38,20 @@ public function test_limit(): void static::assertCount(2, iterator_to_array($extractor->extract(flow_context(config())))); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + static::assertSame( + [ + ['text' => 'line a', 'date' => '2026-01-01'], + ['text' => 'line b'], + ], + data_frame() + ->read(from_text(__DIR__ . '/../Fixtures/cross_stream/*/data.txt')) + ->fetch() + ->toArray(), + ); + } + public function test_signal_stop(): void { $extractor = from_text(path_real(__DIR__ . '/../Fixtures/orders_flow.csv')); diff --git a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/date=2026-01-01/file.xml b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/date=2026-01-01/file.xml new file mode 100644 index 0000000000..869bb13642 --- /dev/null +++ b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/date=2026-01-01/file.xml @@ -0,0 +1,3 @@ + + 1 + diff --git a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/nopart/file.xml b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/nopart/file.xml new file mode 100644 index 0000000000..dafd1c3785 --- /dev/null +++ b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/nopart/file.xml @@ -0,0 +1,3 @@ + + 2 + diff --git a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLParserExtractorTest.php b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLParserExtractorTest.php index db0e8d8f09..6844153d5c 100644 --- a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLParserExtractorTest.php +++ b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLParserExtractorTest.php @@ -4,9 +4,11 @@ namespace Flow\ETL\Adapter\XML\Tests\Integration; +use Flow\ETL\Config; use Flow\ETL\Extractor\Signal; use Flow\ETL\Tests\FlowIntegrationTestCase; +use function array_keys; use function Flow\ETL\Adapter\XML\from_xml; use function Flow\ETL\DSL\config; use function Flow\ETL\DSL\df; @@ -19,6 +21,19 @@ final class XMLParserExtractorTest extends FlowIntegrationTestCase { + public function test_extract_does_not_mutate_user_provided_schema(): void + { + $schema = schema(xml_schema('node')); + + $extractor = from_xml(__DIR__ . '/../Fixtures/cross_stream/*/file.xml', 'root/item')->withSchema($schema); + + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + df(Config::builder()->putInputIntoRows())->read($extractor)->run(); + + static::assertNull($schema->findDefinition('date')); + static::assertNull($schema->findDefinition('_input_file_uri')); + } + public function test_limit(): void { $extractor = from_xml(path_real(__DIR__ . '/../Fixtures/flow_orders.xml'))->withXMLNodePath('root/row'); @@ -29,6 +44,18 @@ public function test_limit(): void static::assertCount(2, $rows); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + $rows = df() + ->read(from_xml(__DIR__ . '/../Fixtures/cross_stream/*/file.xml', 'root/item')) + ->fetch() + ->toArray(); + + static::assertSame(['node', 'date'], array_keys($rows[0])); + static::assertSame('2026-01-01', $rows[0]['date']); + static::assertSame(['node'], array_keys($rows[1])); + } + public function test_reading_deep_xml(): void { static::assertSame( diff --git a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLReaderExtractorTest.php b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLReaderExtractorTest.php index b384ac3d09..9f9893909a 100644 --- a/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLReaderExtractorTest.php +++ b/src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Integration/XMLReaderExtractorTest.php @@ -9,6 +9,7 @@ use Flow\ETL\Extractor\Signal; use Flow\ETL\Tests\FlowIntegrationTestCase; +use function array_keys; use function Flow\ETL\DSL\config; use function Flow\ETL\DSL\data_frame; use function Flow\ETL\DSL\flow_context; @@ -28,6 +29,21 @@ public function test_limit(): void static::assertCount(2, iterator_to_array($extractor->extract(flow_context(config())))); } + public function test_partition_columns_are_not_leaking_between_streams(): void + { + $rows = data_frame() + ->read( + // @mago-ignore analysis:deprecated-class + new XMLReaderExtractor(path(__DIR__ . '/../Fixtures/cross_stream/*/file.xml'), 'root/item'), + ) + ->fetch() + ->toArray(); + + static::assertSame(['node', 'date'], array_keys($rows[0])); + static::assertSame('2026-01-01', $rows[0]['date']); + static::assertSame(['node'], array_keys($rows[1])); + } + public function test_reading_deep_xml(): void { static::assertEquals( diff --git a/src/cli/src/Flow/CLI/Command/DatabaseTableSchemaCommand.php b/src/cli/src/Flow/CLI/Command/DatabaseTableSchemaCommand.php index d55f273dc5..653f8efd08 100644 --- a/src/cli/src/Flow/CLI/Command/DatabaseTableSchemaCommand.php +++ b/src/cli/src/Flow/CLI/Command/DatabaseTableSchemaCommand.php @@ -110,7 +110,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } - $schema->keep(...$columns); + $schema = $schema->keep(...$columns); } if (option_bool('output-ascii', $input)) { diff --git a/src/core/etl/src/Flow/ETL/Row/PhpRowHydrator.php b/src/core/etl/src/Flow/ETL/Row/PhpRowHydrator.php index d476f4ca6c..cccd2523e6 100644 --- a/src/core/etl/src/Flow/ETL/Row/PhpRowHydrator.php +++ b/src/core/etl/src/Flow/ETL/Row/PhpRowHydrator.php @@ -103,7 +103,7 @@ private function instantiate(array $batch, Schema $schema, callable $prepare, bo } if (array_key_exists($name, $rowValues->metadata)) { - $definition = (clone $definition)->setMetadata($rowValues->metadata[$name]); + $definition = $definition->setMetadata($rowValues->metadata[$name]); } // @mago-ignore analysis:mixed-assignment diff --git a/src/core/etl/src/Flow/ETL/Schema.php b/src/core/etl/src/Flow/ETL/Schema.php index 8780ec9705..b7b4b2d44d 100644 --- a/src/core/etl/src/Flow/ETL/Schema.php +++ b/src/core/etl/src/Flow/ETL/Schema.php @@ -20,7 +20,6 @@ use function array_key_exists; use function array_keys; use function array_map; -use function array_merge; use function array_search; use function array_splice; use function array_values; @@ -32,7 +31,7 @@ use function sprintf; use function usort; -final class Schema implements Countable +final readonly class Schema implements Countable { /** * @var array> @@ -123,9 +122,7 @@ public static function fromPipeline(Pipeline $pipeline, FlowContext $context, in */ public function add(Definition ...$definitions): self { - $this->setDefinitions(...array_merge(array_values($this->definitions), $definitions)); - - return $this; + return new self(...array_values($this->definitions), ...$definitions); } /** @@ -167,9 +164,7 @@ public function addBefore(string|Reference $reference, Definition ...$definition */ public function addMetadata(string $definition, string $name, int|string|bool|float|array $value): self { - $this->get($definition)->addMetadata($name, $value); - - return $this; + return $this->replace($definition, $this->get($definition)->addMetadata($name, $value)); } public function count(): int @@ -230,9 +225,7 @@ public function gracefulRemove(string|Reference ...$entries): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -259,9 +252,7 @@ public function insertAt(int $index, Definition ...$definitions): self array_splice($definitionsList, $index, 0, $definitions); - $this->setDefinitions(...$definitionsList); - - return $this; + return new self(...$definitionsList); } public function isSame(self $schema): bool @@ -304,9 +295,7 @@ public function keep(string|Reference ...$entries): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -324,9 +313,7 @@ public function makeNullable(): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } public function merge(self $schema): self @@ -359,9 +346,7 @@ public function merge(self $schema): self } } - $this->setDefinitions(...array_values($newDefinitions)); - - return $this; + return new self(...array_values($newDefinitions)); } /** @@ -416,9 +401,7 @@ public function moveTo(string|Reference $name, int $index): self $moved = array_splice($definitionsList, $from, 1); array_splice($definitionsList, $index, 0, $moved); - $this->setDefinitions(...$definitionsList); - - return $this; + return new self(...$definitionsList); } /** @@ -444,9 +427,7 @@ public function normalize(): array */ public function prepend(Definition ...$definitions): self { - $this->setDefinitions(...$definitions, ...array_values($this->definitions)); - - return $this; + return new self(...$definitions, ...array_values($this->definitions)); } public function references(): References @@ -481,9 +462,7 @@ public function remove(string|Reference ...$entries): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -505,9 +484,7 @@ public function rename(string|Reference $entry, string $newName): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -541,9 +518,7 @@ public function reorder(string|Reference ...$names): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -567,9 +542,7 @@ public function replace(string|Reference $entry, Definition $definition): self } } - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } /** @@ -581,9 +554,7 @@ public function replace(string|Reference $entry, Definition $definition): self */ public function setMetadata(string $definition, Metadata $metadata): self { - $this->get($definition)->setMetadata($metadata); - - return $this; + return $this->replace($definition, $this->get($definition)->setMetadata($metadata)); } /** @@ -595,9 +566,7 @@ public function sort(SortingStrategy $strategy = new AlphabeticalStrategy()): se usort($definitions, static fn(Definition $left, Definition $right): int => $strategy->compare($left, $right)); - $this->setDefinitions(...$definitions); - - return $this; + return new self(...$definitions); } private function indexOf(string|Reference $reference): int @@ -640,9 +609,7 @@ private function moveRelative(string|Reference $name, string|Reference $referenc array_splice($definitionsList, $referenceIndex + $offset, 0, $moved); - $this->setDefinitions(...$definitionsList); - - return $this; + return new self(...$definitionsList); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition.php b/src/core/etl/src/Flow/ETL/Schema/Definition.php index bc8646af15..e8ee674e40 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition.php @@ -16,7 +16,7 @@ interface Definition /** * @param array|bool|float|int|string $value * - * @return static + * @return static a new definition, the original is left untouched */ public function addMetadata(string $key, int|string|bool|float|array $value): static; @@ -68,7 +68,7 @@ public function normalize(): array; public function rename(string $newName): static; /** - * @return static + * @return static a new definition, the original is left untouched */ public function setMetadata(Metadata $metadata): static; diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/BooleanDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/BooleanDefinition.php index d4354a15b1..af6271fb15 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/BooleanDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/BooleanDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition */ -final class BooleanDefinition implements Definition +final readonly class BooleanDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -163,9 +161,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/DateDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/DateDefinition.php index 9784f46ab2..705e92dca7 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/DateDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/DateDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition<\DateTimeInterface> */ -final class DateDefinition implements Definition +final readonly class DateDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type<\DateTimeInterface> */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -187,9 +185,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/DateTimeDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/DateTimeDefinition.php index 7e77eb1bcc..c5fbd35df7 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/DateTimeDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/DateTimeDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition<\DateTimeInterface> */ -final class DateTimeDefinition implements Definition +final readonly class DateTimeDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type<\DateTimeInterface> */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -183,9 +181,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/EnumDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/EnumDefinition.php index 2c2058cbf0..a713583c20 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/EnumDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/EnumDefinition.php @@ -24,24 +24,24 @@ * * @implements Definition */ -final class EnumDefinition implements Definition +final readonly class EnumDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var EnumType */ - private readonly EnumType $type; + private EnumType $type; /** * @param class-string $enumClass */ public function __construct( string|Reference $ref, - private readonly string $enumClass, - private readonly bool $nullable = false, + private string $enumClass, + private bool $nullable = false, ?Metadata $metadata = null, ) { if ($enumClass !== UnitEnum::class && !enum_exists($enumClass)) { @@ -60,9 +60,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->enumClass, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -186,9 +184,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->enumClass, $this->nullable, $metadata); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/FloatDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/FloatDefinition.php index 6a7f18cfd4..fc9ddceebc 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/FloatDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/FloatDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition */ -final class FloatDefinition implements Definition +final readonly class FloatDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -179,9 +177,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLDefinition.php index 776778931c..56ba16f8a4 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLDefinition.php @@ -21,20 +21,20 @@ /** * @implements Definition */ -final class HTMLDefinition implements Definition +final readonly class HTMLDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -164,9 +162,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLElementDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLElementDefinition.php index f1d69f2697..50331334f3 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLElementDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/HTMLElementDefinition.php @@ -21,20 +21,20 @@ /** * @implements Definition */ -final class HTMLElementDefinition implements Definition +final readonly class HTMLElementDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -164,9 +162,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/IntegerDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/IntegerDefinition.php index a1ee2be2a0..d0d855c752 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/IntegerDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/IntegerDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition */ -final class IntegerDefinition implements Definition +final readonly class IntegerDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -179,9 +177,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/JsonDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/JsonDefinition.php index 98bb919681..b9089ba2d9 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/JsonDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/JsonDefinition.php @@ -21,20 +21,20 @@ /** * @implements Definition */ -final class JsonDefinition implements Definition +final readonly class JsonDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -164,9 +162,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/ListDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/ListDefinition.php index 6dad210b12..636db169e3 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/ListDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/ListDefinition.php @@ -28,19 +28,19 @@ * * @implements Definition> */ -final class ListDefinition implements Definition +final readonly class ListDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @param ListType $type */ public function __construct( string|Reference $ref, - private readonly ListType $type, - private readonly bool $nullable = false, + private ListType $type, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -52,9 +52,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->type, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -223,9 +221,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->type, $this->nullable, $metadata); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/MapDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/MapDefinition.php index 3faaf9bb0d..939b7a4a6f 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/MapDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/MapDefinition.php @@ -23,19 +23,19 @@ * * @implements Definition> */ -final class MapDefinition implements Definition +final readonly class MapDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @param MapType $type */ public function __construct( string|Reference $ref, - private readonly MapType $type, - private readonly bool $nullable = false, + private MapType $type, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->type, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -205,9 +203,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->type, $this->nullable, $metadata); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/NullDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/NullDefinition.php index 32b01f6dab..586e68acde 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/NullDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/NullDefinition.php @@ -19,16 +19,16 @@ /** * @implements Definition */ -final class NullDefinition implements Definition +final readonly class NullDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct(string|Reference $ref, ?Metadata $metadata = null) { @@ -42,9 +42,7 @@ public function __construct(string|Reference $ref, ?Metadata $metadata = null) */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -131,9 +129,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/StringDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/StringDefinition.php index 613dd57b65..63e8b0952c 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/StringDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/StringDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition */ -final class StringDefinition implements Definition +final readonly class StringDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -151,9 +149,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/StructureDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/StructureDefinition.php index 9d8cb21b65..066aeb8c3a 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/StructureDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/StructureDefinition.php @@ -24,19 +24,19 @@ * * @implements Definition> */ -final class StructureDefinition implements Definition +final readonly class StructureDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @param StructureType $type */ public function __construct( string|Reference $ref, - private readonly StructureType $type, - private readonly bool $nullable = false, + private StructureType $type, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -48,9 +48,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->type, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -221,9 +219,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->type, $this->nullable, $metadata); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/TimeDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/TimeDefinition.php index b1beb9b15a..b3a8a39eab 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/TimeDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/TimeDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition<\DateInterval> */ -final class TimeDefinition implements Definition +final readonly class TimeDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type<\DateInterval> */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -171,9 +169,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/UnionDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/UnionDefinition.php index 02e8d5791a..f32f37ee7a 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/UnionDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/UnionDefinition.php @@ -20,19 +20,19 @@ /** * @implements Definition */ -final class UnionDefinition implements Definition +final readonly class UnionDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @param UnionType $type */ public function __construct( string|Reference $ref, - private readonly UnionType $type, - private readonly bool $nullable = false, + private UnionType $type, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -44,9 +44,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->type, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -162,9 +160,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->type, $this->nullable, $metadata); } /** diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/UuidDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/UuidDefinition.php index c450c291ad..6c3188de5f 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/UuidDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/UuidDefinition.php @@ -21,20 +21,20 @@ /** * @implements Definition */ -final class UuidDefinition implements Definition +final readonly class UuidDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -164,9 +162,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/XMLDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/XMLDefinition.php index 50d0518898..50e301592a 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/XMLDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/XMLDefinition.php @@ -21,20 +21,20 @@ /** * @implements Definition<\DOMDocument|XMLDocument> */ -final class XMLDefinition implements Definition +final readonly class XMLDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type<\DOMDocument|XMLDocument> */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -47,9 +47,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -164,9 +162,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/src/Flow/ETL/Schema/Definition/XMLElementDefinition.php b/src/core/etl/src/Flow/ETL/Schema/Definition/XMLElementDefinition.php index 73c22c55c6..c62943aede 100644 --- a/src/core/etl/src/Flow/ETL/Schema/Definition/XMLElementDefinition.php +++ b/src/core/etl/src/Flow/ETL/Schema/Definition/XMLElementDefinition.php @@ -20,20 +20,20 @@ /** * @implements Definition<\DOMElement|\Dom\Element> */ -final class XMLElementDefinition implements Definition +final readonly class XMLElementDefinition implements Definition { private Metadata $metadata; - private readonly Reference $ref; + private Reference $ref; /** * @var Type<\DOMElement|\Dom\Element> */ - private readonly Type $type; + private Type $type; public function __construct( string|Reference $ref, - private readonly bool $nullable = false, + private bool $nullable = false, ?Metadata $metadata = null, ) { $this->ref = EntryReference::init($ref); @@ -46,9 +46,7 @@ public function __construct( */ public function addMetadata(string $key, int|string|bool|float|array $value): static { - $this->metadata = $this->metadata->add($key, $value); - - return $this; + return new self($this->ref, $this->nullable, $this->metadata->add($key, $value)); } public function entry(): Reference @@ -163,9 +161,7 @@ public function rename(string $newName): static public function setMetadata(Metadata $metadata): static { - $this->metadata = $metadata; - - return $this; + return new self($this->ref, $this->nullable, $metadata); } public function type(): Type diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Row/NativeRowHydratorTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Row/NativeRowHydratorTest.php index 4137ab3ade..eb10e09448 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Row/NativeRowHydratorTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Row/NativeRowHydratorTest.php @@ -477,7 +477,7 @@ public function test_native_cast_without_schema_delegates_to_php_inference(): vo ); } - public function test_native_cast_follows_in_place_schema_mutations(): void + public function test_native_cast_follows_schema_changes(): void { if (!NativeRowHydrator::isSupported()) { static::markTestSkipped('flow_php extension with the native hydrator is not loaded.'); @@ -490,12 +490,12 @@ public function test_native_cast_follows_in_place_schema_mutations(): void $batch = [new RawRowValues(['id' => '1', 'name' => 7])]; static::assertSame(serialize($php->cast($batch, $schema)), serialize($native->cast($batch, $schema))); - $schema->add(bool_schema('active', nullable: true)); + $schema = $schema->add(bool_schema('active', nullable: true)); $batch = [new RawRowValues(['id' => '2', 'name' => 'b', 'active' => 'yes'])]; static::assertSame(serialize($php->cast($batch, $schema)), serialize($native->cast($batch, $schema))); - $schema->makeNullable(); + $schema = $schema->makeNullable(); $batch = [new RawRowValues(['id' => null, 'name' => null, 'active' => null])]; static::assertSame(serialize($php->cast($batch, $schema)), serialize($native->cast($batch, $schema))); @@ -522,7 +522,7 @@ public function test_native_moves_markup_values_verbatim(): void static::assertEquals($phpDehydrated[0]->types['doc'], $nativeDehydrated[0]->types['doc']); } - public function test_native_hydrate_follows_in_place_schema_mutations(): void + public function test_native_hydrate_follows_schema_changes(): void { if (!NativeRowHydrator::isSupported()) { static::markTestSkipped('flow_php extension with the native hydrator is not loaded.'); @@ -535,12 +535,12 @@ public function test_native_hydrate_follows_in_place_schema_mutations(): void $batch = [new RawRowValues(['id' => 1, 'name' => 'a'])]; static::assertSame(serialize($php->hydrate($batch, $schema)), serialize($native->hydrate($batch, $schema))); - $schema->add(bool_schema('active', nullable: true)); + $schema = $schema->add(bool_schema('active', nullable: true)); $batch = [new RawRowValues(['id' => 2, 'name' => 'b', 'active' => true])]; static::assertSame(serialize($php->hydrate($batch, $schema)), serialize($native->hydrate($batch, $schema))); - $schema->makeNullable(); + $schema = $schema->makeNullable(); $batch = [new RawRowValues(['id' => null, 'name' => null, 'active' => null])]; static::assertSame(serialize($php->hydrate($batch, $schema)), serialize($native->hydrate($batch, $schema))); diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/RowsTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/RowsTest.php index f58d6b70c4..7d87c646ea 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/RowsTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/RowsTest.php @@ -850,6 +850,16 @@ public function test_rows_schema(): void ); } + public function test_rows_schema_does_not_mutate_row_schemas(): void + { + $first = row(int_entry('id', 1)); + + $rows = rows($first, row(int_entry('id', 2), str_entry('name', 'foo'))); + + static::assertEquals(schema(integer_schema('id'), string_schema('name', true)), $rows->schema()); + static::assertEquals(schema(integer_schema('id')), $first->schema()); + } + public function test_rows_schema_when_rows_have_different_list_types(): void { $rows = rows( diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/BooleanDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/BooleanDefinitionTest.php index 0609da3a72..bfaf021fcc 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/BooleanDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/BooleanDefinitionTest.php @@ -78,6 +78,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -241,6 +242,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_boolean_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateDefinitionTest.php index 8a0f83c716..472895f38b 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateDefinitionTest.php @@ -96,6 +96,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -273,6 +274,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_date_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateTimeDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateTimeDefinitionTest.php index ea037ca627..cca0491b16 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateTimeDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/DateTimeDefinitionTest.php @@ -95,6 +95,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -272,6 +273,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_datetime_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/EnumDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/EnumDefinitionTest.php index 3c2110db90..7e1bd9784e 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/EnumDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/EnumDefinitionTest.php @@ -81,6 +81,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -260,6 +261,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_throws_exception_for_non_existing_enum_class(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/FloatDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/FloatDefinitionTest.php index 17799a1b02..19dacc2de1 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/FloatDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/FloatDefinitionTest.php @@ -88,6 +88,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -265,6 +266,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_float_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLDefinitionTest.php index cd05be5b12..b3aca0a759 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLDefinitionTest.php @@ -79,6 +79,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } #[RequiresPhp('>= 8.4.0')] @@ -244,6 +245,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_html_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLElementDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLElementDefinitionTest.php index 27cd653bd8..db422e5f95 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLElementDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/HTMLElementDefinitionTest.php @@ -79,6 +79,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } #[RequiresPhp('>= 8.4.0')] @@ -244,6 +245,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_html_element_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/IntegerDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/IntegerDefinitionTest.php index 0c7fba9e62..e82373204e 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/IntegerDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/IntegerDefinitionTest.php @@ -89,6 +89,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -266,6 +267,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_integer_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/JsonDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/JsonDefinitionTest.php index 1bd8ecfb4e..d50484deab 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/JsonDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/JsonDefinitionTest.php @@ -78,6 +78,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -241,6 +242,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_json_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/ListDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/ListDefinitionTest.php index ae20f41224..feb23bc837 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/ListDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/ListDefinitionTest.php @@ -104,6 +104,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -281,6 +282,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_list_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/MapDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/MapDefinitionTest.php index 655202075a..34f65347da 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/MapDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/MapDefinitionTest.php @@ -103,6 +103,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -284,6 +285,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_map_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/NullDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/NullDefinitionTest.php index b01e7d4869..5caebb3a69 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/NullDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/NullDefinitionTest.php @@ -21,10 +21,13 @@ final class NullDefinitionTest extends FlowTestCase { public function test_add_metadata(): void { - $withMeta = null_schema('id')->addMetadata('key', 'value'); + $def = null_schema('id'); + + $withMeta = $def->addMetadata('key', 'value'); static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_entry(): void @@ -171,9 +174,13 @@ public function test_rename(): void public function test_set_metadata(): void { + $def = null_schema('id'); $metadata = Metadata::with('key', 'value'); - static::assertTrue(null_schema('id')->setMetadata($metadata)->metadata()->isEqual($metadata)); + $withMeta = $def->setMetadata($metadata); + + static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_is_null_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StringDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StringDefinitionTest.php index 53c3953722..95def6b7ca 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StringDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StringDefinitionTest.php @@ -77,6 +77,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -231,6 +232,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_string_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StructureDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StructureDefinitionTest.php index 7b1e604836..b43f043b2f 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StructureDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/StructureDefinitionTest.php @@ -103,6 +103,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -312,6 +313,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_structure_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/TimeDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/TimeDefinitionTest.php index 7795cc7af0..edcc202424 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/TimeDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/TimeDefinitionTest.php @@ -96,6 +96,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -273,6 +274,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_time_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UnionDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UnionDefinitionTest.php index 668b6b80ed..1916db8ac0 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UnionDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UnionDefinitionTest.php @@ -96,6 +96,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_definition_from_type_creates_union_definition(): void @@ -303,6 +304,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_union_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UuidDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UuidDefinitionTest.php index b5e0c2919c..e02888d42a 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UuidDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/UuidDefinitionTest.php @@ -78,6 +78,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -241,6 +242,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_uuid_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLDefinitionTest.php index 2edf72aaf1..c9fc14c43b 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLDefinitionTest.php @@ -78,6 +78,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -241,6 +242,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_xml_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLElementDefinitionTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLElementDefinitionTest.php index 83c24b9e63..3e40ce6b17 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLElementDefinitionTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/Definition/XMLElementDefinitionTest.php @@ -78,6 +78,7 @@ public function test_add_metadata(): void static::assertTrue($withMeta->metadata()->has('key')); static::assertSame('value', $withMeta->metadata()->get('key')); + static::assertFalse($def->metadata()->has('key')); } public function test_does_not_match_entry_with_different_name(): void @@ -241,6 +242,7 @@ public function test_set_metadata(): void $withMeta = $def->setMetadata($metadata); static::assertTrue($withMeta->metadata()->isEqual($metadata)); + static::assertTrue($def->metadata()->isEmpty()); } public function test_type_returns_xml_element_type(): void diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/SchemaTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/SchemaTest.php index 065e5d3051..29713a95c8 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/SchemaTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Schema/SchemaTest.php @@ -235,6 +235,31 @@ public static function provide_move_to_reference_inputs(): Generator yield 'Reference name' => [ref('active')]; } + public static function provide_mutators(): Generator + { + yield 'add' => [static fn(Schema $schema) => $schema->add(bool_schema('active'))]; + yield 'addAfter' => [static fn(Schema $schema) => $schema->addAfter('id', bool_schema('active'))]; + yield 'addBefore' => [static fn(Schema $schema) => $schema->addBefore('id', bool_schema('active'))]; + yield 'addMetadata' => [static fn(Schema $schema) => $schema->addMetadata('id', 'primary_key', true)]; + yield 'gracefulRemove' => [static fn(Schema $schema) => $schema->gracefulRemove('name')]; + yield 'insertAt' => [static fn(Schema $schema) => $schema->insertAt(1, bool_schema('active'))]; + yield 'keep' => [static fn(Schema $schema) => $schema->keep('id')]; + yield 'makeNullable' => [static fn(Schema $schema) => $schema->makeNullable()]; + yield 'merge' => [static fn(Schema $schema) => $schema->merge(schema(bool_schema('active')))]; + yield 'moveAfter' => [static fn(Schema $schema) => $schema->moveAfter('id', 'name')]; + yield 'moveBefore' => [static fn(Schema $schema) => $schema->moveBefore('name', 'id')]; + yield 'moveTo' => [static fn(Schema $schema) => $schema->moveTo('name', 0)]; + yield 'prepend' => [static fn(Schema $schema) => $schema->prepend(bool_schema('active'))]; + yield 'remove' => [static fn(Schema $schema) => $schema->remove('name')]; + yield 'rename' => [static fn(Schema $schema) => $schema->rename('name', 'title')]; + yield 'reorder' => [static fn(Schema $schema) => $schema->reorder('name', 'id')]; + yield 'replace' => [static fn(Schema $schema) => $schema->replace('name', str_schema('title'))]; + yield 'setMetadata' => [ + static fn(Schema $schema) => $schema->setMetadata('id', schema_metadata(['primary_key' => true])), + ]; + yield 'sort' => [static fn(Schema $schema) => $schema->sort()]; + } + public static function provide_reorder_reference_inputs(): Generator { yield 'string names' => [['id', 'name', 'email']]; @@ -564,6 +589,19 @@ public function test_move_to_out_of_range(): void schema(int_schema('id'), str_schema('name'))->moveTo('id', 2); } + /** + * @param callable(Schema) : Schema $mutator + */ + #[DataProvider('provide_mutators')] + public function test_mutators_return_new_instance(callable $mutator): void + { + $schema = schema(int_schema('id'), str_schema('name')); + $before = $schema->normalize(); + + static::assertNotSame($schema, $mutator($schema)); + static::assertSame($before, $schema->normalize()); + } + public function test_normalizing_and_recreating_schema(): void { $schema = schema( @@ -582,13 +620,6 @@ public function test_normalizing_and_recreating_schema(): void static::assertEquals($schema, Schema::fromArray($schema->normalize())); } - public function test_positional_mutation_returns_same_instance(): void - { - $schema = schema(int_schema('id'), str_schema('name')); - - static::assertSame($schema, $schema->prepend(bool_schema('active'))); - } - public function test_prepend(): void { $schema = schema( @@ -846,11 +877,4 @@ public function test_sort_empty_schema(): void { static::assertSame([], array_keys(schema()->sort()->definitions())); } - - public function test_sort_returns_same_instance(): void - { - $schema = schema(str_schema('name'), int_schema('id')); - - static::assertSame($schema, $schema->sort()); - } } diff --git a/src/extension/flow-php-ext/src/ctx.rs b/src/extension/flow-php-ext/src/ctx.rs index 445c794730..c9ebd19c7f 100644 --- a/src/extension/flow-php-ext/src/ctx.rs +++ b/src/extension/flow-php-ext/src/ctx.rs @@ -287,24 +287,6 @@ pub fn construct_with_zvals( Ok(obj) } -/// Runs an object's engine `clone` handler, mirroring PHP `clone $object`: a -/// shallow copy that shares the readonly sub-objects (ref/type) by refcount, the -/// exact semantics `(clone $definition)->setMetadata(...)` relies on. -pub fn clone_object(object: &ZendObject) -> Result, PhpException> { - let handler = unsafe { object.handlers.as_ref() } - .and_then(|handlers| handlers.clone_obj) - .ok_or_else(|| ext_exception("flow_php failed to resolve a clone handler"))?; - - let cloned = unsafe { handler(std::ptr::from_ref(object).cast_mut()) }; - ensure_no_pending_exception("clone a definition")?; - - if cloned.is_null() { - return Err(ext_exception("flow_php failed to clone a definition")); - } - - Ok(unsafe { ZBox::from_raw(cloned) }) -} - fn method_handle(class: &str, method: &str) -> Result { Function::try_from_method(class, method).ok_or_else(|| { ext_exception(format!( diff --git a/src/extension/flow-php-ext/src/hydrate.rs b/src/extension/flow-php-ext/src/hydrate.rs index f031821d8c..7327f6201a 100644 --- a/src/extension/flow-php-ext/src/hydrate.rs +++ b/src/extension/flow-php-ext/src/hydrate.rs @@ -11,9 +11,9 @@ use ext_php_rs::types::{ZendHashTable, ZendObject, Zval}; use ext_php_rs::zend::{ClassEntry, Function}; use crate::ctx::{ - array_key_index, call_handle, call_handle_on, ce_method_ref, clone_object, - construct_with_zvals, find_class, ht_add, ht_find_key, ht_insert, ht_insert_key, - property_offset, write_slot, zval_str, Ctx, HtKey, + array_key_index, call_handle, call_handle_on, ce_method_ref, construct_with_zvals, + find_class, ht_add, ht_find_key, ht_insert, ht_insert_key, property_offset, + write_slot, zval_str, Ctx, HtKey, }; use crate::encode::{expect_object, ht_for_each, read_slot}; use crate::exception::ext_exception; @@ -400,13 +400,13 @@ impl HydrateColumn { } } -/// `Schema` is mutated in place (`add()`/`keep()`/`makeNullable()` return -/// `$this`), so object identity cannot key the plan cache. Every structural -/// mutation swaps the `definitions` array (`setDefinitions` builds a fresh one) -/// and copy-on-write separates external writes, so the array's address -/// identifies the definition set; retaining it (refcount++) prevents address -/// reuse. Definition-level `setMetadata` mutates the retained (shared) objects -/// directly and needs no rebuild. +/// `Schema` is immutable - `add()`/`keep()`/`makeNullable()` return a new +/// instance carrying a freshly built `definitions` array - so the array's +/// address identifies the definition set; retaining it (refcount++) prevents +/// address reuse. Keying on the array rather than on `Schema` identity also +/// survives a caller that hands the same definitions to a new `Schema`. +/// Per-value `setMetadata` returns a new `Definition` and leaves the retained +/// ones untouched, so it needs no rebuild. pub struct HydratePlan { pub(crate) definitions_slot: u32, pub(crate) definitions_retained: Zval, @@ -507,9 +507,9 @@ pub(crate) fn build_hydrate_plan( /// Resolves the `(definition, entry class, entry slots)` triple for one column /// occurrence, mirroring `PhpRowHydrator::instantiate` + `EntryFactory::fromDefinition`: -/// the common path shares the retained base `Definition`; per-value metadata clones -/// it (`(clone $def)->setMetadata(...)`), and a null value on a non-nullable -/// definition produces a fresh `makeNullable()` variant. +/// the common path shares the retained base `Definition`; per-value metadata takes +/// the new instance `$def->setMetadata(...)` returns, and a null value on a +/// non-nullable definition produces a fresh `makeNullable()` variant. pub(crate) fn resolve_entry_definition( column: &HydrateColumn, metadata: Option<&Zval>, @@ -527,20 +527,15 @@ pub(crate) fn resolve_entry_definition( .ok_or_else(|| ext_exception("flow_php expected a Definition object"))?; let variant = if let Some(metadata) = metadata { - let metadata = metadata.shallow_clone(); - let mut cloned = clone_object(base_obj)?; - let cloned_ce = unsafe { cloned.ce.as_ref() } + let base_ce = unsafe { base_obj.ce.as_ref() } .ok_or_else(|| ext_exception("flow_php failed to resolve a Definition class"))?; - let fns = def_rare_fns(def_rare_cache, cloned_ce)?; - call_handle( + let fns = def_rare_fns(def_rare_cache, base_ce)?; + call_handle_on( fns.set_metadata, - Some(&mut cloned), - &mut [metadata], + base_obj, + &mut [metadata.shallow_clone()], "set per-value metadata", - )?; - let mut zv = Zval::new(); - zv.set_object(&mut cloned); - zv + )? } else { column.base_def.shallow_clone() };