Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Updated: Comment #0
Problem/Motivation
In #2143069: Add getProvider() and hasCustomStorage to FieldDefinitionInterface we introduced yet another place where we check whether our field definition extends FieldDefinition, in this case to avoid the need of adding a setter on the FieldDefinitionInterface. This is problematic because right now our system is working correctly only for configurable fields and classes extending FieldDefinition, any other class implementing the interface is SOL. In this particular case the latter would not get their provider key populated, which may lead to all sort of broken behaviors.
Proposed resolution
Add the required setters on the FieldDefinitionInterface, remove the FieldDefinition checks, and provide an implementation in Field(Instance)Config that will throw an exception if a value differing from the expected/hard-coded value is passed.
Remaining tasks
- Find consensus on the solution
- Write a patch
User interface changes
None
API changes
None, just an API addition.
Comments
Comment #1
plachThis could be even considered a bug report actually.
Comment #2
fagoHaving an interface means people are free to use their own field definition classes if they really want to - but for that to work out the entity manager needs to set some basic stuff for them as well. Thus, I could see us adding only those setters used by manager to the interface + throw exception if someone tries to use them to set something other than the hard-coded value on configurable fields?
Comment #3
plachComment #4
plachComment #5
tstoecklerYes, I actually did not at all understand what the problem with adding those methods on the interface is.
I have nothing against throwing exceptions in addition to that, where it makes sense.
Comment #6
YesCT CreditAttribution: YesCT commentedThe critical #2143291: Clarify handling of field translatability is postponed on this. I think that makes this critical.
Also tagging. Not sure what other tags this might need.
Comment #7
plachCurrently the only places where we are checking
FieldDefinitions areEntityManagerand (guess who) Content Translation. If we fix the former, as per #2, we need to add the following setters:setProvider()setName()setTargetEntityTypeId()If we want to fix also the CT case we need to add also the (guess what)
setTranslatable()method. I have the feeling that in this case the logic behind what gets a setter becomes quite arbitrary, OTOH we really need that setter to properly fix #2143291: Clarify handling of field translatability. Maybe we should just add a setter for each property, unless there's a hard reason to make it read-only?The main problem with CT is that it was designed to work with definition arrays, which are natively mutable, objects broke this assumption badly unfortunately, as now field definitions are no longer alterable in a strict sense. At least not reliably alterable.
Comment #8
plachComment #9
plachThis is no longer critical.
I think it might be partially addressed in #2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration, btw.
Comment #10
larowlanTouching parts of this in #2289063: Change contact message entity to behave more like a normal entity
Comment #11
BerdirDuplicate of #2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods I think. This has more info, but the other is based on a discussion that we had today and reflects the latest plans.
I've also already closed #2143297: setters on FieldDefinitionInterface as a duplicate of that one.
Comment #12
fagoComment #13
heddnThere's better info and action items outlined in #2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods.