Miscellaneous cleanup.

image.php

@@ -41,7 +41,7 @@
 if (!Config::get('resize_images')) { unset($_GET['thumb']); } 
 
 // FIXME: Legacy stuff - should be removed after a version or so
-if (!$_GET['object_type']) { 
+if (!isset($_GET['object_type'])) { 
 	$_GET['object_type'] = 'album'; 
 } 
 

index.php

@@ -24,7 +24,7 @@
 
 show_header();
 
-$action = scrub_in($_REQUEST['action']);
+$action = isset($_REQUEST['action']) ? scrub_in($_REQUEST['action']) : null;
 
 /**
  * Check for the refresh mojo, if it's there then require the

lib/batch.lib.php

@@ -23,8 +23,7 @@
 
 /**
  * get_song_files
- * tmakes array of song ids and returns
- *	array of path to actual files
+ * Takes an array of song ids and returns an array of the actual filenames
  */
 function get_song_files($media_ids) {
 
@@ -57,13 +56,13 @@ function get_song_files($media_ids) {
  */
 function send_zip( $name, $song_files ) {
 
-	// Check if they want to save it to a file, if so then make sure they've got
-	// a defined path as well and that it's writeable
+	// Check if they want to save it to a file, if so then make sure they've
+	// got a defined path as well and that it's writable.
 	if (Config::get('file_zip_download') && Config::get('file_zip_path')) {
 		// Check writeable
 		if (!is_writable(Config::get('file_zip_path'))) {
 			$in_memory = '1';
-			debug_event('Error','File Zip Path:' . Config::get('file_zip_path') . ' is not writeable','1');
+			debug_event('Error','File Zip Path:' . Config::get('file_zip_path') . ' is not writable','1');
 		}
 		else {
 			$in_memory = '0';

lib/class/access.class.php

@@ -312,13 +312,15 @@ public static function check_network($type,$user,$level,$ip='') {
 
 	/**
 	 * check_access
-	 * This is the global 'has_access' function it can check for any 'type' of object
-	 * everything uses the global 0,5,25,50,75,100 stuff. GLOBALS['user'] is always used
+	 * This is the global 'has_access' function it can check for any 'type'
+	 * of object.
+	 * Everything uses the global 0,5,25,50,75,100 stuff. GLOBALS['user'] is
+	 * always used.
 	 */
 	public static function check($type,$level) {
 
 		if (Config::get('demo_mode')) { return true; }
-		if (INSTALL == '1') { return true; }
+		if (defined('INSTALL')) { return true; }
 
 		$level = intval($level);
 

lib/class/ajax.class.php

@@ -108,15 +108,14 @@ public static function button($action,$icon,$alt,$source='',$post='',$class='')
 
 		// If they passed a span class
 		if ($class) {
-			$class_txt = ' class="' . $class . '"';
+			$class = ' class="' . $class . '"';
 		}
 
-
 		$string = get_user_icon($icon,$alt);
 
 		// Generate a <a> so that it's more compliant with older browsers
 		// (ie :hover actions) and also to unify linkbuttons (w/o ajax) display
-		$string = "<a href=\"javascript:void(0);\" id=\"$source\" $class_txt>".$string."</a>\n";
+		$string = "<a href=\"javascript:void(0);\" id=\"$source\" $class>".$string."</a>\n";
 
 		$string .= self::observe($source,'click',$ajax_string);
 
@@ -126,7 +125,7 @@ public static function button($action,$icon,$alt,$source='',$post='',$class='')
 
 	/**
 	 * text
-	 * This prints out the specified text as a link and setups the required
+	 * This prints out the specified text as a link and sets up the required
 	 * ajax for the link so it works correctly
 	 */
 	public static function text($action,$text,$source,$post='',$class='') {
@@ -136,11 +135,11 @@ public static function text($action,$text,$source,$post='',$class='') {
 
 		// If they passed a span class
 		if ($class) {
-			$class_txt = ' class="' . $class . '"';
+			$class = ' class="' . $class . '"';
 		}
 
 		// If we pass a source put it in the ID
-		$string = "<a href=\"javascript:void(0);\" id=\"$source\" $class_txt>$text</a>\n";
+		$string = "<a href=\"javascript:void(0);\" id=\"$source\" $class>$text</a>\n";
 
 		$string .= self::observe($source,'click',$ajax_string);
 
@@ -177,7 +176,7 @@ public static function set_include_override($value) {
 	 */
 	public static function start_container($name) {
 
-		if (AJAX_INCLUDE == '1' AND !self::$include_override) { return true; }
+		if (defined('AJAX_INCLUDE') && !self::$include_override) { return true; }
 
 		echo '<div id="' . scrub_out($name) . '">';
 
@@ -189,7 +188,7 @@ public static function start_container($name) {
 	 */
 	public static function end_container() {
 
-		if (AJAX_INCLUDE == '1' AND !self::$include_override) { return true; }
+		if (defined('AJAX_INCLUDE') && !self::$include_override) { return true; }
 
 		echo "</div>";
 

lib/class/album.class.php

@@ -30,12 +30,13 @@ class Album extends database_object {
 	/* Variables from DB */
 	public $id;
 	public $name;
-	public $full_name; // Prefix + Name, genereated by format();
 	public $disk;
 	public $year;
 	public $prefix;
 	public $mbid; // MusicBrainz ID
 
+	public $full_name; // Prefix + Name, generated
+
 	// cached information
 	public $_songs=array();
 
@@ -58,7 +59,7 @@ public function __construct($id='') {
 			$this->$key = $value;
 		}
 
-		// Little bit of formating here
+		// Little bit of formatting here
 		$this->full_name = trim(trim($info['prefix']) . ' ' . trim($info['name']));
 
 		return true;
@@ -140,11 +141,16 @@ private function _get_extra_info() {
 			return parent::get_from_cache('album_extra',$this->id);
 		}
 
-		$sql = "SELECT COUNT(DISTINCT(song.artist)) as artist_count,COUNT(song.id) AS song_count,artist.name AS artist_name" .
-			",artist.prefix AS artist_prefix, artist.id AS artist_id ".
-			"FROM `song` " .
-			"INNER JOIN `artist` ON `artist`.`id`=`song`.`artist` " .
-			"WHERE `song`.`album`='$this->id' GROUP BY `song`.`album`";
+		$sql = "SELECT " .
+			"COUNT(DISTINCT(`song`.`artist`)) AS `artist_count`, " .
+			"COUNT(`song`.`id`) AS `song_count`, " .
+			"`artist`.`name` AS `artist_name`, " .
+			"`artist`.`prefix` AS `artist_prefix`, " .
+			"`artist`.`id` AS `artist_id` " .
+			"FROM `song` INNER JOIN `artist` " . 
+			"ON `artist`.`id`=`song`.`artist` " .
+			"WHERE `song`.`album`='$this->id' " .
+			"GROUP BY `song`.`album`";
 		$db_results = Dba::read($sql);
 
 		$results = Dba::fetch_assoc($db_results);
@@ -170,12 +176,16 @@ public function get_songs($limit = 0,$artist='') {
 
 		$results = array();
 
+		$artist = Dba::escape($artist);
+
+		$sql = "SELECT `id` FROM `song` WHERE `album`='$this->id' ";
 		if ($artist) {
-			$artist_sql = "AND `artist`='" . Dba::escape($artist) . "'";
+			$sql .= "AND `artist`='$artist'";
+		}
+		$sql .= "ORDER BY `track`, `title`";
+		if ($limit) {
+			$sql .= " LIMIT $limit";
 		}
-
-		$sql = "SELECT `id` FROM `song` WHERE `album`='$this->id' $artist_sql ORDER BY `track`, `title`";
-		if ($limit) { $sql .= " LIMIT $limit"; }
 		$db_results = Dba::read($sql);
 
 		while ($r = Dba::fetch_assoc($db_results)) {
@@ -228,7 +238,7 @@ public function format() {
 		$this->f_name_link .="</a>";
 
 		$this->f_link 		= $this->f_name_link;
-		$this->f_title		= $full_name;
+		$this->f_title		= $this->full_name; // FIXME: Legacy?
 		if ($this->artist_count == '1') {
 			$artist = trim(trim($this->artist_prefix) . ' ' . trim($this->artist_name));
 			$this->f_artist_name = $artist;

lib/class/art.class.php

@@ -55,15 +55,20 @@ public function __construct($uid, $type) {
 	 * Called on creation of the class
 	 */
 	public static function _auto_init() {
-		self::$enabled = make_bool($_SESSION['art_enabled']);
+		if (!isset($_SESSION['art_enabled'])) {
+			$_SESSION['art_enabled'] = (Config::get('bandwidth') > 25);
+		}
+		else {
+			self::$enabled = make_bool($_SESSION['art_enabled']);
+		}
 	}
 
 	/**
 	 * is_enabled
 	 * Checks whether the user currently wants art
 	 */
 	public static function is_enabled() {
-		if (self::$enabled || (Config::get('bandwidth') > 25)) {
+		if (self::$enabled) {
 			return true;
 		}
 
@@ -718,7 +723,7 @@ public function gather_musicbrainz($limit=0) {
 			$arurl = $ar->getTargetId();
 			debug_event('mbz-gatherart', "Found URL AR: " . $arurl , '5');
 			foreach ($coverartsites as $casite) {
-				if (strstr($arurl, $casite['domain'])) {
+				if (strpos($arurl, $casite['domain']) !== false) {
 					debug_event('mbz-gatherart', "Matched coverart site: " . $casite['name'], '5');
 					if (preg_match($casite['regexp'], $arurl, $matches) == 1) {
 						$num_found++;
@@ -1085,7 +1090,7 @@ public function gather_lastfm($limit,$options=false) {
 			$url = $coverart[$key];
 
 			// We need to check the URL for the /noimage/ stuff
-			if (strstr($url,"/noimage/")) {
+			if (strpos($url,"/noimage/") !== false) {
 				debug_event('LastFM','Detected as noimage, skipped ' . $url,'3');
 				continue;
 			}

lib/class/artist.class.php

@@ -227,7 +227,7 @@ public function format() {
 		// If this is a fake object, we're done here
 		if ($this->_fake) { return true; }
 
-		$this->f_name_link = "<a href=\"" . Config::get('web_path') . "/artists.php?action=show&amp;artist=" . $this->id . "\" title=\"" . $this->full_name . "\">" . $name . "</a>";
+		$this->f_name_link = "<a href=\"" . Config::get('web_path') . "/artists.php?action=show&amp;artist=" . $this->id . "\" title=\"" . $this->f_full_name . "\">" . $name . "</a>";
 		$this->f_link = Config::get('web_path') . '/artists.php?action=show&amp;artist=' . $this->id;
 
 		// Get the counts

lib/class/browse.class.php

@@ -103,6 +103,7 @@ public function show_objects($object_ids = null) {
 			${$class_name} = new $class_name($id);
 		}
 
+		$match = '';
 		// Format any matches we have so we can show them to the masses
 		if ($filter_value = $this->get_filter('alpha_match')) {
 			$match = ' (' . $filter_value . ')';

lib/class/catalog.class.php

@@ -459,7 +459,7 @@ public function add_files($path,$options) {
 		}
 
 		// Correctly detect the slash we need to use here
-		if (strstr($path,"/")) {
+		if (strpos($path,"/") !== false) {
 			$slash_type = '/';
 		}
 		else {
@@ -952,7 +952,10 @@ public static function dump_album_art($catalog_id, $methods=array()) {
 
 			// Try the preferred filename, if that fails use folder.???
 			$preferred_filename = Config::get('album_art_preferred_filename');
-			if (!$preferred_filename || strstr($preferred_filename,"%")) { $preferred_filename = "folder.$extension"; }
+			if (!$preferred_filename || 
+				strpos($preferred_filename, '%') !== false) {
+				$preferred_filename = "folder.$extension";
+			}
 
 			$file = "$dir/$preferred_filename";
 			if ($file_handle = fopen($file,"w")) {

lib/class/dba.class.php

@@ -424,7 +424,10 @@ public static function reset_db_charset() {
 
 			// Itterate through the columns of the table
 			while ($table = Dba::fetch_assoc($describe_results)) {
-				if (strstr($table['Type'],'varchar') OR strstr($table['Type'],'enum') OR strstr($table['Table'],'text')) {
+				if (
+				(strpos($table['Type'], 'varchar') !== false) ||
+				(strpos($table['Type'], 'enum') !== false) ||
+				(strpos($table['Table'],'text') !== false)) {
 					$sql = "ALTER TABLE `" . $row['0'] . "` MODIFY `" . $table['Field'] . "` " . $table['Type'] . " CHARACTER SET " . $target_charset;
 					$charset_results = Dba::write($sql);
 					if (!$charset_results) {

lib/class/query.class.php

@@ -145,6 +145,7 @@ public function reset() {
 		$this->reset_join();
 		$this->reset_select();
 		$this->reset_having();
+		$this->set_static_content(false);
 		$this->set_is_simple(false);
 		$this->set_start(0);
 		$this->set_offset(Config::get('offset_limit') ? Config::get('offset_limit') : '25');
@@ -219,7 +220,9 @@ public function get_filter($key) {
 
 		// Simple enough, but if we ever move this crap
 		// If we ever move this crap what?
-		return $this->_state['filter'][$key];
+		return isset($this->_state['filter'][$key]) 
+			? $this->_state['filter'][$key]
+			: false;
 
 	} // get_filter
 

lib/class/rating.class.php

@@ -65,6 +65,8 @@ public static function build_cache($type, $ids) {
 		if (!is_array($ids) OR !count($ids)) { return false; }
 
 		$user_id = intval($GLOBALS['user']->id);
+		$ratings = array();
+		$user_ratings = array();
 
 		$idlist = '(' . implode(',', $ids) . ')';
 		$sql = "SELECT `rating`, `object_id` FROM `rating` " .
@@ -73,7 +75,7 @@ public static function build_cache($type, $ids) {
 		$db_results = Dba::read($sql);
 
 		while ($row = Dba::fetch_assoc($db_results)) {
-			$user[$row['object_id']] = $row['rating'];
+			$user_ratings[$row['object_id']] = $row['rating'];
 		}
 
 		$sql = "SELECT AVG(`rating`) as `rating`, `object_id` FROM " .
@@ -82,19 +84,26 @@ public static function build_cache($type, $ids) {
 		$db_results = Dba::read($sql);
 
 		while ($row = Dba::fetch_assoc($db_results)) {
-			$rating[$row['object_id']] = $row['rating'];
+			$ratings[$row['object_id']] = $row['rating'];
   		}
 
 		foreach ($ids as $id) {
-			parent::add_to_cache('rating_' . $type . '_user' . $user_id, $id, intval($user[$id]));
-
-			if (!isset($rating[$id])) {
+			// First store the user-specific rating
+			if (!isset($user_ratings[$id])) {
 				$rating = 0;
 			}
 			else {
-				$rating = round($rating[$id]['rating'], 1);
+				$rating = intval($user_ratings[$id]);
 			}
+			parent::add_to_cache('rating_' . $type . '_user' . $user_id, $id, $rating);
 
+			// Then store the average
+			if (!isset($ratings[$id])) {
+				$rating = 0;
+			}
+			else {
+				$rating = round($ratings[$id]['rating'], 1);
+			}
 			parent::add_to_cache('rating_' . $type . '_all', $id, $rating);
 		}
 
@@ -125,11 +134,14 @@ public function get_user_rating($user_id = null) {
 			"AND `object_id`='$id' AND `object_type`='$type'";
 		$db_results = Dba::read($sql);
 
-		$results = Dba::fetch_assoc($db_results);
+		$rating = 0;
 
-		parent::add_to_cache($key, $id, $results['rating']);
+		if ($results = Dba::fetch_assoc($db_results)) {
+			$rating = $results['rating'];
+		}
 
-		return $results['rating'];
+		parent::add_to_cache($key, $id, $rating);
+		return $rating;
 
 	} // get_user_rating