From 2a4cdab68b1bb3dd1bdc10325e5c997b1ccae081 Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Mon, 27 Jul 2026 11:56:40 +0200 Subject: [PATCH 1/2] fix: extractors no longer mutate user-provided Schem - clone Schema at withSchema() reception in all 16 schema-receiving extractors - clone per extract() run and per stream before extending with _input_file_uri/partition columns - fixes caller schema pollution, extractor-lifetime pollution and cross-stream partition leaks (#2536 regression) - regression tests with mixed partitioned/non-partitioned fixtures per adapter --- documentation/adrs.md | 1 + .../extractors-never-mutate-user-schema.md | 78 ++++++++++++++++++ documentation/upgrading.md | 8 ++ .../src/Flow/ETL/Adapter/CSV/CSVExtractor.php | 6 +- .../cross_stream/date=2026-01-01/data.csv | 2 + .../Fixtures/cross_stream/nopart/data.csv | 2 + .../Tests/Integration/CSVExtractorTest.php | 33 ++++++++ .../Adapter/Doctrine/DbalKeySetExtractor.php | 2 +- .../Doctrine/DbalLimitOffsetExtractor.php | 2 +- .../Adapter/Doctrine/DbalQueryExtractor.php | 2 +- .../Flow/ETL/Adapter/Excel/ExcelExtractor.php | 6 +- .../cross_stream/date=2026-01-01/data.xlsx | Bin 0 -> 5596 bytes .../Fixtures/cross_stream/nopart/data.xlsx | Bin 0 -> 5596 bytes .../Tests/Integration/ExcelExtractorTest.php | 34 ++++++++ .../GoogleSheet/GoogleSheetExtractor.php | 4 +- .../Tests/Unit/GoogleSheetExtractorTest.php | 54 ++++++++++++ .../Http/PsrHttpClientDynamicExtractor.php | 2 +- .../Http/PsrHttpClientPaginatedExtractor.php | 2 +- .../Http/PsrHttpClientStaticExtractor.php | 2 +- .../JSON/JSONMachine/JsonExtractor.php | 6 +- .../JSON/JSONMachine/JsonLinesExtractor.php | 6 +- .../cross_stream/date=2026-01-01/data.json | 1 + .../cross_stream/date=2026-01-01/data.jsonl | 1 + .../Fixtures/cross_stream/nopart/data.json | 1 + .../Fixtures/cross_stream/nopart/data.jsonl | 1 + .../JSONMachine/JsonExtractorTest.php | 33 ++++++++ .../JSONMachine/JsonLinesExtractorTest.php | 33 ++++++++ .../PostgreSql/PostgreSqlCursorExtractor.php | 2 +- .../PostgreSql/PostgreSqlKeySetExtractor.php | 2 +- .../PostgreSqlLimitOffsetExtractor.php | 2 +- .../Flow/ETL/Adapter/Text/TextExtractor.php | 2 +- .../cross_stream/date=2026-01-01/data.txt | 1 + .../Fixtures/cross_stream/nopart/data.txt | 1 + .../Tests/Integration/TextExtractorTest.php | 14 ++++ .../ETL/Adapter/XML/XMLParserExtractor.php | 6 +- .../ETL/Adapter/XML/XMLReaderExtractor.php | 2 +- .../cross_stream/date=2026-01-01/file.xml | 3 + .../Fixtures/cross_stream/nopart/file.xml | 3 + .../Integration/XMLParserExtractorTest.php | 27 ++++++ .../Integration/XMLReaderExtractorTest.php | 16 ++++ .../src/Flow/ETL/Extractor/ArrayExtractor.php | 2 +- 41 files changed, 376 insertions(+), 29 deletions(-) create mode 100644 documentation/adrs/extractors-never-mutate-user-schema.md create mode 100644 src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/date=2026-01-01/data.csv create mode 100644 src/adapter/etl-adapter-csv/tests/Flow/ETL/Adapter/CSV/Tests/Fixtures/cross_stream/nopart/data.csv create mode 100644 src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/date=2026-01-01/data.xlsx create mode 100644 src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Fixtures/cross_stream/nopart/data.xlsx create mode 100644 src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.json create mode 100644 src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/date=2026-01-01/data.jsonl create mode 100644 src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.json create mode 100644 src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Fixtures/cross_stream/nopart/data.jsonl create mode 100644 src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/date=2026-01-01/data.txt create mode 100644 src/adapter/etl-adapter-text/tests/Flow/ETL/Adapter/Text/Tests/Fixtures/cross_stream/nopart/data.txt create mode 100644 src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/date=2026-01-01/file.xml create mode 100644 src/adapter/etl-adapter-xml/tests/Flow/ETL/Adapter/XML/Tests/Fixtures/cross_stream/nopart/file.xml diff --git a/documentation/adrs.md b/documentation/adrs.md index 9a95eb2758..22babf242e 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: Extractors Never Mutate a User-Provided Schema](/documentation/adrs/extractors-never-mutate-user-schema.md) ### [Proposed AD](https://github.com/flow-php/flow/pulls?q=is%3Apr+is%3Aopen+label%3AAD+) diff --git a/documentation/adrs/extractors-never-mutate-user-schema.md b/documentation/adrs/extractors-never-mutate-user-schema.md new file mode 100644 index 0000000000..45012f61f5 --- /dev/null +++ b/documentation/adrs/extractors-never-mutate-user-schema.md @@ -0,0 +1,78 @@ +# Extractors Never Mutate a User-Provided Schema + +[TOC] + +Proposed by: @norberttech +Date: 2026-07-27 + +## Context +--- + +`Schema` is intentionally mutable — `add()`, `remove()`, `rename()` and similar methods mutate `$this` and return +`$this`. 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 that stores the caller's `Schema` instance therefore writes every internal extension into the +caller's object. This produced three observable defects (#2536 regression): + +1. **Caller schema pollution** — a `Schema` the user holds for other purposes (e.g. passed later to a loader) + gains non-nullable columns it never declared. +2. **Extractor-lifetime pollution** — columns added during one `extract()` run persist on the extractor's schema + into subsequent runs. +3. **Cross-stream pollution** — partition columns of one stream leak into the schema used for the next stream, + and `Hydrator::cast(fillMissing: true)` injects `null` into a non-nullable definition. + +## Decision +--- + +**Extractors never mutate a user-provided `Schema`. Isolation is achieved by cloning, not by changing Schema's +mutable contract.** + +Two rules apply to every extractor that accepts a `Schema`: + +- **Clone at reception**: `withSchema()` (or a constructor argument) stores `clone $schema`, never the instance + itself. This applies to *all* schema-receiving extractors, including those that do not extend the schema today — + the rule is structural, so future mutations can never alias the caller. +- **Clone before extending**: an extractor that adds columns clones its own copy once per `extract()` run, and + once more per stream before adding partition columns, so runs and streams never share a `Schema` instance. + +A shallow `clone` is sufficient: `Schema`'s only state is `array`, `add()` rebuilds the array, +and extractors never mutate existing `Definition` objects. + +DSL `from_*()` functions stay pure delegation — the clone lives in the extractor, never in the DSL. + +## Pros & Cons +--- + +**Advantages:** + +- **No API change**: Schema's mutable contract and every call site stay untouched. +- **Structural safety**: aliasing is impossible regardless of what an extractor does to its copy later. +- **Cheap**: one shallow clone per reception, per run and per stream — negligible against I/O-bound extraction. + +**Disadvantages:** + +- The rule is convention, not compiler-enforced — a new extractor can forget to clone; regression tests asserting + the caller's schema is untouched guard each extractor. +- Reception clones in non-extending extractors are unobservable dead weight until a mutation is introduced. + +## Alternatives Considered +--- + +### 1. Make `Schema` immutable (`add()` returns a new instance) + +**Rejected because:** it changes a core contract used across the entire codebase for a problem local to +extractors, and the mutable contract is a deliberate performance choice. + +### 2. 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..19fdd9941e 100644 --- a/documentation/upgrading.md +++ b/documentation/upgrading.md @@ -478,6 +478,14 @@ 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. + --- ## Upgrading from 0.40.x to 0.41.x diff --git a/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php b/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php index 32719d6170..d2e49bfc2c 100644 --- a/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php +++ b/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php @@ -59,7 +59,7 @@ public function extract(FlowContext $context): Generator $shouldPutInputIntoRows = $context->config->shouldPutInputIntoRows(); $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $baseSchema = $this->schema; + $baseSchema = $this->schema !== null ? clone $this->schema : null; if ( $baseSchema !== null @@ -78,7 +78,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = $baseSchema !== null ? clone $baseSchema : null; if ($schema !== null) { foreach ($partitions as $partition) { @@ -229,7 +229,7 @@ public function withHeader(bool $withHeader): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } 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..9f2a1898d0 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,11 @@ 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_to_ascii; +use function Flow\ETL\DSL\str_schema; use function Flow\Filesystem\DSL\path_real; use function iterator_to_array; @@ -145,6 +148,19 @@ public function test_bom_removal_utf8(): void ); } + 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 +454,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-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php index ac0c2f6404..1dcc5de565 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php @@ -222,7 +222,7 @@ public function withPageSize(int $pageSize): self */ public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php index a240348fe4..452a932d15 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php @@ -164,7 +164,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php index 1da60a9fe4..6caafae2f6 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php @@ -90,7 +90,7 @@ public function withParameters(ParametersSet $parametersSet): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php b/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php index f25ecbb31b..11377a2062 100644 --- a/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php +++ b/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php @@ -75,7 +75,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $baseSchema = $this->schema; + $baseSchema = $this->schema !== null ? clone $this->schema : null; if ( $baseSchema !== null @@ -89,7 +89,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = $baseSchema !== null ? clone $baseSchema : null; if ($schema !== null) { foreach ($partitions as $partition) { @@ -212,7 +212,7 @@ public function withReader(ExcelReader $reader): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } 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 0000000000000000000000000000000000000000..658d23787a8f5cdca49474c6d45c183ba07fdf62 GIT binary patch literal 5596 zcmaJ_1z1$;)*cwTrJF%oVn9$r5R@J|<$w~x&^bd&N_TgsNJ@v40@B?`NeDEzO?w%CH*zI&?TaLyQ1e+qTVYSONaOzm(HZw`TFNs?^^R$jqHn{eb} zu3F<&z6@GUzZ>VUlmw_NVdI#W`Zs<;F8cvG0D1j)z|bO}aJL2jg^im7%;vd+gAJd% zo$X&ZX+E;<;wNd{qIiB-zgHNj$+Ja@*Zk@WFtD{j*;dg~*zW5^sfr~%P9Ann_mH(k zu*u-G)Ak1rxt&%mbu&>M=bqUt8w#(zO7!3-il7GaB_%EDpA{3?7Z@<7#(uUy1ShnLe zw5Cf2K8cP6ERhuVSLYVSeMrBJ32Uw2?r@X&!z%o2EP!>SQ`o^gJ6U*3|Ajoit|djS zBQ-yyMIt+n8+lY3qZ)SrSX09DC8V$e2|BY_hl;N-r6<;y>cXsE(pH>&(K;i96-GBD4snY- z79sBO9$+G&*Pq-9SwmLRg zWzBQRVkb$DiR8NXdKavORLmzWNh-R=K?DvQ0d56HX0gk!xK#M&S&vURz6|8?Z7YhM zQxaaX606&T>lWa})%67%TDl0=p&y2%Wkx8ssmLi&IY$Ek9Dx56Uev$b25t$3I>W&~ ze*{nw)~2iCupmOxcKC+GdAp?ER#Y`zTQ^SZv_Xm%+vzRlnOsDoY~b2!R%q|{QwGhP z>ZtpiuDbzjL?I5FZFwS*%aWU&3;ac*eI=5#1l}?I7@R4)CD*4uSDO`wR?@6&=&Tv% z4_;j&j*P}BrvgT(1sTy5Bu)iQ!y`ariaoG=Y<2!HL9W-C)byn;x%eM&Tq0n%0=rhN zV#)X}@m3$VVE`u*Iri}4?^;Y~gTis2^Lu9zg`MS=V!^o)feuZQrFWw1E8&wid^Gw|cWl-fY%M?!^C+8CF37I$l2-_D zwvX_4*(9e^PQ*^f<0ezfUF=d8-J&*Y9()4Q2MlOx;+bl_bIhf^z!J_Be>%OHtPkTh}HnGZw#EaqWqd`nsxS-f!+E&iem1k&7NzH|cREWz@(^UbcM zyPX)Eyja)F95-)cB{fYm%M*mLU>c~2nv^!_Fnm;h7KVVmZuQlKQ(gHn#5b(&xlWG& zsch=(*G$N5ZT;(eL!@KI;W6`h@^ddR`pVd-u1sJiU;z)FQ{s-3)FAAQp5FebYyRdV za6iL~K+laH>ESal(t6uR2bosFNI`={ycc6 zedyVRAG_MWDx-e-4lGk4ibYx3On`0SFv{-I`z6?&y#;@YRouq+^$$D(h_@{}oS_1! zfTrv~Uw7Ha^XD8mRsSo;kNi=MCc_9P?`F~BT@z3Ki|imhPO6Yj-DL}9zF1ye11Hv9 zK2@y_0F-=nPF>fZl{h8dXRyi2peK!4j zdl`or2qh;S$Hk6h1+9RQc&YbxD$fo}r_vupi4RPY@3Ka99ZuK|;X@?t-(E7xOIf+R z3^j^tbr*(TziCe~US3+Cmh}{N=WPzi?3;D7jY5#>sEBoFY%`{4jJ?rXsS5DmyK+`ZH!RVlHB2kiFqiCqJuL21U3-()#c#ar6{aBJB^w^u z_VzlD)kDrywuFp!NtyG}@Syg#c-~68{R)_~n#dWiW-4`zMgK*IPosYkw>tCD>PJ52 zydd~=yue@^-JZ%Hd1sz~Sdoi3q-Pdk8McTV`0(Js!w<0P(8I zQ!s$U*|!N&qqRiJ>U?7(3{T1WxPx&C$Gp#@v{NZ-NGcX%QCt=<&|&4}aH*+d^|)e$ z3oY$p6wQsD01ho*@Z;Sf?^JXNNp!SKMZao2Km_ z?_Y~{Tz2YhISgVkoz+@glA&tXS=Et0Ba-fnyN7b#U zvG+()NPzXpGpBQVS&Z8`@p((g`)p%R0Ivp_G>>O8Lqp>m1J1T2dfyX8$-d0%&T`6u zH5LQ+1()2=3lVolUN2v7`K z)$9ckg6FRc6(q}_W6b7bNE`?wdB;o;m^QnX*d3jk?XO@*uTak>>|T%`?kyCl{islYyP=E+G^p z91GwM#E-9guU!)&l0I$6S$8F7u5NR7hYl!zR!$c5YDx7K@9NXL4ev}U6kJpf08bEy zRTCBqQxs9Y^cNC*`iFrp9(sa~HVM4Y`p&9iULA-s_$`5Q1y_F*3ofH_rz0&+C#x!1 zGdbLZa9~PPziM2%Pk03eC>D@2Jb-*Q{>+`EO%@#>ge&$^h%0e%yL%33^*no=WfesF z7N&@Cx~??b`uGvXdK5NCMIpz56fq)r`92?cj|bBLh9^kH`Z@13j0iY&N~J>B57Dy* zPtIHxy`3ryW9I-qUU=7vfNWLO2M_KrCkkrbQT%2n(S^AhyvBH*pS?k=Os0yD-cx7v1J6xmDdqzl78 z%aj{V5Pe%rj?7gWfpBd{blOz#4BK9$)jOugx-gd}nva6^DXaDx>M!o_4$^O|P_ZaI zISZ-jF7qqqP}#xs^A1(s-0%K#NIShhBq+W1N}yXCai8SLJaBlHojV!mqE1sDPNf)> zk(vJtzT?$SV2wkZpzjcXFg1tq9|xypbyNhT{r~Z7sJr*#M2SY;^E3N0w{U_t41Sd} zjM25;A~{4nax`Aq`%R~M-)nn|siGr5(0fc)vNEy5%Edz^9k;FG2twe-VV{XR)Nl(WKQ1VxFWL`YzZ9Z3AkRb7S2#*NE;Hs9NSNSR}3%}6Qnk@S_e*ff{la%C2WTDJo&Tc`J<269jSw#yQhwQy9)QQoq03)1a8%%;RJfd z;yx_FU)OH4T@hOE_pZ&ETK=K%HMIt}`MGJP+Lm_vGc|>po_=mhM>4m%1W-0u!1R4Y za!3L_;$sq7+=jcXh!3ORx|c6$wOkmU`Gx+~f(wEWg|dT0{W}h1eh~i0evsrQ$|jDE zD3G(_4AeUL$$M8W$(7c*ZG>Ifp5cyDGYL^=bqi0pSg6<559{N4dE(1_nm=~7|9&|d zG=h+3+B2@xEW?v!euT!EXHasV;=DYE9TTs7z4UXNu6iC&PsqaU#BK^pj^jO6cCKwu z!$=xUkFKrBr~4<+&)ivz*au!B_EiUBT%R-|a?ldH1ej@p+kn%L9|%x^#K+5WcPwe`(sK;*X*wp@JE7hFxpR3_jW8WWHBcTS{~CiHTM zH&1ya+h-y^Ty(*cF@z;-Iz9;Fgeaz@C?-65x~V8LaC;Cp=RWkKIcqN5ek3y zvr9@GM0u_(!b^687qVC*g~PV*-%EN&W2SE|pNu{(e7td4fKLSt_9ddbkaS3N(^EbVE%1t~_{LnpC!-tqHL-?VG-r8Au1T22B;sU)CG?Xd_ z+pJcWPIBGh5weeQ%zu$N4c&997drUr;IA=FB-LP2Q?*j@jem?c40B3+-N}Fp zl*YfCqZdJV!mIqaHhb&KZQcyRJ*|cLmYG_DjBDK1DDkLQgPJgs^ihLEvk^Vx&S?N+ zSMNQWG;R4mO=}CBXwK+2datF3VkC*mbGGC|?_>K_np1X1G=4K^SyuBMyih0Vu+BvQ z#+{jHg2>9Ey2|=GznmPZNxaqykTY%*j}M)(h{0cl-2(a4f?>1{o8wfdEy8eeFMN*N zo<2-eYC%IM2mG3&-5eyL=4k&ZH>PU8E8d)}pyo`!ga-Mlzn?n&Zs%sJjhe&!l1IoG zMcTPJmHA!yCY45Y4`8|e}W2}yyGE(N4J1_^1T;Yuqa z|KPpnR#U{lqyXUJ;sS{55qf}YMu7Tm1hcS*gYI0N z%M*qbF!)Hr_I)Ftd%M?CN}D5E!bdXYeFM@4ewYZRgK1rTy@PP|iVJY!Aby8~f>Ifu zZde{izL1NQg_h`XN5_7r@qU#}e7a5FV)tZ_Wk_3rmBg&CWTMml2$g;^BC>0)8N#~;%(V5_fNa_Cf=>_P5dy;X2Ik@{7O;m%q0nuzGMHdm2`Fj$rt`Fe205BP(6 zT-mrE%?Zkz{nnH3Mmem-0BVXjxRxb>b&IHFKST$huKx)bI@BE=_Mktpafia}OrcP_ zJ01@9f8wMrVcP*FZCXs`mq&qVzpu+vcQ;;RBfgWvw22Wezd7^LIlf?txp=@ z7LpVq>GJJkB&Fj@?0b%I6~PmlUigA^33s0f-`8VGot?AhG7J6eR4yz1c=a;UepW*C zVBlov5TO?G=tH!&i>K?Oh7|;7WgeD_9Kw0O38ShMY>9BX#Yf_KGD3XYGBKbcxn0La zh5^4wC06`cFN*HK$aU2X=`Xs5J)`o~4s6?yaM+qNrot&-^{SFNQF@vO6KlYi5B6M? z_&9SM9WyZZyEG$(Gp@cmb{|P2F#KMw!3t=$VTS&BPsAD+x1EY==xjdk+^~T4Ue;wo zB1QT$6>$~Cfrm6?2u5hps6e{%D~Y$nazWtcEW-Jiu0#p1%tC<%^TdJi?0oL9#vG2# z4Sj*5L=o|ZmJ!F{tS)41f~xMPS-wMWNSPnnRo+Z`_Hv%10RT{pf2%K=UwH$!wy<_Hf%> zev3P$iYW5aH{*zorA$vKC-Dl2D%PQ!^zGYcbBuA43oIg(X$QM=mj{(T=R>DSvJ5Qj zLh0u?dEZHTp?<5&sz0N%R?tmRL$XFv%q85Qm^^T zG~i86F@=J;S5jo2ibKxfIrU`rswOj&dY0wAHc3zhLnReHKwgQC|T3Bv9M zP!!-*0i$U%T2tv3R0G5w9=2F9MY=}PgbNUN%%36eh}F_PfR0FLYNvQ1eRS!WZpHL2 zh8D`Fgv8fLf95Vj{2_{oyt!Gu+2vSV)H;Q=xn&V>zOQVP;?^x*Y{w=Z0;SkyIG~Fw zOx*Mkjw&oyQA>{Nv?d-O#Z`&0dM#^Kd`zvfL0`>6_}xL$mtNZGdmuF)D_?lYPFs9x zu6jB;GRLOiE$7z2(+F53-x5d1Od6I~AkPw{!S}W=nC-6V!?bD6=xFgvhtAirG+E)e z2^S<}9Il{l4ybE-9lfJXzFsiOz@ua}v(2%$spa-84t~Hnysue%CZ38lGA{ObY zu;y-t@AyKU(8dIAs$f>&_{qW)nZSraHioRWhuO%RMfC0I_8_(KjpmDI2lTlTTN?Zg zyVf_w*@O3Df@&|d*%;2*;_bc>8XaNbPTBF*f&|xHMD0l|7x+oJZB~q{)=3^LaT^m% zrcm_EM2*jZLMCoJw&tV{9qZ0A!bWH zlDcPTM)JfBb{Ev&;o32h$}g6%g2+IWl^ynBw!G!Cw+eN!U6*H;-EHoFrtI?HYrJ^b ziEifpOnyEm&!?^O659GivdurBH0r={Ne>3X@APlu(63LG7qq$PiuZl`K?28&)ZngI zn6_%Q`3$D=|pI9Sw`KfhQF^GuF$HX8-#0t}4hA*jiP~qao3aToM%2I~LNo=~H2Ib>{}xFP6Kw$i)?1U7rbp42A#;_yuMJ~YNr(4y z`s6l>8!5O0g5C|Iss5U29 z+<01ufSAH7fTzN$ISOzW-Q-E5g^!)1jHLA?+6oRN;2)|s~Sl6_eZ z6?`gl<@FWf9b5Qm(3+xUw9eRwB2%+wwlFW?Soe7pw<=@|ZwSX(6_p0{wb}SUFVwYc zB+G}m(9+L`(cIYy;TC1H0lZrj?GV?nBq!^P(?q|_zB2y@X%;=7Kq61Z_a~WOHTg4I znx*d^?p{7-qY1jf8r60t}; z@+NUQWGObLI=!%5OX&7m^IXuPeKEoeIPr&c=qp{$&pcbVt3sL9J)S4bqP47Ph0D9b z2zqCqnr*Rl2iLBk@tr?*%&_EWamCCd3aJj|4VTaj! zwYxg1sgxOBqbF)injg;3&S8AqZcS zE;Ae>KATI4&QTmv;M$04w=3rvv_DU;amsk^%2bkMISkyTs@$oqIls-@&#<~c&7%0^ zB&@o#G@yt>c?&DRH(crSZs#8Zni;)eAsL7te4U!7_eq~jV+>BQbEjaqs?nB3QbR&A zU*s9Uw|rU&ZE;Bw^`JqgW|lDUVQ6|*TX|6W{~rbeo!#e0inMaxYwYvff{DH`_>b&C z%#M{Nu>q2yFQXORXIfReJ{#+dVd~qDqjFHdnFG#7CvQVk|Z=4-E~Yq zdS#sHR1gf@{JaX*JHo38dtL%+d4W$S|m8==rzhy7#M}RrhObrHgf`iPUFpH~tb%-BS zL2cxM_eEq@y_YdrgY#IEg`8{)6aqRYKh92k@UN~@K0gt{yC<_+%YW`^zkk0Jp*@Y~ z^Bnm$|F%NHvn6f$@w(NET{eGaU2#iH4Wzi@9nzkXCcFC5hU3a(_y+nKvxK?nD!6>X9~b_9N2I9{Fbc3EZaZu`|$s z#bZzu{I=C@qddGO;9ZLgjoc%_OPa^rmfy@?sIF_a8mP)wclB~pIgz`+O$5FK@tM7k zN(oD3IGsr*k6-nW5%Rkgu;Sx?qe?ac-|}4V$E@oq(G|*2(VBN0D1H$9*Zm;rb(Bq< zoUTC5iq}_d2UBz}Tu>;kaN7yGu^Hfv(l83pWOWLTxmu|qY6kW2e7p&y7pD(h9KT)+ zhYX!cG42?@RWHStVUj@O%+)WxPx-AZn;i?kY^7waMO!TwLr1{M{m5YgTbAQJ_RAdm zklLYi+AeK-lg0Z-7HiyDw{Z4+gd8jPgt-yW101T55$r@Xq-Ll?j9F z>6@oKVy(!i59b~56ih);yS5L4xM7ggR7j%4)6Wp;KIVSB?E4lomaM%5FAI1fxlx4_ z3ne!To~#>HauP+=IHE^kjy|72ZJvq};Q6rxT4%DPudMqdi*=wjjMVx;NmWC1t$ zxk*auzsg*hQy-aCUWK`8aa^|D!0sEjA0vA{CUy!`@y_z@F7Ue%lqhUS#eD3HwAvGO zFYEm3F@3)my)03W(mahw9zD{INlWYb)TWu2RM)F0pT@h3H+HltfHkL|Q`Ecdtke77 zqJ<=@&!%|#)KK1w9@fD+vIW-{KIJCqD|+Pd{IMSiTc+S6Po3qHj3`*bT=_YA32-1y z7WTPHNh;ZGi$}mQ&MCK3rO_pkN6KQyxkg~`02=ssl32Xfq`GpU{0uz88-X<;vf`}I z1xyETXX``}9q}qjB3`a6&tXm?2&{)1EM_c3+AU$B7Y_Wv|PH-^U54u%zmYsyowfnpgGjKUX7ev&}gL zjJhz=hLD%Wbd>hAZyxQdioVtek~MA-NeD+;#StvQ$bfzb5R4A`d6fDp=`o!>@=Z~% zrw8kbTF}rb0KevF*9S>gbF}}qYg4t~9j{MTuI5a?j288%f1W!19_RW_`)UsJ%Op@W zii&f6D)YPZbs2rN8UJOTC~^Il^WV1Se~WTWEnOw*FPlc`CsdT*1^Dj)t}Bl#QT5A& zP?(|u{7qv09^pFsUn!Jd#!L8Tg#YN3-(9a$+Esh_W!uF6qgnj!e!YILc<+}zMqU1= n*ZjqTzk6R#t^bDZ73#nIkBV1Q#KOLM1U~8rK)qnupI-k5re#;Y literal 0 HcmV?d00001 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/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php b/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php index 0a8b0bbc4a..642e4fabca 100644 --- a/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php +++ b/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php @@ -83,7 +83,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $schema = $this->schema; + $schema = $this->schema !== null ? clone $this->schema : null; if ($schema !== null && $shouldPutInputIntoRows) { if ($schema->findDefinition('_spread_sheet_id') === null) { @@ -200,7 +200,7 @@ public function withRowsPerPage(int $rowsPerPage): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } 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-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php index 34f6629499..7c175d84fe 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php @@ -92,7 +92,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php index 4d62331c0c..49df972035 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php @@ -101,7 +101,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php index 82a0b86f93..fec0e43bb6 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php @@ -90,7 +90,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php index 0cb43f6e58..f9e1942045 100644 --- a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php +++ b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php @@ -48,7 +48,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); $encoder = new JSONEncoder(); - $baseSchema = $this->schema; + $baseSchema = $this->schema !== null ? clone $this->schema : null; if ( $baseSchema !== null @@ -62,7 +62,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = $baseSchema !== null ? clone $baseSchema : null; if ($schema !== null) { foreach ($partitions as $partition) { @@ -150,7 +150,7 @@ public function withPointer(string $pointer, bool $pointerToEntryName = false): public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php index e592fecb6d..7a9f611fc5 100644 --- a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php +++ b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php @@ -49,7 +49,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); $encoder = new JSONEncoder(); - $baseSchema = $this->schema; + $baseSchema = $this->schema !== null ? clone $this->schema : null; if ( $baseSchema !== null @@ -76,7 +76,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = $baseSchema !== null ? clone $baseSchema : null; if ($schema !== null) { foreach ($partitions as $partition) { @@ -166,7 +166,7 @@ public function withPointer(string $pointer, bool $pointerToEntryName = false): public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } 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-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php index ebde047a60..65ea6eb10f 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php @@ -143,7 +143,7 @@ public function withMaximum(int $maximum): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php index c7aa8021d8..0a82c7d33f 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php @@ -121,7 +121,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php index 2446ab98e1..60637885b1 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php @@ -115,7 +115,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php b/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php index 2f0b6685ec..98f8043dbe 100644 --- a/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php +++ b/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php @@ -48,7 +48,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = clone $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { 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/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php index 4e73b94d0c..ce9b8556e1 100644 --- a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php +++ b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php @@ -116,7 +116,7 @@ public function extract(FlowContext $context): Generator $batchSize = $context->config->extractorBatchSize(); $encoder = new XMLEncoder(); - $baseSchema = $this->schema ?? schema(xml_schema('node')); + $baseSchema = $this->schema !== null ? clone $this->schema : schema(xml_schema('node')); if ($shouldPutInputIntoRows && $baseSchema->findDefinition('_input_file_uri') === null) { $baseSchema = $baseSchema->add(str_schema('_input_file_uri')); @@ -126,7 +126,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = clone $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { @@ -281,7 +281,7 @@ public function withBufferSize(int $bufferSize): self public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } diff --git a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php index edb9cc4edf..4de1ae9169 100644 --- a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php +++ b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php @@ -81,7 +81,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema; + $schema = clone $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { 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/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php b/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php index 0049eae94f..e6323b64ed 100644 --- a/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php +++ b/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php @@ -38,7 +38,7 @@ public function extract(FlowContext $context): Generator public function withSchema(Schema $schema): self { - $this->schema = $schema; + $this->schema = clone $schema; return $this; } From fd44cd2a5bc6fc40f9463676036b2bfb23f8840a Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Mon, 27 Jul 2026 14:18:08 +0200 Subject: [PATCH 2/2] refactor(flow-php/etl): make Schema and Definition immutable - all Schema/Definition mutators return a new instance; both are final readonly - revert extractor Schema clones, redundant under immutability - native hydrator: use setMetadata() return value instead of mutating a clone - assign mutator results in DatabaseTableSchemaCommand and PhpRowHydrator - replace clone ADR with schema-immutability, add upgrading entry --- documentation/adrs.md | 2 +- .../extractors-never-mutate-user-schema.md | 78 ------------ documentation/adrs/schema-immutability.md | 116 ++++++++++++++++++ documentation/upgrading.md | 18 ++- .../src/Flow/ETL/Adapter/CSV/CSVExtractor.php | 6 +- .../Tests/Integration/CSVExtractorTest.php | 15 +++ .../Adapter/Doctrine/DbalKeySetExtractor.php | 2 +- .../Doctrine/DbalLimitOffsetExtractor.php | 2 +- .../Adapter/Doctrine/DbalQueryExtractor.php | 2 +- .../Flow/ETL/Adapter/Excel/ExcelExtractor.php | 6 +- .../GoogleSheet/GoogleSheetExtractor.php | 4 +- .../Http/PsrHttpClientDynamicExtractor.php | 2 +- .../Http/PsrHttpClientPaginatedExtractor.php | 2 +- .../Http/PsrHttpClientStaticExtractor.php | 2 +- .../JSON/JSONMachine/JsonExtractor.php | 6 +- .../JSON/JSONMachine/JsonLinesExtractor.php | 6 +- .../PostgreSql/PostgreSqlCursorExtractor.php | 2 +- .../PostgreSql/PostgreSqlKeySetExtractor.php | 2 +- .../PostgreSqlLimitOffsetExtractor.php | 2 +- .../Flow/ETL/Adapter/Text/TextExtractor.php | 2 +- .../ETL/Adapter/XML/XMLParserExtractor.php | 6 +- .../ETL/Adapter/XML/XMLReaderExtractor.php | 2 +- .../Command/DatabaseTableSchemaCommand.php | 2 +- .../src/Flow/ETL/Extractor/ArrayExtractor.php | 2 +- .../etl/src/Flow/ETL/Row/PhpRowHydrator.php | 2 +- src/core/etl/src/Flow/ETL/Schema.php | 67 +++------- .../etl/src/Flow/ETL/Schema/Definition.php | 4 +- .../Schema/Definition/BooleanDefinition.php | 16 +-- .../ETL/Schema/Definition/DateDefinition.php | 16 +-- .../Schema/Definition/DateTimeDefinition.php | 16 +-- .../ETL/Schema/Definition/EnumDefinition.php | 18 ++- .../ETL/Schema/Definition/FloatDefinition.php | 16 +-- .../ETL/Schema/Definition/HTMLDefinition.php | 16 +-- .../Definition/HTMLElementDefinition.php | 16 +-- .../Schema/Definition/IntegerDefinition.php | 16 +-- .../ETL/Schema/Definition/JsonDefinition.php | 16 +-- .../ETL/Schema/Definition/ListDefinition.php | 16 +-- .../ETL/Schema/Definition/MapDefinition.php | 16 +-- .../ETL/Schema/Definition/NullDefinition.php | 14 +-- .../Schema/Definition/StringDefinition.php | 16 +-- .../Schema/Definition/StructureDefinition.php | 16 +-- .../ETL/Schema/Definition/TimeDefinition.php | 16 +-- .../ETL/Schema/Definition/UnionDefinition.php | 16 +-- .../ETL/Schema/Definition/UuidDefinition.php | 16 +-- .../ETL/Schema/Definition/XMLDefinition.php | 16 +-- .../Definition/XMLElementDefinition.php | 16 +-- .../Tests/Unit/Row/NativeRowHydratorTest.php | 12 +- .../tests/Flow/ETL/Tests/Unit/RowsTest.php | 10 ++ .../Definition/BooleanDefinitionTest.php | 2 + .../Schema/Definition/DateDefinitionTest.php | 2 + .../Definition/DateTimeDefinitionTest.php | 2 + .../Schema/Definition/EnumDefinitionTest.php | 2 + .../Schema/Definition/FloatDefinitionTest.php | 2 + .../Schema/Definition/HTMLDefinitionTest.php | 2 + .../Definition/HTMLElementDefinitionTest.php | 2 + .../Definition/IntegerDefinitionTest.php | 2 + .../Schema/Definition/JsonDefinitionTest.php | 2 + .../Schema/Definition/ListDefinitionTest.php | 2 + .../Schema/Definition/MapDefinitionTest.php | 2 + .../Schema/Definition/NullDefinitionTest.php | 11 +- .../Definition/StringDefinitionTest.php | 2 + .../Definition/StructureDefinitionTest.php | 2 + .../Schema/Definition/TimeDefinitionTest.php | 2 + .../Schema/Definition/UnionDefinitionTest.php | 2 + .../Schema/Definition/UuidDefinitionTest.php | 2 + .../Schema/Definition/XMLDefinitionTest.php | 2 + .../Definition/XMLElementDefinitionTest.php | 2 + .../Flow/ETL/Tests/Unit/Schema/SchemaTest.php | 52 +++++--- src/extension/flow-php-ext/src/ctx.rs | 18 --- src/extension/flow-php-ext/src/hydrate.rs | 43 +++---- 70 files changed, 430 insertions(+), 418 deletions(-) delete mode 100644 documentation/adrs/extractors-never-mutate-user-schema.md create mode 100644 documentation/adrs/schema-immutability.md diff --git a/documentation/adrs.md b/documentation/adrs.md index 22babf242e..7828c155bf 100644 --- a/documentation/adrs.md +++ b/documentation/adrs.md @@ -38,7 +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: Extractors Never Mutate a User-Provided Schema](/documentation/adrs/extractors-never-mutate-user-schema.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/extractors-never-mutate-user-schema.md b/documentation/adrs/extractors-never-mutate-user-schema.md deleted file mode 100644 index 45012f61f5..0000000000 --- a/documentation/adrs/extractors-never-mutate-user-schema.md +++ /dev/null @@ -1,78 +0,0 @@ -# Extractors Never Mutate a User-Provided Schema - -[TOC] - -Proposed by: @norberttech -Date: 2026-07-27 - -## Context ---- - -`Schema` is intentionally mutable — `add()`, `remove()`, `rename()` and similar methods mutate `$this` and return -`$this`. 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 that stores the caller's `Schema` instance therefore writes every internal extension into the -caller's object. This produced three observable defects (#2536 regression): - -1. **Caller schema pollution** — a `Schema` the user holds for other purposes (e.g. passed later to a loader) - gains non-nullable columns it never declared. -2. **Extractor-lifetime pollution** — columns added during one `extract()` run persist on the extractor's schema - into subsequent runs. -3. **Cross-stream pollution** — partition columns of one stream leak into the schema used for the next stream, - and `Hydrator::cast(fillMissing: true)` injects `null` into a non-nullable definition. - -## Decision ---- - -**Extractors never mutate a user-provided `Schema`. Isolation is achieved by cloning, not by changing Schema's -mutable contract.** - -Two rules apply to every extractor that accepts a `Schema`: - -- **Clone at reception**: `withSchema()` (or a constructor argument) stores `clone $schema`, never the instance - itself. This applies to *all* schema-receiving extractors, including those that do not extend the schema today — - the rule is structural, so future mutations can never alias the caller. -- **Clone before extending**: an extractor that adds columns clones its own copy once per `extract()` run, and - once more per stream before adding partition columns, so runs and streams never share a `Schema` instance. - -A shallow `clone` is sufficient: `Schema`'s only state is `array`, `add()` rebuilds the array, -and extractors never mutate existing `Definition` objects. - -DSL `from_*()` functions stay pure delegation — the clone lives in the extractor, never in the DSL. - -## Pros & Cons ---- - -**Advantages:** - -- **No API change**: Schema's mutable contract and every call site stay untouched. -- **Structural safety**: aliasing is impossible regardless of what an extractor does to its copy later. -- **Cheap**: one shallow clone per reception, per run and per stream — negligible against I/O-bound extraction. - -**Disadvantages:** - -- The rule is convention, not compiler-enforced — a new extractor can forget to clone; regression tests asserting - the caller's schema is untouched guard each extractor. -- Reception clones in non-extending extractors are unobservable dead weight until a mutation is introduced. - -## Alternatives Considered ---- - -### 1. Make `Schema` immutable (`add()` returns a new instance) - -**Rejected because:** it changes a core contract used across the entire codebase for a problem local to -extractors, and the mutable contract is a deliberate performance choice. - -### 2. 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/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 19fdd9941e..122ee384a6 100644 --- a/documentation/upgrading.md +++ b/documentation/upgrading.md @@ -478,7 +478,8 @@ 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 +### 32) `flow-php/etl-adapter-csv`, `-excel`, `-json`, +`-xml` - explicit schema no longer projects partition columns away | Before | After | |------------------------------------------------------------------|------------------------------------------------------| @@ -486,6 +487,19 @@ Custom aggregators must implement `references()` - return the references the agg 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 @@ -2241,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/src/Flow/ETL/Adapter/CSV/CSVExtractor.php b/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php index d2e49bfc2c..32719d6170 100644 --- a/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php +++ b/src/adapter/etl-adapter-csv/src/Flow/ETL/Adapter/CSV/CSVExtractor.php @@ -59,7 +59,7 @@ public function extract(FlowContext $context): Generator $shouldPutInputIntoRows = $context->config->shouldPutInputIntoRows(); $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $baseSchema = $this->schema !== null ? clone $this->schema : null; + $baseSchema = $this->schema; if ( $baseSchema !== null @@ -78,7 +78,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema !== null ? clone $baseSchema : null; + $schema = $baseSchema; if ($schema !== null) { foreach ($partitions as $partition) { @@ -229,7 +229,7 @@ public function withHeader(bool $withHeader): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } 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 9f2a1898d0..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 @@ -21,6 +21,7 @@ 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; @@ -148,6 +149,20 @@ 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')); diff --git a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php index 1dcc5de565..ac0c2f6404 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalKeySetExtractor.php @@ -222,7 +222,7 @@ public function withPageSize(int $pageSize): self */ public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php index 452a932d15..a240348fe4 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalLimitOffsetExtractor.php @@ -164,7 +164,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php index 6caafae2f6..1da60a9fe4 100644 --- a/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php +++ b/src/adapter/etl-adapter-doctrine/src/Flow/ETL/Adapter/Doctrine/DbalQueryExtractor.php @@ -90,7 +90,7 @@ public function withParameters(ParametersSet $parametersSet): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php b/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php index 11377a2062..f25ecbb31b 100644 --- a/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php +++ b/src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php @@ -75,7 +75,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $baseSchema = $this->schema !== null ? clone $this->schema : null; + $baseSchema = $this->schema; if ( $baseSchema !== null @@ -89,7 +89,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema !== null ? clone $baseSchema : null; + $schema = $baseSchema; if ($schema !== null) { foreach ($partitions as $partition) { @@ -212,7 +212,7 @@ public function withReader(ExcelReader $reader): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php b/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php index 642e4fabca..0a8b0bbc4a 100644 --- a/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php +++ b/src/adapter/etl-adapter-google-sheet/src/Flow/ETL/Adapter/GoogleSheet/GoogleSheetExtractor.php @@ -83,7 +83,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); - $schema = $this->schema !== null ? clone $this->schema : null; + $schema = $this->schema; if ($schema !== null && $shouldPutInputIntoRows) { if ($schema->findDefinition('_spread_sheet_id') === null) { @@ -200,7 +200,7 @@ public function withRowsPerPage(int $rowsPerPage): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php index 7c175d84fe..34f6629499 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientDynamicExtractor.php @@ -92,7 +92,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php index 49df972035..4d62331c0c 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientPaginatedExtractor.php @@ -101,7 +101,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php index fec0e43bb6..82a0b86f93 100644 --- a/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php +++ b/src/adapter/etl-adapter-http/src/Flow/ETL/Adapter/Http/PsrHttpClientStaticExtractor.php @@ -90,7 +90,7 @@ public function withPreRequest(callable $preRequest): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php index f9e1942045..0cb43f6e58 100644 --- a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php +++ b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonExtractor.php @@ -48,7 +48,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); $encoder = new JSONEncoder(); - $baseSchema = $this->schema !== null ? clone $this->schema : null; + $baseSchema = $this->schema; if ( $baseSchema !== null @@ -62,7 +62,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema !== null ? clone $baseSchema : null; + $schema = $baseSchema; if ($schema !== null) { foreach ($partitions as $partition) { @@ -150,7 +150,7 @@ public function withPointer(string $pointer, bool $pointerToEntryName = false): public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php index 7a9f611fc5..e592fecb6d 100644 --- a/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php +++ b/src/adapter/etl-adapter-json/src/Flow/ETL/Adapter/JSON/JSONMachine/JsonLinesExtractor.php @@ -49,7 +49,7 @@ public function extract(FlowContext $context): Generator $hydrator = $context->hydrator(); $batchSize = $context->config->extractorBatchSize(); $encoder = new JSONEncoder(); - $baseSchema = $this->schema !== null ? clone $this->schema : null; + $baseSchema = $this->schema; if ( $baseSchema !== null @@ -76,7 +76,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = $baseSchema !== null ? clone $baseSchema : null; + $schema = $baseSchema; if ($schema !== null) { foreach ($partitions as $partition) { @@ -166,7 +166,7 @@ public function withPointer(string $pointer, bool $pointerToEntryName = false): public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php index 65ea6eb10f..ebde047a60 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlCursorExtractor.php @@ -143,7 +143,7 @@ public function withMaximum(int $maximum): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php index 0a82c7d33f..c7aa8021d8 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlKeySetExtractor.php @@ -121,7 +121,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php index 60637885b1..2446ab98e1 100644 --- a/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php +++ b/src/adapter/etl-adapter-postgresql/src/Flow/ETL/Adapter/PostgreSql/PostgreSqlLimitOffsetExtractor.php @@ -115,7 +115,7 @@ public function withPageSize(int $pageSize): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php b/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php index 98f8043dbe..2f0b6685ec 100644 --- a/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php +++ b/src/adapter/etl-adapter-text/src/Flow/ETL/Adapter/Text/TextExtractor.php @@ -48,7 +48,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = clone $baseSchema; + $schema = $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { diff --git a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php index ce9b8556e1..4e73b94d0c 100644 --- a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php +++ b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLParserExtractor.php @@ -116,7 +116,7 @@ public function extract(FlowContext $context): Generator $batchSize = $context->config->extractorBatchSize(); $encoder = new XMLEncoder(); - $baseSchema = $this->schema !== null ? clone $this->schema : schema(xml_schema('node')); + $baseSchema = $this->schema ?? schema(xml_schema('node')); if ($shouldPutInputIntoRows && $baseSchema->findDefinition('_input_file_uri') === null) { $baseSchema = $baseSchema->add(str_schema('_input_file_uri')); @@ -126,7 +126,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = clone $baseSchema; + $schema = $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { @@ -281,7 +281,7 @@ public function withBufferSize(int $bufferSize): self public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } diff --git a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php index 4de1ae9169..edb9cc4edf 100644 --- a/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php +++ b/src/adapter/etl-adapter-xml/src/Flow/ETL/Adapter/XML/XMLReaderExtractor.php @@ -81,7 +81,7 @@ public function extract(FlowContext $context): Generator $streamUri = $shouldPutInputIntoRows ? $stream->path()->uri() : null; $partitions = $stream->path()->partitions(); - $schema = clone $baseSchema; + $schema = $baseSchema; foreach ($partitions as $partition) { if ($schema->findDefinition($partition->name) === null) { 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/Extractor/ArrayExtractor.php b/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php index e6323b64ed..0049eae94f 100644 --- a/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php +++ b/src/core/etl/src/Flow/ETL/Extractor/ArrayExtractor.php @@ -38,7 +38,7 @@ public function extract(FlowContext $context): Generator public function withSchema(Schema $schema): self { - $this->schema = clone $schema; + $this->schema = $schema; return $this; } 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() };