Skip to content

Restored 'text', 'text_long', 'file', 'image' field-handler semantics and added type-coverage safety net.#360

Merged
AlexSkrypnyk merged 1 commit intomasterfrom
feature/field-coverage
Apr 28, 2026
Merged

Restored 'text', 'text_long', 'file', 'image' field-handler semantics and added type-coverage safety net.#360
AlexSkrypnyk merged 1 commit intomasterfrom
feature/field-coverage

Conversation

@AlexSkrypnyk
Copy link
Copy Markdown
Collaborator

Summary

The v3 field-handler registry introduced a loud-failure policy in DefaultHandler for multi-column field types, but shipped without handlers for text and text_long - both multi-column (value/format) types. This caused an immediate regression for any consumer that stubbed those fields. FileHandler and ImageHandler also lost the 2.x reuse behaviour: they always uploaded a new managed file instead of looking up an existing one by URI or basename, breaking assertions that relied on a stable file ID after pre-creating a managed file. This PR closes both gaps and adds a safety net that prevents the same class of regression from shipping silently in future.

Stack

This PR is #3 of 4 in a stacked series:

Merge in order. Depends on #359.

Changes

New handlers - text / text_long

  • Added TextHandler and TextLongHandler - minimal pass-through implementations of FieldHandlerInterface. Both auto-register via the directory scan in Core::registerDefaultFieldHandlers().
  • Unit tests (TextHandlerTest, TextLongHandlerTest) and kernel round-trip tests (TextHandlerKernelTest, TextLongHandlerKernelTest).

FileHandler reuse refactor

  • Extracted resolveExistingFile(string $value): ?object - looks up a managed file by URI (scheme present) or by bare basename (probes public:// then private://). Absolute disk paths (contain / but no scheme) fall through unchanged.
  • Extracted uploadAndSave(string $file_path): object - the original upload-and-rename path, now a named method.
  • expand() delegates: $this->resolveExistingFile($file_path) ?? $this->uploadAndSave($file_path).
  • Unit tests: three new cases covering URI reuse, public-basename reuse, private-basename fallback. Existing upload-path tests updated to register the new entity_type.manager stub alongside file.repository.
  • Kernel test: FileHandlerReuseKernelTest verifies the round-trip - pre-create a file, reference it by URI or basename, assert same target_id and exactly one file entity in storage.

ImageHandler subclass

  • ImageHandler now extends FileHandler (previously extended AbstractHandler). expand() calls the inherited resolveExistingFile/uploadAndSave pair, then wraps the result in the image-specific target_id/alt/title shape.
  • Unit and kernel reuse tests added (ImageHandlerReuseKernelTest).

Coverage safety net - FieldTypeCoverageKernelTest

  • Enumerates every field_type plugin in the loaded install.
  • Asserts each type is: (a) backed by a registered handler, (b) DefaultHandler-safe (single value column), or (c) listed in a documented SKIP map.
  • A new core type without a handler or SKIP entry fails immediately, surfacing the gap before it reaches consumers.
  • Companion assertion: testDefaultHandlerIsNotRegistered() guards against DefaultHandler being inadvertently added to the registry (which would mask gaps in the coverage check itself).

README / field-handler table

  • H2 row: added text and text_long alongside existing compound types.
  • H4 row: documented the reuse-before-upload behaviour for FileHandler/ImageHandler.
  • New "Handler-coverage safety net" section describing FieldTypeCoverageKernelTest.

Before / After

# text_long field before this PR

scenario stub -> Core::expandEntityFields()
  -> registry miss (no TextLongHandler)
  -> DefaultHandler::expand()
  -> throws: "DefaultHandler cannot handle multi-column type 'text_long'"

# text_long field after this PR

scenario stub -> Core::expandEntityFields()
  -> registry hit: TextLongHandler
  -> TextLongHandler::expand() -> (array) $values
  -> value stored intact
# file field (pre-created managed file) before this PR

scenario stub 'public://logo.png'
  -> FileHandler::expand()
  -> file_get_contents('public://logo.png')
  -> file.repository->writeData(...) -> NEW managed file (id=42)
  -> assertion: file id == 7 -> FAIL

# file field (pre-created managed file) after this PR

scenario stub 'public://logo.png'
  -> FileHandler::expand()
  -> resolveExistingFile('public://logo.png') -> file entity id=7
  -> assertion: file id == 7 -> PASS

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Warning

Rate limit exceeded

@AlexSkrypnyk has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 48 minutes and 7 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b063ae35-943a-4da5-8cfe-8d82602dacce

📥 Commits

Reviewing files that changed from the base of the PR and between ff6efb1 and 2e3e4c3.

📒 Files selected for processing (14)
  • src/Drupal/Driver/Core/Field/FileHandler.php
  • src/Drupal/Driver/Core/Field/ImageHandler.php
  • src/Drupal/Driver/Core/Field/README.md
  • src/Drupal/Driver/Core/Field/TextHandler.php
  • src/Drupal/Driver/Core/Field/TextLongHandler.php
  • tests/Drupal/Tests/Driver/Kernel/Core/Field/FieldTypeCoverageKernelTest.php
  • tests/Drupal/Tests/Driver/Kernel/Core/Field/FileHandlerReuseKernelTest.php
  • tests/Drupal/Tests/Driver/Kernel/Core/Field/ImageHandlerReuseKernelTest.php
  • tests/Drupal/Tests/Driver/Kernel/Core/Field/TextHandlerKernelTest.php
  • tests/Drupal/Tests/Driver/Kernel/Core/Field/TextLongHandlerKernelTest.php
  • tests/Drupal/Tests/Driver/Unit/Core/Field/FileHandlerTest.php
  • tests/Drupal/Tests/Driver/Unit/Core/Field/ImageHandlerTest.php
  • tests/Drupal/Tests/Driver/Unit/Core/Field/TextHandlerTest.php
  • tests/Drupal/Tests/Driver/Unit/Core/Field/TextLongHandlerTest.php
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/field-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from feature/field-classifier to master April 28, 2026 08:14
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/field-coverage branch from 51b0797 to 2e3e4c3 Compare April 28, 2026 08:16
@github-actions
Copy link
Copy Markdown

Code coverage (threshold: 95%)



Code Coverage Report Summary:
  Classes: 88.46% (23/26)
  Methods: 96.11% (173/180)
  Lines:   99.00% (890/899)

Per-class coverage
Drupal\Driver\BlackboxDriver                                 100.00%
Drupal\Driver\Core\Core                                       99.38%
Drupal\Driver\Core\Field\AbstractHandler                     100.00%
Drupal\Driver\Core\Field\AddressHandler                      100.00%
Drupal\Driver\Core\Field\BooleanHandler                      100.00%
Drupal\Driver\Core\Field\DaterangeHandler                    100.00%
Drupal\Driver\Core\Field\DatetimeHandler                     100.00%
Drupal\Driver\Core\Field\DefaultHandler                      100.00%
Drupal\Driver\Core\Field\EmbridgeAssetItemHandler              0.00%
Drupal\Driver\Core\Field\EntityReferenceHandler              100.00%
Drupal\Driver\Core\Field\EntityReferenceRevisionsHandler      91.11%
Drupal\Driver\Core\Field\FieldClassifier                      94.44%
Drupal\Driver\Core\Field\FileHandler                         100.00%
Drupal\Driver\Core\Field\ImageHandler                        100.00%
Drupal\Driver\Core\Field\LinkHandler                         100.00%
Drupal\Driver\Core\Field\ListFloatHandler                      0.00%
Drupal\Driver\Core\Field\ListHandlerBase                     100.00%
Drupal\Driver\Core\Field\ListIntegerHandler                    0.00%
Drupal\Driver\Core\Field\ListStringHandler                     0.00%
Drupal\Driver\Core\Field\NameHandler                         100.00%
Drupal\Driver\Core\Field\OgStandardReferenceHandler            0.00%
Drupal\Driver\Core\Field\SupportedImageHandler               100.00%
Drupal\Driver\Core\Field\TextHandler                         100.00%
Drupal\Driver\Core\Field\TextLongHandler                     100.00%
Drupal\Driver\Core\Field\TextWithSummaryHandler              100.00%
Drupal\Driver\Core\Field\TimeHandler                         100.00%
Drupal\Driver\DrupalDriver                                   100.00%
Drupal\Driver\DrushDriver                                    100.00%
Drupal\Driver\Exception\BootstrapException                   100.00%
Drupal\Driver\Exception\Exception                            100.00%
Drupal\Driver\Exception\UnsupportedDriverActionException     100.00%

@AlexSkrypnyk AlexSkrypnyk merged commit 32ca8d3 into master Apr 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant