For DX sake, the Field API tries to make sure that $field and $instances always contain all the expected (field|instance|widget|formatter) settings, avoiding the need for $foo = isset($field['settings']['foo']) ? $field['settings']['some_setting'] : 'default_for_foo'; code all over the place.
Currently, field_create_field() and field_create_instance() add missing settings on field / instance creation.
However, by the time we read the definition back, the execution context might have changed:
- the module implementing the field type, widget or formatter has been updated and has a new setting
- a 3rd party module, using the new *_info_alter hooks (#502522: Allow drupal_alter() on the various Field API declarative hooks) to add new settings, has been enabled.
The attached patch :
- makes sure missing settings and their default values are filled in field_read_fields() / field_read_instances() (was a TODO so far)
- adds some tests for this : testReadField(), testReadFieldInstance()
- reworks the existing testCreateField(), testCreateFieldInstance() so that they only test field_create_*(), not field_read_*()
- burns a kitten's whiskers by unifying minor things between field and instances CRUD test classes, and adding missing PHPdocs.test functions.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | field_settings_on_read-509736-5.patch | 12.58 KB | yched |
| #3 | field_settings_on_read-509736-3.patch | 13.54 KB | yched |
| field_settings_on_read.patch | 13.53 KB | yched |
Comments
Comment #1
yched commentedEr, typo in the sample ugly-code-we-want-to-avoid :
$foo = isset($field['settings']['foo']) ? $field['settings']['foo'] : 'default_for_foo';, of course.Comment #3
yched commentedFix test error.
Comment #5
yched commentedRerolled, moved a few kittens over to #518412: Minor cleanup in field.test
Would be cool to get in, there are other cleanups pending in this area ;-)
Comment #6
yched commentedNote to reviewers / core committers: the actual patch is like 6 lines, the rest is test adjustments :-)
Comment #7
bjaspan commentedThis patch increases the consistency of Field API by making field and instance objects more complete in more places.
Comment #8
dries commentedCommitted to CVS HEAD. Thanks.
Comment #9
philipnet commentedThis fails on the PIFR #2 clients :(
The change:
Because of the name change, when the tests are run this causes a PHP exception which is not capture by the testbot.
Running the tests via the CLI/PHP script, reports:
Comment #10
yched commented@philipnet: thanks for the report, but I'm a little confused.
This patch is now committed. So do you mean that current HEAD fails on PIFR #2 because a test class name is different from what it was 3 days ago ?
If so, I'd be tempted to call this a bug in PIFR ?
Comment #11
philipnet commentedSorry, my bad :(
It looks like the registry in D7 cached the old class name and the PHP script tried to call that. After a fresh install of HEAD I can't replicate the problem.