And that goes against all the documentation.

In elysia_cron_module_jobs(), the default options for each job are added, but 'callback' and 'arguments' rely on the $job key being an existing function. If I specify 'file' but not 'callback', the callback function I want to use hasn't been loaded yet, and callback gets set to {$module}_cronapi instead of $job.

Putting an include_once() in this function may not be the best idea, so perhaps what needs to happen is an update to the documentation, saying 'file' requires the 'callback' or the 'execute' $op for hook_cronapi().

Comments

gotheric’s picture

Assigned: Unassigned » gotheric

Interesting issue.

You're right, if someone declares a "file", and in the same file he declares the standard cron function (= a function named as the job name) without declaring the "callback" it doesn't work.

A solution to avoid the inclusion of "file" in elysia_cron_module_jobs() is to setting the default function not to "{$module}_cronapi", but to a new "croncall_proxy" function that, when called, includes the file and choose what to do.

I'll thing about it.
Thanks for the issue.

kala4ek’s picture

Version: 7.x-2.1 » 7.x-2.x-dev
Assigned: gotheric » Unassigned
Issue summary: View changes
Island Usurper’s picture

Assigned: Unassigned » Island Usurper

So last night I spent 4 hours wrestling with my debugger only to find this issue again. Imagine my surprise when I realize I wrote this issue in the first place! :-|

I think the way to fix this is not to check the callability when the jobs are collected, but when we need to execute them. Patch is forthcoming.

Island Usurper’s picture

Assigned: Island Usurper » Unassigned
Status: Active » Needs review
StatusFileSize
new1.2 KB

I haven't tested this yet, but if someone sees any problems before I get to, here is the patch.

Island Usurper’s picture

Initial tests say it works for me.

kala4ek’s picture

Assigned: Unassigned » kala4ek

  • kala4ek committed 71c0b8d on 7.x-2.x authored by Island Usurper
    Issue #2078173 by Island Usurper, kala4ek, gotheric: Setting 'file' but...
kala4ek’s picture

Assigned: kala4ek » Unassigned
Status: Needs review » Fixed

Thanks, committed to latest dev.

imre.horjan’s picture

Latest dev is broken after this commit:
http://cgit.drupalcode.org/elysia_cron/commit/?id=71c0b8d

These change seem to cause this error:
- include_once((!empty($_elysia_cron_settings[$job]['file path']) ? $_elysia_cron_settings[$job]['file path'] : drupal_get_path('module', $_elysia_cron_settings[$job]['module'])) . DIRECTORY_SEPARATOR . $_elysia_cron_settings[$job]['file']);
+ include_once !empty($_elysia_cron_settings[$job]['file path']) ? $_elysia_cron_settings[$job]['file path'] : drupal_get_path('module', $_elysia_cron_settings[$job]['module']) . DIRECTORY_SEPARATOR . $_elysia_cron_settings[$job]['file'];

If you define 'callback', 'file' and 'file path' in your cronapi hook, Elysia cron tryies to include empty filenames @ line 1448, and cron callback is not executed.
(2.3 works fine)

// Try this.
function mymodule_cronapi($op, $job = NULL) {
  $items['mymodule_cron'] = array(
    'callback' => 'mymodule_cron_callback',
    'rule' => '* * * * *',
    'time' => 60,
    'weight' => 1,
    'file' => 'mymodule.cron.inc',
    'file path' => drupal_get_path('module', 'mymodule'),
  );
  return $items;
}
imre.horjan’s picture

Status: Fixed » Active
Island Usurper’s picture

Concatenation of strings within and around the conditional operator is goofy. While the outermost parentheses for include_once weren't needed, the ones from the !empty() to the DIRECTORY_SEPARATOR are.

imre.horjan’s picture

Status: Active » Needs work
imre.horjan’s picture

StatusFileSize
new945 bytes

Since the other patch is already merged, I've created a new one based on latest dev based on Island Usurper's suggestion.
Please check if the patch can be applied...it's exported from PHPStorm.

imre.horjan’s picture

Status: Needs work » Needs review

  • kala4ek committed 26a04d6 on 7.x-2.x
    Issue #2078173 by Island Usurper, imre.horjan, kala4ek: Setting 'file'...
kala4ek’s picture

Status: Needs review » Fixed

Fixed it, also reformatted this ugly line into several lines, made it easy to understand.

Status: Fixed » Closed (fixed)

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