Clean up user class. Implement start of unit testing framework for appdb.

Implement start of user class unit test.

account.php

@@ -114,17 +114,26 @@ function cmd_do_new()
 
     $result = $user->create($aClean['ext_email'], $aClean['ext_password'], $aClean['ext_realname'], $aClean['CVSrelease'] );
 
-    if($result == true)
+    if($result == SUCCESS)
     {
         /* if we can log the user in, log them in automatically */
-        if($user->login($aClean['ext_email'], $aClean['ext_password']))
+        if($user->login($aClean['ext_email'], $aClean['ext_password']) == SUCCESS)
             $_SESSION['current'] = $user;
 
         addmsg("Account created! (".$aClean['ext_email'].")", "green");
         redirect(apidb_fullurl());
     }
-    else
+    else if($result == USER_CREATE_EXISTS)
     {
+        addmsg("An account with this e-mail exists already.", "red");
+        retry("new", "Failed to create account");
+    } else if($result = USER_CREATE_FAILED)
+    {
+        addmsg("Error while creating a new user.", "red");
+        retry("new", "Failed to create account");
+    } else
+    {
+        addmsg("Unknown failure while creating new user.  Please report this problem to appdb admins.", "red");
         retry("new", "Failed to create account");
     }
 }
@@ -193,7 +202,7 @@ function cmd_do_login()
     $user = new User();
     $result = $user->login($aClean['ext_email'], $aClean['ext_password']);
 
-    if($result == true)
+    if($result == SUCCESS)
     {
         $_SESSION['current'] = $user;
         addmsg("You are successfully logged in as '$user->sRealname'.", "green");

include/user.php

@@ -6,6 +6,14 @@
 require_once(BASE."include/version.php");
 require_once(BASE."include/util.php");
 
+define(SUCCESS, 0);
+define(USER_CREATE_EXISTS, 1);
+define(USER_CREATE_FAILED, 2);
+define(USER_LOGIN_FAILED, 3);
+define(USER_UPDATE_FAILED, 4);
+define(USER_UPDATE_FAILED_EMAIL_EXISTS, 5); /* user updating to an email address that is already in use */
+define(USER_UPDATE_FAILED_NOT_LOGGED_IN, 6); /* user::update() called but user not logged in */
+
 /**
  * User class for handling users
  */
@@ -65,31 +73,31 @@ class User {
         {
             // Update timestamp and clear the inactivity flag if it was set
             query_appdb("UPDATE user_list SET stamp=NOW(), inactivity_warned='false' WHERE userid=".$this->iUserId);
-            return true;
+            return SUCCESS;
         }
-        return false;
+        return USER_LOGIN_FAILED;
     }
 
 
     /*
      * Creates a new user.
-     * returns true on success, false on failure
+     * returns SUCCESS on success, USER_CREATE_EXISTS if the user already exists
      */
     function create($sEmail, $sPassword, $sRealname, $sWineRelease)
     {
         if(user_exists($sEmail))
         {
-            addMsg("An account with this e-mail exists already.","red");
+            return USER_CREATE_EXISTS;
-            return false;
         } else
         {
             $hResult = query_parameters("INSERT INTO user_list (realname, email, CVSrelease, password, stamp,".
                                         "created) VALUES ('?', '?', '?', password('?'), ?, ?)",
                                         $sRealname, $sEmail, $sWineRelease, $sPassword, "NOW()", "NOW()");
 
-            if(!$hResult) addMsg("Error while creating a new user.", "red");
+            if(!$hResult) return USER_CREATE_FAILED;
 
             $retval = $this->login($sEmail, $sPassword);
+            if($retval == SUCCESS)
                 $this->setPref("comments:mode", "threaded"); /* set the users default comments:mode to threaded */
 
             return $retval;
@@ -100,44 +108,57 @@ class User {
     /**
      * Update User Account;
      */
-    function update($sEmail = null, $sPassword = null, $sRealname = null, $sWineRelease = null)
+    function update()
     {
-        if(!$this->isLoggedIn()) return false;
+        if(!$this->isLoggedIn()) return USER_UPDATE_FAILED_NOT_LOGGED_IN;
 
-        if ($sEmail)
+        /* create an instance of ourselves so we can see what has changed */
+        $oUser = new User($this->iUserId);
+
+        if($this->sEmail && ($this->sEmail != $oUser->sEmail))
         {
-            if(user_exists($sEmail) && $sEmail != $this->sEmail)
+            /* make sure this email isn't already in use */
+            if(user_exists($this->sEmail))
             {
                 addMsg("An account with this e-mail exists already.","red");
-                return false;
+                return USER_UPDATE_FAILED_EMAIL_EXISTS;
             }
-            if (!query_appdb("UPDATE user_list SET email = '".addslashes($sEmail)."' WHERE userid = ".$this->iUserId))
+            if (!query_appdb("UPDATE user_list SET email = '".addslashes($this->sEmail)."' WHERE userid = ".$this->iUserId))
-                return false;
+                return USER_UPDATE_FAILED;
-            $this->sEmail = $sEmail;
         }
 
-        if ($sPassword)
+        if ($this->sRealname && ($this->sRealname != $oUser->sRealname))
         {
-            if (!query_appdb("UPDATE user_list SET password = password('$sPassword') WHERE userid = ".$this->iUserId))
+            if (!query_appdb("UPDATE user_list SET realname = '".addslashes($this->sRealname)."' WHERE userid = ".$this->iUserId))
-                return false;
+                return USER_UPDATE_FAILED;
         }
 
-        if ($sRealname)
+        if ($this->sWineRelease && ($this->sWineRelease != $oUser->sWineRelease))
         {
-            if (!query_appdb("UPDATE user_list SET realname = '".addslashes($sRealname)."' WHERE userid = ".$this->iUserId))
+            if (!query_appdb("UPDATE user_list SET CVSrelease = '".addslashes($this->sWineRelease)."' WHERE userid = ".$this->iUserId))
-                return false;
+                return USER_UPDATE_FAILED;
-            $this->sRealname = $sRealname;
+        }
+        return SUCCESS;
     }
 
-        if ($sWineRelease)
+    /** 
+     * NOTE: we can't update the users password like we can update other
+     * fields such as their email or username because the password is hashed
+     * in the database so we can't keep the users password in a class member variable
+     * and use update() because we can't check if the password changed without hashing
+     * the newly supplied one
+     */
+    function update_password($sPassword)
     {
-            if (!query_appdb("UPDATE user_list SET CVSrelease = '".addslashes($sWineRelease)."' WHERE userid = ".$this->iUserId))
+        if($sPassword)
-                return false;
+        {
-            $this->sWineRelease = $sWineRelease;
+            if (query_appdb("UPDATE user_list SET password = password('$sPassword') WHERE userid = ".$this->iUserId))
-        }
                 return true;
         }
 
+        return false;
+    }
+
 
     /**
      * Removes the current, or specified user and preferences from the database.

preferences.php

@@ -111,6 +111,7 @@ if($_POST)
         $oUser->setPref($arr[1], $value);
     }
     
+    /* make sure the user enters the same password twice */
     if ($aClean['ext_password'] == $aClean['ext_password2'])
     {
         $str_passwd = $aClean['ext_password'];
@@ -119,7 +120,20 @@ if($_POST)
     {
         addmsg("The Passwords you entered did not match.", "red");
     }
-    if ($oUser->update($aClean['ext_email'], $str_passwd, $aClean['ext_realname'], $aClean['CVSrelease']))
+
+    /* update user data fields */
+    $oUser->sEmail = $aClean['ext_email'];
+    $oUser->sRealname = $aClean['ext_realname'];
+    $oUser->sWineRelease = $aClean['CVSrelease'];
+
+    /* if the password was empty in both cases then skip updating the users password */
+    if($str_passwd != "")
+    {
+        if(!$oUser->update_password($str_passwd))
+            addmsg("Failed to update password", "red");
+    }
+
+    if ($oUser->update() == SUCCESS)
     {
         addmsg("Preferences Updated", "green");
         // we were managing an user, let's go back to the admin after updating tha admin status

unit_test/path.php

@@ -0,0 +1,3 @@
+<?php
+define("BASE","../");
+?>

unit_test/run_tests

@@ -0,0 +1 @@
+php -f run_tests.php

unit_test/run_tests.php

@@ -0,0 +1,11 @@
+<?php
+
+/* Main test function.  To add new tests include_once() */
+/* your test file here */
+
+/* TODO: test the rest of the classes we have */
+
+include_once("test_user.php");
+
+
+?>

unit_test/test_user.php

@@ -0,0 +1,285 @@
+<?php
+
+/* unit tests for user class */
+
+require_once("path.php");
+require_once(BASE."include/incl.php");
+require_once(BASE."include/user.php");
+
+/* TODO: check permissions functions */
+
+$test_email = "testemail@somesite.com";
+$test_password = "password";
+
+function test_start($sFunctionName)
+{
+    echo $sFunctionName."() starting\n";
+}
+
+/* NOTE: test_user_login() relies on this function leaving the test user */
+/* in the database */
+function test_user_create()
+{
+    test_start(__FUNCTION__);
+
+    global $test_email, $test_password;
+
+    $oUser = new User();
+
+    /* delete the user if they already exist */
+    if($oUser->login($test_email, $test_password) == SUCCESS)
+    {
+        $oUser->delete();
+        $oUser = new User();
+    }
+
+    /* create the user */
+    $retval = $oUser->create("testemail@somesite.com", "password", "Test user", "20051020");
+    if($retval != SUCCESS)
+    {
+        if($retval == USER_CREATE_EXISTS)
+            echo "The user already exists!\n";
+        else if($retval == USER_LOGIN_FAILED)
+            echo "User login failed!\n";
+        else
+            echo "ERROR: UNKNOWN ERROR!!\n";
+            
+        return false;
+    }
+
+    /* try creating the user again, see that we get USER_CREATE_EXISTS */
+    $retval = $oUser->create("testemail@somesite.com", "password", "Test user", "20051020");
+    if($retval != USER_CREATE_EXISTS)
+    {
+        echo "Got '".$retval."' instead of USER_CREATE_EXISTS(".USER_CREATE_EXISTS.")\n";
+        return false;
+    }
+
+    return true;
+}
+
+/* NOTE: relies on test_create_user() being run first and leaving a user */
+/*  created in the db */
+function test_user_login()
+{
+    test_start(__FUNCTION__);
+
+    global $test_email, $test_password;
+
+    /* test that correct information results in a correct login */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != SUCCESS)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* test that incorrect user results in a login failed */
+    $oUser = new User();
+    $retval = $oUser->login("some nutty username", $testpassword);
+    if($retval != USER_LOGIN_FAILED)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* test that incorrect password results in a login failed */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, "some password");
+    if($retval != USER_LOGIN_FAILED)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    return true;
+}
+
+
+function test_user_update_set_test($realname, $winerelease)
+{
+    global $test_email, $test_password;
+
+    /* log the user in */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != SUCCESS)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* modify the users realname and wine release */
+    $oUser->sRealname = $realname;
+    $oUser->sWineRelease = $winerelease;
+    $oUser->update(); /* save the changes */
+
+    /* log the user in again */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != SUCCESS)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* make sure the realname and wine release match */
+    if($oUser->sRealname != $realname)
+    {
+        echo "Realname of '".$oUser->sRealname."' doesn't match expected realname of '".$realname."'\n";
+        return false;
+    }
+
+    if($oUser->sWineRelease != $winerelease)
+    {
+        echo "Wine release of '".$oUser->sWineRelease."' doesn't match expected wine release of '".$winerelease."'\n";
+        return false;
+    }
+
+    return true;
+}
+
+/* test that we can set values and call user::update() and have the values be saved */
+function test_user_update()
+{
+    test_start(__FUNCTION__);
+
+    global $test_email, $test_password;
+
+    if(!test_user_update_set_test("some bogus realname", "some crazy wine release"))
+    {
+        return false;
+    }
+
+    if(!test_user_update_set_test("some new bogus realname", "some new crazy wine release"))
+    {
+        return false;
+    }
+
+    return true;
+}
+
+function test_user_delete()
+{
+    test_start(__FUNCTION__);
+
+    global $test_email, $test_password;
+
+    /* login the user */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != SUCCESS)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* delete the user */
+    $oUser->delete();
+
+    /* try to log in again */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != USER_LOGIN_FAILED)
+    {
+        echo "Got '".$retval."' instead of USER_LOGIN_FAILED(".USER_LOGIN_FAILED.")\n";
+        return false;
+    }
+
+
+    /* now create the user again and see that it is created successfully */
+
+    /* create the user */
+    $oUser = new User();
+    $retval = $oUser->create($test_email, $test_password, "Test user", "20051020");
+    if($retval != SUCCESS)
+    {
+        if($retval == USER_CREATE_EXISTS)
+            echo "The user already exists!\n";
+        else if($retval == USER_LOGIN_FAILED)
+            echo "User login failed!\n";
+        else
+            echo "ERROR: UNKNOWN ERROR!!\n";
+            
+        return false;
+    }
+
+    return true;
+}
+
+function test_user_getpref_setpref()
+{
+    test_start(__FUNCTION__);
+
+    global $test_email, $test_password;
+
+    /* login the user */
+    $oUser = new User();
+    $retval = $oUser->login($test_email, $test_password);
+    if($retval != SUCCESS)
+    {
+        echo "Got '".$retval."' instead of SUCCESS(".SUCCESS.")\n";
+        return false;
+    }
+
+    /* set a preference and retrieve it */
+    $pref_key = "testpreference";
+    $pref_value = "test value";
+    $oUser->setPref($pref_key, $pref_value);
+
+    $got_pref = $oUser->getPref($pref_key);
+    if($got_pref != $pref_value)
+    {
+        echo "Expected preference value of '".$pref_value."' got preference value of '".$got_pref."'\n";
+        return false;
+    }
+    
+    return true;
+}
+
+
+/*************************/
+/* Main testing routines */
+
+if(!test_user_create())
+    echo "test_user_create() failed!\n";
+else
+    echo "test_user_create() passed\n";
+
+if(!test_user_login())
+    echo "test_user_login() failed!\n";
+else
+    echo "test_user_login() passed\n";
+
+if(!test_user_update())
+    echo "test_user_update() failed!\n";
+else
+    echo "test_user_update() passed\n";
+
+if(!test_user_delete())
+    echo "test_user_delete() failed!\n";
+else
+    echo "test_user_delete() passed\n";
+
+if(!test_user_getpref_setpref())
+    echo "test_user_getpref_setpref() failed!\n";
+else
+    echo "test_user_getpref_setpref() passed\n";
+
+/* TODO: the rest of the user member functions we don't currently test */
+
+
+
+/* clean up the user we created during testing */
+/* so the unit test leaves no trace that it ran */
+$oUser = new User();
+
+/* delete the user if they already exist */
+if($oUser->login($test_email, $test_password) == SUCCESS)
+{
+    $oUser->delete();
+    $oUser = new User();
+}
+
+?>