@swentel pointed that Fieldinfo::getFieldMap()
- has a comment saying: "Filter out (...) instances on unknown entity types"
- yet contains no code that does anything about unknown entity types...

This did vaguely ring a bell about me doing some hairy back and forth dance in this area during #1735118: Convert Field API to CMI.
More specifically this happened in #1906218-118: Helper issue for "Field CMI".

Digging around, I found out my local commits that:
- introduced the comment and some check around entity_get_info($instance_config['entity_type']) in getFieldMap()
- then, removed the check in getFieldMap(), added it in getBundleInstances() instead, but didn't update the comment in getFieldMap()
I think the reason was that $field->getBundles() relies on getFieldMap(), and is used for various housekeeping tasks, so it needed data whether the entity type was "real" or not.

It might be worth re-evaluating this after #1497374: Switch from Field-based storage to Entity-based storage.

CommentFileSizeAuthor
#3 2059965-3.patch802 bytesswentel
#2 2059965-2.patch2.46 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Title: Should FieldInfo::getFieldMap() return data about unknown (disabled) entity types » Should FieldInfo::getFieldMap() filter data about unknown (disabled) entity types

more accurate title - currently we don't filter, even though a code comment says we do.

swentel’s picture

Status: Active » Needs review
FileSize
2.46 KB

Maybe just remove the docs ? With disabled modules out, this shouldn't happen anymore - even if we need to add some more code later to make that actually work nice.

swentel’s picture

FileSize
802 bytes

Wrong patch

amateescu’s picture

Title: Should FieldInfo::getFieldMap() filter data about unknown (disabled) entity types » Should FieldInfo::getFieldMap() filter data about unknown entity types
Status: Needs review » Reviewed & tested by the community

Unknown entity types should throw exceptions from the entity manager or something, so yes, I think it's not field api's problem.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

Issue summary: View changes

unfinished sentence