diff --git a/composer.json b/composer.json index 86c10689..08891a88 100644 --- a/composer.json +++ b/composer.json @@ -37,6 +37,7 @@ "drupal/core-composer-scaffold": "^11.2", "drupal/core-recommended": "^11.2", "drupal/date_recur": "^3.6", + "drupal/driver_field_test": "*", "drupal/mailsystem": "^4.4", "drupal/name": "^1.2", "drupal/smart_date": "^4.2", @@ -57,6 +58,10 @@ "drupal": { "type": "composer", "url": "https://packages.drupal.org/8" + }, + "driver_field_test": { + "type": "path", + "url": "tests/fixtures/modules/driver_field_test" } }, "minimum-stability": "beta", diff --git a/src/Drupal/Driver/Core/Core.php b/src/Drupal/Driver/Core/Core.php index 0b4e5b99..03ac9255 100644 --- a/src/Drupal/Driver/Core/Core.php +++ b/src/Drupal/Driver/Core/Core.php @@ -15,6 +15,8 @@ use Drupal\Driver\Core\Field\FieldClassifier; use Drupal\Driver\Core\Field\FieldClassifierInterface; use Drupal\Driver\Core\Field\FieldHandlerInterface; +use Drupal\Driver\Core\Field\FieldShapeClassifier; +use Drupal\Driver\Core\Field\FieldShapeClassifierInterface; use Drupal\Driver\Core\Alias\AuthorAlias; use Drupal\Driver\Core\Alias\ParentTermAlias; use Drupal\Driver\Core\Alias\VocabularyMachineNameAlias; @@ -84,6 +86,11 @@ class Core implements CoreInterface, CreationAliasCapabilityInterface { */ protected ?FieldClassifierInterface $fieldClassifier = NULL; + /** + * Lazily created field shape classifier instance. + */ + protected ?FieldShapeClassifierInterface $fieldShapeClassifier = NULL; + /** * Set up the Core implementation. * @@ -232,6 +239,28 @@ public function getFieldClassifier(): FieldClassifierInterface { return $this->fieldClassifier; } + /** + * Creates the field shape classifier instance for this Core. + * + * Subclasses override this method when they ship a version-specific value + * shape classifier. The default returns the base 'FieldShapeClassifier' which + * covers Drupal 10 and 11. + */ + protected function createFieldShapeClassifier(): FieldShapeClassifierInterface { + return new FieldShapeClassifier(); + } + + /** + * {@inheritdoc} + */ + public function getFieldShapeClassifier(): FieldShapeClassifierInterface { + if (!$this->fieldShapeClassifier instanceof FieldShapeClassifierInterface) { + $this->fieldShapeClassifier = $this->createFieldShapeClassifier(); + } + + return $this->fieldShapeClassifier; + } + /** * {@inheritdoc} */ @@ -243,11 +272,66 @@ public function getFieldHandler(EntityStubInterface $stub, string $entity_type, throw new \RuntimeException(sprintf('Field "%s" not found on entity type "%s".', $field_name, $entity_type)); } - $class = $this->fieldHandlers[$field_types[$field_name]] ?? DefaultHandler::class; + $field_type = $field_types[$field_name]; + $class = $this->fieldHandlers[$field_type] ?? DefaultHandler::class; + + if ($class === DefaultHandler::class) { + $this->assertDefaultHandlerCanMarshal($entity_type, $field_name, $field_type, $bundle); + } return new $class($stub, $entity_type, $field_name); } + /** + * Rejects a field the DefaultHandler fallback cannot marshal. + * + * Consulted only when no dedicated handler is registered for the field type. + * Delegates the value-shape decision to the field shape classifier and, when + * it reports the field is an entity reference or a complex/nested value, + * throws an actionable exception naming the field and why the default cannot + * relay it. + * + * @param string $entity_type + * The entity type ID. + * @param string $field_name + * The field name. + * @param string $field_type + * The field type ID, for the exception message. + * @param string $bundle + * The bundle name, for the exception message. + * + * @throws \RuntimeException + * When the field's stored shape is not one the default handler can relay. + */ + protected function assertDefaultHandlerCanMarshal(string $entity_type, string $field_name, string $field_type, string $bundle): void { + $storage = $this->getEntityFieldManager()->getFieldStorageDefinitions($entity_type)[$field_name] ?? NULL; + + if ($storage === NULL) { + return; + } + + $shape = $this->getFieldShapeClassifier(); + $reason = NULL; + + if ($shape->fieldIsEntityReference($storage)) { + $reason = 'it is an entity-reference value a dedicated handler must resolve to an id'; + } + elseif ($shape->fieldIsComplexValue($storage)) { + $reason = 'it holds a complex or nested value with no single scalar shape'; + } + + if ($reason !== NULL) { + throw new \RuntimeException(sprintf( + 'No dedicated handler is registered for field "%s" (type "%s") on entity type "%s" bundle "%s", and DefaultHandler cannot marshal it: %s. Register a dedicated handler via Core::registerFieldHandler().', + $field_name, + $field_type, + $entity_type, + $bundle, + $reason, + )); + } + } + /** * Expands values on the given stub through the field-handler pipeline. * diff --git a/src/Drupal/Driver/Core/CoreInterface.php b/src/Drupal/Driver/Core/CoreInterface.php index 34edc8c2..324b236c 100644 --- a/src/Drupal/Driver/Core/CoreInterface.php +++ b/src/Drupal/Driver/Core/CoreInterface.php @@ -19,6 +19,7 @@ use Drupal\Driver\Capability\WatchdogCapabilityInterface; use Drupal\Driver\Core\Field\FieldClassifierInterface; use Drupal\Driver\Core\Field\FieldHandlerInterface; +use Drupal\Driver\Core\Field\FieldShapeClassifierInterface; use Drupal\Driver\Entity\EntityStubInterface; /** @@ -150,4 +151,16 @@ public function getEntityFieldTypes(string $entity_type, ?string $bundle = NULL) */ public function getFieldClassifier(): FieldClassifierInterface; + /** + * Returns the field shape classifier, lazily instantiating on first access. + * + * Consumers call into the field shape classifier to ask a field's stored + * value shape - whether it is an entity reference or a complex/nested value - + * during handler selection. See 'src/Drupal/Driver/Core/Field/README.md'. + * + * @return \Drupal\Driver\Core\Field\FieldShapeClassifierInterface + * The field shape classifier instance. + */ + public function getFieldShapeClassifier(): FieldShapeClassifierInterface; + } diff --git a/src/Drupal/Driver/Core/Field/ColorFieldTypeHandler.php b/src/Drupal/Driver/Core/Field/ColorFieldTypeHandler.php index 893930e7..66b42e47 100644 --- a/src/Drupal/Driver/Core/Field/ColorFieldTypeHandler.php +++ b/src/Drupal/Driver/Core/Field/ColorFieldTypeHandler.php @@ -12,6 +12,13 @@ * formatting and the opacity-disabled case, so the handler only relays the * multi-column records through. * + * @deprecated in drupal-driver:3.x and is removed from drupal-driver:4.0.0. + * The 'color'/'opacity' columns are plain scalars the generic DefaultHandler + * now relays, so this pass-through handler is redundant. It is retained for + * consumers that extend or reference it. Register a dedicated handler only + * for a field type whose author-facing input differs from its stored value. + * + * @see \Drupal\Driver\Core\Field\DefaultHandler * @see https://www.drupal.org/project/color_field */ class ColorFieldTypeHandler extends AbstractHandler { diff --git a/src/Drupal/Driver/Core/Field/DefaultHandler.php b/src/Drupal/Driver/Core/Field/DefaultHandler.php index 13ec88ff..44bd123d 100644 --- a/src/Drupal/Driver/Core/Field/DefaultHandler.php +++ b/src/Drupal/Driver/Core/Field/DefaultHandler.php @@ -5,11 +5,18 @@ namespace Drupal\Driver\Core\Field; /** - * Fallback handler for field types that have no dedicated handler. + * Fallback handler for field types with no dedicated handler. * - * Only correct for H1 (single-column scalar) fields. See - * 'src/Drupal/Driver/Core/Field/README.md' for the full handler-selection - * table and the loud-failure policy this class enforces. + * Relays the normalised records to storage verbatim. It is the resolved + * handler for any field type without a registered handler class. 'Core' asks + * the field shape classifier whether the field is a plain scalar before it + * falls back here (see 'FieldShapeClassifierInterface') and rejects a field + * this handler cannot marshal - an entity-reference target or a complex/nested + * value - so by the time this handler runs the field is known to be a + * plain-scalar shape safe to pass through. + * + * See 'src/Drupal/Driver/Core/Field/README.md' for the full handler-selection + * table. */ class DefaultHandler extends AbstractHandler { @@ -17,20 +24,6 @@ class DefaultHandler extends AbstractHandler { * {@inheritdoc} */ protected function doExpand(array $records): array { - $columns = $this->fieldInfo->getColumns(); - - if (count($columns) !== 1 || !array_key_exists('value', $columns)) { - throw new \RuntimeException(sprintf( - 'No dedicated handler is registered for field "%s" (type "%s") on entity type "%s" bundle "%s", and DefaultHandler cannot marshal it: the field has %d column(s) (%s) and DefaultHandler only supports single-column scalar fields keyed by "value". Implement a dedicated handler for this field type and register it via Core::registerFieldHandler().', - $this->fieldInfo->getName(), - $this->fieldInfo->getType(), - $this->fieldInfo->getTargetEntityTypeId(), - $this->fieldConfig->getTargetBundle() ?? '(none)', - count($columns), - implode(', ', array_keys($columns)), - )); - } - return $records; } diff --git a/src/Drupal/Driver/Core/Field/FieldShapeClassifier.php b/src/Drupal/Driver/Core/Field/FieldShapeClassifier.php new file mode 100644 index 00000000..550205e1 --- /dev/null +++ b/src/Drupal/Driver/Core/Field/FieldShapeClassifier.php @@ -0,0 +1,66 @@ +storedProperties($storage) as $definition) { + if ($definition instanceof DataReferenceTargetDefinition) { + return TRUE; + } + } + + return FALSE; + } + + /** + * {@inheritdoc} + */ + public function fieldIsComplexValue(FieldStorageDefinitionInterface $storage): bool { + foreach ($this->storedProperties($storage) as $definition) { + if ($definition instanceof ComplexDataDefinitionInterface || $definition instanceof ListDataDefinitionInterface) { + return TRUE; + } + } + + return FALSE; + } + + /** + * Yields a field's stored (non-computed) property definitions. + * + * Computed properties are storage-derived, not author-supplied, so they never + * bear on whether the caller can express the field as a plain scalar. + * + * @param \Drupal\Core\Field\FieldStorageDefinitionInterface $storage + * The field storage definition to inspect. + * + * @return iterable<\Drupal\Core\TypedData\DataDefinitionInterface> + * The stored property definitions. + */ + protected function storedProperties(FieldStorageDefinitionInterface $storage): iterable { + foreach ($storage->getPropertyDefinitions() as $definition) { + if (!$definition->isComputed()) { + yield $definition; + } + } + } + +} diff --git a/src/Drupal/Driver/Core/Field/FieldShapeClassifierInterface.php b/src/Drupal/Driver/Core/Field/FieldShapeClassifierInterface.php new file mode 100644 index 00000000..7552d36f --- /dev/null +++ b/src/Drupal/Driver/Core/Field/FieldShapeClassifierInterface.php @@ -0,0 +1,53 @@ + ...]` shape for all single-column scalars. | -| H2 | Multi-column compound | `text`, `text_long`, `text_with_summary`, `link`, `address`, `daterange` | Typed handler required (`TextHandler`, `TextLongHandler`, `TextWithSummaryHandler`, `LinkHandler`, `AddressHandler`, `DaterangeHandler`) | `DefaultHandler` must throw if it is invoked on a field type outside H1. See "DefaultHandler loud-failure policy" below. | +| H1 | Plain-scalar columns | `string`, `integer`, `boolean`, `float`, `decimal`, `email`, `telephone`, `uri`, `timestamp`, `created`, `changed` | `DefaultHandler` | Relayed verbatim by `DefaultHandler`. `Core` gates the fallback on the classifier, so an unhandled reference or complex field throws instead. | +| H2 | Multi-column compound | `text`, `text_long`, `text_with_summary`, `link`, `address`, `daterange` | `LinkHandler`, `AddressHandler`, `DaterangeHandler`; `TextHandler`/`TextLongHandler`/`TextWithSummaryHandler` (deprecated) | `link`/`address`/`daterange` transform the compound value. The `text*` columns are plain scalars the `DefaultHandler` now relays, so their handlers are deprecated for 4.0.0. See "DefaultHandler classification policy" below. | | H3 | Simple datetime | `datetime` | `DatetimeHandler` | Parses human date strings to ISO 8601 storage shape. | | H4 | Entity reference (single target) | `entity_reference`, `file`, `image` | `EntityReferenceHandler`, `FileHandler`, `ImageHandler` | Resolve human-readable label/path/filename to `target_id`. `FileHandler`/`ImageHandler` first try to reuse an existing managed file at the given URI or bare basename (searching `public://` and `private://`) before falling back to uploading a new file under `public://.`. | | H5 | Entity reference with revision | `entity_reference_revisions` (paragraphs) | `EntityReferenceRevisionsHandler` | Composite `target_id` + `target_revision_id`. Resolves target and auto-populates the current revision id. | @@ -48,7 +48,7 @@ field-type string returned by `FieldDefinitionInterface::getType()`. | H9 | Organic Groups reference (contrib) | `og_standard_reference` | `OgStandardReferenceHandler` | OG-specific lookup. | | H10 | Embedded asset reference (contrib) | `embridge_asset_item` | `EmbridgeAssetItemHandler` | Embridge-specific shape. | | H11 | Smart date range (contrib) | `smartdate` | `SmartdateHandler` | Six-column timestamp range with auto-derived duration; accepts numeric Unix timestamps or `strtotime()` strings. | -| H12 | Color with opacity (contrib) | `color_field_type` | `ColorFieldTypeHandler` | Two columns (`color` hex + optional `opacity` float); `preSave()` owns hex formatting, so the handler relays records unchanged. | +| H12 | Color with opacity (contrib) | `color_field_type` | `ColorFieldTypeHandler` (deprecated) | Two plain-scalar columns (`color` hex + optional `opacity` float) the `DefaultHandler` now relays; `preSave()` owns hex formatting. Handler deprecated for 4.0.0. | | H13 | Recurring date (contrib) | `date_recur` | `DateRecurHandler` | Five columns (`value`/`end_value`/`rrule`/`timezone`/`infinite`); dates stored verbatim in the record's own timezone and `preSave()` derives `infinite`, so the handler relays records unchanged. | ## Cardinality @@ -57,40 +57,63 @@ Independent of resolution. Handlers must accept either a scalar or an array. Internally, they normalize a scalar to `[$scalar]` before returning the storage shape. No category in the primary table changes behavior based on cardinality. -## DefaultHandler loud-failure policy +## DefaultHandler classification policy `DefaultHandler` is the fallback when no typed handler matches a field's type -string. It runs `(array) $value`, which is only correct for H1 (single-column -scalars). For H2-H10 it would silently produce a malformed storage shape that -the entity layer then persists as broken data (entity reference by string -instead of id, datetime stored as raw user input, address fields left null, -etc.). - -**`DefaultHandler` loudly fails when invoked on a field type outside H1.** - -Detection criterion: the field's storage definition has exactly one column (the -canonical `value` column). If the field has multiple columns or its single -column is not named `value`, `DefaultHandler::expand()` throws a clearly-worded -exception identifying the field name, entity type, bundle, and field-type -string, and stating that a dedicated handler must be implemented for this -field type. The error is a direct call to action: implement the handler (or -register one), then re-run. - -In typical scenarios (node title, boolean status, integer counters, etc.) -nothing changes - `DefaultHandler` works as today. In edge cases where a user -stubs a compound field that has no registered handler, they get an immediate, -actionable error instead of silently corrupted data downstream. +string. It is a pure pass-through: it relays the normalised records to storage +verbatim. Deciding whether that is safe is not its job - `Core` consults the +field shape classifier first. + +Value shape (scalar vs entity-reference vs complex) is orthogonal to the F-row +(origin/storage) axis `FieldClassifier` owns, so it has its own classifier. +Before `getFieldHandler()` falls back to `DefaultHandler`, `Core` calls +`FieldShapeClassifierInterface` on the field's storage definition. It reads only +the stored (non-computed) property definitions and stays deliberately generic - +it enumerates no field types or data-type strings. `Core` throws when either +predicate reports one of the two shapes the generic type system says cannot be +authored as a plain scalar: + +- **Entity-reference target** (`fieldIsEntityReference()` - a + `DataReferenceTargetDefinition`, e.g. `target_id`): the caller supplies a + label, path, or name that must be resolved to an id the author cannot know. +- **Complex or nested value** (`fieldIsComplexValue()` - a + `ComplexDataDefinitionInterface` such as a `map`, or a + `ListDataDefinitionInterface`): there is no single scalar shape to relay. + +Everything else - including datetime strings, booleans, and list keys - is a +scalar the default relays as-is. A field whose author-facing input differs from +its stored scalar (a timezone-relative date, a boolean label, an allowed-value +label, split name/address components) is served by a dedicated handler that +performs the translation; that knowledge lives in the handlers, never in the +default or either classifier. + +The check is proactive, not try/catch. Both rejected shapes fail silently - a +label persisted as a bogus id, a nested value flattened - so `Core` refuses the +field at handler resolution with an exception identifying the field name, type, +entity type, bundle, and why the default cannot relay it, rather than after a +corrupt save. The error is a direct call to action: register a dedicated +handler, then re-run. + +### Deprecated pass-through handlers + +`TextHandler`, `TextLongHandler`, `TextWithSummaryHandler`, and +`ColorFieldTypeHandler` handle field types whose columns are plain scalars the +generic `DefaultHandler` now relays, so they are redundant. They remain +registered and functional but are `@deprecated` for removal in +`drupal-driver:4.0.0`, kept only for consumers that extend or reference them. On +removal, their field types fall through to `DefaultHandler` with no change in +behaviour. ## Handler-coverage safety net `FieldTypeCoverageKernelTest` enumerates every field-type plugin the loaded Drupal install exposes and asserts that each one is either (a) backed by a -registered handler, (b) schema-compatible with `DefaultHandler` (single -`value` column), or (c) listed in the test's `SKIP` map with a documented -reason (computed, write-only, composite-lifecycle, etc.). Adding a new core -field type without a handler or a SKIP entry fails that test, preventing -the type from silently falling through to `DefaultHandler` and blowing up -the first time a scenario references it. +registered handler, (b) default-safe per the field shape classifier +(`FieldShapeClassifierInterface` - no entity-reference target or complex/nested +property), or (c) listed in the test's `SKIP` map with a documented reason +(computed, write-only, composite-lifecycle, etc.). Adding a new core field type +that needs translation without a handler or a SKIP entry fails that test, +preventing the type from silently falling through to `DefaultHandler`. ## What each resolution actually means in code @@ -157,9 +180,11 @@ protected function createFieldClassifier(): FieldClassifierInterface { } ``` -The default implementation returns the base `FieldClassifier`. The pattern -mirrors `registerDefaultFieldHandlers()`, which likewise allows subclasses to -extend handler registration per version. +The default implementation returns the base `FieldClassifier`. The value-shape +classifier follows the identical pattern - `Core::createFieldShapeClassifier()` +returns the base `FieldShapeClassifier`, overridable by a +`Core{N}\Field\FieldShapeClassifier`. Both mirror `registerDefaultFieldHandlers()`, +which likewise allows subclasses to extend registration per version. ## Pipeline walk-through diff --git a/src/Drupal/Driver/Core/Field/TextHandler.php b/src/Drupal/Driver/Core/Field/TextHandler.php index 54ffad54..6bac2457 100644 --- a/src/Drupal/Driver/Core/Field/TextHandler.php +++ b/src/Drupal/Driver/Core/Field/TextHandler.php @@ -6,6 +6,14 @@ /** * Field handler for 'text' fields. + * + * @deprecated in drupal-driver:3.x and is removed from drupal-driver:4.0.0. + * The 'text' columns are plain scalars the generic DefaultHandler now relays, + * so this pass-through handler is redundant. It is retained for consumers + * that extend or reference it. Register a dedicated handler only for a field + * type whose author-facing input differs from its stored value. + * + * @see \Drupal\Driver\Core\Field\DefaultHandler */ class TextHandler extends AbstractHandler { diff --git a/src/Drupal/Driver/Core/Field/TextLongHandler.php b/src/Drupal/Driver/Core/Field/TextLongHandler.php index 670b5581..9cc1ead3 100644 --- a/src/Drupal/Driver/Core/Field/TextLongHandler.php +++ b/src/Drupal/Driver/Core/Field/TextLongHandler.php @@ -6,6 +6,14 @@ /** * Field handler for 'text_long' fields. + * + * @deprecated in drupal-driver:3.x and is removed from drupal-driver:4.0.0. + * The 'text_long' columns are plain scalars the generic DefaultHandler now + * relays, so this pass-through handler is redundant. It is retained for + * consumers that extend or reference it. Register a dedicated handler only + * for a field type whose author-facing input differs from its stored value. + * + * @see \Drupal\Driver\Core\Field\DefaultHandler */ class TextLongHandler extends AbstractHandler { diff --git a/src/Drupal/Driver/Core/Field/TextWithSummaryHandler.php b/src/Drupal/Driver/Core/Field/TextWithSummaryHandler.php index 69bc3f17..0c69dbe9 100644 --- a/src/Drupal/Driver/Core/Field/TextWithSummaryHandler.php +++ b/src/Drupal/Driver/Core/Field/TextWithSummaryHandler.php @@ -6,6 +6,15 @@ /** * Field handler for 'text_with_summary' fields. + * + * @deprecated in drupal-driver:3.x and is removed from drupal-driver:4.0.0. + * The 'text_with_summary' columns are plain scalars the generic + * DefaultHandler now relays, so this pass-through handler is redundant. It is + * retained for consumers that extend or reference it. Register a dedicated + * handler only for a field type whose author-facing input differs from its + * stored value. + * + * @see \Drupal\Driver\Core\Field\DefaultHandler */ class TextWithSummaryHandler extends AbstractHandler { diff --git a/tests/Drupal/Tests/Driver/Kernel/Core/Field/CustomModuleFieldKernelTest.php b/tests/Drupal/Tests/Driver/Kernel/Core/Field/CustomModuleFieldKernelTest.php new file mode 100644 index 00000000..60feaefc --- /dev/null +++ b/tests/Drupal/Tests/Driver/Kernel/Core/Field/CustomModuleFieldKernelTest.php @@ -0,0 +1,70 @@ + + */ + protected static $modules = [ + ...self::BASE_MODULES, + 'driver_field_test', + ]; + + /** + * Tests a custom plain-scalar field with no handler rides the fallback. + */ + public function testScalarFieldWithoutHandlerRoundTrips(): void { + $this->attachField('field_scalar', 'driver_test_scalar'); + + $this->assertFieldRoundTripViaDriver('field_scalar', [ + ['value' => 'a plain value', 'weight' => 5], + ]); + } + + /** + * Tests a custom entity-reference field with no handler is refused. + */ + public function testReferenceFieldWithoutHandlerIsRejected(): void { + $this->attachField('field_ref', 'driver_test_reference'); + + $stub = new EntityStub(self::ENTITY_TYPE, self::BUNDLE, [ + 'name' => 'test entity', + 'field_ref' => [['target_id' => 1]], + ]); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageMatches('/No dedicated handler is registered.*entity-reference/s'); + + $this->core->entityCreate($stub); + } + +} diff --git a/tests/Drupal/Tests/Driver/Kernel/Core/Field/FieldTypeCoverageKernelTest.php b/tests/Drupal/Tests/Driver/Kernel/Core/Field/FieldTypeCoverageKernelTest.php index fff0f7cd..ea58b676 100644 --- a/tests/Drupal/Tests/Driver/Kernel/Core/Field/FieldTypeCoverageKernelTest.php +++ b/tests/Drupal/Tests/Driver/Kernel/Core/Field/FieldTypeCoverageKernelTest.php @@ -16,9 +16,9 @@ * that each one is either: * (a) backed by a dedicated handler registered with Core (the registry map * has an entry for this type), or - * (b) safe for DefaultHandler - its storage schema declares exactly one - * column named 'value', which is the only shape DefaultHandler can - * marshal, or + * (b) safe for DefaultHandler per the field shape classifier - every stored + * property is a plain scalar it can relay verbatim (no entity-reference + * target or complex/nested value), or * (c) documented in the SKIP map with a reason (computed, write-only, or * otherwise not stub-expansion-compatible). * @@ -119,29 +119,28 @@ private function isHandlerRegistered(string $type): bool { } /** - * Returns TRUE when the field type's schema matches DefaultHandler's shape. + * Returns TRUE when the field type can ride DefaultHandler's pass-through. * - * DefaultHandler only marshals fields whose storage schema declares exactly - * one column named 'value'. Any other shape triggers a loud throw at - * expand() time, meaning the type needs a dedicated handler. + * DefaultHandler relays a field verbatim only when every stored property is + * a plain scalar. The field shape classifier flags an entity-reference target + * or a complex/nested value, and Core throws for those when it would + * otherwise fall back to the default. */ private function isDefaultHandlerSafe(string $type): bool { try { $storage = BaseFieldDefinition::create($type); - $plugin_class = \Drupal::service('plugin.manager.field.field_type')->getPluginClass($type); - $schema = $plugin_class::schema($storage); + $shape = $this->core->getFieldShapeClassifier(); + $unsafe = $shape->fieldIsEntityReference($storage) || $shape->fieldIsComplexValue($storage); } catch (\Throwable) { - // Schema construction fails for types that require settings we haven't + // Property construction fails for types that require settings we haven't // supplied (e.g. entity_reference without target_type). Treat those as - // unsafe: if DefaultHandler cannot reason about the schema, neither + // unsafe: if the classifier cannot reason about the properties, neither // can this coverage test, and a dedicated handler is the right answer. return FALSE; } - $columns = $schema['columns'] ?? []; - - return count($columns) === 1 && array_key_exists('value', $columns); + return !$unsafe; } /** diff --git a/tests/Drupal/Tests/Driver/Unit/Core/CoreFieldHandlerLookupTest.php b/tests/Drupal/Tests/Driver/Unit/Core/CoreFieldHandlerLookupTest.php index 37ac01d3..c1fa6612 100644 --- a/tests/Drupal/Tests/Driver/Unit/Core/CoreFieldHandlerLookupTest.php +++ b/tests/Drupal/Tests/Driver/Unit/Core/CoreFieldHandlerLookupTest.php @@ -144,6 +144,10 @@ protected function setUpDrupalContainer(): void { $storage_definition = $this->createMock(FieldStorageDefinitionInterface::class); $storage_definition->method('getType')->willReturn('string'); + // Consulted by Core's classifier gate when a field type falls back to + // DefaultHandler; a plain scalar (no properties) keeps the field + // default-expandable. + $storage_definition->method('getPropertyDefinitions')->willReturn([]); $entity_field_manager = $this->createMock(EntityFieldManagerInterface::class); $entity_field_manager->method('getFieldStorageDefinitions') diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/ColorFieldTypeHandlerTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/ColorFieldTypeHandlerTest.php deleted file mode 100644 index 0b407d7d..00000000 --- a/tests/Drupal/Tests/Driver/Unit/Core/Field/ColorFieldTypeHandlerTest.php +++ /dev/null @@ -1,82 +0,0 @@ -newInstanceWithoutConstructor(); - - $property = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); - $property->setValue($handler, 'color'); - - return $handler; - } - - /** - * {@inheritdoc} - */ - public static function dataProviderExpand(): \Iterator { - yield 'bare scalar maps to color column' => [ - '#1A2B3C', - [['color' => '#1A2B3C']], - NULL, - NULL, - ]; - yield 'list of scalars is multiple color deltas' => [ - ['#1A2B3C', '#FFFFFF'], - [['color' => '#1A2B3C'], ['color' => '#FFFFFF']], - NULL, - NULL, - ]; - yield 'single record with color and opacity' => [ - ['color' => '#1A2B3C', 'opacity' => 0.5], - [['color' => '#1A2B3C', 'opacity' => 0.5]], - NULL, - NULL, - ]; - yield 'opacity omitted when not supplied' => [ - ['color' => '#1A2B3C'], - [['color' => '#1A2B3C']], - NULL, - NULL, - ]; - yield 'list of records' => [ - [['color' => '#1A2B3C', 'opacity' => 0.5], ['color' => '#FFFFFF']], - [['color' => '#1A2B3C', 'opacity' => 0.5], ['color' => '#FFFFFF']], - NULL, - NULL, - ]; - - yield 'mixed positional and named keys rejected' => [ - ['#1A2B3C', 'opacity' => 0.5], - NULL, - \InvalidArgumentException::class, - 'Field value cannot mix positional and named keys', - ]; - yield 'record missing main property rejected' => [ - ['opacity' => 0.5], - NULL, - \InvalidArgumentException::class, - 'Field record must include the main property "color"', - ]; - } - -} diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/DefaultHandlerTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/DefaultHandlerTest.php index 5e34ffad..7b920a3d 100644 --- a/tests/Drupal/Tests/Driver/Unit/Core/Field/DefaultHandlerTest.php +++ b/tests/Drupal/Tests/Driver/Unit/Core/Field/DefaultHandlerTest.php @@ -4,8 +4,6 @@ namespace Drupal\Tests\Driver\Unit\Core\Field; -use Drupal\Core\Field\FieldDefinitionInterface; -use Drupal\Core\Field\FieldStorageDefinitionInterface; use Drupal\Driver\Core\Field\AbstractHandler; use Drupal\Driver\Core\Field\DefaultHandler; use Drupal\Driver\Core\Field\FieldHandlerInterface; @@ -14,6 +12,11 @@ /** * Tests the DefaultHandler field handler. * + * DefaultHandler is a pure pass-through: it relays the normalised records to + * storage unchanged. 'Core' rejects fields the default cannot marshal before it + * resolves this handler, so that classification is exercised in FieldClassifier + * and Core, not here. + * * @group fields */ #[Group('fields')] @@ -23,7 +26,7 @@ class DefaultHandlerTest extends FieldHandlerUnitTestBase { * {@inheritdoc} */ protected function createHandler(): FieldHandlerInterface { - return $this->handlerWithColumns(['value' => []]); + return $this->handlerWithMainProperty('value'); } /** @@ -48,6 +51,12 @@ public static function dataProviderExpand(): \Iterator { NULL, NULL, ]; + yield 'multi-column scalar record passes through unchanged' => [ + [['value' => 'label', 'format' => 'plain_text']], + [['value' => 'label', 'format' => 'plain_text']], + NULL, + NULL, + ]; yield 'integer scalar' => [ 42, [['value' => 42]], @@ -70,57 +79,16 @@ public static function dataProviderExpand(): \Iterator { } /** - * Tests that a multi-column field triggers the loud-failure policy. - */ - public function testExpandThrowsForMultipleColumns(): void { - $handler = $this->handlerWithColumns(['value' => [], 'format' => []]); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('No dedicated handler is registered'); - $this->expectExceptionMessage('2 column(s) (value, format)'); - - $handler->expand([['value' => 'hello']]); - } - - /** - * Tests that a single-column field not keyed by 'value' triggers failure. - */ - public function testExpandThrowsForSingleColumnNotNamedValue(): void { - $handler = $this->handlerWithColumns(['target_id' => []]); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('target_id'); - - $handler->expand([['value' => 42]]); - } - - /** - * Builds a DefaultHandler wired to a mocked field storage/config pair. + * Builds a DefaultHandler with only its main property set. * - * @param array> $columns - * Column descriptors keyed by column name. + * DefaultHandler's pass-through 'doExpand()' touches no field metadata, so + * the handler needs only the main property the base 'normalise()' reads. */ - protected function handlerWithColumns(array $columns): DefaultHandler { - $storage = $this->createMock(FieldStorageDefinitionInterface::class); - $storage->method('getColumns')->willReturn($columns); - $storage->method('getName')->willReturn('field_example'); - $storage->method('getType')->willReturn('example_type'); - $storage->method('getTargetEntityTypeId')->willReturn('node'); - - $config = $this->createMock(FieldDefinitionInterface::class); - $config->method('getTargetBundle')->willReturn('article'); - - $reflection = new \ReflectionClass(DefaultHandler::class); - $handler = $reflection->newInstanceWithoutConstructor(); - - $info_prop = $reflection->getParentClass()->getProperty('fieldInfo'); - $info_prop->setValue($handler, $storage); - - $config_prop = $reflection->getParentClass()->getProperty('fieldConfig'); - $config_prop->setValue($handler, $config); + protected function handlerWithMainProperty(string $main_property): DefaultHandler { + $handler = (new \ReflectionClass(DefaultHandler::class))->newInstanceWithoutConstructor(); - $main_property = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); - $main_property->setValue($handler, 'value'); + $main_prop = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); + $main_prop->setValue($handler, $main_property); return $handler; } diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/FieldShapeClassifierTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/FieldShapeClassifierTest.php new file mode 100644 index 00000000..1bc35a00 --- /dev/null +++ b/tests/Drupal/Tests/Driver/Unit/Core/Field/FieldShapeClassifierTest.php @@ -0,0 +1,84 @@ +assertTrue($classifier->fieldIsEntityReference($this->storageWithProperties([ + 'target_id' => DataReferenceTargetDefinition::create('integer'), + ]))); + + // A plain scalar is not a reference. + $this->assertFalse($classifier->fieldIsEntityReference($this->storageWithProperties([ + 'value' => DataDefinition::create('string'), + ]))); + + // A datetime column is a scalar, not a reference. + $this->assertFalse($classifier->fieldIsEntityReference($this->storageWithProperties([ + 'value' => DataDefinition::create('datetime_iso8601'), + ]))); + + // A computed reference is storage-derived, not author-supplied, so it is + // ignored. + $this->assertFalse($classifier->fieldIsEntityReference($this->storageWithProperties([ + 'value' => DataDefinition::create('string'), + 'entity' => DataReferenceTargetDefinition::create('integer')->setComputed(TRUE), + ]))); + } + + /** + * Tests complex-value detection by a ComplexDataDefinitionInterface. + */ + public function testFieldIsComplexValue(): void { + $classifier = new FieldShapeClassifier(); + + $this->assertTrue($classifier->fieldIsComplexValue($this->storageWithProperties([ + 'value' => DataDefinition::create('string'), + 'options' => MapDataDefinition::create(), + ]))); + + // Plain scalars are not complex. + $this->assertFalse($classifier->fieldIsComplexValue($this->storageWithProperties([ + 'value' => DataDefinition::create('string'), + 'format' => DataDefinition::create('string'), + ]))); + } + + /** + * Builds a storage definition mock exposing the given property definitions. + * + * @param array $properties + * Property definitions keyed by property name. + */ + protected function storageWithProperties(array $properties): FieldStorageDefinitionInterface { + $storage = $this->createMock(FieldStorageDefinitionInterface::class); + $storage->method('getPropertyDefinitions')->willReturn($properties); + + return $storage; + } + +} diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextHandlerTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/TextHandlerTest.php deleted file mode 100644 index dadfb4c6..00000000 --- a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextHandlerTest.php +++ /dev/null @@ -1,76 +0,0 @@ -newInstanceWithoutConstructor(); - - $property = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); - $property->setValue($handler, 'value'); - - return $handler; - } - - /** - * {@inheritdoc} - */ - public static function dataProviderExpand(): \Iterator { - yield 'bare scalar' => [ - 'Inline text.', - [['value' => 'Inline text.']], - NULL, - NULL, - ]; - yield 'list of scalars' => [ - ['a', 'b'], - [['value' => 'a'], ['value' => 'b']], - NULL, - NULL, - ]; - yield 'single record with value and format' => [ - ['value' => 'Inline text.', 'format' => 'plain_text'], - [['value' => 'Inline text.', 'format' => 'plain_text']], - NULL, - NULL, - ]; - yield 'list of records' => [ - [['value' => 'a', 'format' => 'plain_text'], ['value' => 'b']], - [['value' => 'a', 'format' => 'plain_text'], ['value' => 'b']], - NULL, - NULL, - ]; - - yield 'mixed positional and named keys rejected' => [ - ['a', 'format' => 'plain_text'], - NULL, - \InvalidArgumentException::class, - 'Field value cannot mix positional and named keys', - ]; - yield 'record missing main property rejected' => [ - ['format' => 'plain_text'], - NULL, - \InvalidArgumentException::class, - 'Field record must include the main property "value"', - ]; - } - -} diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextLongHandlerTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/TextLongHandlerTest.php deleted file mode 100644 index dbc87d29..00000000 --- a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextLongHandlerTest.php +++ /dev/null @@ -1,64 +0,0 @@ -newInstanceWithoutConstructor(); - - $property = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); - $property->setValue($handler, 'value'); - - return $handler; - } - - /** - * {@inheritdoc} - */ - public static function dataProviderExpand(): \Iterator { - yield 'bare scalar' => [ - 'Body copy.', - [['value' => 'Body copy.']], - NULL, - NULL, - ]; - yield 'single record with value and format' => [ - ['value' => 'Body copy.', 'format' => 'plain_text'], - [['value' => 'Body copy.', 'format' => 'plain_text']], - NULL, - NULL, - ]; - - yield 'mixed positional and named keys rejected' => [ - ['Body.', 'format' => 'plain_text'], - NULL, - \InvalidArgumentException::class, - 'Field value cannot mix positional and named keys', - ]; - yield 'record missing main property rejected' => [ - ['format' => 'plain_text'], - NULL, - \InvalidArgumentException::class, - 'Field record must include the main property "value"', - ]; - } - -} diff --git a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextWithSummaryHandlerTest.php b/tests/Drupal/Tests/Driver/Unit/Core/Field/TextWithSummaryHandlerTest.php deleted file mode 100644 index df7eeec6..00000000 --- a/tests/Drupal/Tests/Driver/Unit/Core/Field/TextWithSummaryHandlerTest.php +++ /dev/null @@ -1,64 +0,0 @@ -newInstanceWithoutConstructor(); - - $property = new \ReflectionProperty(AbstractHandler::class, 'mainProperty'); - $property->setValue($handler, 'value'); - - return $handler; - } - - /** - * {@inheritdoc} - */ - public static function dataProviderExpand(): \Iterator { - yield 'bare scalar' => [ - 'body text', - [['value' => 'body text']], - NULL, - NULL, - ]; - yield 'single record with summary' => [ - ['value' => 'body text', 'summary' => 'short'], - [['value' => 'body text', 'summary' => 'short']], - NULL, - NULL, - ]; - - yield 'mixed positional and named keys rejected' => [ - ['body text', 'summary' => 'short'], - NULL, - \InvalidArgumentException::class, - 'Field value cannot mix positional and named keys', - ]; - yield 'record missing main property rejected' => [ - ['summary' => 'short'], - NULL, - \InvalidArgumentException::class, - 'Field record must include the main property "value"', - ]; - } - -} diff --git a/tests/fixtures/modules/driver_field_test/composer.json b/tests/fixtures/modules/driver_field_test/composer.json new file mode 100644 index 00000000..03e88a9d --- /dev/null +++ b/tests/fixtures/modules/driver_field_test/composer.json @@ -0,0 +1,7 @@ +{ + "name": "drupal/driver_field_test", + "description": "Test fixture: custom field types with no driver handler.", + "license": "GPL-2.0-or-later", + "type": "drupal-module", + "version": "1.0.0" +} diff --git a/tests/fixtures/modules/driver_field_test/driver_field_test.info.yml b/tests/fixtures/modules/driver_field_test/driver_field_test.info.yml new file mode 100644 index 00000000..4b4fab0c --- /dev/null +++ b/tests/fixtures/modules/driver_field_test/driver_field_test.info.yml @@ -0,0 +1,5 @@ +name: 'Driver field test' +type: module +description: 'Provides custom field types with no driver handler, for kernel tests.' +package: Testing +core_version_requirement: ^10 || ^11 diff --git a/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestReferenceItem.php b/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestReferenceItem.php new file mode 100644 index 00000000..e54b8bc8 --- /dev/null +++ b/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestReferenceItem.php @@ -0,0 +1,68 @@ +setLabel(new TranslatableMarkup('Referenced entity ID')) + ->setRequired(TRUE); + + return $properties; + } + + /** + * {@inheritdoc} + */ + public static function schema(FieldStorageDefinitionInterface $field_definition): array { + return [ + 'columns' => [ + 'target_id' => [ + 'type' => 'int', + 'unsigned' => TRUE, + ], + ], + ]; + } + + /** + * {@inheritdoc} + */ + public function isEmpty(): bool { + $value = $this->get('target_id')->getValue(); + + return $value === NULL || $value === ''; + } + +} diff --git a/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestScalarItem.php b/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestScalarItem.php new file mode 100644 index 00000000..c601df90 --- /dev/null +++ b/tests/fixtures/modules/driver_field_test/src/Plugin/Field/FieldType/DriverTestScalarItem.php @@ -0,0 +1,65 @@ +setLabel(new TranslatableMarkup('Value')) + ->setRequired(TRUE); + + $properties['weight'] = DataDefinition::create('integer') + ->setLabel(new TranslatableMarkup('Weight')); + + return $properties; + } + + /** + * {@inheritdoc} + */ + public static function schema(FieldStorageDefinitionInterface $field_definition): array { + return [ + 'columns' => [ + 'value' => [ + 'type' => 'varchar', + 'length' => 255, + ], + 'weight' => [ + 'type' => 'int', + ], + ], + ]; + } + + /** + * {@inheritdoc} + */ + public function isEmpty(): bool { + $value = $this->get('value')->getValue(); + + return $value === NULL || $value === ''; + } + +}