Registering rewrite rules is hard, so let's introduce remove_rewrite_rule.

[ericlewis]

A plugin cannot use the current Rewrite API to register/deregister rewrite rules on plugin activation/deactivation elegantly, without being extremely hacky.

Current best practice is to add rewrite rules via init, and flush properly on activation/deactivation hooks in your plugin:

function add_some_rewrite_rules() {
        add_rewrite_rule( ... );
}
add_action( 'init', 'add_some_rewrite_rules' );

register_activation_hook( __FILE__, function() {
        add_some_rewrite_rules();
        flush_rewrite_rules();
} );

register_deactivation_hook( __FILE__, function() {
        flush_rewrite_rules();
} );

However, there is a race condition on the deactivate request: init triggers first, adding your rewrite rule, so when you flush rewrite rules in the deactivation function, the rewrite rule persists.

You have some dirty options: checking the script being run in add_some_rewrite_rules() to sniff for a request for the plugin being deactivated comes to mind, I'm sure there are others.

This would be much easier if we had a remove_rewrite_rule() function we could hit on deactivation, and probably read better.

[ericlewis]

In attachment:29118.diff​, introduce remove_rewrite_rule(), which is a facade for the remove_rule() method on the WP_Rewrite class.

This allows for a more intuitive API, and comprehensible boilerplate for plugin developers:

function add_some_rewrite_rules() {
        add_rewrite_rule( ... );
}
add_action( 'init', 'add_some_rewrite_rules' );

function remove_some_rewrite_rules() {
        remove_rewrite_rule( ... );
}

register_activation_hook( __FILE__, function() {
        add_some_rewrite_rules();
        flush_rewrite_rules();
} );

register_deactivation_hook( __FILE__, function() {
        remove_some_rewrite_rules();
        flush_rewrite_rules();
} );

[mboynes]

I really dig this idea, and that race condition has bugged me for longer than I'd care to say. My workaround for that has always been, on activation or deactivation, just delete the rewrite_rules option -- this will cause them to be regenerated in the next request. Not ideal.

My main concern with your patch is that it pretty much requires repetition. If I call add_rewrite_rule() 20 times, that also means I'd have to call remove_rewrite_rule() 20 times with the same arguments. There would be ways around this, e.g. using call_user_func_array(), but I think any solution would be a bit much for the average developer. Besides keeping code DRY, having to repeat rules is asking for trouble. It would be very simple for someone to add a rule to their "add" function/method, but forget to add it to the "remove" counterpart.

I'm not sure of the best route on this. The first idea that popped to mind is to add another argument (ugh) to add_rewrite_rule() indicating the source/name/group. For instance:

add_rewrite_rule( 'bar/?$', 'index.php?foo=bar', 'top', 'my-plugin' );
add_rewrite_rule( 'bar/(\d+)/?$', 'index.php?foo=bar&bat=$matches[1]', 'top', 'my-plugin' );
add_rewrite_rule( 'bar.json$', 'index.php?foo=bar&format=json', 'top', 'my-plugin' );

This opens up the possibility to then call something like remove_rewrite_rules( 'my-plugin' ); and remove them all. As an added bonus, we could then filter added rewrite rules as we can with add_permastruct(), e.g. add_filter( 'my-plugin_rewrite_rules', '__return_empty_array' ).

Regardless of the solution, this is a great initiative. Thanks for getting it started!

[nacin]

rmccue recently made a pitch (I think in a gist somewhere, not in a ticket yet) to eliminate rewrite rule storage and essentially make flushing no-op. By converting everything to runtime, it's a bit more WP-like. I'm not against this assuming it can be made faster. But it may be a solid alternative to cleaning up this mess.

[mboynes]

Replying to nacin:

rmccue recently made a pitch (I think in a gist somewhere, not in a ticket yet) to eliminate rewrite rule storage and essentially make flushing no-op. By converting everything to runtime, it's a bit more WP-like. I'm not against this assuming it can be made faster. But it may be a solid alternative to cleaning up this mess.

I'd be very interested in seeing that pitch (and some benchmarks). I looked around, but couldn't find it.

[rmccue]

It was ​integrated into the Human Made rewrite helper (alternatively, ​this gist).

Here's my description from the pull request (and the crux of how it works):

This automatically flushes rewrite rules when they need to be. It does this by "generating" them on every request, which could potentially theoretically be slow.

In practice, the slowest part of rewrite rules is deleting the option, and then saving it again, which WP does every time you call flush_rewrite_rules. If your rewrites haven't changed, deleting the option is unnecessary, as update_option can work out whether it needs to actually update the DB/object cache.

I'm not entirely sure on whether we can integrate this, as it may be tricky with backwards compatibility. Let's say you have a filter that's adding one-rule-per-tag (I hope not, but who knows). It'll now go from only being regenerated when needed (rare) to on-load. This isn't a break of backwards compatibility, but could cause extreme performance degradation with poorly coded plugins.

I've been running it personally on my own projects and on Happytables for a while now, and haven't seen any brokenness. There's negligible additional load (since internally, all you're doing is creating an array, which isn't a slow process) and extra time.

Happy to create a patch (and a new ticket if this one isn't suitable), but there is a bit of developer education and evangelism we'd need to do here.

[danielbachhuber]

It's not uncommon for me to hook expensive processes (for instance, a call to get_terms()) onto a rewrite filter because, as a developer, I know flushing rewrite rules only happens in contexts where it's fine for me to have an expensive process. I think flushing rewrite rules on every page load, and retaining all of the filters we have, is a non-starter.

[ericlewis]

Replying to danielbachhuber:

It's not uncommon for me to hook expensive processes (for instance, a call to get_terms()) onto a rewrite filter because, as a developer, I know flushing rewrite rules only happens in contexts where it's fine for me to have an expensive process.

Can you expand on this use case?

[danielbachhuber]

Replying to ericlewis:

Can you expand on this use case?

See ​http://vip.wordpress.com/documentation/remove-the-category-base-from-your-category-permastructs/

[mboynes]

I'm with @danielbachhuber. I've also hooked relatively expensive processes into a generate rewrite rules filter because I understood how and when it would be run. I've done this in cases to improve performance for the average page load (e.g. generating a rewrite rule for a taxonomy with very few terms to be something like (foo|bar|bat)/?$ so I don't have to sort out collisions with greedy rules).

[rmccue]

FWIW, I'd tend to agree, which is why I hadn't patched this up. It's easy to write for if you're expecting it to get called every page. I'd also like to note that the result of get_terms is cached, so that specific example is fine on sites with object caching. (If you're writing rules like that for performance reasons, it'd be crazy not to have object caching installed. That doesn't help for the general case of plugins doing it though, obviously.)

That said, we could consider bringing this functionality in as part of a "lazy" reload system. Basically, we'd have the normal stored rewrite rules, plus an additional part generated on load and mixed in (but not saved). That's new functionality that you'd have to opt-in to and I'd prefer not to do that personally, but it is an option.

[MikeSchinkel]

Is there any appetite for re-imagining the URL rewrite process, one that would be forward incompatible but backward compatible, i.e. that old rewrites would still work?

The current system has limitations in that URLs must be context free, i.e. that a URL pattern can mostly only be used for one type of rewrite. What that means, for example, is that only slugs for post_type='page' can route from the first URL path segment, i.e. we can't ever have this:

example.com/about/ <-- A page
example.com/mikeschinkel/ <-- an author
example.com/atlanta/ <-- an location post type

Thanks to the addition of the 'do_parse_request' hook and an insight gained from Otto we've been able to implement complex routing in our client's web applications, and it works brilliantly. And if we are given a URL path we don't need to route it just falls back to WordPress' default URL rewrites.

Otto's insight is that you can segment the URLs by number of path segments and then only need to test for patterns that match the path segments of the incoming UR: path which significantly reduces the number regular expressions needed to test for a match, except for pages of course. Add to that the ability to cache common URL slugs, such as categories, and we could eliminate most of the time spent looping and evaluating the 100+ rewrite Regexes on ever page load.

It would be great if we could see an new URL Routing API introduced to WordPress core, phased in over a period of numerous releases. Is there any stomach for considering this?

[danielpataki]

I don't want to edit the Codex on this one because I'm not certain, but isn't the Codex on flush_rewrite_riles incorrect?
​http://codex.wordpress.org/Function_Reference/flush_rewrite_rules

It recommends doing the following:

function myplugin_flush_rewrites() {
	// call your CPT registration function here (it should also be hooked into 'init')
	myplugin_custom_post_types_registration();
	flush_rewrite_rules();
}

register_activation_hook( __FILE__, 'myplugin_flush_rewrites' );

function myplugin_flush_rewrites_deactivate() {
	flush_rewrite_rules();
}

register_deactivation_hook( __FILE__, 'myplugin_flush_rewrites_deactivate' );

This is fine for activation but simply doesn't work on deactivation for reasons discussed in this thread.

[Chouby]

However, there is a race condition on the deactivate request: init triggers first, adding your rewrite rule, so when you flush rewrite rules in the deactivation function, the rewrite rule persists.

I use this test just after calling register_deactivation_hook() to detect if the plugin is currently being de-activated:

if (isset($_GET['action'], $_GET['plugin']) && 'deactivate' == $_GET['action'] && plugin_basename(__FILE__) == $_GET['plugin'])
	return;

So when the plugin is de-activated, only the function hooked to register_deactivation_hook() is executed.

[jsherk]

I am working on a plugin and just discovered this problem.

You need to use 'init' to keep adding your rule in case another plugin calls flush_rewrite_rules(), otherwise it will be removed.

But calling flush_rewrite_rules() on deactivation will not remove the rule because it was just added on the init call first.

I like ericlewis suggestion of adding a remove_rewrite_rule that could be called on deactivation so the rule is actually removed.

[jsherk]

If using the INIT hook to add_rewrite_rule, will this potentially add the same rule multiple times to the queue until flush_rewrite_rules() is called, and then will this same rule get added multiple times (and therefore have to be removed multiple times)?

[mdgl]

See also #42563.

[Krstarica]

It’s been a decade since this ticket was opened. Having a solution for this issue would be greatly appreciated.