Merge lp:~mhr3/libunity/more-api-changes into lp:libunity

[Michal Hruby]

More API changes for Previews

A few more API changes, hopefully the last time :)

Tests updated to reflect the changes.

[Paweł Stołowski (stolowski)]

Looks good!

review: Approve

[lp:~mhr3/libunity/more-api-changes]

148. By Michal Hruby on 2012-06-27

Update parts of the API to match the spec. Fixes: . Approved by Michal Hruby, Mikkel Kamstrup Erlandsen.

149. By Michal Hruby on 2012-06-28

More API changes for Previews. Fixes: . Approved by Pawel Stolowski.

=== modified file 'protocol/protocol-previews.vala'
--- protocol/protocol-previews.vala	2012-06-25 12:23:25 +0000
+++ protocol/protocol-previews.vala	2012-06-27 12:19:20 +0000
@@ -444,6 +444,18 @@
       properties["track-data-address"] = track_data_address;
   }
 
+  public void play_uri (string uri)
+  {
+    add_update ("action", "play");
+    add_update ("uri", uri);
+  }
+
+  public void pause_uri (string uri)
+  {
+    add_update ("action", "pause");
+    add_update ("uri", uri);
+  }
+
   static construct
   {
     Dee.Serializable.register_parser (typeof (MusicPreview),
@@ -544,19 +556,19 @@
     Object ();
   }
 
-  public override void begin_updates()
+  public override void begin_updates ()
   {
     base.begin_updates();
     if (_selected_item_sig_id == 0)
     {
-      _selected_item_sig_id = notify["selected-item"].connect ((obj, spec) =>
-        {
-          add_update("series-active-index", selected_item);
-        });
+      _selected_item_sig_id = notify["selected-item"].connect (() =>
+      {
+        add_update("series-active-index", selected_item);
+      });
     }
   }
 
-  public override HashTable<string, Variant>? end_updates_as_hashtable()
+  public override HashTable<string, Variant>? end_updates_as_hashtable ()
   {
     if (_selected_item_sig_id > 0)
     {

=== modified file 'src/unity-previews.vala'
--- src/unity-previews.vala	2012-06-27 12:19:20 +0000
+++ src/unity-previews.vala	2012-06-27 12:19:20 +0000
@@ -351,6 +351,28 @@
     }
   }
 
+  internal override HashTable<string, Variant> update_property (HashTable<string, Variant> values)
+  {
+    Variant? action_v = values["action"];
+    Variant? uri_v = values["uri"];
+    if (action_v != null && uri_v != null)
+    {
+      switch (action_v.get_string ())
+      {
+        case "play":
+          play (uri_v.get_string ());
+          break;
+        case "pause":
+          pause (uri_v.get_string ());
+          break;
+        default:
+          warning ("Unknown MusicPreview action: %s", action_v.get_string ());
+          break;
+      }
+    }
+    return base.update_property (values);
+  }
+
   public signal void play (string uri);
   public signal void pause (string uri);
 
@@ -413,50 +435,91 @@
 }
 
 
-public class SeriesPreview : Preview
-{
-  public int selected_item
-  {
-    get { return _raw.selected_item; }
-    private set { _raw.selected_item = value; }
-  }
-
-  public SeriesPreview ()
-  {
-    Object ();
-  }
-
-  public void add_item (string title, string uri, Icon? icon)
-  {
-    _raw.add_series_item (title, uri, icon);
+public class SeriesItem : Object
+{
+  public string uri { get; construct; }
+  public string title { get; construct; }
+  public Icon? icon_hint { get; construct; }
+
+  public SeriesItem (string uri, string title, Icon? icon_hint)
+  {
+    Object (uri: uri, title: title, icon_hint: icon_hint);
+  }
+}
+
+public class SeriesPreview : Preview, Dee.Serializable
+{
+  private string _current_item_uri;
+  public string current_item_uri
+  {
+    get { return _current_item_uri; }
+    construct { _current_item_uri = value; }
+  }
+
+  // FIXME: why do we even keep this?
+  private int _selected_item = -1;
+
+  public SeriesPreview (SeriesItem[] items, string selected_item_uri)
+  {
+    // careful current_item_uri will be set before items are
+    Object (current_item_uri: selected_item_uri);
+    foreach (unowned SeriesItem item in items)
+    {
+      add_item (item);
+    }
+  }
+
+  private void add_item (SeriesItem item)
+  {
+    _raw.add_series_item (item.title, item.uri, item.icon_hint);
+  }
+
+  /* Emits request_item_preview and updates internal state */
+  private void update_child_preview (string uri, int item_index = -1)
+  {
+    var preview = request_item_preview (uri);
+    if (preview != null)
+    {
+      if (item_index < 0)
+      {
+        // try to auto-update _selected_item
+        int i = 0;
+        foreach (unowned Protocol.SeriesItemRaw item in _raw.get_items ())
+        {
+          if (item.uri == uri)
+          {
+            _selected_item = i;
+            break;
+          }
+          i++;
+        }
+        if (_selected_item < 0) warning ("Unable to find URI in series items");
+      }
+      else _selected_item = item_index;
+
+      _child_preview = preview;
+      _raw.child_preview = preview.get_raw ();
+      _current_item_uri = uri;
+    }
   }
 
   private Preview _child_preview;
 
-  public void set_active_preview (int item_index, Preview? active_preview)
-    requires (item_index < _raw.get_items ().length)
-  {
-    selected_item = item_index;
-    _child_preview = active_preview;
-    _raw.child_preview = active_preview.get_raw ();
-  }
-
-  public signal Preview? selected_item_changed (int new_index);
+  public signal Preview? request_item_preview (string uri);
 
   internal override HashTable<string, Variant> update_property (HashTable<string, Variant> values)
   {
-    int selected = selected_item;
     var response = base.update_property (values);
+    int new_selected_item = _raw.selected_item;
     Preview? preview = null;
-    if (selected != selected_item)
-    {
-      preview = selected_item_changed (selected_item);
-    }
-    else
-    {
-      preview = _child_preview;
-    }
-    response["preview"] = preview.serialize();
+    if (new_selected_item != _selected_item &&
+        new_selected_item < _raw.get_items ().length)
+    {
+      unowned Protocol.SeriesItemRaw item = _raw.get_items ()[new_selected_item];
+      update_child_preview (item.uri, new_selected_item);
+    }
+    preview = _child_preview;
+    response["preview"] = preview.serialize ();
     return response;
   }
 
@@ -465,6 +528,16 @@
     return _child_preview;
   }
 
+  private Variant serialize ()
+  {
+    // ensures that we emit selected_item_changed shortly after construction
+    if (_selected_item < 0 && _raw.get_items ().length > 0)
+    {
+      update_child_preview (current_item_uri);
+    }
+    return _raw.serialize ();
+  }
+
   private unowned Protocol.SeriesPreview _raw;
   internal override Object create_raw ()
   {

=== modified file 'test/vala/test-lens.vala'
--- test/vala/test-lens.vala	2012-06-25 11:58:23 +0000
+++ test/vala/test-lens.vala	2012-06-27 12:19:20 +0000
@@ -812,8 +812,8 @@
 
     bool got_activate_uri_signal = false;
     bool got_preview_uri_signal = false;
-    bool got_selected_item_changed_signal = false;
-    int selected_item_signal_index = -1;
+    string? request_item_signal_uri = null;
+    int request_item_signal_count = 0;
     Unity.PreviewAction? action = null;
 
     signals += new SignalWrapper (local_scope,
@@ -823,18 +823,19 @@
       return null;
     }));
 
-    var series_preview = new Unity.SeriesPreview ();
-    var generic_preview = new Unity.GenericPreview ("child preview", "this is a child preview", null);
+    var item1 = new Unity.SeriesItem ("file:///series_item_1.jpg", "Item #1", null);
+    var item2 = new Unity.SeriesItem ("file:///series_item_2.jpg", "Item #2", null);
+    var series_preview = new Unity.SeriesPreview ({item1, item2}, "file:///series_item_1.jpg");
 
-    signals += new SignalWrapper (series_preview, series_preview.selected_item_changed.connect ((index) => 
-        {
-          got_selected_item_changed_signal = true;
-          selected_item_signal_index = index;
-          return generic_preview;
-        }));
-    series_preview.add_item ("file:///media/series_item_1.jpg", "Item #1", null);
-    series_preview.add_item ("file:///media/series_item_2.jpg", "Item #2", null);
-    series_preview.set_active_preview (0, generic_preview);
+    signals += new SignalWrapper (series_preview,
+      series_preview.request_item_preview.connect ((uri) =>
+    {
+      request_item_signal_count++;
+      request_item_signal_uri = uri;
+      return new Unity.GenericPreview ("child preview",
+                                       "this is a child preview",
+                                       null);
+    }));
       
     signals += new SignalWrapper (local_scope,
       local_scope.preview_uri.connect ((uri) =>
@@ -860,7 +861,6 @@
     assert (sp_reconstructed.selected_item == 0);
     assert (got_preview_uri_signal);
     assert (!got_activate_uri_signal);
-    assert (!got_selected_item_changed_signal);
 
     sp_reconstructed.begin_updates();
     sp_reconstructed.selected_item = 1; // this should result in series-active-index property update
@@ -886,18 +886,17 @@
     run_with_timeout (ml, 5000);
 
     assert (gp_reconstructed != null);
-    assert (selected_item_signal_index == 1);
-    assert (got_selected_item_changed_signal);
+    assert (request_item_signal_uri == item2.uri);
+    assert (request_item_signal_count > 1);
 
     ml = new MainLoop ();
-    got_selected_item_changed_signal = false;
-    
+    request_item_signal_count = 0;
+
     sp_reconstructed.begin_updates();
     sp_reconstructed.selected_item = 1;
     Variant updates_var = sp_reconstructed.end_updates();
     updates = (HashTable<string, Variant>)updates_var;
 
-    //
     // sending update for unchanged selected_item property return preview as well, but no signal is emitted
     call_lens_update_preview_property (mangled_uri, updates, (reply) =>
     {
@@ -909,8 +908,8 @@
     run_with_timeout (ml, 5000);
 
     assert (gp_reconstructed != null);
-    assert (selected_item_signal_index == 1);
-    assert (!got_selected_item_changed_signal);
+    assert (request_item_signal_uri == item2.uri);
+    assert (request_item_signal_count == 0);
   }
 
 }

=== modified file 'test/vala/test-previews.vala'
--- test/vala/test-previews.vala	2012-06-27 12:19:20 +0000
+++ test/vala/test-previews.vala	2012-06-27 12:19:20 +0000
@@ -187,7 +187,7 @@
 
     static void test_preview_series ()
     {
-      var preview = new SeriesPreview ();
+      var preview = new SeriesPreview ({}, "");
 
       // check if serialization works properly
       var data = preview.serialize ();
@@ -203,16 +203,25 @@
       var artwork = new FileIcon (File.new_for_path ("/usr/share/icons/beatles.jpg"));
       string title = "A title";
       string desc = "Description";
+      bool got_generate_preview_signal = false;
 
-      var preview = new SeriesPreview ();
-      preview.add_item ("file:///media/series_item_1.jpg", "Item #1", artwork);
-      preview.add_item ("file:///media/series_item_2.jpg", "Item #2", null);
-      preview.set_active_preview (0, new GenericPreview (title, desc, null));
+      SeriesItem item1 = new SeriesItem ("file:///media/series_item_1.jpg",
+                                         "Item #1", artwork);
+      SeriesItem item2 = new SeriesItem ("file:///media/series_item_2.jpg",
+                                         "Item #2", null);
+      var preview = new SeriesPreview ({item1, item2},
+                                       "file:///media/series_item_2.jpg");
+      preview.request_item_preview.connect (() =>
+      {
+        got_generate_preview_signal = true;
+        return new GenericPreview (title, desc, null);
+      });
 
       // check if serialization works properly
       var data = preview.serialize ();
       var reconstructed = Protocol.Preview.parse (data) as Protocol.SeriesPreview;
 
+      assert (got_generate_preview_signal);
       assert (preview.thumbnail.equal (reconstructed.thumbnail));
       assert (reconstructed.get_items().length == 2);
       assert (reconstructed.child_preview.title == title);