Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#40279 closed defect (bug) (wontfix)

switch_to_blog() and restore_current_blog() gives a fatal error in a multisite

Reported by: kestutisit's profile KestutisIT Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: close
Focuses: multisite Cc:

Description

So I've built a multisite plugin, and my plugin uses namespaces, and that plugin does it's checks based on column.
So it works well in multisite, but when site is not a multisite, plugin gives fatal error:

switch_to_blog($this->conf->getBlogId());

This also gives the fatal error (even despite that current blog is really = 1 when the multisite is off):

switch_to_blog(1);

And this also:

restore_current_blog();

The fatal error is like this:

Fatal error: Call to undefined function <MyPluginName>\Models\Booking\switch_to_blog() in C:\<..>\<MyWebsite>\wp-content\plugins\<MyPluginName>\Models\Booking\class.BookingsObserver.php on line 561.

And the error is really interesting, because it is trying to load that WordPress function, from my current namespace, while it don't do with any other WordPress function like that, so I guess there is a missing namespace definition in that class.

A workaround is this code:

<?php
if(is_multisite())
{
   switch_to_blog($this->conf->getBlogId());
}

But it should not be mandatory.

Change History (6)

#1 follow-up: @ocean90
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
  • Version 4.7.3 deleted

Hello @KestutisIT, thanks for your report.

switch_to_blog() is defined in wp-includes/ms-blogs.php which is only loaded if the current install is a multisite install.
If you have a plugin that is only useful for multisite installs you should check that appropriately and prevent an activation.

#2 in reply to: ↑ 1 ; follow-up: @KestutisIT
8 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to ocean90:

Hello @KestutisIT, thanks for your report.

switch_to_blog() is defined in wp-includes/ms-blogs.php which is only loaded if the current install is a multisite install.
If you have a plugin that is only useful for multisite installs you should check that appropriately and prevent an activation.

No, my plugin works in both ways, but it should work with multisite disabled, you SHOULD NOT have to write different code if multisite is enabled, otherwise it breaks the logic of PHP, OOP, and whatever what is called clean code, good programming, PSR-4/PSR-2 coding standards. It just a design error. At least no action blank function should be made for that. My plugin is made to have blog_id column in tables, and it allows easily manage cross-side data, but if the multisite is disabled, it just have "1" saved there, and all WP functions works, except that switch_to_blog. So that has to be fixed. I'm sure.
The great programming logic says - you have to keep the same interface, just implementation can be different (so 'internal function code of WordPress functions based of enabled/disabled multisite, but the function should be there, just do nothing in WP non-multisite). That is a basic of OOP, of any modern programming.

Last edited 8 years ago by KestutisIT (previous) (diff)

#3 in reply to: ↑ 2 ; follow-up: @jeremyfelt
8 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

Replying to KestutisIT:

you SHOULD NOT have to write different code if multisite is enabled

For better or worse, this is the required pattern when developing something that works in single and multisite configurations. The same is_multisite() check included in the original ticket description is exactly what WordPress core does throughout its code base.

#4 in reply to: ↑ 3 @KestutisIT
8 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to jeremyfelt:

Replying to KestutisIT:

you SHOULD NOT have to write different code if multisite is enabled

For better or worse, this is the required pattern when developing something that works in single and multisite configurations. The same is_multisite() check included in the original ticket description is exactly what WordPress core does throughout its code base.

Please read the BASICS of PHP and coding. Please read why they decided to create PHP7, what is predefined types, and where PHP is going. You are wrong, you can't do all your code on IF/ELSE. There is a thing called design patterns, and we have to follow them. It says that if you have PUBLIC method (same is everywhere available WordPress function) is has to be callable, just can be empty by the abstract class inheritance if it is not defined. That is a BASICS of the code. I'm reopening this for WordPress Lead developer to review. Please don't close it.

"Object interfaces allow you to create code which specifies which methods a class must implement, without having to define how these methods are handled.

Interfaces are defined in the same way as a class, but with the interface keyword replacing the class keyword and without any of the methods having their contents defined.

All methods declared in an interface must be public; this is the nature of an interface."

http://php.net/manual/en/language.oop5.interfaces.php

#5 @flixos90
8 years ago

  • Keywords close added

WordPress is obviously not following PSR or anything related. It might be arguable whether this is good or not, but the fact is that WordPress also has its patterns, one of which is that one has to enable multisite to get access to multisite functionality. This has been established over years as the way WordPress works. Single site is different from multisite, and related pieces of code should account for that.

I'm not closing this since you asked for a lead developer review, but I suggest closing as well.

#6 @johnbillion
8 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

@KestutisIT Thanks for being passionate about improving WordPress!

With Multisite enabled, WordPress operates in a very different manner to a single site installation. In your code example, what return value is expected from $this->conf->getBlogId() on a single site installation? Single site installations don't even have the concept of a blog ID, so the application logic here is flawed.

It may make more sense for single site installations to also operate with the concept of a blog ID, purely so interoperability with multisite installations is improved, but that ship sailed about 7 years ago.

The somewhat confusing error message you're seeing is due to PHP attempting to resolve the procedural function name in the current namespace after attempting to resolve it in the global namespace, before throwing the fatal error. You'll see the same format error in any situation where you attempt to call an undefined function from within a namespace.

Note: See TracTickets for help on using tickets.