Fixes for r28797.
authorAryeh Gregor <simetrical@users.mediawiki.org>
Sun, 23 Dec 2007 19:53:49 +0000 (19:53 +0000)
committerAryeh Gregor <simetrical@users.mediawiki.org>
Sun, 23 Dec 2007 19:53:49 +0000 (19:53 +0000)
* Mark private methods private using a keyword.
* Reject arrays with count == 2: these will fail when you do array_slice( ... , 1 ).
* Treat xor consistent with the other operations: if there's only one parameter the result should just evaluate that, not always return false; and any number of parameters should be allowed.
* Fail fast on bad input: throw an exception if Autopromote encounters a condition it can't understand (after asking extensions).
* Code documentation!  There were five lines of comments in the original commit.
* APCONDS_INGROUPS is not used, or for that matter defined.
* Editcount should use >=, not >, for consistency with past behavior and intuitiveness.
* "autopromoteUser" sounds like it's actually promoting the user somehow.  Renamed the function to getAutopromoteGroups.
* Make sure we don't return the same group more than once, when we're returning a group.  Probably not going to hurt, but may as well be clean.

RELEASE-NOTES
includes/Autopromote.php
includes/DefaultSettings.php
includes/Defines.php
includes/User.php

index b5b73fd..6347359 100644 (file)
@@ -34,8 +34,7 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
      any groups they are in.
 * New permission userrights-interwiki for changing user rights on foreign wikis.
 * $wgImplictGroups for groups that are hidden from Special:Listusers, etc.
-* $wgAutopromote: automatically promote user flags when user matches
-  criterias
+* $wgAutopromote: automatically promote users who match specified criteria
 
 === New features in 1.12 ===
 * (bug 10735) Add a warning for non-descriptive filenames at Special:Upload
index 6b4d976..4220bd9 100644 (file)
@@ -4,21 +4,43 @@
  * This class checks if user can get extra rights
  * because of conditions specified in $wgAutopromote
  */
-class Autopromote {    
-       public static function autopromoteUser( $user ) {
+class Autopromote {
+       /**
+        * Get the groups for the given user based on $wgAutopromote.
+        *
+        * @param User $user The user to get the groups for
+        * @return array Array of groups to promote to.
+        */
+       public static function getAutopromoteGroups( User $user ) {
                global $wgAutopromote;
                $promote = array();
                foreach( $wgAutopromote as $group => $cond ) {
                        if( self::recCheckCondition( $cond, $user ) )
                                $promote[] = $group;
                }
-               return $promote;
+               return array_unique( $promote );
        }
 
-       //@private
-       static function recCheckCondition( $cond, $user ) {
+       /**
+        * Recursively check a condition.  Conditions are in the form
+        *   array( '&' or '|' or '^', cond1, cond2, ... )
+        * where cond1, cond2, ... are themselves conditions; *OR*
+        *   APCOND_EMAILCONFIRMED, *OR*
+        *   array( APCOND_EMAILCONFIRMED ), *OR*
+        *   array( APCOND_EDITCOUNT, number of edits ), *OR*
+        *   array( APCOND_AGE, seconds since registration ), *OR*
+        *   similar constructs defined by extensions.
+        * This function evaluates the former type recursively, and passes off to
+        * self::checkCondition for evaluation of the latter type.
+        *
+        * @param mixed $cond A condition, possibly containing other conditions
+        * @param User  $user The user to check the conditions against
+        * @return bool Whether the condition is true
+        */
+       private static function recCheckCondition( $cond, User $user ) {
                $validOps = array( '&', '|', '^' );
-               if( is_array( $cond ) && count( $cond ) > 0 && in_array( $cond[0], $validOps ) ) {
+               if( is_array( $cond ) && count( $cond ) >= 2 && in_array( $cond[0], $validOps ) ) {
+                       # Recursive condition
                        if( $cond[0] == '&' ) {
                                foreach( array_slice( $cond, 1 ) as $subcond )
                                        if( !self::recCheckCondition( $subcond, $user ) )
@@ -30,23 +52,41 @@ class Autopromote {
                                                return true;
                                return false;
                        } elseif( $cond[0] == '^' ) {
-                               if( count( $cond ) < 3 )
-                                       return false;
-                               return self::recCheckCondition( $cond[1], $user )
-                                       xor self::recCheckCondition( $cond[2], $user );
+                               $res = null;
+                               foreach( array_slice( $cond, 1 ) as $subcond ) {
+                                       if( is_null( $res ) )
+                                               $res = self::recCheckCondition( $subcond, $user );
+                                       else
+                                               $res = ($res xor self::recCheckCondition( $subcond, $user ));
+                               }
+                               return $res;
                        }
                }
+               # If we got here, the array presumably does not contain other condi-
+               # tions; it's not recursive.  Pass it off to self::checkCondition.
                if( !is_array( $cond ) )
                        $cond = array( $cond );
                return self::checkCondition( $cond, $user );
        }
 
-       static function checkCondition( $cond, $user ) {
+       /**
+        * As recCheckCondition, but *not* recursive.  The only valid conditions
+        * are those whose first element is APCOND_EMAILCONFIRMED/APCOND_EDITCOUNT/
+        * APCOND_AGE.  Other types will throw an exception if no extension evalu-
+        * ates them.
+        *
+        * @param array $cond A condition, which must not contain other conditions
+        * @param User  $user The user to check the condition against
+        * @return bool Whether the condition is true for the user
+        */
+       private static function checkCondition( $cond, User $user ) {
                if( count( $cond ) < 1 )
                        return false;
                switch( $cond[0] ) {
                        case APCOND_EMAILCONFIRMED:
                                if( User::isValidEmailAddr( $user->getEmail() ) ) {
+                                       # FIXME: EMAILCONFIRMED is always true if
+                                       # wgEmailAuthentication if false, this is confusing.
                                        global $wgEmailAuthentication;
                                        if( $wgEmailAuthentication ) {
                                                return $user->getEmailAuthenticationTimestamp() ? true : false;
@@ -56,15 +96,17 @@ class Autopromote {
                                }
                                return false;
                        case APCOND_EDITCOUNT:
-                               return $user->getEditCount() > $cond[1];
+                               return $user->getEditCount() >= $cond[1];
                        case APCOND_AGE:
                                $age = time() - wfTimestampOrNull( TS_UNIX, $user->getRegistration() );
                                return $age >= $cond[1];
-                       case APCOND_INGROUPS:
                        default:
-                               $result = false;
+                               $result = null;
                                wfRunHooks( 'AutopromoteCondition', array( $cond[0], array_slice( $cond, 1 ), $user, &$result ) );
-                               return $result;
+                               if( $result === null ) {
+                                       throw new MWException( "Unrecognized condition {$cond[0]} for autopromotion!" );
+                               }
+                               return $result ? true : false;
                }
        }
-}
\ No newline at end of file
+}
index fde53de..44076b4 100644 (file)
@@ -1189,7 +1189,18 @@ $wgAutoConfirmCount = 0;
 //$wgAutoConfirmCount = 50;
 
 /**
- * Automatically promote user flags to users that matchs some conditions
+ * Automatically add a usergroup to any user who matches certain conditions.
+ * The format is
+ *   array( '&' or '|' or '^', cond1, cond2, ... )
+ * where cond1, cond2, ... are themselves conditions; *OR*
+ *   APCOND_EMAILCONFIRMED, *OR*
+ *   array( APCOND_EMAILCONFIRMED ), *OR*
+ *   array( APCOND_EDITCOUNT, number of edits ), *OR*
+ *   array( APCOND_AGE, seconds since registration ), *OR*
+ *   similar constructs defined by extensions.
+ *
+ * Note that EMAILCONFIRMED will always be true if $wgEmailAuthentication is
+ * off!
  */
 $wgAutopromote = array(
        'autoconfirmed' => array( '&',
index d76b347..69096be 100644 (file)
@@ -277,7 +277,8 @@ define( 'SFH_OBJECT_ARGS', 2 );
 # Flags for Parser::replaceLinkHolders
 define( 'RLH_FOR_UPDATE', 1 );
 
-#Autopromote conditions
+# Autopromote conditions (must be here and not in Autopromote.php, so that
+# they're loaded for DefaultSettings.php before AutoLoader.php)
 define( 'APCOND_EDITCOUNT', 1 );
 define( 'APCOND_AGE', 2 );
 define( 'APCOND_EMAILCONFIRMED', 3 );
index 10048a6..e7bbda9 100644 (file)
@@ -1652,10 +1652,10 @@ class User {
                        if( $this->mId ) {
                                $this->mEffectiveGroups[] = 'user';
 
-                               $this->mEffectiveGroups = array_merge(
+                               $this->mEffectiveGroups = array_unique( array_merge(
                                        $this->mEffectiveGroups,
-                                       Autopromote::autopromoteUser( $this )
-                               );
+                                       Autopromote::getAutopromoteGroups( $this )
+                               ) );
 
                                # Hook for additional groups
                                wfRunHooks( 'UserEffectiveGroups', array( &$this, &$this->mEffectiveGroups ) );