some bugfixes and subsonic usability (#2157)

* Create RELEASE-PROCESS.md * Api::stats require filter * add subsonic_stream_scrobble to config * Update migrate_config.inc * can't download new config without nohtml class * use cache instead of noscrobble in subsonic
ampache · Nov 11, 2019 · 87eb0f2 · 87eb0f2
1 parent fd79187
commit 87eb0f2
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 34 deletions.
diff --git a/.gitignore b/.gitignore
@@ -41,3 +41,4 @@ core/
 develop/
 master/
 *.swp
+*.save
diff --git a/bin/migrate_config.inc b/bin/migrate_config.inc
@@ -47,7 +47,7 @@ $old_config = file_get_contents($prefix . '/config/ampache.cfg.php');
 
 $data = explode("\n",$old_config);
 
-echo T_("Parsing old config file...");
+echo "Parsing old config file...";
 echo "\n";
 
 foreach ($data as $line) {
@@ -77,18 +77,18 @@ foreach ($data as $line) {
 
 } // end foreach lines
 
-echo T_("Parse complete, writing");
+echo "Parse complete, writing";
 echo "\n";
 
 $handle = fopen($prefix . '/config/ampache.cfg.php','w');
 
 $worked = fwrite($handle,$new_config);
 
 if ($worked) {
- echo T_("Write success, config migrated");
+ echo "Write success, config migrated";
  echo "\n";
 }
 else {
- echo T_("Access denied, config migration failed");
+ echo "Access denied, config migration failed";
  echo "\n";
 }
diff --git a/config/ampache.cfg.php.dist b/config/ampache.cfg.php.dist
@@ -653,6 +653,12 @@ show_footer_statistics = "true"
 ; DEFAULT: false
 ;allow_php_themes = "false"
 
+; Subsonic clients all seem to ignore the download method
+; This setting will not scrobble actions from stream actions.
+; (This means you can sync playlists without overloading your server)
+; Make sure you enable scrobbling in your client to keep recording stats!
+; DEFAULT: "true"
+;subsonic_stream_scrobble = "false"
 
 ;#########################################################
 ; Debugging #
@@ -803,7 +809,6 @@ site_charset = UTF-8
 ;ldap_member_attribute = "memberuid"
 
 
-
 ;#########################################################
 ; OpenID login info (optional) #
 ;#########################################################

diff --git a/docs/RELEASE-PROCESS.md b/docs/RELEASE-PROCESS.md
@@ -0,0 +1,11 @@
+# Ampache Release Guide
+
+## Wagnerd's process
+
+1. Merge/squash develop branch with master.
+2. Create a zip package named "ampache-*.*.*_all.zip and add the entire ampache directory tree.
+3. From the zip file, I then removed all files that are used for installation/development and are not necessary for ampache to function.
+4. I then unpacked the zip into a separate path, created a server pointing to that path and tested basic functionality. This might be where unit testing would be helpful.
+5. When drafting a new release, set target to master branch. When publishing, github will add a reference to the two compressed packages (zip and tar.gz) which will need to be installed.
+6. The "ampache-*.*.*_all.zip" drop-in package is then uploaded.
+7. After setting version and title, it should be ready to publish.
diff --git a/lib/class/api.class.php b/lib/class/api.class.php
@@ -1147,6 +1147,12 @@ public static function playlist_remove_song($input)
  */
  public static function search_songs($input)
  {
+ if (!self::check_parameter($input, array('filter'))) {
+ debug_event('api.class', "'filter' required on search_songs function call.", 2);
+ echo XML_Data::error('401', T_("Missing mandatory parameter") . " 'filter'");
+
+ return false;
+ }
  $array = array();
  $array['type'] = 'song';
  $array['rule_1'] = 'anywhere';
@@ -1177,6 +1183,12 @@ public static function search_songs($input)
  */
  public static function videos($input)
  {
+ if (!self::check_parameter($input, array('filter'))) {
+ debug_event('api.class', "'filter' required on videos function call.", 2);
+ echo XML_Data::error('401', T_("Missing mandatory parameter") . " 'filter'");
+
+ return false;
+ }
  self::$browse->reset_filters();
  self::$browse->set_type('video');
  self::$browse->set_sort('title', 'ASC');
@@ -1219,17 +1231,17 @@ public static function video($input)
  *
  * @param array $input
  * type = (string) 'song'|'album'|'artist'
- * filter = (string) 'newest'|'highest'|'frequent'|'recent'|'forgotten'|'flagged'|null //optional
+ * filter = (string) 'newest'|'highest'|'frequent'|'recent'|'forgotten'|'flagged'|'random'
  * user_id = (integer) //optional
  * username = (string) //optional
  * offset = (integer) //optional
  * limit = (integer) //optional
  */
  public static function stats($input)
  {
- if (!self::check_parameter($input, array('type'))) {
+ if (!self::check_parameter($input, array('type', 'filter'))) {
  debug_event('api.class', "'type' required on stats function call.", 2);
- echo XML_Data::error('401', T_("Missing mandatory parameter") . " 'type'");
+ echo XML_Data::error('401', T_("Missing mandatory parameter") . " 'type', 'filter'");
 
  return false;
  }
@@ -1289,6 +1301,7 @@ public static function stats($input)
  debug_event('api.class', 'stats flagged', 4);
  $results = Userflag::get_latest($type, $user_id);
  break;
+ case "random":
  default:
  debug_event('api.class', 'stats random ' . $type, 4);
  switch ($type) {
@@ -1876,7 +1889,7 @@ public static function scrobble($input)
 
  return;
  }
- $user->update_stats('song', $scrobble_id, $agent, array(), false, $date);
+ $user->update_stats('song', $scrobble_id, $agent, array(), $date);
  echo XML_Data::success('successfully scrobbled: ' . $scrobble_id);
  }
  Session::extend($input['auth']);
@@ -2199,7 +2212,7 @@ public static function download($input)
  $user_id = User::get_from_username(Session::username($input['auth']))->id;
 
  $url = '';
- $params = '&action=download' . '&client=api' . '&noscrobble=1';
+ $params = '&action=download' . '&client=api' . '&cache=1';
  if ($original) {
  $params .= '&transcode_to=' . $format;
  }

diff --git a/lib/class/stats.class.php b/lib/class/stats.class.php
@@ -252,11 +252,12 @@ public static function get_last_song($user_id = '')
  * This returns the objects that have happened for $user_id sometime after $time
  * used primarily by the democratic cooldown code
  */
- public static function get_object_history($user_id, $time)
+ public static function get_object_history($user_id, $time, $newest = true)
  {
  if (!in_array((string) $user_id, User::get_valid_users())) {
  $user_id = Core::get_global('user')->id;
  }
+ $order = ($newest) ? 'DESC' : 'ASC';
 
  $sql = "SELECT * FROM `object_count` " .
  "LEFT JOIN `song` ON `song`.`id` = `object_count`.`object_id` ";
@@ -267,7 +268,7 @@ public static function get_object_history($user_id, $time)
  if (AmpConfig::get('catalog_disable')) {
  $sql .= "AND `catalog`.`enabled` = '1' ";
  }
- $sql .= "ORDER BY `object_count`.`date` DESC";
+ $sql .= "ORDER BY `object_count`.`date` " . $order;
  $db_results = Dba::read($sql, array($user_id, $time));
 
  $results = array();

diff --git a/lib/class/subsonic_api.class.php b/lib/class/subsonic_api.class.php
@@ -1007,6 +1007,9 @@ public static function stream($input)
  if ($timeOffset) {
  $params .= '&frame=' . $timeOffset;
  }
+ if (AmpConfig::get('subsonic_stream_scrobble') == 'false') {
+ $params .= '&cache=1';
+ }
 
  $url = '';
  if (Subsonic_XML_Data::isSong($fileid)) {
@@ -1029,7 +1032,7 @@ public static function download($input)
  {
  $fileid = self::check_parameter($input, 'id', true);
 
- $url = Song::play_url(Subsonic_XML_Data::getAmpacheId($fileid), '&action=download' . '&client=' . rawurlencode($input['c']) . '&noscrobble=1', 'api', function_exists('curl_version'));
+ $url = Song::play_url(Subsonic_XML_Data::getAmpacheId($fileid), '&action=download' . '&client=' . rawurlencode($input['c']) . '&cache=1', 'api', function_exists('curl_version'));
  self::follow_stream($url);
  }
 

diff --git a/lib/class/user.class.php b/lib/class/user.class.php
@@ -885,7 +885,7 @@ public function update_last_seen()
  * updates the playcount mojo for this specific user
  * @param string $media_type
  */
- public function update_stats($media_type, $media_id, $agent = '', $location = array(), $noscrobble = false, $date = null)
+ public function update_stats($media_type, $media_id, $agent = '', $location = array(), $date = null)
  {
  debug_event('user.class', 'Updating stats for {' . $media_type . '/' . $media_id . '} {' . $agent . '}...', 5);
  $media = new $media_type($media_id);
@@ -897,24 +897,19 @@ public function update_stats($media_type, $media_id, $agent = '', $location = ar
  return false;
  }
 
- if (!$noscrobble) {
- $this->set_preferences();
- // If pthreads available, we call save_mediaplay in a new thread to quickly return
- if (class_exists("Thread", false)) {
- debug_event('user.class', 'Calling save_mediaplay plugins in a new thread...', 5);
- $thread = new scrobbler_async(Core::get_global('user'), $media);
- if ($thread->start()) {
- //$thread->join();
- } else {
- debug_event('user.class', 'Error when starting the thread.', 1);
- }
+ $this->set_preferences();
+ // If pthreads available, we call save_mediaplay in a new thread to quickly return
+ if (class_exists("Thread", false)) {
+ debug_event('user.class', 'Calling save_mediaplay plugins in a new thread...', 5);
+ $thread = new scrobbler_async(Core::get_global('user'), $media);
+ if ($thread->start()) {
+ //$thread->join();
  } else {
- self::save_mediaplay(Core::get_global('user'), $media);
+ debug_event('user.class', 'Error when starting the thread.', 1);
  }
  } else {
- debug_event('user.class', 'Scrobbling explicitly skipped', 5);
+ self::save_mediaplay(Core::get_global('user'), $media);
  }
-
  $media->set_played($user_id, $agent, $location, $date);
 
  return true;
@@ -1342,7 +1337,7 @@ public function get_recently_played($limit, $type = '', $newest = true)
  }
  $ordersql = ($newest == true) ? 'DESC' : 'ASC';
 
- $sql = "SELECT * FROM `object_count` WHERE `object_type` = ? AND `user` = ? " .
+ $sql = "SELECT `object_id`, MAX(`date`) AS `date` FROM `object_count` WHERE `object_type` = ? AND `user` = ? " .
  "ORDER BY `date` " . $ordersql . " LIMIT " . $limit;
  $db_results = Dba::read($sql, array($type, $this->id));
 

diff --git a/play/index.php b/play/index.php
@@ -666,13 +666,18 @@
  if ($start > 0) {
  debug_event('play/index', 'Content-Range doesn\'t start from 0, stats should already be registered previously; not collecting stats', 5);
  } else {
+ $sessionkey = $sid ?: Stream::get_session();
+ $agent = Session::agent($sessionkey);
+ $location = Session::get_geolocation($sessionkey);
  if (!$share_id && $record_stats) {
  if (Core::get_server('REQUEST_METHOD') != 'HEAD') {
  debug_event('play/index', 'Registering stream stats for {' . $media->get_stream_name() . '}...', 4);
- $sessionkey = $sid ?: Stream::get_session();
- $agent = Session::agent($sessionkey);
- $location = Session::get_geolocation($sessionkey);
- Core::get_global('user')->update_stats($type, $media->id, $agent, $location, isset($_REQUEST['noscrobble']));
+ Core::get_global('user')->update_stats($type, $media->id, $agent, $location);
+ }
+ } elseif (!$share_id && !$record_stats) {
+ if (Core::get_server('REQUEST_METHOD') != 'HEAD') {
+ debug_event('play/index', 'Registering download stats for {' . $media->get_stream_name() . '}...', 5);
+ Stats::insert($type, $media->id, $uid, $agent, $location, 'download');
  }
  }
  }

diff --git a/templates/show_debug.inc.php b/templates/show_debug.inc.php
@@ -23,7 +23,7 @@
  <div id="information_actions">
  <ul>
  <li>
- <a href="<?php echo AmpConfig::get('web_path'); ?>/admin/system.php?action=generate_config"><?php echo UI::get_icon('cog', T_('Generate Configuration File')) . ' ' . T_('Generate Configuration File'); ?></a>
+ <a class="nohtml" href="<?php echo AmpConfig::get('web_path'); ?>/admin/system.php?action=generate_config"><?php echo UI::get_icon('cog', T_('Generate Configuration File')) . ' ' . T_('Generate Configuration File'); ?></a>
  </li>
  <li>
  <a href="<?php echo AmpConfig::get('web_path'); ?>/admin/system.php?action=write_config"><?php echo UI::get_icon('cog', T_('Write New Config')) . ' ' . T_('Write New Config'); ?></a>