Make WordPress Core

Opened 8 years ago

Last modified 6 years ago

#37395 new defect (bug)

Transients should be replaced with options, e.g. plugin delete notification

Reported by: kitchin's profile kitchin Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Plugins Keywords:
Focuses: Cc:

Description

Transient data is not guaranteed. Even in the shortest possible duration, on a page redirect! In wp-admin/plugins.php :

set_transient('plugins_delete_result_' . $user_ID, $delete_result); //Store the result in a cache rather than a URL param due to object type & length
wp_redirect( self_admin_url("plugins.php?deleted=$plugins_to_delete&plugin_status=$status&paged=$page&s=$s") );

Now a delete notification is not critical, but it would be just as easy to do this with set/get/delete_option(), and set a good example for plugin authors. The retrieval step does an immediate delete:

$delete_result = get_transient( 'plugins_delete_result_' . $user_ID );
delete_transient( 'plugins_delete_result_' . $user_ID );

So it looks easy to write a patch substituting set/get/delete_option(), modulo multi-site perhaps.

I expected to see more of these in the WP codebase related to notifications, but it's mainly other stuff. About half the use of transients in WP is wrong. I'll list them here for future bugs, but I leave this bug for the Plugins component.

  • GOOD: wp-admin/includes/dashboard.php ( $cache_key, 'plugin_slugs' )
  • BAD: wp-admin/includes/template.php ( 'settings_errors' )
  • GOOD: wp-admin/includes/theme-install.php ( 'wporg_theme_feature_list' )
  • BAD: wp-admin/plugins.php ( see above: 'plugins_delete_result_' . $user_ID )
  • ALL GOOD: wp-content/themes/*
  • PROB. BAD: wp-trunk/wp-cron.php AND wp-includes/cron.php ( 'doing_cron' ). Using it as a file lock is either egregious or just confusing, depending on whether it is doubling as a global store for the same run. Why not use *_option() instead?
  • GOOD: wp-includes/author-template.php ( 'is_multi_author' )
  • PROB. BAD: wp-includes/class-feed.php ( $this->name, $this->mod_name )
  • PROB. BAD: wp-includes/media.php ( $regeneration_lock ). Bad if it's to protect successive ajax calls, confusing if it's a global store for the same run.
  • GOOD: wp-includes/ms-functions.php ( 'dirsize_cache' )
  • BAD: wp-includes/pluggable.php ( 'random_seed' )
  • GOOD: wp-includes/rss.php ( $cache_option )
  • BAD: wp-mail.php ( 'mailserver_last_checked' )

You were expecting a Clint Eastwood reference?

Should all be easy fixes.

Change History (1)

#1 @ocean90
8 years ago

  • Version trunk deleted

Related: #33977, #27162

Note: See TracTickets for help on using tickets.