[wp-trac] [WordPress Trac] #37395: Transients should be replaced with options, e.g. plugin delete notification

WordPress Trac noreply at wordpress.org
Mon Jul 18 12:04:05 UTC 2016


#37395: Transients should be replaced with options, e.g. plugin delete notification
--------------------------+-----------------------------
 Reporter:  kitchin       |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Plugins       |    Version:  trunk
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 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.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/37395>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list