Merge lp:~unity-team/unity-lens-music/improved-search-results-5.0 into lp:unity-lens-music/5.0

[Mikkel Kamstrup Erlandsen]

Fix u1 search results to not seem too random. Lowers mem churn in musicstore scope quite a bit.

Backport from trunk: Fix u1 search results to not seem too random. Lowers mem churn in musicstore scope quite a bit.

This was rooted in the fact that musicsearch.ubuntu.com returns a mix of artists, albums, and tracks in the results. We do not show artists, so if the results contains mainly that, then we're screwed (even though there could be many potential track/album matches outside the fetched results). Luckily newer versions of the webservice contains some parameters to tweak this behaviour. This commit makes us use that.

In addition to using the new musicsearch API I also lowered memory churn quite considerably by removing the need to create a lot of Track/Album objects and do excessive string copying.

[Michal Hruby (mhr3)]

Looking good. LGTM

review: Approve

=== modified file 'src/musicstore-collection.vala'
--- src/musicstore-collection.vala	2012-02-06 22:19:44 +0000
+++ src/musicstore-collection.vala	2012-07-11 08:06:22 +0000
@@ -25,6 +25,7 @@
   {
 
     private const string MUSICSTORE_BASE_URI = "http://musicsearch.ubuntu.com/v1/";
+    private const uint PURCHASE_CATEGORY = 2;
 
     public MusicStoreCollection ()
     {
@@ -56,34 +57,51 @@
 	yield parser.load_from_stream_async (dis, cancellable);
 	var root_object = parser.get_root ().get_object ();
 	
-	foreach (var r in root_object.get_array_member ("results").get_elements ()) {
-	  var result_obj = r.get_object ();
-	  
-	  string type = result_obj.get_string_member ("type");
-	  if (type == "track") {
-	    Track track = new Track ();
-	    track.title = result_obj.get_string_member ("title");
-	    track.artist = result_obj.get_string_member ("artist");
-	    track.uri = result_obj.get_string_member ("purchase_url");
-	    track.artwork_path = result_obj.get_string_member ("image");
-	    track.mime_type = "audio-x-generic";
-
-	    // FIXME drag n drop uri needs to be the u1ms:// link
-	    model.append (track.uri, track.artwork_path, 2, track.mime_type, track.title, track.artist, track.uri);
-	  
-	  } else if (type == "album") {
-	    Album album = new Album ();
-	    album.title = result_obj.get_string_member ("title");
-	    album.artist = result_obj.get_string_member ("artist");
-	    album.uri = result_obj.get_string_member ("purchase_url");
-	    album.artwork_path = result_obj.get_string_member ("image");
-	    
-	    model.append (album.uri, album.artwork_path, 2, "audio-x-generic", album.title, album.artist, album.uri);
-	  }
+	Json.Object? results = root_object.get_object_member ("results");
+	if (results == null) {
+	  warning ("Invalid response from server. No 'results' member.");
+	  return;
+	}
+	
+	var albums = results.get_array_member ("album").get_elements ();
+	var tracks = results.get_array_member ("track").get_elements ();
+	
+	debug ("Got %u albums and %u tracks", albums.length (), tracks.length ());
+	
+	unowned string? uri, artwork_path, mimetype, title, artist, dnd_uri;
+	
+	foreach (var album_node in albums) {
+	  var album_obj = album_node.get_object ();
+	  
+	  uri = album_obj.get_string_member ("purchase_url");
+	  artwork_path = album_obj.get_string_member ("image");
+	  mimetype = "audio-x-generic";
+	  title = album_obj.get_string_member ("title");
+	  artist = album_obj.get_string_member ("artist");
+	  dnd_uri = uri;
+	  
+	  model.append (uri, artwork_path, PURCHASE_CATEGORY,
+	                mimetype, title, artist, dnd_uri);
         }
+        
+	foreach (var track_node in tracks) {
+	  var track_obj = track_node.get_object ();
+	  
+	  uri = track_obj.get_string_member ("purchase_url");
+	  artwork_path = track_obj.get_string_member ("image");
+	  mimetype = "audio-x-generic";
+	  title = track_obj.get_string_member ("title");
+	  artist = track_obj.get_string_member ("artist");
+	  dnd_uri = uri;
+          
+	  // FIXME drag n drop uri needs to be the u1ms:// link
+	  
+	  model.append (uri, artwork_path, PURCHASE_CATEGORY,
+	                mimetype, title, artist, dnd_uri);
+        }        
 
 	debug ("Retrieved '%s' in %fms", file.get_uri (), timer.elapsed()*1000);
-	debug ("model has %u rows after search", model.get_n_rows ());
+	debug ("Model has %u rows after search", model.get_n_rows ());
 
       } catch (Error e) {
 	warning ("Error reading URL '%s': %s", file.get_uri (), e.message);
@@ -116,6 +134,13 @@
       }
       
       uri.append ("&pagesize=10");
+      
+      // This makes the service return $pagesize results *per content type*.
+      // Which we need - as it could otherwise return $pagesize results mixed
+      // or artist,album, and track. Since we can't display artists, this can
+      // lead to an empty result set in the dash if there is only artists in
+      // the response from the webservice
+      uri.append ("&grouping=1");
 
       return uri.str;
     }