Cleanup merge of user/group helpers into Identity interface. Reduce redundant code in the user module and remove references to the Identity helper from the user module as the user module should be able to access things directly. Simplify the get_user_list api method to just accept an array of ids to return user objects for.

11 files changed, 113 insertions, 211 deletions

diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 32a27646..bc0efa01 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -77,7 +77,7 @@ class movie_Core {
     $movie->title = $title;
     $movie->description = $description;
     $movie->name = $name;
-    $movie->owner_id = $owner_id ? $owner_id : Identity::active();
+    $movie->owner_id = $owner_id ? $owner_id : Identity::active()->id;
     $movie->width = $movie_info[0];
     $movie->height = $movie_info[1];
     $movie->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv";
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index cf316819..ad23e322 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -76,7 +76,7 @@ class photo_Core {
     $photo->title = $title;
     $photo->description = $description;
     $photo->name = $name;
-    $photo->owner_id = $owner_id ? $owner_id : Identity::active();
+    $photo->owner_id = $owner_id ? $owner_id : Identity::active()->id;
     $photo->width = $image_info[0];
     $photo->height = $image_info[1];
     $photo->mime_type = empty($image_info['mime']) ? "application/unknown" : $image_info['mime'];
diff --git a/modules/gallery/libraries/Identity.php b/modules/gallery/libraries/Identity.php
index 229d0da9..fb553de6 100644
--- a/modules/gallery/libraries/Identity.php
+++ b/modules/gallery/libraries/Identity.php
@@ -191,22 +191,8 @@ class Identity_Core {
   /**
    * @see Identity_Driver::get_user_list.
    */
-  static function get_user_list($filter=array()) {
-    return self::instance()->driver->get_user_list($filter);
-  }
-
-  /**
-   * @see Identity_Driver::get_group_list.
-   */
-  static function get_group_list($filter=array()) {
-    return self::instance()->driver->get_group_list($filter);
-  }
-
-  /**
-   * @see Identity_Driver::get_edit_rules.
-   */
-  static function get_edit_rules($object_type) {
-    return self::instance()->driver->get_edit_rules($object_type);
+  static function get_user_list($ids) {
+    return self::instance()->driver->get_user_list($ids);
   }
 
   static function get_login_form($url) {
diff --git a/modules/gallery/libraries/drivers/Identity.php b/modules/gallery/libraries/drivers/Identity.php
index 0b789908..a9e1a75b 100644
--- a/modules/gallery/libraries/drivers/Identity.php
+++ b/modules/gallery/libraries/drivers/Identity.php
@@ -83,37 +83,11 @@ interface Identity_Driver {
 
   /**
    * List the users
-   * @param mixed      options to apply to the selection of the user
-   *                   currently supported:
-   *                     "orderby" => array(<field name>, "ASC|DESC")
-   *                     "in" => array(<field name>, array(values, ...))
-   *                     "where" => array(<field name>, value)
-   *                        <field name> follows Kohana syntax where it could contain the first
-   *                                     half of a logical expression (i.e. "field IS NOT")
-   * @return array     the group list.
+   * @param array      array of ids to return the user objects for
+   * @return array     the user list.
    */
-  public function get_user_list($filter=array());
+  public function get_user_list($ids);
 
-  /**
-   * List the groups
-   * @param mixed      options to apply to the selection of the group
-   *                   currently supported:
-   *                     "orderby" => array(<field name>, "ASC|DESC")
-   *                     "in" => array(<field name>, array(values, ...))
-   *                     "where" => array(<field name>, value)
-   *                        <field name> follows Kohana syntax where it could contain the first
-   *                                     half of a logical expression (i.e. "field IS NOT")
-   * @return array     the group list.
-   */
-  public function get_group_list($filter=array());
-
-  /**
-   * Return the edit rules associated with an group.
-   *
-   * @param  string   $object_type to return rules for ("user"|"group")
-   * @return stdClass containing the rules
-   */
-  public function get_edit_rules($object_type);
 } // End Identity Driver Definition
 
 /**
@@ -205,19 +179,14 @@ abstract class User_Definition {
    * @param integer $size the target size of the image (default 80px)
    * @return string a url
    */
-  public function avatar_url($size=80, $default=null) {
-    return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s",
-                   md5($this->user->email), $size, $default ? "&d=" . urlencode($default) : "");
-  }
+  abstract public function avatar_url($size=80, $default=null);
 
   /**
    * Return the best version of the user's name.  Either their specified full name, or fall back
    * to the user name.
    * @return string
    */
-  public function display_name() {
-    return empty($this->user->full_name) ? $this->user->name : $this->user->full_name;
-  }
+  abstract public function display_name();
 
   /**
    * Return the internal user object without the wrapper.
diff --git a/modules/notification/helpers/notification.php b/modules/notification/helpers/notification.php
index 4c58bc29..2e22eeae 100644
--- a/modules/notification/helpers/notification.php
+++ b/modules/notification/helpers/notification.php
@@ -67,8 +67,7 @@ class notification {
   }
 
   static function get_subscribers($item) {
-    // @todo only return distinct email addresses
-    $subscriber_ids = array();
+   $subscriber_ids = array();
     foreach (ORM::factory("subscription")
              ->select("user_id")
              ->join("items", "subscriptions.item_id", "items.id")
@@ -79,12 +78,11 @@ class notification {
       $subscriber_ids[] = $subscriber->user_id;
     }
 
-    $users = Identity::get_user_list(array("in" => array("id", $subscriber_ids),
-                                       "where" => array("email IS NOT" => null)));
+    $users = Identity::get_user_list($subscriber_ids);
 
     $subscribers = array();
     foreach ($users as $user) {
-      if (access::user_can($user, "view", $item)) {
+      if (access::user_can($user, "view", $item) && !empty($user->email)) {
         $subscribers[$user->email] = 1;
       }
     }
diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 3465c4b1..fed872a5 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -21,8 +21,12 @@ 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 = user::get_user_list(array("orderby" => array("name" => "ASC")));
-    $view->content->groups = group::get_group_list(array("orderby" => array("name" => "ASC")));
+    $view->content->users = ORM::factory("user")
+      ->orderby("name", "ASC")
+      ->find_all();
+    $view->content->groups = ORM::factory("group")
+      ->orderby("name", "ASC")
+      ->find_all();
     print $view;
   }
 
@@ -303,7 +307,7 @@ 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::get_edit_rules());
+    $form->add_rules_from($user);
     $form->edit_user->password->rules("-required");
 
     module::event("user_edit_form_admin", $user, $form);
@@ -325,8 +329,7 @@ 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");
-    $user = ORM::factory("user");
-    $form->add_rules_from(user::get_edit_rules());
+    $form->add_rules_from(ORM::factory("user"));
 
     module::event("user_add_form_admin", $user, $form);
     $group->submit("")->value(t("Add User"));
@@ -366,7 +369,7 @@ 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::get_edit_rules());
+    $form->add_rules_from($group);
     return $form;
   }
 
@@ -378,8 +381,7 @@ 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"));
-    $group = ORM::factory("group");
-    $form->add_rules_from(group::get_edit_rules());
+    $form->add_rules_from(ORM::factory("group"));
     return $form;
   }
 
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index 6e666ba3..ebce1d8d 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -77,7 +77,7 @@ class Users_Controller extends Controller {
       ->matches($group->password);
     $group->input("email")->label(t("Email"))->id("g-email")->value($user->email);
     $group->input("url")->label(t("URL"))->id("g-url")->value($user->url);
-    $form->add_rules_from(user::get_edit_rules());
+    $form->add_rules_from($user);
 
     module::event("user_edit_form", $user, $form);
     $group->submit("")->value(t("Save"));
diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php
index 295e5f50..cf5c050f 100644
--- a/modules/user/helpers/group.php
+++ b/modules/user/helpers/group.php
@@ -28,7 +28,14 @@ class group_Core {
    * @see Identity_Driver::create.
    */
   static function create($name) {
-    return Identity::instance()->create_group($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;
   }
 
   /**
@@ -51,7 +58,7 @@ class group_Core {
    * @return Group_Definition  the group object, or null if the id was invalid.
    */
   static function lookup($id) {
-    return Identity::instance()->lookup_group_by_field("id", $id);
+    return self::lookup_by_field("id", $id);
   }
 
   /**
@@ -60,20 +67,23 @@ class group_Core {
    * @return Group_Definition  the group object, or null if the name was invalid.
    */
   static function lookup_by_name($name) {
-    return Identity::instance()->lookup_group_by_field("name", $name);
+    return self::lookup_by_field("name", $name);
   }
 
   /**
    * @see Identity_Driver::get_group_list.
    */
-  static function get_group_list($filter=array()) {
-    return Identity::instance()->get_group_list($filter);
-  }
-
-  /**
-   * @see Identity_Driver::get_edit_rules.
-   */
-  static function get_edit_rules() {
-    return Identity::instance()->get_edit_rules("group");
+  static function lookup_by_field($field_name, $value) {
+    try {
+      $user = model_cache::get("group", $value, $field_name);
+      if ($user->loaded) {
+        return $user;
+      }
+    } catch (Exception $e) {
+      if (strpos($e->getMessage(), "MISSING_MODEL") === false) {
+       throw $e;
+      }
+    }
+    return null;
   }
 }
diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php
index 394f8185..fa7b320f 100644
--- a/modules/user/helpers/user.php
+++ b/modules/user/helpers/user.php
@@ -28,28 +28,37 @@ class user_Core {
    * @see Identity_Driver::guest.
    */
   static function guest() {
-    return Identity::guest();
+    return model_cache::get("user", 1);
   }
 
   /**
    * @see Identity_Driver::create_user.
    */
   static function create($name, $full_name, $password) {
-    return Identity::create_user($name, $full_name, $password);
-  }
+    $user = ORM::factory("user")->where("name", $name)->find();
+    if ($user->loaded) {
+      throw new Exception("@todo USER_ALREADY_EXISTS $name");
+    }
 
-  /**
-   * @see Identity_Driver::is_correct_password.
-   */
-  static function is_correct_password($user, $password) {
-    return Identity::is_correct_password($user, $password);
+    $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;
   }
 
   /**
    * @see Identity_Driver::hash_password.
    */
   static function hash_password($password) {
-    return Identity::hash_password($password);
+    require_once(MODPATH . "user/lib/PasswordHash.php");
+    $hashGenerator = new PasswordHash(10, true);
+    return $hashGenerator->HashPassword($password);
   }
 
   /**
@@ -58,7 +67,7 @@ class user_Core {
    * @return User_Definition  the user object, or null if the id was invalid.
    */
   static function lookup($id) {
-    return self::_lookup_user_by_field("id", $id);
+    return self::lookup_by_field("id", $id);
   }
 
   /**
@@ -67,33 +76,10 @@ class user_Core {
    * @return User_Definition  the user object, or null if the name was invalid.
    */
   static function lookup_by_name($name) {
-    return self::_lookup_user_by_field("name", $name);
-  }
-
-  /**
-   * Look up a user by hash.
-   * @param string       $name the user name
-   * @return User_Definition  the user object, or null if the name was invalid.
-   */
-  static function lookup_by_hash($hash) {
-    return self::_lookup_user_by_field("hash", $hash);
-  }
-
-  /**
-   * @see Identity_Driver::get_user_list.
-   */
-  static function get_user_list($filter=array()) {
-    return Identity::get_user_list($filter);
-  }
-
-  /**
-   * @see Identity_Driver::get_edit_rules.
-   */
-  static function get_edit_rules() {
-    return Identity::get_edit_rules("user");
+    return self::lookup_by_field("name", $name);
   }
 
-  private static function _lookup_user_by_field($field_name, $value) {
+  static function lookup_by_field($field_name, $value) {
     try {
       $user = model_cache::get("user", $value, $field_name);
       if ($user->loaded) {
diff --git a/modules/user/libraries/drivers/Identity/Gallery.php b/modules/user/libraries/drivers/Identity/Gallery.php
index 013497b6..77db11a3 100644
--- a/modules/user/libraries/drivers/Identity/Gallery.php
+++ b/modules/user/libraries/drivers/Identity/Gallery.php
@@ -25,28 +25,14 @@ class Identity_Gallery_Driver implements Identity_Driver {
    * @see Identity_Driver::guest.
    */
   public function guest() {
-    return new Gallery_User(model_cache::get("user", 1));
+    return new Gallery_User(user::guest());
   }
 
   /**
    * @see Identity_Driver::create_user.
    */
   public function create_user($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($this->everybody()->_uncloaked());
-    $user->add($this->registered_users()->_uncloaked());
-
-    $user->save();
-    return new Gallery_User($user);
+    return new Gallery_User(user::create($name, $full_name, $password));
   }
 
   /**
@@ -84,126 +70,58 @@ class Identity_Gallery_Driver implements Identity_Driver {
    * @see Identity_Driver::hash_password.
    */
   public function hash_password($password) {
-    require_once(MODPATH . "user/lib/PasswordHash.php");
-    $hashGenerator = new PasswordHash(10, true);
-    return $hashGenerator->HashPassword($password);
+    return user::hash_password($password);
   }
 
   /**
    * @see Identity_Driver::lookup_user_by_field.
    */
   public function lookup_user_by_field($field_name, $value) {
-    try {
-      $user = model_cache::get("user", $value, $field_name);
-      if ($user->loaded) {
-        return new Gallery_User($user);
-      }
-    } catch (Exception $e) {
-      if (strpos($e->getMessage(), "MISSING_MODEL") === false) {
-       throw $e;
-      }
-    }
-    return null;
+    return new Gallery_User(user::lookup_by_field($field_name, $value));
   }
 
   /**
    * @see Identity_Driver::create_group.
    */
   public function create_group($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 new Gallery_Group($group);
+    return new Gallery_Group(group::create($name));
   }
 
   /**
    * @see Identity_Driver::everybody.
    */
   public function everybody() {
-    return new Gallery_Group(model_cache::get("group", 1));
+    return new Gallery_Group(group::everybody());
   }
 
   /**
    * @see Identity_Driver::registered_users.
    */
   public function registered_users() {
-    return new Gallery_Group(model_cache::get("group", 2));
+    return new Gallery_Group(group::registered_users());
   }
 
   /**
    * @see Identity_Driver::lookup_group_by_field.
    */
   public function lookup_group_by_field($field_name, $value) {
-    try {
-      $group = model_cache::get("group", $value, $field_name);
-      if ($group->loaded) {
-        return new Gallery_Group($group);
-      }
-    } catch (Exception $e) {
-      if (strpos($e->getMessage(), "MISSING_MODEL") === false) {
-       throw $e;
-      }
-    }
-    return null;
+    return new Gallery_Group(group::lookup_by_field($field_name, $value));
   }
 
   /**
    * @see Identity_Driver::get_user_list.
    */
-  public function get_user_list($filter=array()) {
-    $results = $this->_do_search("user", $filter);
+  public function get_user_list($ids) {
+    $results = ORM::factory("user")
+      ->in("id", ids)
+      ->find_all()
+      ->as_array();;
     $users = array();
-    foreach ($results->as_array() as $user) {
+    foreach ($results as $user) {
       $users[] = new Gallery_User($user);
     }
     return $users;
   }
-
-  /**
-   * @see Identity_Driver::get_group_list.
-   */
-  public function get_group_list($filter=array()) {
-    $results = $this->_do_search("group", $filter);
-    $groups = array();
-    foreach ($results->as_array() as $group) {
-      $groups[] = new Gallery_Group($group);
-    }
-    return $groups;
-  }
-
-  /**
-   * @see Identity_Driver::get_edit_rules.
-   */
-  public function get_edit_rules($object_type) {
-    return (object)ORM::factory($object_type)->rules;
-  }
-
-  /**
-   * Build the query based on the supplied filters for the specified model.
-   * @param  string   $object_type to return rules for ("user"|"group")
-   * @param  mixed    $filters options to apply to the selection.
-   */
-  private function _do_search($object_type, $filter) {
-    $object = ORM::factory($object_type);
-
-    foreach ($filter as $method => $args) {
-      switch ($method) {
-      case "in":
-        $object->in($args[0], $args[1]);
-        break;
-      default:
-        $object->$method($args);
-      }
-    }
-
-    return $object->find_all();
-  }
-
 } // End Identity Gallery Driver
 
 /**
@@ -217,6 +135,20 @@ class Gallery_User extends User_Definition {
     $this->user = $user;
   }
 
+  /**
+   * @see User_Definition::avatar_url
+   */
+  public function avatar_url($size=80, $default=null) {
+    return $this->user->avatar_url($size, $default);
+  }
+
+  /**
+   * @see User_Definition::display_name
+   */
+  public function display_name() {
+    return $this->user->display_name();
+  }
+
   public function save() {
     $this->user->save();
   }
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 1993bd05..d99603b2 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -51,6 +51,16 @@ class User_Model extends ORM {
     module::event("user_deleted", $old);
   }
 
+  /**
+   * Return a url to the user's avatar image.
+   * @param integer $size the target size of the image (default 80px)
+   * @return string a url
+   */
+  public function avatar_url($size=80, $default=null) {
+    return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s",
+                   md5($this->email), $size, $default ? "&d=" . urlencode($default) : "");
+  }
+
   public function save() {
     if (!$this->loaded) {
         $created = 1;
@@ -63,4 +73,13 @@ class User_Model extends ORM {
     }
     return $this;
   }
+
+  /**
+   * Return the best version of the user's name.  Either their specified full name, or fall back
+   * to the user name.
+   * @return string
+   */
+  public function display_name() {
+    return empty($this->full_name) ? $this->name : $this->full_name;
+  }
 }