[wp-trac] [WordPress Trac] #30261: Split all existing shared taxonomy terms on WP upgrade

[WordPress Trac]

#30261: Split all existing shared taxonomy terms on WP upgrade
-------------------------------------+---------------------------
 Reporter:  boonebgorges             |       Owner:  boonebgorges
     Type:  enhancement              |      Status:  reopened
 Priority:  normal                   |   Milestone:  4.3
Component:  Taxonomy                 |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+---------------------------

Comment (by boonebgorges):

 @dd32 pointed out that the solution previously suggested could result in a
 lag when viewing the about.php page, which seems like an acceptable way to
 introduce someone to the new version of WP. The 'admin_init' trick also
 doesn't work reliably during multisite upgrade routines. Moreover, @pento
 and @dd32 both pointed out that it would be nice to have some automatic
 recovery or retry system, in case the PHP process times out during the
 splitting routine.

 [attachment:30261.6.diff] is an attempt to address all these issues.
 During `upgrade_430()`, an HTTP request is sent to
 `upgrade.php?step=split_shared_terms`. I've modified upgrade.php so that
 `WP_INSTALLING` is not defined during the `split_shared_terms` request.
 (This is the ugliest part of the patch, but we don't have a better system
 for asynchronous loopback requests - ajax-actions seems like the wrong
 place, and with admin-ajax.php we have to deal with nopriv vs priv issues,
 since the loopback request doesn't have a WP auth cookie.) So, during a
 normal upgrade, term splitting happens in an external request, in which
 all plugins are run. This works as expected in both multisite and non-
 multisite environments.

 Second, I kept an 'admin_init' hook in there. This ensures that, if the
 splitting process is killed for some reason (eg, due to a timeout), it
 will retry the next time an Administrator visits the admin. When
 `split_all_shared_terms()` figures out that it's finished splitting all
 shared terms - either by finishing its `foreach` loop, or by detecting
 that there's nothing to split - it will set a flag in the database that
 will prevent the routine from being run again in the future. This should
 be pretty resiliant.

 Note that [attachment:30261.6.diff] also contains the fix to the `$this`
 bug in #33206, which will allow you to test during a network upgrade.

 Feedback and testing is sorely needed, especially from plugin authors who
 are affected by term splitting. Here's how I've been testing:
 - Install WP 4.2.x to a fresh database
 - Optional: to test in multisite, convert installation to multisite and
 create a new site or two
 - Generate some shared terms (on all sites). I've been using this wp-cli
 command: https://gist.github.com/boonebgorges/2beb1379569c3bd85803
 - (Network-)Activate plugins that hook to 'split_shared_term'. I wrote a
 dummy plugin that hooks to 'split_shared_term' and writes to an error log.
 - Replace 4.2.x files with trunk files (`svn switch`, `git checkout`,
 whatever) and apply [attachment:30261.6.diff]
 - Run the db upgrade routine, as appropriate for your config (multisite vs
 non)
 - Check that terms have been split, and that your plugin has detected the
 splits

 @dd32 @pento I would especially value your feedback on:
 - Whether the added overhead of the loopback HTTP request is going to
 cause problems, particularly during network upgrades
 - Whether the failure recovery technique (an 'admin_init' callback, which
 sets an "all done" flag when it detects that the migration is complete)
 seems OK to you
 - Whether you can think of a less horrid way of allowing a non-
 authenticated term-splitting routine than my upgrade.php hack

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30261#comment:29>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform