jhedstrom / DrupalDriver

A collection of drivers for controlling Drupal.
GNU General Public License v2.0
64 stars 97 forks source link

Prevent PHP warning when creating a node with Drupal 7.79 and it's new field storage optimization #233

Closed dsnopek closed 3 years ago

dsnopek commented 3 years ago

Drupal 7.79 (release notes) added a field storage optimization (change record) that can be enabled by setting a variable in the site's settings.php:

$conf['field_sql_storage_skip_writing_unchanged_fields'] = TRUE;

With this enabled, creating nodes in Behat with the Drupal extension, for example:

Given I am viewing my "page" with the title "A page"

... will lead to a PHP warning like this:

Warning: Invalid argument supplied for foreach() in /app/modules/field/modules/field_sql_storage/field_sql_storage.module line 462

This was discovered on a project which has Behat setup to treat PHP notices/warnings as test failures, so it's actually breaking the test suite in this case.

After digging into this, it seems the problem is that the new field storage optimization is comparing $node to $node->original, but Drupal\Driver\Cores\Drupal7::nodeCreate() is doing $node->original = clone $node before the fields are expanded by $this->expandEntityFields(). That means that $node->original->body contains a string, rather than an array in the proper format for a Drupal field, which leads to the PHP warning.

One possible solution would be to move the $node->original = clone $node after the $this->expandEntityFields() so that $node->original->body is expanded. However, in normal operation, Drupal doesn't set $node->original when creating a new node, only when updating an existing node. So, I don't this this actually needed at all, and this PR just removes it.

This fixes the PHP warning in my testing.

pfrenssen commented 3 years ago

This looks OK to me. I think this reasoning makes sense:

However, in normal operation, Drupal doesn't set $node->original when creating a new node, only when updating an existing node.

For reference here is the commit that added this code: https://github.com/jhedstrom/DrupalDriver/commit/f470c561e44f091ebd368b964eeaea9d38121eb5 - the commit message says that this was intended to prevent notices, but I could not find more context about which notices this would be. Possibly notices inside DrupalDriver itself?

jhedstrom commented 3 years ago

Yeah, I think this is good to go--I think it was throwing notices in the DrupalDriver itself.

dsnopek commented 3 years ago

Thanks!

jhedstrom commented 3 years ago

I've added a 2.1.1 release that includes this fix.