You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add interpretation-stage events: PreInterpretFileEvent and PreQueueRowEvent - #680
Adds two events at the interpretation stage (file → rows), so projects can adjust import rows with a small listener instead of copying a whole interpreter (all shipped interpreters are final, so today even one line of row logic means duplicating the CSV/XLSX/JSON/XML reading code, fileValid(), previewData(), setSettings(), and a Studio UI dynamic type).
PreQueueRowEvent
Dispatched in AbstractInterpreter::processImportRow() for every extracted row, before the delta check, identifier cache, and queueing — i.e. early enough that changed values (including the ID column) affect the resolver, the delta check, and the path/location strategies. Listeners can:
modify the row: $event->setRows([$changedRow]) — e.g. add computed key/path columns for the resolver
skip the row: $event->skipRow() — with skipRow(keepInCleanupIdentifierCache: true) the original row's identifier is still registered, so an active cleanup strategy does not delete/unpublish the row's existing element (an easy trap to fall into otherwise; covered in docs and tests)
fan out the row: $event->setRows([$rowA, $rowB, $rowC]) — each row is queued and imported as its own element (1 source row → N elements is impossible at the mapping stage by construction)
Cross-row state (e.g. SAP-style exports where a group marker row applies to all following rows) works naturally: dispatches are sequential within one interpretFile() run, and PreInterpretFileEvent doubles as a per-run reset signal.
PreInterpretFileEvent
Dispatched at the start of interpretFile() with a settable path: normalize the file (transcode, strip a report preamble, rewrite delimiters) while keeping the standard interpreter.
Studio preview parity
Both events are also dispatched (flagged isPreview()) in the preview/column-header/transformation-preview code paths via a new internal PreviewEventApplier, so the mapping UI shows exactly the columns an actual import produces — listener-added synthetic columns are visible and mappable. Skipped rows stay visible in the preview; a fan-out shows the first resulting row and exposes the columns of all resulting rows.
Wiring & BC
AbstractInterpreter gets a null-guarded setEventDispatcher() (constructor unchanged — no BC break for existing custom interpreters).
The interpreter compiler pass adds the setEventDispatcher call to every tagged interpreter service that supports it, so built-in and custom interpreters dispatch the events.
Without listeners, behavior is unchanged (covered by tests, including the existing CsvEncodingInterpreterTest).
Additional info
Follows the invitation in doc/06_Extending/02_Events.md ("More events to come when needed (just provide PRs ;-)"), which this PR also extends with full documentation of both events including the cleanup-interaction warning.
Concrete motivation: our production project ships three custom interpreters (~840 lines PHP plus a Studio UI module-federation remote) whose only purpose is row filtering, cross-row group-marker carry-forward with synthetic resolver columns, and a wide-to-long pivot (1 row → 3 elements). All three reduce to small PreQueueRowEvent listeners on the stock CSV interpreter with these events.
Tests: PreQueueRowEventTest (event API), InterpreterRowEventTest (end-to-end through CsvFileInterpreter + real QueueService: unchanged baseline, modify, skip, fan-out, file replacement, compiler-pass wiring), PreviewEventApplierTest (preview parity). Verified locally: unit suite green, php-cs-fixer clean, phpstan (repo config) clean.
…wEvent
Listeners can now normalize the source file, and modify, skip, or fan out
extracted rows before they are queued - covering row filtering, cross-row
state, synthetic resolver columns, and 1-to-N imports without writing a
custom interpreter. Both events are mirrored into the Studio preview so
the mapping UI shows the columns an actual import would produce.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One or more custom setup steps configured for this repository failed during this Copilot code review run:
Install dependencies with Composer
Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.
Note
You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.
- Add declare(strict_types=1) to both new event classes.
- PreviewEventApplier: skip dispatch for empty preview records, display the
first fan-out row exactly as queued (no value merging from later rows),
and drop headers for columns no resulting row contains.
- Document that the preview dispatches only the displayed record (stateful
listeners should branch on isPreview()) and that interpreters not
extending AbstractInterpreter must dispatch the events themselves.
- Replace the reflection-based dispatcher-wiring test with a container-level
compiler-pass test (fixes Sonar php:S3011, removes the broad Throwable catch).
- Add end-to-end regression tests for skipRow() cleanup interaction using a
stub load strategy: kept identifier survives cleanup, dropped one does not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract label building out of applyToPreviewData (cognitive complexity),
comment the intentionally empty stub methods, and drop the unused closure
parameter in the empty-record preview test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This checks unresolved service metadata. The pass runs before optimization (src/PimcoreDataImporterBundle.php:54): a tagged child inheriting its class from an untagged parent has no local class, while class: '%app.interpreter_class%' still contains a parameter placeholder. method_exists() therefore checks a service ID or placeholder and silently skips dispatcher injection. These interpreters remain registered, but emit neither event through the base class, unlike Studio's separate preview dispatches. Resolve inherited classes and class parameters before checking the setter, or defer wiring until definitions are resolved, while preserving inherited dispatcher method calls. Add regression cases for both definition forms.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in ca63409: the path returned by applyToPath() now lives in its own $interpretedFilePath, used for fileValid() and previewData(); the JSON-validation cleanup only ever unlinks $previewFilePath, the local copy from PreviewService::getLocalPreviewFile(). No dedicated regression test: PreviewDataService is final readonly with final collaborators and resolves its configuration via Configuration::getByName(), so it cannot be exercised in a unit test without a kernel and database — the separate variable makes the replacement path structurally unreachable for the unlink.
- PreviewDataService: keep a listener's replacement path separate from the
local preview copy, so the JSON-validation cleanup can never delete a file
owned by a PreInterpretFileEvent listener.
- InterpreterConfigurationFactoryPass: resolve the class through parent
definitions and container parameters before checking for
setEventDispatcher(), and skip services that already inherit the call.
Parents are processed first so a child never gets the call twice.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regarding the previously missed finding on InterpreterConfigurationFactoryPass (unresolved inherited classes / class parameters): fixed in ca63409. The pass now walks the ChildDefinition parent chain to find the class and resolves it through the parameter bag before checking for setEventDispatcher(). It also skips services whose chain already carries the call, and processes parents before children so an inherited call is never duplicated. InterpreterCompilerPassTest covers a child inheriting its class from an untagged parent, a %parameter% class, and a tagged parent + child pair (call on the parent only).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes in this pull request
Resolves pimcore/platform-version#340
Adds two events at the interpretation stage (file → rows), so projects can adjust import rows with a small listener instead of copying a whole interpreter (all shipped interpreters are
final, so today even one line of row logic means duplicating the CSV/XLSX/JSON/XML reading code,fileValid(),previewData(),setSettings(), and a Studio UI dynamic type).PreQueueRowEventDispatched in
AbstractInterpreter::processImportRow()for every extracted row, before the delta check, identifier cache, and queueing — i.e. early enough that changed values (including the ID column) affect the resolver, the delta check, and the path/location strategies. Listeners can:$event->setRows([$changedRow])— e.g. add computed key/path columns for the resolver$event->skipRow()— withskipRow(keepInCleanupIdentifierCache: true)the original row's identifier is still registered, so an active cleanup strategy does not delete/unpublish the row's existing element (an easy trap to fall into otherwise; covered in docs and tests)$event->setRows([$rowA, $rowB, $rowC])— each row is queued and imported as its own element (1 source row → N elements is impossible at the mapping stage by construction)Cross-row state (e.g. SAP-style exports where a group marker row applies to all following rows) works naturally: dispatches are sequential within one
interpretFile()run, andPreInterpretFileEventdoubles as a per-run reset signal.PreInterpretFileEventDispatched at the start of
interpretFile()with a settable path: normalize the file (transcode, strip a report preamble, rewrite delimiters) while keeping the standard interpreter.Studio preview parity
Both events are also dispatched (flagged
isPreview()) in the preview/column-header/transformation-preview code paths via a new internalPreviewEventApplier, so the mapping UI shows exactly the columns an actual import produces — listener-added synthetic columns are visible and mappable. Skipped rows stay visible in the preview; a fan-out shows the first resulting row and exposes the columns of all resulting rows.Wiring & BC
AbstractInterpretergets a null-guardedsetEventDispatcher()(constructor unchanged — no BC break for existing custom interpreters).setEventDispatchercall to every tagged interpreter service that supports it, so built-in and custom interpreters dispatch the events.CsvEncodingInterpreterTest).Additional info
doc/06_Extending/02_Events.md("More events to come when needed (just provide PRs ;-)"), which this PR also extends with full documentation of both events including the cleanup-interaction warning.PreQueueRowEventlisteners on the stock CSV interpreter with these events.PreQueueRowEventTest(event API),InterpreterRowEventTest(end-to-end throughCsvFileInterpreter+ realQueueService: unchanged baseline, modify, skip, fan-out, file replacement, compiler-pass wiring),PreviewEventApplierTest(preview parity). Verified locally: unit suite green,php-cs-fixerclean,phpstan(repo config) clean.🤖 Generated with Claude Code