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.If you have some nice entity caching going on, it's possible that when doing a node_load on a Webform submission on a Webform with conditionals, you can run into the following error:
Fatal error: Call to undefined function webform_compare_floats() in /vagrant_sites/spire.dev/profiles/spire_profile/modules/contrib/webform/includes/webform.conditionals.inc on line 1346
We're missing the component .inc files, which provide these conditionals functions. Webform would normally load those files for us, but, because we're caching, Webform got left out of the picture.
It's a simple enough fix - we just need to ensure that the component .inc files are loaded before trying to use their functions. Without knowledge of any other points that the files might be needed, I suggest (via patch on the way) adding them just before we try using them to do the conditional checks.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | webform-entitycache-2529246-7.patch | 1.38 KB | DanChadwick |
| #6 | includes_missed_when-2529246-6.patch | 799 bytes | amoebanath |
| #1 | includes_missed_when-2529246-1.patch | 717 bytes | amoebanath |
Comments
Comment #1
amoebanath CreditAttribution: amoebanath commentedPatch! :)
Comment #2
DanChadwick CreditAttribution: DanChadwick commentedIn general, I'd prefer the entity cache take responsibility for re-creating the environment that is being cached. I'm not sure if that's possible in this case. I'm not too keen on adding extra code, particularly in an inner loop, that is only needed for a tiny proportion of Webform users.
What's the execution path that causes this? Can we fix it in a better place that would benefit entity cache users and not add to the Webform codebase or execution time?
That said, the proposed change is very lightweight. webform_component_include already uses static caching.
Comment #3
DanChadwick CreditAttribution: DanChadwick commentedChanging to a feature request, since I think we are really requesting compatibility, rather than reporting a defect. Entity cache has created an environment for Webform which is not identical to what core provides.
Comment #4
bjcooper CreditAttribution: bjcooper commentedI'm seeing this exact issue, which @amoebanath described well.
Here's one way to reproduce it:
Directly after a cache clear the submission will load up, but not a second time. I presume that's because Entity Cache is intercepting and returning some form of cached version, sans the include for the validation function used to check the number field.
@amoebanath, your patch is super handy since I had this combo running in production and I don't think it broke until I recently updated Webform (perhaps I jumped from the 3.x branch to 4.x). Thanks for your sleuth work. I pulled my hair out for an entire day before I randomly disabled Entity Cache and saw my form start working again—at which point I found my way to this post.
@DanChadwick, it certainly does seem that Entity Cache is the faulty party here. However, it may be worth noting this incompatibility someplace, since putting these two modules together will bring at least some Webforms with conditionals to a terrible grinding halt. It's Entity Cache's responsibility really, but it'd be sort of a PSA for Webform users, especially since I believe earlier versions of Webform were compatible (perhaps coincidentally) with Entity Cache.
Comment #5
DanChadwick CreditAttribution: DanChadwick commentedI would like to know:
1) What is the execution path that executes conditionals without loading the components first. This might help to inform the best location to place the includes.
2) It is reasonable for entitycache to load the includes. For example, is there an entitycache hook that webform or entitycache could implement for this.
Where to put integration code that glues two contrib modules together is always tricky. Beyond technical considerations, I look to the installed base. entitycache has about 5% of the installed base of webform. Therefore, many fewer installations that don't need the code would be affected if the code went into entitycache.
Perhaps we should move this issue to the entitycache queue for further comment?
Comment #6
amoebanath CreditAttribution: amoebanath commented1) I think it stems from the call to
node_load()fromwebform_menu_load(). The eventual call toentity_load()is handled by entitycache, which loads a cached version of the webform, bypassing the call towebform_node_load().Normally Drupal's NodeController would be the handler for the call to
entity_load(), and would make a call towebform_node_load()on completion, to let Webform finish things off - this is where the components are normally loaded in.2) We could use
hook_entitycache_entitytype_load()- this is entitycache's hook for allowing other modules to act on entities. Here we can do some loading of the components, neatly out of the way. I'll put up a patch, just as a suggestion.I do see that it's not sensible to expect all modules to magically integrate. Certainly, it would be odd to expect entitycache to make all entities work out-of-the-box, but similarly we can't expect every module to work with every possible add-on like entitycache. Arguably, this could be something to leave to developer's custom implementations... if it can be done in a way that doesn't take up too much code and effort, it would be nice to have the fix in Webform I feel?
Comment #7
DanChadwick CreditAttribution: DanChadwick commentedThere are a few problems with #6, all moot I think. For posterity, using the node type isn't a reliable way to tell which node type(s) have webforms. And for this sort of hook, it is probably safer to load all the types used in the components, not just the conditionals.
I have decided to commit #1, with an added
if(!function_exists(...))around the call for performance reasons. This solves the problem for entitycache and for any other conflicting caching mechanisms.This is a blind commit, since I haven't actually test entitycache compatibility, rather just that webform still works fine without entity cache, and that the call to webform_component_include() works.
Committed to 7.x-4.x and 8.x.
Comment #10
amoebanath CreditAttribution: amoebanath commentedHi Dan, I've given that patch a quick run through, and it doesn't quite solve the problem. It checks whether the comparison callback exists, but the comparison callback existing is not the bit causing trouble - it's the function that the comparison callback then calls that is missing. Unfortunately, we don't have the name of that function yet at this point.
If you're still happy to solve it at this point of execution, I'd say that original patch #1 is the way to go - without the if statement. There's no major performance hit by removing that if statement, because the first thing
webform_component_include()does is check against a static variable whether the component is already included. If it's already included, I think the time taken would be comparable to afunction_exists()call?+ Thanks for your guidance with this, by the way. Greatly appreciated :)
Comment #13
DanChadwick CreditAttribution: DanChadwick commentedWell, that's a bummer. Thanks for testing. I removed the "if", effectively committing your patch. Thanks.
Committed to 7.x-4.x and 8.x. No patch file as I edited each branch by hand.