[wp-trac] [WordPress Trac] #23016: Allow plugins to manipulate WP_Roles ($wp_roles global) on construct

[WordPress Trac]

#23016: Allow plugins to manipulate WP_Roles ($wp_roles global) on construct
-------------------------------------------------+-------------------------
 Reporter:  johnjamesjacoby                      |       Owner:  pento
     Type:  defect (bug)                         |      Status:  closed
 Priority:  normal                               |   Milestone:  4.7
Component:  Role/Capability                      |     Version:  3.5
 Severity:  normal                               |  Resolution:  fixed
 Keywords:  has-patch has-unit-tests commit      |     Focuses:
  needs-dev-note                                 |
-------------------------------------------------+-------------------------

Comment (by johnjamesjacoby):

 @mnelson4 - interesting thoughts. I think the original behavior was
 unintentional, and problematic for the way WordPress (and plugins &
 themes) interact with Roles.

 The old `reinit()` method used `$this` vs. directly touching the
 `$wp_roles` global, so the current `_init()` method is a direct equivalent
 -- either one could be used for the same end result (though, I think I
 found a different consequence.)

 Please consider these (very crude) examples (that should probably be unit
 tests):

 {{{
 add_action( 'init', function() {
         $wp_roles                 = new WP_Roles();
         $my_reference_to_wp_roles = $wp_roles;
         $wp_roles->foo            = 'bar';

         var_dump( $wp_roles->foo );
         var_dump( $my_reference_to_wp_roles->foo );

         die;
 } );
 }}}

 Will output:

 {{{
 string 'bar' (length=3)
 string 'bar' (length=3)
 }}}

 `$my_reference_to_wp_roles` was never modified, but because the variable
 it points to was, it was too.

 ----

 The same happens if you use the global:

 {{{
 add_action( 'init', function() {
         global $wp_roles;

         $my_reference_to_wp_roles = $wp_roles;
         $wp_roles->foo            = 'bar';

         var_dump( $wp_roles->foo );
         var_dump( $my_reference_to_wp_roles->foo );

         die;
 } );
 }}}

 Will output:

 {{{
 string 'bar' (length=3)
 string 'bar' (length=3)
 }}}

 ----

 If we intentionally try to wipe out `$wp_roles`, we can. Our clone keeps
 the changes (stale compared to the global.)

 {{{
 add_action( 'init', function() {
         global $wp_roles;

         $my_reference_to_wp_roles = $wp_roles;
         $wp_roles->foo            = 'bar';

         $wp_roles = $wp_roles->reinit();
         var_dump( $wp_roles->foo );

         $wp_roles->foo = 'bar';
         $wp_roles = new WP_Roles();

         var_dump( $wp_roles->foo );
         var_dump( $my_reference_to_wp_roles->foo );

         die;
 } );

 }}}

 Will output:

 {{{
 string null
 string null
 string 'bar' (length=3)
 }}}

 ----

 Lastly, let's mix & match a changed global reference:

 {{{
 add_action( 'init', function() {
         global $wp_roles, $new_roles;

         $new_roles = $wp_roles;

         $wp_roles->changed = true;
 }, 10 );

 add_action( 'init', function() {
         global $wp_roles;
         $wp_roles->changed = false;
 }, 11 );

 add_action( 'init', function() {
         global $wp_roles, $new_roles;

         var_dump( $wp_roles->changed );
         var_dump( $new_roles->changed );

         die;
 }, 12 );
 }}}

 And we'll get...

 {{{
 boolean false
 boolean false
 }}}

 ----

 There is also `wp_roles()`, which (in)conveniently obscured the problem.
 There might be places (even in core) that would benefit from using
 `wp_roles()` vs `new WP_Roles()`, but we'll need to audit those & make
 decisions based on intent. Basically: yield or not yield. (At a cursory, I
 think all usages are OK.)

 Either way, it appears to my eyes that there's not much of a change here,
 and what change there is creates a more predictable and easier to
 understand interaction with `WP_Roles`.

 ----

 I think that `WP_Roles->reinit()` does need to keep it's `use_db` check,
 though. Otherwise the `$wp_user_roles` isn't going to be respected as the
 omega roles array for the entire installation. Patch imminent for this.

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