This took so long to get to the bottom of! Haha! The domain_node_grants() function checks domain_grant_all() to determine if it should set the 'domain_all' grant. domain_grant_all() uses the following code to check if cron is currently running:<?p>

      $ref = explode('/', request_uri());
      $script = array_pop($ref);
      if (variable_get('domain_cron_rule', 1) && $script == 'cron.php') {

The problem is that there are 6 calls (plus drush) to drupal_cron_run() from index.php, but domain_grant_all() checks only for cron.php. Additionally, on an external cron run (http://example.com/cron.php?cron_key=XXX...) the $script variable is set to "cron.php?cron_key=XXX..." which also fails the if test. The current code seems like it would function only in D6.

Attached is A method to correct the issue. It really is a hack as far as I am concerned, but works and there is not reason it wouldn't be reliable. Still, I'd rather there was a different method. The code implements hook_domain_cron_queue_info(), not to create a queue, but to detect cron start and set a global variable. domain_grant_all() then uses that instead of the script test.

In my use case, I am programmatically updating a Field Collection item entity stored in a node using an EntityWrapper during a cron queue. The host entity information isn't loaded correctly and the entity->save() function fails with the following exception:

Exception: Unable to save a field collection item without a valid reference to a host entity. in FieldCollectionItemEntity->save() (line 473 of /var/www/sites/all/modules/contrib/field_collection/field_collection.module).

If I disable domain.module or apply this patch the code works correctly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentrickard’s picture

Status: Needs review » Needs work

Interesting. That is code leftover from Drupal 6, and that is a hacky fix (especially the global). Looking at drupal_cron_run(), it looks like the hook_cron_queue_info() approach is correct.

Can we change that to set a static lookup rather than a global var. A little bit nicer (arguably) and more consistent code.

I also wonder if we have the same issue with XMLRPC, but we can deal with that if it ever comes up.

agentrickard’s picture

Also a little safe to use hook_cron_queue_info_alter(), since that doesn't require a return value.

agentrickard’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

And a patch.

13rac1’s picture

Status: Needs review » Reviewed & tested by the community

The use of the alter() is arguably better, but the static lookup makes it much cleaner. Sadly the "hack" is required either way. I'm happy with this though.

I've added #1851412: Add $cron_run global set to TRUE during cron runs for modules/developers as a core D8 feature patch.

agentrickard’s picture

Thanks.

agentrickard’s picture

Fixed in 7.x.3.

 8f8fdae..10c7a52  7.x-3.x -> 7.x-3.x
agentrickard’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
1.51 KB

Committed to 7.x.2.

   e793f8d..204dcf2  7.x-2.x -> 7.x-2.x
13rac1’s picture

Thanks for the fast response!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.