WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 2 weeks ago

#14170 closed enhancement (fixed)

Problems with network activation

Reported by: scribu Owned by:
Milestone: 3.1 Priority: high
Severity: major Version:
Component: Multisite Keywords: needs-patch
Focuses: Cc:

Description

When you do a network activation of a plugin, register_activation_hook() fires only for the current site.

This causes plugins to behave incorrectly on every other site.

Attachments (5)

deactivate_plugins.diff (1.2 KB) - added by scribu 4 years ago.
14170.diff (2.9 KB) - added by scribu 3 years ago.
pass $network_wide to callbacks
14170.2.diff (3.4 KB) - added by scribu 3 years ago.
Remove extraneous trim() calls
14170.3.diff (1.5 KB) - added by scribu 3 years ago.
activation for new sites
14170.4.diff (2.2 KB) - added by scribu 3 years ago.
Handle network activation via filters

Download all attachments as: .zip

Change History (78)

comment:1 nacin4 years ago

For starters:

If a plugin has an activation hook, perhaps after network activation, we show a giant notice warning the administrator of their demise.

comment:2 scribu4 years ago

Yeah, that would be a good start.

comment:3 scribu4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

comment:4 wpmuguru4 years ago

With a few lines of code any plugin can be made to handle network activation and function as though it had been activated on each site individually. For an example, the network privacy plugin (http://wordpress.org/extend/plugins/network-privacy/) is coded to check for network vs individual site activation.

comment:5 nacin4 years ago

That plugin uses $_GET['networkwide'], which wouldn't be set in the case of a bulk network activate. It's too internally to be relied on... Realistically should be a is_plugin_active_for_network() call.

comment:6 scribu4 years ago

Another thing to consider is what happens to a network plugin when a new site is added to that network.

comment:7 nacin4 years ago

  • Priority changed from normal to high

Also, uninstall hooks. We should probably store the callback in sitemeta instead of blog options.

Ryan and I previously spoke about requiring network activation/deactivation from the main site only. This ticket strongly reinforces that suggestion as valid. Otherwise you get activation hooks potentially firing any blog. It's much easier if it is confined to the main site.

comment:8 nacin4 years ago

... store the callback in sitemeta for network plugins. I'm not sure how that could work, as it could always be deactivated on another blog and if the activation wasn't fired properly, then that's a problem. Just presenting an additional argument for requiring network actions from is_main_site().

comment:9 nacin4 years ago

What if the plugin was already activated on blog C, when the plugin is then network activated from any blog (main or otherwise)? Then the activation routine could potentially fire twice. Generally a plugin should be mindful of potential activation being called without deactivation first (in the case of file system changes), but again... Another potential for issues.

comment:10 scribu4 years ago

I think it makes sense to alow network (de)activation only from the main site.

comment:11 scribu4 years ago

I found an issue with deactivation as well. Prerequisites: A plugin with a deactivation hook.

On the same blog:

  1. Do a normal activation of plugin X
  1. Do a network activation of plugin X
  1. Do a network deactivation of plugin X

You will find that the deactivation hook was triggered, even though the plugin is still active on that blog.

comment:12 scribu4 years ago

Actually, for step 2, you will have to go to a different blog.

comment:13 follow-up: nacin4 years ago

I wonder if a plugin can easily detect and account for network activation/deactivation when attached to those hooks. Likewise, I wonder if a plugin can easily detect that a plugin about to be deactivated will still be active in some cases.

That said, if we restrict the process to the main site, then those steps won't occur. Once a plugin is activated, it must be deactivated before it can be network activated. Or, as it is currently, you can simply go to another blog that has it deactivated and then network activate it from there.

comment:14 scribu4 years ago

  • Keywords has-patch added; needs-patch removed

deactivate_plugins.diff fixes the above mentioned issue.

comment:15 scribu4 years ago

  • Summary changed from Problem with network activation and activation hook to Problems with network activation

comment:16 in reply to: ↑ 13 scribu4 years ago

Replying to nacin:

I wonder if a plugin can easily detect and account for network activation/deactivation when attached to those hooks. Likewise, I wonder if a plugin can easily detect that a plugin about to be deactivated will still be active in some cases.

I am currently working on a plugin that addresses the first issue. For the second, it's not so easy, hence the patch.

scribu4 years ago

comment:17 nacin4 years ago

An API for detecting exactly which activation and deactivation process is occurring based on the status of the plugin on the main blog, the current blog, and the network will be important. But the patch would no longer be necessary if we limited network activation/deactivation to the main blog.

comment:18 nacin4 years ago

  • Milestone changed from Future Release to 3.1

Moving to 3.1 --

  • Restrict network activation/deactivation to the main site only.
  • Consider better hooks or functions to allow plugins to detect the situation in which the plugin is being activated or deactivated.

comment:19 scribu4 years ago

  • Keywords needs-patch added; has-patch removed

comment:21 scribu4 years ago

With #14435 in, I think this would do:

  • let plugin authors handle network activation; help them by passing a $network_activation flag to the callback.
  • when a new site is created, fire the activation hooks for all active plugins (network activated or not).

comment:22 coolmann4 years ago

Hi everybody, I know this is not a support forum, but please allow me to ask a question about this specific issue. I'm developing a plugin and I'd like to make it multi-site aware. Since things are changing in WP 3.1 about the network activation, per your discussion here above, I wanted to understand if it's safe to use the following code for my activation function

register_activation_hook( __FILE__, 'coolmann_activate' );
function coolmann_activate() {
	global $wpdb;
	if (function_exists('is_multisite') && is_multisite()) {
		if (isset($_GET['networkwide']) && ($_GET['networkwide'] == 1)) {
	                $old_blog = $wpdb->blogid;
			// Get all blog ids
			$blogids = $wpdb->get_col($wpdb->prepare("SELECT blog_id FROM $wpdb->blogs"));
			foreach ($blogids as $blog_id) {
				switch_to_blog($blog_id);
				_single_blog_activate();
			}
			switch_to_blog($old_blog);
			return;
		}	
	} 
	_single_blog_activate();
}

where _single_blog_activate() is the function that I use to initialize options, tables, etc. What do you think?

comment:23 follow-up: scribu4 years ago

Hi coolmann, that's basically what my Proper Network Activation plugin does for all plugins that have an activation hook.

For now, I think it's better you simply call _single_blog_activate().

Instead of making plugin authors go through this for each plugin they have, maybe register_activation_hook() should accept a third parameter, like $do_for_all_blogs = true. This way, plugins that want to handle network activation themselves can do so easily.

comment:24 in reply to: ↑ 23 coolmann4 years ago

Thank you for your quick reply, Scribu, really appreciated. I agree about the extra parameter (which would replace the clunky $_GET value), but the same thing should be added to the deactivation hook. Also, what about the 'wpmu_new_blog' hook, is it safe to assume it will be still available in WP 3.1 to run the activation script when a new blog is created (and the plugin is network-activated?) I would suggest keeping this hook and let plugin developers handle the activation accordingly.

comment:25 jane3 years ago

Freeze is a few days away, so if this is getting patched for 3.1, now's the time.

scribu3 years ago

pass $network_wide to callbacks

comment:26 scribu3 years ago

  • Keywords has-patch added; needs-patch removed

14170.diff takes a conservative approach, simply passing the $network_wide flag to (de)activation hooks.

scribu3 years ago

Remove extraneous trim() calls

comment:27 scribu3 years ago

Related: #14915

comment:28 scribu3 years ago

(In [16011]) Pass $network_wide flag to de/activation hooks. See #14170

comment:29 coolmann3 years ago

Hi Scribu, where can I find some documentation on how to properly retrofit my plugins to make them WP-MU aware with your new approach? My understanding is that now register_activation_hook will have a new parameter that specifies if this is a network activation, am I correct?

comment:30 scribu3 years ago

So, if you were doing this:

register_activation_hook( __FILE__, 'my_plugin_activation' );

function my_plugin_activation() {
  // usual activation code
}

You can now do this:

register_activation_hook( __FILE__, 'my_plugin_activation' );

function my_plugin_activation( $network_wide ) {
  if ( $network_wide ) {
    // network activation code
  } else {
    // usual activation code
  }
}

comment:31 scribu3 years ago

One thing I still want to land is activating network plugins when a new site is created.

comment:32 follow-up: coolmann3 years ago

I think it should be up to each plugin developer to decide if his/her plugin needs to be multisite aware or not. In my experience, most plugins work just fine with no knowledge of being used in a "network", because they don't use any custom tables, and are accessing WP tables via $wpdb->prefix and not $table_prefix.

scribu3 years ago

activation for new sites

comment:33 scribu3 years ago

  • Keywords commit added

comment:34 in reply to: ↑ 32 scribu3 years ago

In my experience, most plugins work just fine with no knowledge of being used in a "network", because they don't use any custom tables, and are accessing WP tables via $wpdb->prefix and not $table_prefix.

For example, If they need to set some options, (options are per-site), and don't account for network activation, problems do appear.

Replying to coolmann:

I think it should be up to each plugin developer to decide if his/her plugin needs to be multisite aware or not.

Indeed. The modification I posted above is entirely optional.

comment:35 coolmann3 years ago

  • Cc coolmann added

According to 14170.3.diff, I would not need to specify the "foreach blogid" loop anymore, am I right? Just to recap it would now be something like:

register_activation_hook( __FILE__, 'my_plugin_activation' );

function my_plugin_activation( $network_wide ) {
  // usual activation code
}

It seems like $network_wide is not needed anymore, since WP is taking care of performing the appropriate actions (foreach), right?

comment:36 scribu3 years ago

No, 14170.3.diff is for when a new site is created, but the network already has plugins active.

comment:37 follow-up: scribu3 years ago

(In [16021]) Trigger activation hooks for netork plugins when new site is created. See #14170

comment:38 in reply to: ↑ 37 coolmann3 years ago

Replying to scribu:

(In [16021]) Trigger activation hooks for netork plugins when new site is created. See #14170

I see. Good point. So, in other words, I can remove the following 'custom' code from my plugins, am I right?

// Make WP SlimStat multi-site aware
add_action( 'wpmu_new_blog', array( &$wp_slimstat,'new_blog_activate'), 10, 6);
function new_blog_activate(){
	global $wpdb;
 	if (is_plugin_active_for_network('path_to_plugin')){
		$old_blog = $wpdb->blogid;
		switch_to_blog($blog_id);
		_single_blog_activate();
		switch_to_blog($old_blog);
	}
}

This is now automatically done by Wordpress, with your patch, if I'm not mistaken.

comment:39 follow-up: scribu3 years ago

Exactly.

comment:40 in reply to: ↑ 39 coolmann3 years ago

Replying to scribu:

Exactly.

Wow, you rock my friend! :) Is this patch going to be applied to 3.1? I think it's one of the best news for plugin developers around the globe! If you happen to be in New York, let me know... I'd love to buy you a case of beer!

comment:41 scribu3 years ago

  • Keywords needs-patch added; has-patch commit removed

It's already in: [16021]

Setting needs-patch for another action that would automatically do the looping when activating a new network plugin.

scribu3 years ago

Handle network activation via filters

comment:42 scribu3 years ago

  • Keywords has-patch added; needs-patch removed

14170.4.diff completes the puzzle.

Since it's done via filters, it can easily be swapped with something else.

comment:43 nacin3 years ago

-1. This won't scale. I don't think we should do r16021 either.

comment:44 scribu3 years ago

What's wrong with [16021] ?

comment:45 follow-up: nacin3 years ago

We're only firing network updates for a single blog. [16021] introduces an inconsistency -- that it'll be applied only to the main blog, and any new blogs -- that can only be fixed by 14070.4.diff. I don't think we should do that -- leave it to plugins -- so I think we should remove this as well.

comment:46 scribu3 years ago

Ok, so nacin explained it to me better over IRC:

(01:57:06 AM) nacin: right, my point is, network activating a plugin needs to run in three cases:
(01:57:16 AM) nacin: 1) main site, when the activate link is clicked.
(01:57:22 AM) nacin: 2) all other sites that exist at that time.
(01:57:28 AM) nacin: 3) any new sites created.
(01:57:34 AM) nacin: 1 was before today.
(01:57:36 AM) nacin: 3 is now.
(01:57:40 AM) nacin: 2 just doesn't scale.
(01:57:49 AM) nacin: hence I suggested, maybe we shouldn't do 3 if we can't do 2.

comment:47 coolmann3 years ago

Which, in other words, means that plugin developers will have to continue using this code when a new blog is created and the plugin is "network activated", right? Just trying to understand. Thank you.

// Make WP SlimStat multi-site aware
add_action( 'wpmu_new_blog', array( &$wp_slimstat,'new_blog_activate'), 10, 6);
function new_blog_activate(){
	global $wpdb;
 	if (is_plugin_active_for_network('path_to_plugin')){
		$old_blog = $wpdb->blogid;
		switch_to_blog($blog_id);
		_single_blog_activate();
		switch_to_blog($old_blog);
	}
}

comment:48 coolmann3 years ago

Sorry, I forgot to ask... what about $network_wide ? Is that patch going to be approved? Thanks.

comment:49 follow-up: nacin3 years ago

  • Type changed from defect (bug) to enhancement

The reason why I am against this in core is demonstrated precisely by your plugin. Looking through it, you're creating an *awful* lot of tables. You are creating six tables *per blog*. That can't scale in any reasonable fashion.

No network administrator would let that on their site, given plenty of other plugins with far less footprint. I'm weary of plugins having an official API to do things like this. If someone wants your plugin on their network, they're going to look to disable all of that so they can generate those tables on their own. (It took something like two weeks to generate commentmeta tables for every site on WordPress.com.)

Sidenote - A lot of those are lookup tables and should probably be global tables. The ones that aren't can be post types. Then you won't need to do this at all.

comment:50 in reply to: ↑ 49 coolmann3 years ago

Hi Andrew, thank you for your notes, I know my plugin still needs to be optimized for network installations. You're right about making the lookup tables global (the one that stores the geolocation data is already global, indeed), I hadn't thought about that. Since I'm afraid we may be off-topic here, may I contact you in some other way to discuss a little bit? I actually wanted to talk to you about the network activation thing at the WordcampNYC, but there where too many people waiting to talk to you and I had a train to catch :) Please let me know.

Camu

comment:51 in reply to: ↑ 45 alexrabe3 years ago

Andrew,

if we remove [16021]. What is the best way to add additional tables in a network installation ? Or is it deprecated for plugin authors to add additional tables at all ?

comment:52 scribu3 years ago

Adding additional tables is not deprecated, just frowned upon.

A way to make network activation scale would be to have a multi-step procedure, similar to Update network (which performs the db update procedure for each site).

Speaking of which, what if we just hooked into that?

comment:53 scribu3 years ago

Ah, yes, the 'wpmu_upgrade_site' can be used to perform the activation, 5 sites at a time.

I will update the Proper Network Activation plugin to use this.

comment:54 coolmann3 years ago

Scribu, could you please elaborate on how to use this hook? Thanks.

comment:55 alexrabe3 years ago

Ok Scribu, I will do my best to reduce the amount of tables , to win your favour again. In the meantime, is there any source/sample how a plugin can reliable install/upgrade tables in a network enviroment ? It's for me hard to understand which filter/hooks I should use now in 3.1

comment:56 scribu3 years ago

  • Keywords needs-patch added; has-patch removed

It turns out network/upgrade.php was not the best place to hook into.

I've updated the Proper Network Activation to do activation on 5 sites at a time, instead of all at once.

http://plugins.trac.wordpress.org/browser/proper-network-activation/trunk/proper-network-activation.php?rev=305871

It involves keeping a queue of de/activations that need to occur and sending AJAX requests until all sites are processed.

Please try it out: http://wordpress.org/extend/plugins/proper-network-activation/

So, alexrabe, that would be a scalable way of doing it.

I think we can all agree that it's a lot of code for each plugin to have to lug around.

I'm going to try and submit it as a patch before November 1st.

comment:58 alexrabe3 years ago

Get the point, didn't think in this way. You are right activating a plugin in network environment should be handled via ajax...

comment:59 scribu3 years ago

  • Milestone changed from 3.1 to Future Release

Didn't come up with a patch in time. Best not to rush.

comment:60 coolmann3 years ago

So this means we should NOT support WP MU in our plugins at all but suggest users to install your plugin, right? No foreach, no $network_install, etc, has yet been rolled into WP 3.1, am I right?

comment:61 scribu3 years ago

Yes, that's what I'm doing with my plugins anyway.

comment:62 follow-up: alexrabe3 years ago

I'm very surprised that there will be not rework for 3.1, just for this holy freeze deadline ?

comment:63 in reply to: ↑ 62 nacin3 years ago

Replying to alexrabe:

I'm very surprised that there will be not rework for 3.1, just for this holy freeze deadline ?

One, deadlines are not arbitrary. Two, there needs to be agreement among the core developers for this to be implemented, and there clearly isn't.

comment:64 nacin3 years ago

(In [16213]) Revert [16021]. See #14170.

comment:65 coolmann3 years ago

Any news on this one, guys?

comment:66 dd323 years ago

Any news on this one, guys?

There'll be no change to this in 3.1, After 3.1, decisions can be made regarding changes for 3.2

comment:68 nacin3 years ago

  • Milestone changed from Future Release to 3.1
  • Resolution set to fixed
  • Status changed from new to closed

Activation hooks are lame, particularly in a network situation. It's far better to use an upgrade routine fired on admin_init, and handle that per-site, basing it on a stored option.

Due to this comment: http://core.trac.wordpress.org/ticket/14170#comment:18, I am closing as fixed on 3.1. Anything else can be covered in new tickets.

comment:69 scribu3 years ago

Activation hooks are lame, particularly in a network situation.

Blasphemy! :)

Although, looking back at it, they do seem to be a hassle, even in a non-multisite environment.

comment:70 maorb20 months ago

  • Cc maor@… added

comment:71 khromov2 weeks ago

What is the status of this issue today as of WP 3.8?

Are activation hooks still only run on the first site (site id = 1) ?

comment:72 coolmann2 weeks ago

We ended up solving the issue by running the activation/upgrade function on a new blog the first time the user visits the corresponding admin screen for that blog ;) This way it scales without any issues.

comment:73 khromov2 weeks ago

Sounds great, thanks coolmann!

Note: See TracTickets for help on using tickets.