Clean up the rating code by moving uniqueness constraints into the database,

using SQL's AVG() when we want an average, updating the cache when a rating
changes, etc.

1 files changed, 96 insertions, 125 deletions

diff --git a/lib/class/rating.class.php b/lib/class/rating.class.php
index 5c2bb3e9..59efd43d 100644
--- a/lib/class/rating.class.php
+++ b/lib/class/rating.class.php
@@ -22,56 +22,53 @@
 
 /**
  * Rating class
- * This is an amalgamation(sp?) of code from SoundOfEmotion
- * to track ratings for songs, albums and artists.
-*/
+ * This tracks ratings for songs, albums and artists.
+ */
 class Rating extends database_object {
 
-	/* Provided vars */
-	var $id; 	// The ID of the object who's ratings we want to pull
-	var $type; 	// The type of object we want
-
-	/* Generated vars */
-	var $rating;	// The average rating as set by all users
-	var $preciserating;  // Rating rounded to 1 decimal
+	// Public variables
+	public $id;		// The ID of the object rated
+	public $type;		// The type of object we want
+	public $rating;		// Integer rating
+	public $preciserating;  // Decimal rating
 
 	/**
 	 * Constructor
-	 * This is run every time a new object is created, it requires
-	 * the id and type of object that we need to pull the raiting for
+	 * This is run every time a new object is created, and requires
+	 * the id and type of object that we need to pull the rating for
 	 */
-	public function __construct($id,$type) {
+	public function __construct($id, $type) {
+		$id = intval($id);
+		$type = Dba::escape($type);
 
-		$this->id 	= intval($id);
-		$this->type 	= Dba::escape($type);
+		$this->id = $id;
+		$this->type = $type;
 
-		// Check for the users rating
-		if ($rating = $this->get_user($GLOBALS['user']->id)) {
-			$this->rating = $rating;
-			$this->preciserating = $rating;
-		}
-		else {
-			$this->get_average();
+		if (! $rating = $this->get_user_rating()) {
+			$rating = $this->get_average_rating();
 		}
 
+		$this->rating = floor($rating);
+		$this->preciserating = $rating;
+
 		return true;
 
 	} // Constructor
 
 	/**
  	 * build_cache
-	 * This attempts to get everything we'll need for this page load in a single query, saving
-	 * the connection overhead
-	 * //FIXME: Improve logic so that misses get cached as average
+	 * This attempts to get everything we'll need for this page load in a 
+	 * single query, saving on connection overhead
 	 */
 	public static function build_cache($type, $ids) {
 
 		if (!is_array($ids) OR !count($ids)) { return false; }
 
-		$user_id = Dba::escape($GLOBALS['user']->id);
+		$user_id = intval($GLOBALS['user']->id);
 
 		$idlist = '(' . implode(',', $ids) . ')';
-		$sql = "SELECT `rating`, `object_id`,`rating`.`rating` FROM `rating` WHERE `user`='$user_id' AND `object_id` IN $idlist " .
+		$sql = "SELECT `rating`, `object_id` FROM `rating` " .
+			"WHERE `user`='$user_id' AND `object_id` IN $idlist " .
 			"AND `object_type`='$type'";
 		$db_results = Dba::read($sql);
 
@@ -79,27 +76,26 @@ class Rating extends database_object {
 			$user[$row['object_id']] = $row['rating'];
 		}
 
-		$sql = "SELECT `rating`,`object_id` FROM `rating` WHERE `object_id` IN $idlist AND `object_type`='$type'";
+		$sql = "SELECT AVG(`rating`) as `rating`, `object_id` FROM " .
+			"`rating` WHERE `object_id` IN $idlist AND " .
+			"`object_type`='$type' GROUP BY `object_id`";
 		$db_results = Dba::read($sql);
 
 		while ($row = Dba::fetch_assoc($db_results)) {
-			$rating[$row['object_id']]['rating'] += $row['rating'];
-			$rating[$row['object_id']]['total']++;
+			$rating[$row['object_id']] = $row['rating'];
   		}
 
 		foreach ($ids as $id) {
-			parent::add_to_cache('rating_' . $type . '_user',$id,intval($user[$id]));
+			parent::add_to_cache('rating_' . $type . '_user' . $user_id, $id, intval($user[$id]));
 
-			// Do the bit of math required to store this
 			if (!isset($rating[$id])) {
-				$entry = array('average'=>'0','percise'=>'0');
+				$rating = 0;
 			}
 			else {
-				$average = round($rating[$id]['rating']/$rating[$id]['total'],1);
-				$entry = array('average'=>floor($average),'percise'=>$average);
+				$rating = round($rating[$id]['rating'], 1);
 			}
 
-			parent::add_to_cache('rating_' . $type . '_all',$id,$entry);
+			parent::add_to_cache('rating_' . $type . '_all', $id, $rating);
 		}
 
 		return true;
@@ -107,108 +103,92 @@ class Rating extends database_object {
 	} // build_cache
 
 	/**
-	 * get_user
-	 * Get the user's rating this is based off the currently logged
-	 * in user. It returns the value
+	 * get_user_rating
+	 * Get a user's rating.  If no userid is passed in, we use the currently
+	 * logged in user.
 	 */
-	 public function get_user($user_id) {
+	 public function get_user_rating($user_id = null) {
 
 		$id = intval($this->id);
-
-		if (parent::is_cached('rating_' . $this->type . '_user',$id)) {
-			return parent::get_from_cache('rating_' . $this->type . '_user',$id);
+		$type = Dba::escape($this->type);
+		if (is_null($user_id)) {
+			$user_id = $GLOBALS['user']->id;
+		}
+		$user_id = intval($user_id);
+		
+		$key = 'rating_' . $type . '_user' . $user_id;
+		if (parent::is_cached($key, $id)) {
+			return parent::get_from_cache($key, $id);
 		}
 
-		$user_id = Dba::escape($user_id);
-
-		$sql = "SELECT `rating` FROM `rating` WHERE `user`='$user_id' AND `object_id`='$id' AND `object_type`='$this->type'";
+		$sql = "SELECT `rating` FROM `rating` WHERE `user`='$user_id' ".
+			"AND `object_id`='$id' AND `object_type`='$type'";
 		$db_results = Dba::read($sql);
 
 		$results = Dba::fetch_assoc($db_results);
 
-		parent::add_to_cache('rating_' . $this->type . '_user',$id,$results['rating']);
+		parent::add_to_cache($key, $id, $results['rating']);
 
 		return $results['rating'];
 
-	} // get_user
+	} // get_user_rating
 
 	/**
-	 * get_average
-	 * Get the users average rating this is based off the floor'd average
-	 * of what everyone has rated this album as. This is shown if there
-	 * is no personal rating, and used for random play mojo. It sets
-	 * $this->average_rating and returns the value
+	 * get_average_rating
+	 * Get the floored average rating of what everyone has rated this object
+	 * as. This is shown if there is no personal rating.
 	 */
-	public function get_average() {
+	public function get_average_rating() {
 
 		$id = intval($this->id);
+		$type = Dba::escape($this->type);
 
-		if (parent::is_cached('rating_' . $this->type . '_all',$id)) {
-			$data = parent::get_from_cache('rating_' . $this->type . '_user',$id);
-			$this->rating = $data['rating'];
-			$this->perciserating = $data['percise'];
-			return true;
+		if (parent::is_cached('rating_' . $type . '_all', $id)) {
+			return parent::get_from_cache('rating_' . $type . '_user', $id);
 		}
 
-		$sql = "SELECT `rating` FROM `rating` WHERE `object_id`='$id' AND `object_type`='$this->type'";
+		$sql = "SELECT AVG(`rating`) as `rating` FROM `rating` WHERE " .
+			"`object_id`='$id' AND `object_type`='$type'";
 		$db_results = Dba::read($sql);
 
-		$i = 0;
-
-		while ($r = Dba::fetch_assoc($db_results)) {
-			$i++;
-			$total += $r['rating'];
-		} // while we're pulling results
-
-		if ($total > 0) {
-			$average = round($total/$i, 1);
-		}
-		elseif ($i >= '1' AND $total == '0') {
-			$average = -1;
-		}
-		else {
-			$average = 0;
-		}
-
-		$this->preciserating = $average;
-		$this->rating = floor($average);
-
-		return $this->rating;
+		$results = Dba::fetch_assoc($db_results);
+		
+		parent::add_to_cache('rating_' . $type . '_all', $id, $results['rating']);
+		return $results['rating'];
 
-	} // get_average
+	} // get_average_rating
 
 	/**
 	 * set_rating
-	 * This function sets a rating for the current $this object.
-	 * This uses the currently logged in user for the 'user' who is rating
-	 * the object. Returns true on success, false on failure
+	 * This function sets the rating for the current object.
+	 * If no userid is passed in, we use the currently logged in user.
 	 */
-	public function set_rating($score) {
-
-		$score = Dba::escape($score);
-
-		// If score is -1, then remove rating
-		if ($score == '-1') {
-			$sql = "DELETE FROM `rating` WHERE `object_id`='$this->id' AND `object_type`='$this->type' " .
-				"AND `user`='" . Dba::escape($GLOBALS['user']->id) . "'";
-			$db_results = Dba::read($sql);
-			return true;
+	public function set_rating($rating, $user_id = null) {
+		$id = intval($this->id);
+		$type = Dba::escape($this->type);
+		$rating = intval($rating);
+		if (is_null($user_id)) {
+			$user_id = $GLOBALS['user']->id;
 		}
+		$user_id = intval($user_id);
 
-		/* Check if it exists */
-		$sql = "SELECT `id` FROM `rating` WHERE `object_id`='$this->id' AND `object_type`='$this->type' " .
-			"AND `user`='" . Dba::escape($GLOBALS['user']->id) . "'";
-		$db_results = Dba::read($sql);
+		debug_event('Rating', "Setting rating for $type $id to $rating", 5);
 
-		if ($existing = Dba::fetch_assoc($db_results)) {
-			$sql = "UPDATE `rating` SET `rating`='$score' WHERE `id`='" . $existing['id'] . "'";
-			$db_results = Dba::write($sql);
+		// If score is -1, then remove rating
+		if ($rating == '-1') {
+			$sql = "DELETE FROM `rating` WHERE " .
+				"`object_id`='$this->id' AND " .
+				"`object_type`='$this->type' AND " .
+				"`user`='$user_id'";
 		}
 		else {
-			$sql = "INSERT INTO `rating` (`object_id`,`object_type`,`rating`,`user`) VALUES " .
-				" ('$this->id','$this->type','$score','" . $GLOBALS['user']->id . "')";
-			$db_results = Dba::write($sql);
+			$sql = "REPLACE INTO `rating` " .
+			"(`object_id`, `object_type`, `rating`, `user`) " .
+			"VALUES ('$id', '$type', '$rating', '$user_id')";
 		}
+		$db_results = Dba::write($sql);
+
+		parent::add_to_cache('rating_' . $type . '_user' . $user_id, $id, $rating);
 
 		return true;
 
@@ -216,33 +196,24 @@ class Rating extends database_object {
 
 	/**
 	 * show
-	 * This takes an id and a type and displays the rating if ratings are enabled.
+	 * This takes an id and a type and displays the rating if ratings are 
+	 * enabled.  If $static is true, the rating won't be editable.
 	 */
-	public static function show ($object_id,$type) {
+	public static function show($object_id, $type, $static=false) {
 
-		// If there aren't ratings don't return anything
+		// If ratings aren't enabled don't do anything
 		if (!Config::get('ratings')) { return false; }
 
-		$rating = new Rating($object_id,$type);
+		$rating = new Rating($object_id, $type);
 
-		require Config::get('prefix') . '/templates/show_object_rating.inc.php';
+		if ($static) {
+			require Config::get('prefix') . '/templates/show_static_object_rating.inc.php';
+		}
+		else {
+			require Config::get('prefix') . '/templates/show_object_rating.inc.php';
+		}
 
 	} // show
 
-	/**
-	 * show_static
-	 * This is a static version of the ratings created by Andy90
-	 */
-	public static function show_static ($object_id,$type) {
-
-		// If there aren't ratings don't return anything
-		if (!Config::get('ratings')) { return false; }
-
-		$rating = new Rating($object_id,$type);
-
-		require Config::get('prefix') . '/templates/show_static_object_rating.inc.php';
-
-	} // show_static
-
 } //end rating class
 ?>