Updated: Comment #N

Problem/Motivation

When you specify a field type that doesn't exist, due to some weird Typed Data magic, it still returns a generic list instance, which then results in a fatal error.

Proposed resolution

Check the type in advance and throw a helpful exception.

Remaining tasks

User interface changes

API changes

Original report by @ivanjaros

I have created a custom entity and I have defined it's fields in baseFieldDefinitions() method.
But if I use non-core type(Drupal\Core\Field\Plugin\Field\FieldType), like 'decimal' which is provided by Number module(Drupal\number\Plugin\Field\FieldType\DecimalItem) I'll end up with error. So I had to change the type to float, which is a "core" type, for time being.

Here is what happened when I've used non-core type #2164961: Call to undefined method setLangcode() when creating a field.

Comments

berdir’s picture

Version: 8.0-alpha7 » 8.x-dev
Assigned: Unassigned » fago

Wow, this is strange, let's see what @fago has to say about this, I don't see a reason why this wouldn't work, looks like something with the list class is messed up?

berdir’s picture

Ok, found some time to debug this, the reason is quite simple.

This happens if the field type is not existing. The typed data manager first uses the data type 'list', which is ItemList and that is then overriden by the FieldDefinition, but only if the referencing field type is exposed as a data type.

This needs better error handling somewhere, but that's the only problem here.

berdir’s picture

Status: Active » Needs review
StatusFileSize
new921 bytes

Here's a patch with some basic validation. We could also validate this when building the field definitions in the entity manager?

Also, the way the list overrides work is really hard to debug, wouldn't it be better to use an actual type for that? Then we might notice this earlier rather than returning a bogus data instance?

berdir’s picture

Title: Non-core FieldType does not work in ContentEntityInterface::baseFieldDefinitions() » Misleading fatal error when specifying a field type that is not available for a base field
Issue summary: View changes
Issue tags: +Entity Field API

Updating title and issue summary.

fago’s picture

StatusFileSize
new797 bytes

Yeah, I think we should do the check when instantiating the object somewhere. I've not tested it, but attached patch should let it throw a better error message while instantiating the field object.

Related, this code did not through any notice because of #2220259: DefaultPluginManager::getDefinition() does not match the interface.

fago’s picture

Assigned: fago » Unassigned
berdir’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new797 bytes

Thanks, manually verified. I don't thin this needs tests, this is a like calling a function when that module is disabled or a wrong schema definition, but we want to be a nicer to the developer and tell him where the problem is.

To reproduce:

Make sure you have a node.
Go to Node::baseFieldDefinitions() and change for example the title field type (string) to bla.
$ drush cr

HEAD: frontpage
Notice: Undefined index: list_class in Drupal\Core\TypedData\ListDataDefinition->getClass()
InvalidArgumentException: Cannot set a list with a non-array value. in Drupal\Core\TypedData\Plugin\DataType\ItemList->setValue()

And when trying to add new content on node/add/article, you get this:
Fatal error: Call to undefined method Drupal\Core\TypedData\Plugin\DataType\ItemList::setLangcode() in .../core/lib/Drupal/Core/Entity/ContentEntityBase.php on line 430

=> Not helpful.

With this patch:
LogicException: An invalid data type field_item:bla has been specified for list items. in Drupal\Core\TypedData\ListDataDefinition->getClass()

then you at least know that there's a bla thingy somewhere that you need to fix. It's still typed data'ified but maybe that will change in the typed data adapter issue.

Just changing %plugin_id to @plugin_id because it's double escaped otherwise.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ebfc1a4 and pushed to 8.x. Thanks!

  • Commit ebfc1a4 on 8.x by alexpott:
    Issue #2165223 by Berdir, fago: Misleading fatal error when specifying a...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.