From ff728b3ccd329e988220ca2857b3dcd989ebad37 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 17:51:57 -0800
Subject: Whitespace.

---
 modules/user/controllers/admin_users.php | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 96b86fff..ab747528 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -21,12 +21,8 @@ class Admin_Users_Controller extends Admin_Controller {
   public function index() {
     $view = new Admin_View("admin.html");
     $view->content = new View("admin_users.html");
-    $view->content->users = ORM::factory("user")
-      ->order_by("name", "ASC")
-      ->find_all();
-    $view->content->groups = ORM::factory("group")
-      ->order_by("name", "ASC")
-      ->find_all();
+    $view->content->users = ORM::factory("user")->order_by("name", "ASC")->find_all();
+    $view->content->groups = ORM::factory("group")->order_by("name", "ASC")->find_all();
     print $view;
   }
 
-- 
cgit v1.2.3


From a691dcc63cb6403784e061997cc85606a8f953b3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 19:58:55 -0800
Subject: Convert Admin_Users::add_user() to use model based validation.  Get
 the rules and business logic out of the form and user::create(), and move it
 into User_Model::save().

---
 modules/user/controllers/admin_users.php | 39 ++++++++---------
 modules/user/helpers/user.php            | 26 ------------
 modules/user/models/user.php             | 72 ++++++++++++++++++++++++++------
 3 files changed, 78 insertions(+), 59 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index ab747528..7f08f8a1 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -30,31 +30,33 @@ class Admin_Users_Controller extends Admin_Controller {
     access::verify_csrf();
 
     $form = $this->_get_user_add_form_admin();
-    $valid = $form->validate();
-    $name = $form->add_user->inputs["name"]->value;
-    if ($user = user::lookup_by_name($name)) {
-      $form->add_user->inputs["name"]->add_error("in_use", 1);
+    try {
+      $user = ORM::factory("user");
+      $valid = $form->validate();
+      $user->name = $form->add_user->inputs["name"]->value;
+      $user->full_name = $form->add_user->full_name->value;
+      $user->password = $form->add_user->password->value;
+      $user->email = $form->add_user->email->value;
+
+      if (!empty($form->add_user->locale->value)) {
+        $user->locale = $form->add_user->locale->value;
+      }
+      $user->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->add_user->inputs[$key]->add_error($error, 1);
+      }
       $valid = false;
     }
 
     if ($valid) {
-      $user = user::create(
-        $name, $form->add_user->full_name->value, $form->add_user->password->value);
-      $user->email = $form->add_user->email->value;
-      $user->admin = $form->add_user->admin->checked;
-
-      if ($form->add_user->locale) {
-        $desired_locale = $form->add_user->locale->value;
-        $user->locale = $desired_locale == "none" ? null : $desired_locale;
-      }
       $user->save();
       module::event("user_add_form_admin_completed", $user, $form);
-
       message::success(t("Created user %user_name", array("user_name" => $user->name)));
       print json_encode(array("result" => "success"));
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -329,11 +331,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $group->input("url")->label(t("URL"))->id("g-url");
     self::_add_locale_dropdown($group);
     $group->checkbox("admin")->label(t("Admin"))->id("g-admin");
-    $form->add_rules_from(ORM::factory("user"));
-
-    $minimum_length = module::get_var("user", "mininum_password_length", 5);
-    $form->add_user->password
-      ->rules($minimum_length ? "required|length[$minimum_length, 40]" : "length[40]");
 
     module::event("user_add_form_admin", $user, $form);
     $group->submit("")->value(t("Add user"));
diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php
index e092aecc..3561021f 100644
--- a/modules/user/helpers/user.php
+++ b/modules/user/helpers/user.php
@@ -35,32 +35,6 @@ class user_Core {
     return model_cache::get("user", 1);
   }
 
-  /**
-   * Create a new user.
-   *
-   * @param string  $name
-   * @param string  $full_name
-   * @param string  $password
-   * @return User_Model
-   */
-  static function create($name, $full_name, $password) {
-    $user = ORM::factory("user")->where("name", "=", $name)->find();
-    if ($user->loaded()) {
-      throw new Exception("@todo USER_ALREADY_EXISTS $name");
-    }
-
-    $user->name = $name;
-    $user->full_name = $full_name;
-    $user->password = $password;
-
-    // Required groups
-    $user->add(group::everybody());
-    $user->add(group::registered_users());
-
-    $user->save();
-    return $user;
-  }
-
   /**
    * Is the password provided correct?
    *
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index edba2a2c..12da5784 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -19,14 +19,16 @@
  */
 class User_Model extends ORM implements User_Definition {
   protected $has_and_belongs_to_many = array("groups");
+  protected $password_length = null;
 
-  var $form_rules = array(
-    "name" => "required|length[1,32]",
-    "full_name" => "length[0,255]",
-    "email" => "required|valid_email|length[1,255]",
-    "password" => "length[1,40]",
-    "url" => "valid_url",
-    "locale" => "length[2,10]");
+  var $rules = array(
+    "name"      => array("rules" => array("length[1,32]", "required")),
+    "locale"    => array("rules" => array("length[2,10]")),
+    "password"  => array("rules" => array("length[5,40]")),  // note: overridden in validate()
+    "email"     => array("rules" => array("length[1,255]", "required", "valid::email")),
+    "full_name" => array("rules" => array("length[0,255]")),
+    "url"       => array("rules" => array("valid::url")),
+  );
 
   public function __set($column, $value) {
     switch ($column) {
@@ -35,6 +37,7 @@ class User_Model extends ORM implements User_Definition {
       break;
 
     case "password":
+      $this->password_length = strlen($value);
       $value = user::hash_password($value);
       break;
     }
@@ -65,18 +68,41 @@ class User_Model extends ORM implements User_Definition {
     return $this->groups->find_all();
   }
 
+  /**
+   * Add some custom per-instance rules.
+   */
+  public function validate($array=null) {
+    // validate() is recursive, only modify the rules on the outermost call.
+    if (!$array) {
+      $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
+    }
+
+    $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
+
+    parent::validate($array);
+  }
+
+  /**
+   * Handle any business logic necessary to create or update a user.
+   * @see ORM::save()
+   *
+   * @return ORM User_Model
+   */
   public function save() {
     if (!$this->loaded()) {
-        $created = 1;
-    }
+      // New user
+      $this->add(group::everybody());
+      $this->add(group::registered_users());
 
-    $original = clone $this->original();
-    parent::save();
-    if (isset($created)) {
+      parent::save();
       module::event("user_created", $this);
     } else {
+      // Updated user
+      $original = clone $this->original();
+      parent::save();
       module::event("user_updated", $original, $this);
     }
+
     return $this;
   }
 
@@ -88,4 +114,26 @@ class User_Model extends ORM implements User_Definition {
   public function display_name() {
     return empty($this->full_name) ? $this->name : $this->full_name;
   }
+
+  /**
+   * Validate the user name.  Make sure there are no conflicts.
+   */
+  public function valid_name(Validation $v, $field) {
+    if (db::build()->from("users")
+        ->where("name", "=", $this->name)
+        ->where("id", "<>", $this->id)
+        ->count_records() == 1) {
+      $v->add_error("name", "in_use");
+    }
+  }
+
+  /**
+   * Validate the password.
+   */
+  public function valid_password(Validation $v, $field) {
+    $minimum_length = module::get_var("user", "mininum_password_length", 5);
+    if ($this->password_length < $minimum_length || $this->password_length > 40) {
+      $v->add_error("password", "length");
+    }
+  }
 }
-- 
cgit v1.2.3


From 6a4dda9bdef81bcf79abe5601fd7309e593078f3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 21:15:12 -0800
Subject: Convert Admin_Users_Controller, User_Model and Group_Model to use
 model based validation.

---
 modules/user/controllers/admin_users.php | 115 +++++++++++++------------------
 modules/user/controllers/users.php       |  17 +++--
 modules/user/helpers/group.php           |  17 -----
 modules/user/models/group.php            |  39 ++++++++---
 modules/user/models/user.php             |  18 ++++-
 5 files changed, 103 insertions(+), 103 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 7f08f8a1..c35eba73 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -37,10 +37,9 @@ class Admin_Users_Controller extends Admin_Controller {
       $user->full_name = $form->add_user->full_name->value;
       $user->password = $form->add_user->password->value;
       $user->email = $form->add_user->email->value;
-
-      if (!empty($form->add_user->locale->value)) {
-        $user->locale = $form->add_user->locale->value;
-      }
+      $user->url = $form->edit_user->url->value;
+      $user->locale = $form->add_user->locale->value;
+      $user->admin = $form->edit_user->admin->checked;
       $user->validate();
     } catch (ORM_Validation_Exception $e) {
       // Translate ORM validation errors into form error messages
@@ -110,43 +109,34 @@ class Admin_Users_Controller extends Admin_Controller {
     }
 
     $form = $this->_get_user_edit_form_admin($user);
-    $valid = $form->validate();
-    if ($valid) {
-      $new_name = $form->edit_user->inputs["name"]->value;
-      $temp_user = user::lookup_by_name($new_name);
-      if ($new_name != $user->name &&
-          ($temp_user && $temp_user->id != $user->id)) {
-        $form->edit_user->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
-      } else {
-        $user->name = $new_name;
-      }
-    }
-
-    if ($valid) {
+    try {
+      $valid = $form->validate();
+      $user->name = $form->edit_user->inputs["name"]->value;
       $user->full_name = $form->edit_user->full_name->value;
-      if ($form->edit_user->password->value) {
-        $user->password = $form->edit_user->password->value;
-      }
+      $user->password = $form->edit_user->password->value;
       $user->email = $form->edit_user->email->value;
       $user->url = $form->edit_user->url->value;
-      if ($form->edit_user->locale) {
-        $desired_locale = $form->edit_user->locale->value;
-        $user->locale = $desired_locale == "none" ? null : $desired_locale;
-      }
-
-      // An admin can change the admin status for any user but themselves
+      $user->locale = $form->edit_user->locale->value;
       if ($user->id != identity::active_user()->id) {
         $user->admin = $form->edit_user->admin->checked;
       }
+
+      $user->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->edit_user->inputs[$key]->add_error($error, 1);
+      }
+      $valid = false;
+    }
+
+    if ($valid) {
       $user->save();
       module::event("user_edit_form_admin_completed", $user, $form);
-
       message::success(t("Changed user %user_name", array("user_name" => $user->name)));
       print json_encode(array("result" => "success"));
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -191,25 +181,26 @@ class Admin_Users_Controller extends Admin_Controller {
     access::verify_csrf();
 
     $form = $this->_get_group_add_form_admin();
-    $valid = $form->validate();
-    if ($valid) {
-      $new_name = $form->add_group->inputs["name"]->value;
-      $group = group::lookup_by_name($new_name);
-      if (!empty($group)) {
-        $form->add_group->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
+    try {
+      $valid = $form->validate();
+      $group = ORM::factory("group");
+      $group->name = $form->add_group->inputs["name"]->value;
+      $group->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->add_group->inputs[$key]->add_error($error, 1);
       }
+      $valid = false;
     }
 
     if ($valid) {
-      $group = group::create($new_name);
       $group->save();
       message::success(
         t("Created group %group_name", array("group_name" => $group->name)));
       print json_encode(array("result" => "success"));
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -258,19 +249,19 @@ class Admin_Users_Controller extends Admin_Controller {
     }
 
     $form = $this->_get_group_edit_form_admin($group);
-    $valid = $form->validate();
-
-    if ($valid) {
-      $new_name = $form->edit_group->inputs["name"]->value;
-      $group = group::lookup_by_name($name);
-      if ($group->loaded()) {
-        $form->edit_group->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
+    try {
+      $valid = $form->validate();
+      $group->name = $form->edit_group->inputs["name"]->value;
+      $group->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->edit_group->inputs[$key]->add_error($error, 1);
       }
+      $valid = false;
     }
 
     if ($valid) {
-      $group->name = $form->edit_group->inputs["name"]->value;
       $group->save();
       message::success(
         t("Changed group %group_name", array("group_name" => $group->name)));
@@ -278,8 +269,7 @@ class Admin_Users_Controller extends Admin_Controller {
     } else {
       message::error(
         t("Failed to change group %group_name", array("group_name" => $group->name)));
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -308,10 +298,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $group->input("email")->label(t("Email"))->id("g-email")->value($user->email);
     $group->input("url")->label(t("URL"))->id("g-url")->value($user->url);
     $group->checkbox("admin")->label(t("Admin"))->id("g-admin")->checked($user->admin);
-    $form->add_rules_from($user);
-    $minimum_length = module::get_var("user", "mininum_password_length", 5);
-    $form->edit_user->password
-      ->rules($minimum_length ? "length[$minimum_length, 40]" : "length[40]");
 
     module::event("user_edit_form_admin", $user, $form);
     $group->submit("")->value(t("Modify User"));
@@ -342,15 +328,14 @@ class Admin_Users_Controller extends Admin_Controller {
     foreach ($locales as $locale => $display_name) {
       $locales[$locale] = SafeString::of_safe_html($display_name);
     }
-    if (count($locales) > 1) {
-      // Put "none" at the first position in the array
-      $locales = array_merge(array("" => t("« none »")), $locales);
-      $selected_locale = ($user && $user->locale) ? $user->locale : "";
-      $form->dropdown("locale")
-        ->label(t("Language Preference"))
-        ->options($locales)
-        ->selected($selected_locale);
-    }
+
+    // Put "none" at the first position in the array
+    $locales = array_merge(array("" => t("« none »")), $locales);
+    $selected_locale = ($user && $user->locale) ? $user->locale : "";
+    $form->dropdown("locale")
+      ->label(t("Language Preference"))
+      ->options($locales)
+      ->selected($selected_locale);
   }
 
   private function _get_user_delete_form_admin($user) {
@@ -370,7 +355,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group->inputs["name"]->error_messages(
       "in_use", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Save"));
-    $form->add_rules_from($group);
     return $form;
   }
 
@@ -381,7 +365,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group->inputs["name"]->error_messages(
       "in_use", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Add group"));
-    $form->add_rules_from(ORM::factory("group"));
     return $form;
   }
 
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index ca218393..71f9a889 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -95,14 +95,13 @@ class Users_Controller extends Controller {
     foreach ($locales as $locale => $display_name) {
       $locales[$locale] = SafeString::of_safe_html($display_name);
     }
-    if (count($locales) > 1) {
-      // Put "none" at the first position in the array
-      $locales = array_merge(array("" => t("« none »")), $locales);
-      $selected_locale = ($user && $user->locale) ? $user->locale : "";
-      $form->dropdown("locale")
-        ->label(t("Language Preference"))
-        ->options($locales)
-        ->selected($selected_locale);
-    }
+
+    // Put "none" at the first position in the array
+    $locales = array_merge(array("" => t("« none »")), $locales);
+    $selected_locale = ($user && $user->locale) ? $user->locale : "";
+    $form->dropdown("locale")
+      ->label(t("Language Preference"))
+      ->options($locales)
+      ->selected($selected_locale);
   }
 }
diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php
index 2ada0ac1..38124b0d 100644
--- a/modules/user/helpers/group.php
+++ b/modules/user/helpers/group.php
@@ -24,23 +24,6 @@
  * Note: by design, this class does not do any permission checking.
  */
 class group_Core {
-  /**
-   * Create a new group.
-   *
-   * @param string  $name
-   * @return Group_Definition the group object
-   */
-  static function create($name) {
-    $group = ORM::factory("group")->where("name", "=", $name)->find();
-    if ($group->loaded()) {
-      throw new Exception("@todo GROUP_ALREADY_EXISTS $name");
-    }
-
-    $group->name = $name;
-    $group->save();
-    return $group;
-  }
-
   /**
    * The group of all possible visitors.  This includes the guest user.
    *
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 10f6f4b3..16d6adb7 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -20,8 +20,7 @@
 class Group_Model extends ORM implements Group_Definition {
   protected $has_and_belongs_to_many = array("users");
 
-  var $form_rules = array(
-    "name" => "required|length[4,255]");
+  var $rules = array("name" => array("rules" => array("required", "length[4,255]")));
 
   /**
    * @see ORM::delete()
@@ -37,18 +36,42 @@ class Group_Model extends ORM implements Group_Definition {
     return $this->users->find_all();
   }
 
-  public function save() {
-    if (!$this->loaded()) {
-        $created = 1;
+  /**
+   * Add some custom per-instance rules.
+   */
+  public function validate($array=null) {
+    // validate() is recursive, only modify the rules on the outermost call.
+    if (!$array) {
+      $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
     }
 
-    $original = clone $this->original();
-    parent::save();
-    if (isset($created)) {
+    parent::validate($array);
+  }
+
+  public function save() {
+    if (!$this->loaded()) {
+      // New group
+      parent::save();
       module::event("group_created", $this);
     } else {
+      // Updated group
+      $original = clone $this->original();
+      parent::save();
       module::event("group_updated", $original, $this);
     }
+
     return $this;
   }
+
+  /**
+   * Validate the user name.  Make sure there are no conflicts.
+   */
+  public function valid_name(Validation $v, $field) {
+    if (db::build()->from("groups")
+        ->where("name", "=", $this->name)
+        ->where("id", "<>", $this->id)
+        ->count_records() == 1) {
+      $v->add_error("name", "in_use");
+    }
+  }
 }
\ No newline at end of file
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 12da5784..c45f88ac 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -78,6 +78,7 @@ class User_Model extends ORM implements User_Definition {
     }
 
     $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
+    $this->rules["admin"]["callbacks"] = array(array($this, "valid_admin"));
 
     parent::validate($array);
   }
@@ -131,9 +132,20 @@ class User_Model extends ORM implements User_Definition {
    * Validate the password.
    */
   public function valid_password(Validation $v, $field) {
-    $minimum_length = module::get_var("user", "mininum_password_length", 5);
-    if ($this->password_length < $minimum_length || $this->password_length > 40) {
-      $v->add_error("password", "length");
+    if (!$this->loaded() || $this->password_length) {
+      $minimum_length = module::get_var("user", "mininum_password_length", 5);
+      if ($this->password_length < $minimum_length || $this->password_length > 40) {
+        $v->add_error("password", "length");
+      }
+    }
+  }
+
+  /**
+   * Validate the admin bit.
+   */
+  public function valid_admin(Validation $v, $field) {
+    if ($this->id == identity::active_user()->id && !$this->admin) {
+      $v->add_error("admin", "locked");
     }
   }
 }
-- 
cgit v1.2.3


From 5c49c041e740b8bb8eb6afae8563731ab858aa97 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 22:42:02 -0800
Subject: Use "(string) $form" instead of "$form->__toString()"

---
 modules/gallery/controllers/login.php              |  4 +---
 modules/tag/controllers/admin_tags.php             |  4 +---
 modules/tag/controllers/tags.php                   |  4 +---
 modules/user/controllers/admin_users.php           |  6 ++----
 modules/watermark/controllers/admin_watermarks.php | 12 +++---------
 5 files changed, 8 insertions(+), 22 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/gallery/controllers/login.php b/modules/gallery/controllers/login.php
index 75ee6b9c..464db491 100644
--- a/modules/gallery/controllers/login.php
+++ b/modules/gallery/controllers/login.php
@@ -33,9 +33,7 @@ class Login_Controller extends Controller {
       print json_encode(
         array("result" => "success"));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php
index e20b8ac8..3b605a4e 100644
--- a/modules/tag/controllers/admin_tags.php
+++ b/modules/tag/controllers/admin_tags.php
@@ -60,9 +60,7 @@ class Admin_Tags_Controller extends Admin_Controller {
         array("result" => "success",
               "location" => url::site("admin/tags")));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php
index 992c7411..e28b7a83 100644
--- a/modules/tag/controllers/tags.php
+++ b/modules/tag/controllers/tags.php
@@ -71,9 +71,7 @@ class Tags_Controller extends Controller {
         array("result" => "success",
               "cloud" => tag::cloud(30)->__toString()));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index c35eba73..91468250 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -82,8 +82,7 @@ class Admin_Users_Controller extends Admin_Controller {
       $name = $user->name;
       $user->delete();
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
 
     $message = t("Deleted user %user_name", array("user_name" => $name));
@@ -221,8 +220,7 @@ class Admin_Users_Controller extends Admin_Controller {
       $name = $group->name;
       $group->delete();
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
 
     $message = t("Deleted group %group_name", array("group_name" => $name));
diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php
index 2a1d5f60..f535ad08 100644
--- a/modules/watermark/controllers/admin_watermarks.php
+++ b/modules/watermark/controllers/admin_watermarks.php
@@ -52,9 +52,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
         array("result" => "success",
               "location" => url::site("admin/watermarks")));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -84,9 +82,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
         array("result" => "success",
               "location" => url::site("admin/watermarks")));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -127,9 +123,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
         array("result" => "success",
               "location" => url::site("admin/watermarks")));
     } else {
-      print json_encode(
-        array("result" => "error",
-              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
-- 
cgit v1.2.3


From 9488684220cbf4121dea12a28083f6c34b648da8 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sun, 17 Jan 2010 12:30:24 -0800
Subject: Move model rules down into their validate() function for consistency.
 Change "in_use" error to "conflict" for consistency.

---
 modules/user/controllers/admin_users.php |  8 ++++----
 modules/user/models/group.php            | 10 +++++-----
 modules/user/models/user.php             | 31 ++++++++++++++-----------------
 3 files changed, 23 insertions(+), 26 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 91468250..bc68d154 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -287,7 +287,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $group = $form->group("edit_user")->label(t("Edit user"));
     $group->input("name")->label(t("Username"))->id("g-username")->value($user->name);
     $group->inputs["name"]->error_messages(
-      "in_use", t("There is already a user with that username"));
+      "conflict", t("There is already a user with that username"));
     $group->input("full_name")->label(t("Full name"))->id("g-fullname")->value($user->full_name);
     self::_add_locale_dropdown($group, $user);
     $group->password("password")->label(t("Password"))->id("g-password");
@@ -306,7 +306,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form = new Forge("admin/users/add_user", "", "post", array("id" => "g-add-user-form"));
     $group = $form->group("add_user")->label(t("Add user"));
     $group->input("name")->label(t("Username"))->id("g-username")
-      ->error_messages("in_use", t("There is already a user with that username"));
+      ->error_messages("conflict", t("There is already a user with that username"));
     $group->input("full_name")->label(t("Full name"))->id("g-fullname");
     $group->password("password")->label(t("Password"))->id("g-password");
     $group->password("password2")->label(t("Confirm password"))->id("g-password2")
@@ -351,7 +351,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group = $form->group("edit_group")->label(t("Edit group"));
     $form_group->input("name")->label(t("Name"))->id("g-name")->value($group->name);
     $form_group->inputs["name"]->error_messages(
-      "in_use", t("There is already a group with that name"));
+      "conflict", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Save"));
     return $form;
   }
@@ -361,7 +361,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group = $form->group("add_group")->label(t("Add group"));
     $form_group->input("name")->label(t("Name"))->id("g-name");
     $form_group->inputs["name"]->error_messages(
-      "in_use", t("There is already a group with that name"));
+      "conflict", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Add group"));
     return $form;
   }
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 16d6adb7..c00bf5c9 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -20,8 +20,6 @@
 class Group_Model extends ORM implements Group_Definition {
   protected $has_and_belongs_to_many = array("users");
 
-  var $rules = array("name" => array("rules" => array("required", "length[4,255]")));
-
   /**
    * @see ORM::delete()
    */
@@ -37,12 +35,14 @@ class Group_Model extends ORM implements Group_Definition {
   }
 
   /**
-   * Add some custom per-instance rules.
+   * Specify our rules here so that we have access to the instance of this model.
    */
   public function validate($array=null) {
     // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
-      $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
+      $this->rules = array(
+        "name" => array("rules" => array("required", "length[4,255]"),
+                        "callbacks" => array(array($this, "valid_name"))));
     }
 
     parent::validate($array);
@@ -71,7 +71,7 @@ class Group_Model extends ORM implements Group_Definition {
         ->where("name", "=", $this->name)
         ->where("id", "<>", $this->id)
         ->count_records() == 1) {
-      $v->add_error("name", "in_use");
+      $v->add_error("name", "conflict");
     }
   }
 }
\ No newline at end of file
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index c45f88ac..451b5ffb 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -21,15 +21,6 @@ class User_Model extends ORM implements User_Definition {
   protected $has_and_belongs_to_many = array("groups");
   protected $password_length = null;
 
-  var $rules = array(
-    "name"      => array("rules" => array("length[1,32]", "required")),
-    "locale"    => array("rules" => array("length[2,10]")),
-    "password"  => array("rules" => array("length[5,40]")),  // note: overridden in validate()
-    "email"     => array("rules" => array("length[1,255]", "required", "valid::email")),
-    "full_name" => array("rules" => array("length[0,255]")),
-    "url"       => array("rules" => array("valid::url")),
-  );
-
   public function __set($column, $value) {
     switch ($column) {
     case "hashed_password":
@@ -69,17 +60,23 @@ class User_Model extends ORM implements User_Definition {
   }
 
   /**
-   * Add some custom per-instance rules.
+   * Specify our rules here so that we have access to the instance of this model.
    */
   public function validate($array=null) {
     // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
-      $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
+      $this->rules = array(
+        "admin"     => array("callbacks" => array(array($this, "valid_admin"))),
+        "email"     => array("rules"     => array("length[1,255]", "required", "valid::email")),
+        "full_name" => array("rules"     => array("length[0,255]")),
+        "locale"    => array("rules"     => array("length[2,10]")),
+        "name"      => array("rules"     => array("length[1,32]", "required"),
+                             "callbacks" => array(array($this, "valid_name"))),
+        "password"  => array("callbacks" => array(array($this, "valid_password"))),
+        "url"       => array("rules"     => array("valid::url")),
+      );
     }
 
-    $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
-    $this->rules["admin"]["callbacks"] = array(array($this, "valid_admin"));
-
     parent::validate($array);
   }
 
@@ -124,7 +121,7 @@ class User_Model extends ORM implements User_Definition {
         ->where("name", "=", $this->name)
         ->where("id", "<>", $this->id)
         ->count_records() == 1) {
-      $v->add_error("name", "in_use");
+      $v->add_error("name", "conflict");
     }
   }
 
@@ -134,8 +131,8 @@ class User_Model extends ORM implements User_Definition {
   public function valid_password(Validation $v, $field) {
     if (!$this->loaded() || $this->password_length) {
       $minimum_length = module::get_var("user", "mininum_password_length", 5);
-      if ($this->password_length < $minimum_length || $this->password_length > 40) {
-        $v->add_error("password", "length");
+      if ($this->password_length < $minimum_length) {
+        $v->add_error("password", "min_length");
       }
     }
   }
-- 
cgit v1.2.3


From f0780486eebba9d1f77241e979cedab4df94135a Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 20 Jan 2010 22:53:51 -0800
Subject: Fix typo: edit_user -> add_user.

---
 modules/user/controllers/admin_users.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index bc68d154..1b0e48a4 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -37,9 +37,9 @@ class Admin_Users_Controller extends Admin_Controller {
       $user->full_name = $form->add_user->full_name->value;
       $user->password = $form->add_user->password->value;
       $user->email = $form->add_user->email->value;
-      $user->url = $form->edit_user->url->value;
+      $user->url = $form->add_user->url->value;
       $user->locale = $form->add_user->locale->value;
-      $user->admin = $form->edit_user->admin->checked;
+      $user->admin = $form->add_user->admin->checked;
       $user->validate();
     } catch (ORM_Validation_Exception $e) {
       // Translate ORM validation errors into form error messages
-- 
cgit v1.2.3


From 79a1365991557d64776110b0a3a99ea42b1a70fe Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 30 Jan 2010 20:43:53 -0800
Subject: Don't override the password in the database if it's empty in the
 form. Fixes ticket #995.

---
 modules/user/controllers/admin_users.php | 4 +++-
 modules/user/controllers/users.php       | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 1b0e48a4..c11b0596 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -112,7 +112,9 @@ class Admin_Users_Controller extends Admin_Controller {
       $valid = $form->validate();
       $user->name = $form->edit_user->inputs["name"]->value;
       $user->full_name = $form->edit_user->full_name->value;
-      $user->password = $form->edit_user->password->value;
+      if ($form->edit_user->password->value) {
+        $user->password = $form->edit_user->password->value;
+      }
       $user->email = $form->edit_user->email->value;
       $user->url = $form->edit_user->url->value;
       $user->locale = $form->edit_user->locale->value;
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index ede1f0de..d0c67dd1 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -29,7 +29,9 @@ class Users_Controller extends Controller {
     try {
       $valid = $form->validate();
       $user->full_name = $form->edit_user->full_name->value;
-      $user->password = $form->edit_user->password->value;
+      if ($form->edit_user->password->value) {
+        $user->password = $form->edit_user->password->value;
+      }
       $user->email = $form->edit_user->email->value;
       $user->url = $form->edit_user->url->value;
 
-- 
cgit v1.2.3


From c050acf30a7351bf0ef5b8ee206704c073e881c7 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sun, 31 Jan 2010 16:07:41 -0800
Subject: Fix lots of warnings that pop up when we're in E_STRICT mode. 
 They're mostly issues around uninitialized variables, calling non-static
 functions in a static context, calling Session functions directly instead of
 on its singleton, passing non-variables by reference, and subclasses not
 using the same interface as the parent class.

---
 modules/comment/controllers/admin_comments.php          |  1 +
 modules/comment/helpers/comment_rss.php                 |  1 +
 modules/comment/models/comment.php                      |  3 ++-
 modules/digibug/controllers/digibug.php                 |  2 +-
 modules/forge/libraries/Form_Group.php                  |  2 +-
 modules/g2_import/controllers/admin_g2_import.php       |  5 +++++
 modules/g2_import/helpers/g2_import.php                 | 11 +++++++++++
 modules/g2_import/helpers/g2_import_task.php            | 10 ++++++++--
 modules/g2_import/views/admin_g2_import.html.php        |  2 +-
 modules/gallery/controllers/admin_modules.php           |  1 +
 modules/gallery/controllers/combined.php                |  2 +-
 modules/gallery/controllers/file_proxy.php              |  2 +-
 modules/gallery/helpers/gallery_block.php               |  2 +-
 modules/gallery/helpers/gallery_rss.php                 |  1 +
 modules/gallery/helpers/gallery_task.php                | 11 ++++++++---
 modules/gallery/helpers/graphics.php                    |  3 +++
 modules/gallery/helpers/l10n_client.php                 |  1 +
 modules/gallery/helpers/module.php                      |  3 ++-
 modules/gallery/libraries/Form_Script.php               |  4 ++--
 modules/gallery/libraries/MY_Database.php               |  2 +-
 modules/gallery/libraries/MY_View.php                   |  2 +-
 modules/gallery/libraries/ORM_MPTT.php                  |  2 +-
 modules/gallery/models/item.php                         |  2 +-
 modules/gallery/models/task.php                         |  4 ++--
 modules/gallery/tests/Database_Test.php                 |  8 ++++----
 modules/gallery/tests/Item_Rest_Helper_Test.php         | 17 +++++++++++++++++
 .../gallery_unit_test/controllers/gallery_unit_test.php |  6 +++++-
 modules/rest/controllers/rest.php                       |  1 +
 modules/rest/helpers/rest.php                           |  2 +-
 modules/rest/tests/Rest_Controller_Test.php             |  8 ++++----
 modules/search/helpers/search.php                       |  3 ++-
 modules/tag/helpers/tag_rss.php                         |  2 ++
 modules/tag/helpers/tags_rest.php                       |  1 -
 modules/tag/models/tag.php                              |  2 +-
 modules/tag/tests/Tag_Item_Rest_Helper_Test.php         |  4 +++-
 modules/tag/tests/Tag_Rest_Helper_Test.php              |  8 ++++++++
 modules/tag/tests/Tags_Rest_Helper_Test.php             |  4 ++++
 modules/user/controllers/admin_users.php                |  2 +-
 38 files changed, 111 insertions(+), 36 deletions(-)

(limited to 'modules/user/controllers/admin_users.php')

diff --git a/modules/comment/controllers/admin_comments.php b/modules/comment/controllers/admin_comments.php
index b7dc5fb3..3dd45919 100644
--- a/modules/comment/controllers/admin_comments.php
+++ b/modules/comment/controllers/admin_comments.php
@@ -92,6 +92,7 @@ class Admin_Comments_Controller extends Admin_Controller {
   }
 
   private function _counts() {
+    $counts = new stdClass();
     $counts->unpublished = 0;
     $counts->published = 0;
     $counts->spam = 0;
diff --git a/modules/comment/helpers/comment_rss.php b/modules/comment/helpers/comment_rss.php
index 77044884..79fa07df 100644
--- a/modules/comment/helpers/comment_rss.php
+++ b/modules/comment/helpers/comment_rss.php
@@ -42,6 +42,7 @@ class comment_rss_Core {
       $comments->where("item_id", "=", $id);
     }
 
+    $feed = new stdClass();
     $feed->view = "comment.mrss";
     $feed->children = array();
     foreach ($comments->find_all($limit, $offset) as $comment) {
diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php
index add15ce8..d9d05995 100644
--- a/modules/comment/models/comment.php
+++ b/modules/comment/models/comment.php
@@ -116,7 +116,8 @@ class Comment_Model extends ORM {
 
     // We only notify on the related items if we're making a visible change.
     if ($visible_change) {
-      module::event("item_related_update", $this->item());
+      $item = $this->item();
+      module::event("item_related_update", $item);
     }
 
     return $this;
diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php
index e3b06196..c98ae20c 100644
--- a/modules/digibug/controllers/digibug.php
+++ b/modules/digibug/controllers/digibug.php
@@ -91,7 +91,7 @@ class Digibug_Controller extends Controller {
     }
 
     // We don't need to save the session for this request
-    Session::abort_save();
+    Session::instance()->abort_save();
 
     if (!TEST_MODE) {
       // Dump out the image
diff --git a/modules/forge/libraries/Form_Group.php b/modules/forge/libraries/Form_Group.php
index e0601321..0a04912b 100644
--- a/modules/forge/libraries/Form_Group.php
+++ b/modules/forge/libraries/Form_Group.php
@@ -80,7 +80,7 @@ class Form_Group_Core extends Forge {
 		}
 	}
 
-	public function render()
+	public function render($template = 'forge_template', $custom = FALSE)
 	{
 		// No Sir, we don't want any html today thank you
 		return;
diff --git a/modules/g2_import/controllers/admin_g2_import.php b/modules/g2_import/controllers/admin_g2_import.php
index 1c65f482..6dd155b9 100644
--- a/modules/g2_import/controllers/admin_g2_import.php
+++ b/modules/g2_import/controllers/admin_g2_import.php
@@ -19,6 +19,7 @@
  */
 class Admin_g2_import_Controller extends Admin_Controller {
   public function index() {
+    g2_import::lower_error_reporting();
     if (g2_import::is_configured()) {
       g2_import::init();
     }
@@ -31,6 +32,7 @@ class Admin_g2_import_Controller extends Admin_Controller {
     $view = new Admin_View("admin.html");
     $view->content = new View("admin_g2_import.html");
     $view->content->form = $this->_get_import_form();
+    $view->content->version = g2_import::version();
 
     if (g2_import::is_initialized()) {
       $view->content->g2_stats = $g2_stats;
@@ -38,11 +40,13 @@ class Admin_g2_import_Controller extends Admin_Controller {
       $view->content->thumb_size = module::get_var("gallery", "thumb_size");
       $view->content->resize_size = module::get_var("gallery", "resize_size");
     }
+    g2_import::restore_error_reporting();
     print $view;
   }
 
   public function save() {
     access::verify_csrf();
+    g2_import::lower_error_reporting();
 
     $form = $this->_get_import_form();
     if ($form->validate()) {
@@ -63,6 +67,7 @@ class Admin_g2_import_Controller extends Admin_Controller {
     $view = new Admin_View("admin.html");
     $view->content = new View("admin_g2_import.html");
     $view->content->form = $form;
+    g2_import::restore_error_reporting();
     print $view;
   }
 
diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php
index fa95e547..0fcc0539 100644
--- a/modules/g2_import/helpers/g2_import.php
+++ b/modules/g2_import/helpers/g2_import.php
@@ -24,6 +24,7 @@ class g2_import_Core {
   public static $g2_base_url = null;
 
   private static $current_g2_item = null;
+  private static $error_reporting = null;
 
   static function is_configured() {
     return module::get_var("g2_import", "embed_path");
@@ -931,6 +932,16 @@ class g2_import_Core {
             "useAuthToken" => false));
     return str_replace(self::$g2_base_url, "", $url);
   }
+
+  static function lower_error_reporting() {
+    // Gallery 2 was not designed to run in E_STRICT mode and will barf out errors.  So dial down
+    // the error reporting when we make G2 calls.
+    self::$error_reporting = error_reporting(error_reporting() & ~E_STRICT);
+  }
+
+  static function restore_error_reporting() {
+    error_reporting(self::$error_reporting);
+  }
 }
 
 /**
diff --git a/modules/g2_import/helpers/g2_import_task.php b/modules/g2_import/helpers/g2_import_task.php
index e80b88b9..21ba4c3a 100644
--- a/modules/g2_import/helpers/g2_import_task.php
+++ b/modules/g2_import/helpers/g2_import_task.php
@@ -19,17 +19,19 @@
  */
 class g2_import_task_Core {
   static function available_tasks() {
+    g2_import::lower_error_reporting();
     if (g2_import::is_configured()) {
       g2_import::init();
     }
-
+    $version = g2_import::version();
+    g2_import::restore_error_reporting();
 
     if (class_exists("GalleryCoreApi")) {
       return array(Task_Definition::factory()
                    ->callback("g2_import_task::import")
                    ->name(t("Import from Gallery 2"))
                    ->description(
-                     t("Gallery %version detected", array("version" => g2_import::version())))
+                     t("Gallery %version detected", array("version" => $version)))
                    ->severity(log::SUCCESS));
     }
 
@@ -37,6 +39,8 @@ class g2_import_task_Core {
   }
 
   static function import($task) {
+    g2_import::lower_error_reporting();
+
     $start = microtime(true);
     g2_import::init();
 
@@ -207,5 +211,7 @@ class g2_import_task_Core {
     $task->set("mode", $mode);
     $task->set("queue", $queue);
     $task->set("done", $done);
+
+    g2_import::restore_error_reporting();
   }
 }
diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php
index 0875e7f7..6a5214a3 100644
--- a/modules/g2_import/views/admin_g2_import.html.php
+++ b/modules/g2_import/views/admin_g2_import.html.php
@@ -34,7 +34,7 @@
       <h2> <?= t("Import") ?> </h2>
       <ul id="g-action-status" class="g-message-block">
         <li class="g-success">
-          <?= t("Gallery version %version detected", array("version" => g2_import::version())) ?>
+          <?= t("Gallery version %version detected", array("version" => $version)) ?>
         </li>
         <? if ($g2_sizes["thumb"]["size"] && $thumb_size != $g2_sizes["thumb"]["size"]): ?>
         <li class="g-warning">
diff --git a/modules/gallery/controllers/admin_modules.php b/modules/gallery/controllers/admin_modules.php
index 84fee25d..081b3f12 100644
--- a/modules/gallery/controllers/admin_modules.php
+++ b/modules/gallery/controllers/admin_modules.php
@@ -67,6 +67,7 @@ class Admin_Modules_Controller extends Admin_Controller {
   }
 
   private function _do_save() {
+    $changes = new stdClass();
     $changes->activate = array();
     $changes->deactivate = array();
     $activated_names = array();
diff --git a/modules/gallery/controllers/combined.php b/modules/gallery/controllers/combined.php
index e90a2f1a..7f3a3c7d 100644
--- a/modules/gallery/controllers/combined.php
+++ b/modules/gallery/controllers/combined.php
@@ -41,7 +41,7 @@ class Combined_Controller extends Controller {
     $input = Input::instance();
 
     // We don't need to save the session for this request
-    Session::abort_save();
+    Session::instance()->abort_save();
 
     // Our data is immutable, so if they already have a copy then it needs no updating.
     if ($input->server("HTTP_IF_MODIFIED_SINCE")) {
diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php
index 646edf17..33952366 100644
--- a/modules/gallery/controllers/file_proxy.php
+++ b/modules/gallery/controllers/file_proxy.php
@@ -121,7 +121,7 @@ class File_Proxy_Controller extends Controller {
     expires::check(2592000, $item->updated);
 
     // We don't need to save the session for this request
-    Session::abort_save();
+    Session::instance()->abort_save();
 
     expires::set(2592000, $item->updated);  // 30 days
 
diff --git a/modules/gallery/helpers/gallery_block.php b/modules/gallery/helpers/gallery_block.php
index 9d4e81b6..be0f11b8 100644
--- a/modules/gallery/helpers/gallery_block.php
+++ b/modules/gallery/helpers/gallery_block.php
@@ -72,7 +72,7 @@ class gallery_block_Core {
       $block->content = new View("admin_block_platform.html");
       if (is_readable("/proc/loadavg")) {
         $block->content->load_average =
-          join(" ", array_slice(explode(" ", array_shift(file("/proc/loadavg"))), 0, 3));
+          join(" ", array_slice(explode(" ", current(file("/proc/loadavg"))), 0, 3));
       } else {
         $block->content->load_average = t("Unavailable");
       }
diff --git a/modules/gallery/helpers/gallery_rss.php b/modules/gallery/helpers/gallery_rss.php
index d422636f..c1790d28 100644
--- a/modules/gallery/helpers/gallery_rss.php
+++ b/modules/gallery/helpers/gallery_rss.php
@@ -25,6 +25,7 @@ class gallery_rss_Core {
   }
 
   static function feed($feed_id, $offset, $limit, $id) {
+    $feed = new stdClass();
     switch ($feed_id) {
     case "latest":
       $feed->children = ORM::factory("item")
diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php
index c75e050a..b2f18d7c 100644
--- a/modules/gallery/helpers/gallery_task.php
+++ b/modules/gallery/helpers/gallery_task.php
@@ -111,6 +111,7 @@ class gallery_task_Core {
         site_status::clear("graphics_dirty");
       }
     } catch (Exception $e) {
+      Kohana_Log::add("error",(string)$e);
       $task->done = true;
       $task->state = "error";
       $task->status = $e->getMessage();
@@ -214,6 +215,7 @@ class gallery_task_Core {
         Cache::instance()->delete("update_l10n_cache:{$task->id}");
       }
     } catch (Exception $e) {
+      Kohana_Log::add("error",(string)$e);
       $task->done = true;
       $task->state = "error";
       $task->status = $e->getMessage();
@@ -233,10 +235,10 @@ class gallery_task_Core {
     try {
       $start = microtime(true);
       $data = Cache::instance()->get("file_cleanup_cache:{$task->id}");
-      if ($data) {
-        $files = unserialize($data);
-      }
+      $files = $data ? unserialize($data) : array();
       $i = 0;
+      $current = 0;
+      $total = 0;
 
       switch ($task->get("mode", "init")) {
       case "init":  // 0%
@@ -262,6 +264,7 @@ class gallery_task_Core {
         if (count($files) == 0) {
           break;
         }
+
       case "delete_files":
         $current = $task->get("current");
         $total = $task->get("total");
@@ -279,8 +282,10 @@ class gallery_task_Core {
       if ($total == $current) {
         $task->done = true;
         $task->state = "success";
+        $task->percent_complete = 100;
       }
     } catch (Exception $e) {
+      Kohana_Log::add("error",(string)$e);
       $task->done = true;
       $task->state = "error";
       $task->status = $e->getMessage();
diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index 5a290905..c85c7750 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -262,6 +262,9 @@ class graphics_Core {
    */
   static function detect_toolkits() {
     $toolkits = new stdClass();
+    $toolkits->gd = new stdClass();
+    $toolkits->imagemagick = new stdClass();
+    $toolkits->graphicsmagick = new stdClass();
 
     // GD is special, it doesn't use exec()
     $gd = function_exists("gd_info") ? gd_info() : array();
diff --git a/modules/gallery/helpers/l10n_client.php b/modules/gallery/helpers/l10n_client.php
index 086245e8..c27e4e5b 100644
--- a/modules/gallery/helpers/l10n_client.php
+++ b/modules/gallery/helpers/l10n_client.php
@@ -77,6 +77,7 @@ class l10n_client_Core {
    *     translations for.
    */
   static function fetch_updates(&$num_fetched) {
+    $request = new stdClass();
     $request->locales = array();
     $request->messages = new stdClass();
 
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index 95e426c4..9523d1d2 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -430,7 +430,8 @@ class module_Core {
             // This could happen if there's a race condition
             continue;
           }
-          self::$var_cache->{$row->module_name}->{$row->name} = $row->value;
+          // Mute the "Creating default object from empty value" warning below
+          @self::$var_cache->{$row->module_name}->{$row->name} = $row->value;
         }
         $cache = ORM::factory("var");
         $cache->module_name = "gallery";
diff --git a/modules/gallery/libraries/Form_Script.php b/modules/gallery/libraries/Form_Script.php
index e841408d..1f965767 100644
--- a/modules/gallery/libraries/Form_Script.php
+++ b/modules/gallery/libraries/Form_Script.php
@@ -50,7 +50,7 @@ class Form_Script_Core extends Forge {
     return $this;
   }
 
-  public function render() {
+  public function render($template="forge_template", $custom=false) {
     $script = array();
     if (!empty($this->data["url"])) {
       $script[] = html::script($this->data["url"]);
@@ -63,4 +63,4 @@ class Form_Script_Core extends Forge {
     return implode("\n", $script);
   }
 
-} // End Form Script
\ No newline at end of file
+}
\ No newline at end of file
diff --git a/modules/gallery/libraries/MY_Database.php b/modules/gallery/libraries/MY_Database.php
index 61f23fb0..e2ef68cd 100644
--- a/modules/gallery/libraries/MY_Database.php
+++ b/modules/gallery/libraries/MY_Database.php
@@ -38,7 +38,7 @@ abstract class Database extends Database_Core {
    * Parse the query string and convert any strings of the form `\([a-zA-Z0-9_]*?)\]
    * table prefix . $1
    */
-  public function query($sql = '') {
+  public function query($sql) {
     if (!empty($sql)) {
       $sql = $this->add_table_prefixes($sql);
     }
diff --git a/modules/gallery/libraries/MY_View.php b/modules/gallery/libraries/MY_View.php
index cec59ec1..83e0d0be 100644
--- a/modules/gallery/libraries/MY_View.php
+++ b/modules/gallery/libraries/MY_View.php
@@ -27,7 +27,7 @@ class View extends View_Core {
     View::$global_data[$key] = $value;
   }
 
-  public function is_set($key) {
+  public function is_set($key=null) {
     return parent::is_set($key) ? true : array_key_exists($key, View::$global_data);
   }
 
diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php
index 83f9b51e..3668d42d 100644
--- a/modules/gallery/libraries/ORM_MPTT.php
+++ b/modules/gallery/libraries/ORM_MPTT.php
@@ -85,7 +85,7 @@ class ORM_MPTT_Core extends ORM {
   /**
    * Delete this node and all of its children.
    */
-  public function delete() {
+  public function delete($ignored_id=null) {
     $children = $this->children();
     if ($children) {
       foreach ($this->children() as $item) {
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 083fd06b..dbd56fa2 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -70,7 +70,7 @@ class Item_Model extends ORM_MPTT {
     return $this->type == 'movie';
   }
 
-  public function delete() {
+  public function delete($ignored_id=null) {
     if ($this->id == 1) {
       $v = new Validation(array("id"));
       $v->add_error("id", "cant_delete_root_album");
diff --git a/modules/gallery/models/task.php b/modules/gallery/models/task.php
index f40be492..24d909cb 100644
--- a/modules/gallery/models/task.php
+++ b/modules/gallery/models/task.php
@@ -27,7 +27,7 @@ class Task_Model extends ORM {
     }
   }
 
-  public function set($key, $value) {
+  public function set($key, $value=null) {
     $context = unserialize($this->context);
     $context[$key] = $value;
     $this->context = serialize($context);
@@ -40,7 +40,7 @@ class Task_Model extends ORM {
     return parent::save();
   }
 
-  public function delete() {
+  public function delete($ignored_id=null) {
     Cache::instance()->delete($this->_cache_key());
     return parent::delete();
   }
diff --git a/modules/gallery/tests/Database_Test.php b/modules/gallery/tests/Database_Test.php
index e58f73eb..861f7bba 100644
--- a/modules/gallery/tests/Database_Test.php
+++ b/modules/gallery/tests/Database_Test.php
@@ -168,12 +168,12 @@ class Database_Mock extends Database {
     return array("test");
   }
 
-  public function quote_column($val) {
-    return "[$val]";
+  public function quote_column($val, $alias=null) {
+    return $alias ? "[$val,$alias]" : "[$val]";
   }
 
-  public function quote_table($val) {
-    return "[$val]";
+  public function quote_table($val, $alias=null) {
+    return $alias ? "[$val,$alias]" : "[$val]";
   }
 
   public function quote($val) {
diff --git a/modules/gallery/tests/Item_Rest_Helper_Test.php b/modules/gallery/tests/Item_Rest_Helper_Test.php
index 088b1cbd..6d1dd864 100644
--- a/modules/gallery/tests/Item_Rest_Helper_Test.php
+++ b/modules/gallery/tests/Item_Rest_Helper_Test.php
@@ -32,6 +32,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1->reload();
 
     // No scope is the same as "direct"
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
     $request->params = new stdClass();
     $this->assert_equal_array(
@@ -84,7 +85,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $photo2->save();
     $album1->reload();
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->name = "foo";
     $this->assert_equal_array(
       array("url" => rest::url("item", $album1),
@@ -104,7 +107,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album2 = test::random_album($album1);
     $album1->reload();
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->type = "album";
     $this->assert_equal_array(
       array("url" => rest::url("item", $album1),
@@ -122,7 +127,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->title = "my new title";
 
     item_rest::put($request);
@@ -133,7 +140,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->title = "my new title";
     $request->params->slug = "not url safe";
 
@@ -150,7 +159,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->type = "album";
     $request->params->name = "my album";
     $request->params->title = "my album";
@@ -165,7 +176,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->type = "album";
     $request->params->name = "my album";
     $request->params->title = "my album";
@@ -185,7 +198,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
+    $request->params = new stdClass();
     $request->params->type = "photo";
     $request->params->name = "my photo.jpg";
     $request->file = MODPATH . "gallery/tests/test.jpg";
@@ -200,6 +215,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album1 = test::random_album();
     access::allow(identity::everybody(), "edit", $album1);
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
     item_rest::delete($request);
 
@@ -212,6 +228,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     access::deny(identity::everybody(), "edit", $album1);
     identity::set_active_user(identity::guest());
 
+    $request = new stdClass();
     $request->url = rest::url("item", $album1);
     try {
       item_rest::delete($request);
diff --git a/modules/gallery_unit_test/controllers/gallery_unit_test.php b/modules/gallery_unit_test/controllers/gallery_unit_test.php
index e05fcbaa..2690ad24 100644
--- a/modules/gallery_unit_test/controllers/gallery_unit_test.php
+++ b/modules/gallery_unit_test/controllers/gallery_unit_test.php
@@ -18,11 +18,15 @@
  * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
  */
 class Gallery_Unit_Test_Controller extends Controller {
-  function Index() {
+  function index() {
     if (!TEST_MODE) {
       throw new Kohana_404_Exception();
     }
 
+    // Force strict behavior to flush out bugs early
+    ini_set("display_errors", true);
+    error_reporting(-1);
+
     // Jump through some hoops to satisfy the way that we check for the site_domain in
     // config.php.  We structure this such that the code in config will leave us with a
     // site_domain of "." (for historical reasons)
diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php
index 9141d6d4..374ae0d2 100644
--- a/modules/rest/controllers/rest.php
+++ b/modules/rest/controllers/rest.php
@@ -40,6 +40,7 @@ class Rest_Controller extends Controller {
 
   public function __call($function, $args) {
     $input = Input::instance();
+    $request = new stdClass();
     switch ($method = strtolower($input->server("REQUEST_METHOD"))) {
     case "get":
       $request->params = (object) $input->get();
diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php
index b3f80a55..a61aba2f 100644
--- a/modules/rest/helpers/rest.php
+++ b/modules/rest/helpers/rest.php
@@ -19,7 +19,7 @@
  */
 class rest_Core {
   static function reply($data=array()) {
-    Session::abort_save();
+    Session::instance()->abort_save();
 
     if ($data) {
       if (Input::instance()->get("output") == "html") {
diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php
index 5e624112..9f73bed9 100644
--- a/modules/rest/tests/Rest_Controller_Test.php
+++ b/modules/rest/tests/Rest_Controller_Test.php
@@ -138,8 +138,8 @@ class Rest_Controller_Test extends Gallery_Unit_Test_Case {
 }
 
 class mock_rest {
-  function get($request)    { return $request; }
-  function post($request)   { return $request; }
-  function put($request)    { return $request; }
-  function delete($request) { return $request; }
+  static function get($request)    { return $request; }
+  static function post($request)   { return $request; }
+  static function put($request)    { return $request; }
+  static function delete($request) { return $request; }
 }
\ No newline at end of file
diff --git a/modules/search/helpers/search.php b/modules/search/helpers/search.php
index b2497eae..9018ffa2 100644
--- a/modules/search/helpers/search.php
+++ b/modules/search/helpers/search.php
@@ -65,7 +65,8 @@ class search_Core {
       $record->item_id = $item->id;
     }
 
-    module::event("item_index_data", $record->item(), $data);
+    $item = $record->item();
+    module::event("item_index_data", $item, $data);
     $record->data = join(" ", (array)$data);
     $record->dirty = 0;
     $record->save();
diff --git a/modules/tag/helpers/tag_rss.php b/modules/tag/helpers/tag_rss.php
index f09a4530..5d42caab 100644
--- a/modules/tag/helpers/tag_rss.php
+++ b/modules/tag/helpers/tag_rss.php
@@ -34,6 +34,8 @@ class tag_rss_Core {
       if (!$tag->loaded()) {
         throw new Kohana_404_Exception();
       }
+
+      $feed = new stdClass();
       $feed->children = $tag->items($limit, $offset, "photo");
       $feed->max_pages = ceil($tag->count / $limit);
       $feed->title = $tag->name;
diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php
index ac0eb81d..f28be7b5 100644
--- a/modules/tag/helpers/tags_rest.php
+++ b/modules/tag/helpers/tags_rest.php
@@ -35,7 +35,6 @@ class tags_rest_Core {
         $query->or_where("edit_{$group->id}", "=", access::ALLOW);
       }
       $has_any_edit_perm = $query->close()->count_records();
-
       if (!$has_any_edit_perm) {
         access::forbidden();
       }
diff --git a/modules/tag/models/tag.php b/modules/tag/models/tag.php
index 2b33c30d..38a8ed69 100644
--- a/modules/tag/models/tag.php
+++ b/modules/tag/models/tag.php
@@ -95,7 +95,7 @@ class Tag_Model extends ORM {
    * Overload ORM::delete() to trigger an item_related_update event for all items that are
    * related to this tag.
    */
-  public function delete() {
+  public function delete($ignored_id=null) {
     $related_item_ids = array();
     foreach (db::build()
              ->select("item_id")
diff --git a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php
index 69c580f1..cb368790 100644
--- a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php
+++ b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php
@@ -28,6 +28,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   public function get_test() {
     $tag = tag::add(item::root(), "tag1")->reload();
 
+    $request = new stdClass();
     $request->url = rest::url("tag_item", $tag, item::root());
     $this->assert_equal_array(
       array("url" => rest::url("tag_item", $tag, item::root()),
@@ -38,6 +39,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   }
 
   public function get_with_invalid_url_test() {
+    $request = new stdClass();
     $request->url = "bogus";
     try {
       tag_item_rest::get($request);
@@ -50,6 +52,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   public function delete_test() {
     $tag = tag::add(item::root(), "tag1")->reload();
 
+    $request = new stdClass();
     $request->url = rest::url("tag_item", $tag, item::root());
     tag_item_rest::delete($request);
 
@@ -60,7 +63,6 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $album = test::random_album();
     $tag = tag::add($album, "tag1")->reload();
 
-
     $tuple = rest::resolve(rest::url("tag_item", $tag, $album));
     $this->assert_equal_array($tag->as_array(), $tuple[0]->as_array());
     $this->assert_equal_array($album->as_array(), $tuple[1]->as_array());
diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php
index d3cae0fb..838de975 100644
--- a/modules/tag/tests/Tag_Rest_Helper_Test.php
+++ b/modules/tag/tests/Tag_Rest_Helper_Test.php
@@ -28,6 +28,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   public function get_test() {
     $tag = tag::add(item::root(), "tag1")->reload();
 
+    $request = new stdClass();
     $request->url = rest::url("tag", $tag);
     $this->assert_equal_array(
       array("url" => rest::url("tag", $tag),
@@ -41,6 +42,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   }
 
   public function get_with_invalid_url_test() {
+    $request = new stdClass();
     $request->url = "bogus";
     try {
       tag_rest::get($request);
@@ -53,6 +55,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   public function get_with_no_relationships_test() {
     $tag = test::random_tag();
 
+    $request = new stdClass();
     $request->url = rest::url("tag", $tag);
     $this->assert_equal_array(
       array("url" => rest::url("tag", $tag),
@@ -72,7 +75,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     access::allow(identity::everybody(), "edit", $album);
 
     // Add the album to the tag
+    $request = new stdClass();
     $request->url = rest::url("tag", $tag);
+    $request->params = new stdClass();
     $request->params->url = rest::url("item", $album);
     $this->assert_equal_array(
       array("url" => rest::url("tag_item", $tag, $album)),
@@ -93,7 +98,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function put_test() {
     $tag = test::random_tag();
+    $request = new stdClass();
     $request->url = rest::url("tag", $tag);
+    $request->params = new stdClass();
     $request->params->name = "new name";
 
     tag_rest::put($request);
@@ -102,6 +109,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function delete_tag_test() {
     $tag = test::random_tag();
+    $request = new stdClass();
     $request->url = rest::url("tag", $tag);
     tag_rest::delete($request);
 
diff --git a/modules/tag/tests/Tags_Rest_Helper_Test.php b/modules/tag/tests/Tags_Rest_Helper_Test.php
index a1713811..cdf7bfdf 100644
--- a/modules/tag/tests/Tags_Rest_Helper_Test.php
+++ b/modules/tag/tests/Tags_Rest_Helper_Test.php
@@ -43,6 +43,8 @@ class Tags_Rest_Helper_Test extends Gallery_Unit_Test_Case {
   public function post_test() {
     access::allow(identity::everybody(), "edit", item::root());
 
+    $request = new stdClass();
+    $request->params = new stdClass();
     $request->params->name = "test tag";
     $this->assert_equal(
       array("url" => url::site("rest/tag/1")),
@@ -55,6 +57,8 @@ class Tags_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     identity::set_active_user(identity::guest());
 
     try {
+      $request = new stdClass();
+      $request->params = new stdClass();
       $request->params->name = "test tag";
       tags_rest::post($request);
     } catch (Exception $e) {
diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index c11b0596..03d9858b 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -323,7 +323,7 @@ class Admin_Users_Controller extends Admin_Controller {
     return $form;
   }
 
-  private function _add_locale_dropdown(&$form, $user=null) {
+  private static function _add_locale_dropdown(&$form, $user=null) {
     $locales = locales::installed();
     foreach ($locales as $locale => $display_name) {
       $locales[$locale] = SafeString::of_safe_html($display_name);
-- 
cgit v1.2.3