Use common naming for context switching functions / classes

[swissspidy]

I'm creating this ticket after discussions in #26511 and #25293. A quick overview:

As we all know, there are switch_to_blog() and restore_current_blog() to switch between different sites by adding the sites to the global $_wp_switched_stack and doing a bunch of other stuff.

Now, #26511 aims to introduce switch_to_locale() to do the same for the site's locale. To avoid using (too many) global variables, a WP_Locale_Switcher class was suggested. The locale names are stored in a stack as well

#25293 is about improving switch_to_blog() and introducing switch_to_site() as well as switch_to_network(). #37958 suggests using Network_Sate objects for the stack instead of simply the blog IDs.

#19572 is somewhat related as well, though I don't think a switch_to_post() function makes sense. Fun fact: WP_Query was once named WP_Query_State, see [1449].

---

Now, I want to suggest using some kind of naming convention for new WP_Locale_State and WP_Site_State / WP_Network_State classes to make things easier for developers. Those essentially do the same thing:

• Switch to a new state
• Add this state to a stack
• Pop an item off the stack
• Pop all items off the stack (#37958)

Solid naming is very important to avoid confusion. As mentioned, there are switch_to_blog() and restore_current_blog(). #26511 suggests adding switch_to_locale() & restore_previous_locale(). #19572 suggests switch_to_post() and restore_post(). Ugh.

The simplest solution for that is to introduce an interface that these new classes would need to implement. Basically something along the lines of this:

<?php

interface WP_State {
        /**
         * Switch to new context.
         */
        public function switch_to();

        /**
         * Restore previous context.
         */
        public function restore();

        /**
         * Restore the original context.
         */
        public function clear();

        /**
         * Is context switched?
         */
        public function is_switched();
}

After that, the functions would need to be called similarly switch_to_*() and restore_previous_*(). IMHO restore_previous_*() makes more sense thanrestore_current_*(), but well…

I know that adding an interface or maybe an abstract class for these seems like over-engineering, but it's a good way to communicate the idea.

The least thing we should do is use common naming when introducing such new classes. An interface could always follow later.

[flixos90]

Ah, now it makes sense to me! :)

I like the idea of adding such an interface. About the existing function names, on the one hand I think we should try to align with them as we can't change them. Since the only functions that already exist (of those that you mentioned) are switch_to_blog() and restore_current_blog(), I think we should use those prefixes for the method names of such an interface. On the other hand restore_previous() really would be more accurate than restore_current().

Regarding the interface you suggested:

• I would prefer not to use clear() as method name. Think about wrapping functions like clear_blog(), that sounds more like a destructive action. Instead I'd propose to use restore_original(), and the method you called restore() could become restore_current() or restore_previous() (depending on what we decide, see above).
• I think the suffix in switch_to() is unnecessary, we could also call the method switch() (also, I assume you simply missed that in the above snippet, but let's keep in mind that we need a parameter there :) ).

[flixos90]

Also, note that #37958 also includes popping all items off the stack until a certain "breakpoint" that has been set prior. This often will be the same as "popping all items off the stack", but it's a little more flexibility that this interface would need to support as well I think.

[ChriCo]

Howdy and good evening.

When i see the word "State", than the first think i think of is is somehow in the direction of the "State" or a "Memento"-Pattern.

Also important should be the functionality undo and redo for quick access/switching between states. Clear naming is here very important. When choosing something like clear or delete, we need to add at least state to the -function name. Additionally it would be useful to remove the switch_to-functions completly - yep i know..in future! Set it to deprected and call the new class inside - and just use it in via the WP_Service_Container which is currently discussed in #37699.

Just some random note: the restore-naming is also somehow missleading...do i need a database backup to restore my blog? (just joking).

[swissspidy]

Replying to flixos90:

I think the suffix in switch_to() is unnecessary, we could also call the method switch() (also, I assume you simply missed that in the above snippet, but let's keep in mind that we need a parameter there :) ).

switch is a reserved keyword in PHP. Using it as the method name is only allowed in PHP 7+ IIRC. It was only an example anyway :-)

Replying to flixos90:

Also, note that #37958 also includes popping all items off the stack until a certain "breakpoint" that has been set prior. This often will be the same as "popping all items off the stack", but it's a little more flexibility that this interface would need to support as well I think.

Not sure how likely it is that you want to go to a specific point in the stack and how feasible it would be to implement this for locales for example. Sounds complex.

Replying to ChriCo:

When i see the word "State", then the first i think of is is somehow in the direction of the "State" or a "Memento"-Pattern.

Maybe context is more appropriate.

Also important should be the functionality undo and redo for quick access/switching between states. Clear naming is here very important. When choosing something like clear or delete, we need to add at least state to the -function name. Additionally it would be useful to remove the switch_to-functions completely - yep i know..in future! Set it to deprecated and call the new class inside - and just use it in via the WP_Service_Container which is currently discussed in #37699.

undo would be the current restore I guess? redo sounds unnecessary, but could be solved with a second stack.

I haven't followed #37699 in a while, but it shouldn't affect this ticket in any way for now.

[flixos90]

Replying to swissspidy:

switch is a reserved keyword in PHP. Using it as the method name is only allowed in PHP 7+ IIRC. It was only an example anyway :-)

Oops. :)

Not sure how likely it is that you want to go to a specific point in the stack and how feasible it would be to implement this for locales for example. Sounds complex.

I don't think it's too complex, both the snippet in #37958 and the initial patch in #25293 have a possible implementation.
About the likeliness, this functionality shouldn't exist because a developer might wanna restore a certain point, but rather to prevent weird bugs when multiple parties (like plugins) are involved. Let's say one plugin iterates through sites and then restores back, and then another plugin iterates through some sites itself from within the loop of the other plugin - if the latter did a global restore then, it would mess up the state because the first plugin would already have some sites in the stack. Yes, it is unlikely, but not impossible, and it could easily lead to errors. It actually might be best not to even offer a way to clear the entire stack in order to prevent this problem from happening and instead encourage restoring to a certain point.

[jdgrimes]

It sounds like maybe we need each stack to be a stack of stacks, to keep track of the state of the state. :-)

As far as context vs state, I think that context might make more sense here. When I hear state, I think of a snapshot of the entire state of the application being saved. More like switching to the previous site state will roll back the database to that point or something. Context indicates to me what is the current "stuff": site, user, etc.

I'd definitely favor introducing an interface. As far as the method/function naming, I think that the term "switch" makes sense, but "restore" could be replaced with something better. "Restore" was fine when switch_to_blog() was always used just once and paired with restore_current_blog(), but now we are talking about switching through several states, and maybe switching back, and maybe not. So restore_current_blog() is no longer only used to switch back to the "current" blog, and "restore" without "current" doesn't make as much sense to me. switch_back, unswitch, previous, or pop would be probably be better/more familiar, though obviously not all of those pair with switch very well. I'd say restore would really make more sense if it was taking us back to the original state. I realize that that would break with current naming conventions, though.

[Mista-Flo]

It's a good idea to implement an interface :)

[jeremyfelt]

Replying to swissspidy:

Replying to ChriCo:

When i see the word "State", then the first i think of is is somehow in the direction of the "State" or a "Memento"-Pattern.

Maybe context is more appropriate.

Yes, that seems more descriptive than state.

[swissspidy]

Given the lack of traction and the unnecessary burden this would put on devs and maintainers, closing this as wontfix.