WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 5 months ago

#37542 closed defect (bug) (duplicate)

Taxonomies in cron jobs cause problems

Reported by: blatan Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.5.3
Component: Cron API Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description (last modified by ocean90)

The problem is similiar to this one: #19373

I had figured out the workaround for my issue before I found the above ticket.

I created an import data from an external database. It can be called by admin manually or in daily cron jobs.
In the cron job I have to check if a taxonomy exists in my DB, so I get it with

get_term_by('slug', ... );

If user calls it everything worked fine, but in cron job didn't. I noticed the problem is in get_term_by -> taxonomy_exists.

Taxonomy_exists checks if the given taxonomy is in global $wp_taxonomies array.

I tried to use set_current_user in beginning of the cron job, but it does not work. It seems to $wp_taxonomies is set before the job.

I searched if there a hook like 'before_cron_job', but I haven't found any.

My workaround is quite silly but it works. In the beginning of my cron job callback I call:

function updateTaxonomiesForCron() {
    global $wp_taxonomies;
    if ( defined('DOING_CRON') ){
	$wp_taxonomies['region'] = [];
    }
}

I can see the problem with current_user_can for taxonomies is quite old and probably my ticket will be set as duplicated but I just want to show my case of the issue.

Best regards
Bernard

Attachments (1)

cron-tax.zip (970 bytes) - added by jrfoell 2 years ago.
example plugin to illustrate issue.

Download all attachments as: .zip

Change History (14)

#1 @ocean90
3 years ago

  • Description modified (diff)
  • Keywords reporter-feedback added

Hello @blatan, thanks for your report.

This doesn't sound like #19373. Are you sure that you register your taxonomy correctly? Can you provide some code and steps to reproduce this issue? Thanks!

#2 @blatan
3 years ago

Wwe register taxonomies with this function:

public function init()
    {
        register_taxonomy($this->taxonomy_key, $this->object_type, $this->getArgs());
        if (count($this->default_terms)) {
            foreach ($this->default_terms as $term) {
                wp_insert_term($term, $this->taxonomy_key);
            }
        }
    }

To reproduce the problem:

function abp_daily_callback() {
		update_option('abp_daily_callback', date('Y-m-d H:i'));

		MyImporter::importTermsFromExternalDb();
	}

// in MyImporter class
public static function importTerms() {
    // get $termSlug and $taxonomyName from external DB to check if it exists in my DB
    $term = get_term_by('slug', $termSlug, $taxonomyName, ARRAY_A);
    // $term returned from get_term_by is set to invalid because taxonomy_exists returns false
}

#3 @ocean90
3 years ago

@blatan Where and when is the init() method called?

#4 @blatan
3 years ago

add_action('init', array($this, 'register_taxonomies'));

function register_taxonomies() {
		TaxonomyLoader::loadTaxonomies();
	}


array and $this added because we've got twig in the project

#5 @jrfoell
2 years ago

This is actually an issue if using the "Alternative Cron"

define( 'ALTERNATE_WP_CRON', true );

I attached an example plugin to show a use case. You'll have to set the define in wp-config.php to see it in action. Changing the taxonomy registration init hook to priority 9 will alleviate this problem, but it would be better to instead change the alternate cron to use a much higher init priority so any start-up code has a chance to execute.

@jrfoell
2 years ago

example plugin to illustrate issue.

#6 @blatan
2 years ago

Thanks for the reply, but it was 5 months ago and honestly speaking I completly forgot about. The workaround which I figured out still works.

I don't know if I can help. Tell me please if you want me to do something.

Happy New Year :-)

#7 @jrfoell
2 years ago

@blatan what was your workaround? Curious to know if we're addressing the same issue from two different angles.

#8 follow-up: @dd32
2 years ago

  • Component changed from Taxonomy to Cron API
  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.8

Hey @jrfoell

Thanks for your demo plugin, and mention of ALTERNATE_WP_CRON - It makes a lot more sense now!

wp_cron() is hooked on init at default; when ALTERNATE_WP_CRON is defined cron jobs are processed within that function.
Due to the order of action registration, if a plugin registers something on init at the default 10 it'll run after the cron has already spawned.

A temporary fix for plugin authors would be to enqueue on init at an earlier priority - such as 9 or to register them on an earlier hook such as after_setup_theme (Note: many WordPress functionalities are not available before init, such as capabilities for users, etc).

I think we should move wp_cron() to being fired on something later - wp_loaded looks like a better spot. It's after init and before template functionality. However, it's after the multisite suspended checks, so it we'd no longer be spawning cron for suspended multisites.. which is what #20537 wants anyway. (FYI @jeremyfelt)

A safer bet, would be to simply move wp_cron to running much later on init, at say priority 100.

#9 in reply to: ↑ 8 @SergeyBiryukov
2 years ago

Replying to dd32:

I think we should move wp_cron() to being fired on something later - wp_loaded looks like a better spot.

I agree. Previously moved from sanitize_comment_cookies to init in #19818.

This ticket was mentioned in Slack in #core by flixos90. View the logs.


2 years ago

#11 @flixos90
2 years ago

  • Milestone changed from 4.8 to Future Release

This should land early in a cycle, so let's come back in a future release.

#12 @peterwilsoncc
10 months ago

  • Keywords needs-unit-tests added

I agree with Felix, it's best this lands early in a cycle. This puts it in 5.1 at the earliest.

Along with setting the hook to run later on init when adding the filter, removing the filter will need to be modified in src/wp-includes/class-wp-customize-manager.php.

#13 @peterwilsoncc
5 months ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#24160 raises the same issue, running cron jobs on init, 10 causes problems for sites using ALTERNATE_WP_CRON. I'll add a note to the other ticket to highlight that it is an issue for sites registering custom taxonomies and close this as a duplicate.

Note: See TracTickets for help on using tickets.