Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 7, 2021

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Apr 23, 2021

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
Copy link
Collaborator

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: f470c56 - 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
Copy link
Owner

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

@jhedstrom jhedstrom merged commit c6fc5ea into jhedstrom:master May 7, 2021
@dsnopek
Copy link
Contributor Author

dsnopek commented May 7, 2021

Thanks!

@jhedstrom
Copy link
Owner

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

xurizaemon added a commit to xurizaemon/behat-steps that referenced this pull request Jul 27, 2021
jhedstrom added a commit that referenced this pull request Apr 25, 2022
Prevent PHP warning when creating a node with Drupal 7.79 and it's new field storage optimization

Signed-off-by: Jonathan Hedstrom <jhedstrom@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants