Improvements on classes responsabilities

meson/find_unit_test_sources.sh

@@ -1,5 +1,5 @@
 #!/bin/sh
 
 find ./src -type f -name "*.vala" ! -name "main.vala" > /tmp/lyrics_build_source.txt
-find ./tests/unit -type f -name "*.vala" > /tmp/lyrics_build_test_sources.txt
+find ./tests -type f -name "*.vala" > /tmp/lyrics_build_test_sources.txt
 cat /tmp/lyrics_build_source.txt /tmp/lyrics_build_test_sources.txt

src/Application.vala

@@ -32,6 +32,8 @@ public class Lyrics.Application : Gtk.Application {
         main_stack = new MainStack (display_view);
         players.on_active_player_changed.connect (() => main_stack.set_player (players.active_player));
         players.on_active_player_changed.connect (() => display_view.set_player (players.active_player));
+        players.on_active_player_changed.connect (() => lyrics_service.set_player (players.active_player));
+        lyrics_service.notify["lyric"].connect (() => display_view.set_lyric (lyrics_service.lyric));
 
         var main_window = new MainWindow (this, players, main_stack);
         var header_bar = new Lyrics.HeaderBar (players, lyrics_service);
@@ -57,7 +59,8 @@ public class Lyrics.Application : Gtk.Application {
         var syncedLyrics = new SyncedLyrics.Shim();
         syncedLyrics.install();
 
-        lyrics_service = new LyricsService ();
+        var repository = new Repository ();
+        lyrics_service = new LyricsService (repository);
         players = new Players ();
     }
 

src/Core/Lyric.vala

@@ -1,5 +1,5 @@
 
-public class Lyrics.Lyric {
+public class Lyrics.Lyric : Object {
     private Gee.HashMap<string, string> metadata = new Gee.HashMap<string, string> ();
     private Gee.BidirMapIterator<int64?, string> lrc_iterator;
     private Gee.TreeMap<int64?, string> treemap;

src/Core/LyricsService.vala

@@ -1,26 +1,30 @@
 public class Lyrics.LyricsService : Object {
     public State state { get; set; }
-    IRepository lyric_repository = new Repository ();
+    IRepository lyric_repository;
+    public Lyric? lyric  { get; set; }
 
-    public virtual signal void push_lyric (Lyric lyric) {
-        state = State.DOWNLOADED;
+    public LyricsService (IRepository repository) {
+        lyric_repository = repository;
+        state = State.UNKNOWN;
     }
 
-    public Lyric? request_lyric (Metasong song) {
-        state = State.DOWNLOADING;
-
-        var lyricfile = lyric_repository.find_first (song);
-        if (lyricfile != null) {
-            state = State.DOWNLOADED;
+    public void set_player (Player player) {
+        GLib.MainLoop loop = new GLib.MainLoop ();
 
-            var lyric = lyricfile.to_lyric ();
-            //  Emit a signal notifying that a new lyric arrived
-            push_lyric (lyric);
-            return lyric;
+        if (player.current_song == null) {
+            state = State.UNKNOWN;
+            return;
         }
 
-        state = State.LYRICS_NOT_FOUND;
-        return  null;
+        request_lyric.begin (player.current_song, () => {
+            loop.quit ();
+        });
+        loop.run ();
+    }
+
+    public signal void set_lyric (Lyric lyric) {
+        this.lyric = lyric;
+        state = State.DOWNLOADED;
     }
 
     public enum State {
@@ -44,4 +48,20 @@ public class Lyrics.LyricsService : Object {
             }
         }
     }
+
+    private async void request_lyric (Metasong song) {
+        state = State.DOWNLOADING;
+
+        var lyricfile = lyric_repository.find_first (song);
+
+        if (lyricfile != null) {
+            lyric = lyricfile.to_lyric ();
+            state = State.DOWNLOADED;
+            return;
+        }
+
+        state = State.LYRICS_NOT_FOUND;
+
+        lyric = null;
+    }
 }

src/View/SearchLyricDialog.vala

@@ -58,7 +58,7 @@ public class Lyrics.SearchLyric : Gtk.Dialog {
             debug (@"Retrieving lyric for row id $id\n");
 
             repository.save (song_metadata, result[id]);
-            lyrics_service.push_lyric (result[id].to_lyric ());
+            lyrics_service.set_lyric (result[id].to_lyric ());
         }
     }
 

src/View/Widgets/Displays/IDisplay.vala

@@ -2,6 +2,7 @@ public interface Lyrics.IDisplay : Gtk.Widget {
     public abstract LyricsService lyrics_service { get; set; }
 
     public abstract void set_player (Player player);
+    public abstract void set_lyric (Lyric? lyric);
     public abstract void start (uint64 position);
     public abstract void stop ();
     public abstract void clear ();

src/View/Widgets/Displays/ScrolledDisplay.vala

@@ -15,9 +15,6 @@ public class Lyrics.ScrolledDisplay : Gtk.ScrolledWindow, IDisplay {
         adjustment = vadjustment;
         vscrollbar_policy = Gtk.PolicyType.EXTERNAL;
 
-        lyrics_service = lrservice;
-        lyrics_service.push_lyric.connect (on_lyric_update);
-
         box = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
         box.expand = true;
         box.valign = Gtk.Align.END;
@@ -28,12 +25,11 @@ public class Lyrics.ScrolledDisplay : Gtk.ScrolledWindow, IDisplay {
         stop ();
         debug (@"Player changed");
         if (player.state.to_string () == "PLAYING") {
-            lyrics_service.request_lyric (player.current_song);
             start (player.position);
         }
     }
 
-    public void on_lyric_update (Lyric lyric) {
+    public void set_lyric (Lyric? lyric) {
         this.lyric = lyric;
         return_if_fail (lyric != null);
 
@@ -56,7 +52,7 @@ public class Lyrics.ScrolledDisplay : Gtk.ScrolledWindow, IDisplay {
         remove_source ();
 
         //  Start transition
-        if (lyric != null) {
+        if (lyric != null && lyric.size > 0) {
             transition_to (labels[lyric.get_next_lyric_timestamp (start_time).to_string ()]);
         }
 

tests/MockClass.vala

@@ -0,0 +1,10 @@
+using GLib;
+
+public abstract class MockClass : Object {
+    public MockMethod should_call (string method_name) {
+        GLib.Value value = GLib.Value (typeof (MockMethod));
+        var prop = (method_name + "_mock");
+        this.get_property (prop, ref value);
+        return value.get_object () as MockMethod;
+    }
+}

tests/MockMethod.vala

@@ -0,0 +1,42 @@
+public errordomain ExpectationFailed {
+    METHOD_NOT_CALLED,
+    METHOD_CALLED_TOO_MANY_TIMES,
+}
+
+public class MockMethod : Object {
+    int expected = 0;
+    int executed_count = 0;
+
+    ~MockMethod () {
+        if (executed_count < expected) {
+            throw new ExpectationFailed.METHOD_NOT_CALLED("Method not called");
+        }
+    }
+
+    public void call () throws ExpectationFailed {
+        debug ("MockMethod.call()");
+
+        if (executed_count >= expected) {
+            throw new ExpectationFailed.METHOD_CALLED_TOO_MANY_TIMES("Method called too many times");
+        }
+
+        executed_count = executed_count + 1;
+    }
+
+    public MockMethod never () {
+        return times(0);
+    }
+
+    public MockMethod once () {
+        return times(1);
+    }
+
+    public MockMethod twice () {
+        return times(2);
+    }
+
+    public MockMethod times (int times) {
+        expected = expected + 1;
+        return this;
+    }
+}

tests/mocks/FileMock.vala

@@ -0,0 +1,13 @@
+public class FileMock : MockClass, Lyrics.ILyricFile {
+    public string? get_metadata (string attribute_name) {
+        assert_not_reached ();
+    }
+
+    public string get_content () {
+        assert_not_reached ();
+    }
+
+    public Lyrics.Lyric to_lyric () {
+        assert_not_reached ();
+    }
+}

tests/mocks/PlayerMock.vala

@@ -0,0 +1,21 @@
+
+public class PlayerMock : MockClass, Lyrics.Player  {
+    public int64 position { get; }
+    public Lyrics.Metasong current_song { get; set; }
+    public Lyrics.Player.State state { get; protected set; }
+    public string busname { get; protected set; }
+    public string? identity { get; protected set; }
+
+    public void toggle_play_pause () {
+        assert_not_reached ();
+    }
+
+    public void previous () {
+        assert_not_reached ();
+    }
+
+    public void next () {
+        assert_not_reached ();
+    }
+
+}

tests/mocks/RepositoryMock.vala

@@ -0,0 +1,15 @@
+
+public class RepositoryMock : MockClass, Lyrics.IRepository  {
+    protected MockMethod find_first_mock { get; set; default = new MockMethod ();}
+    protected MockMethod find_mock { get; set; default = new MockMethod ();}
+
+    public Lyrics.ILyricFile? find_first (Lyrics.Metasong song) {
+        find_first_mock.call ();
+        return null;
+    }
+
+    public Gee.Collection<Lyrics.ILyricFile>? find (Lyrics.Metasong song) {
+        find_mock.call ();
+        return null;
+    }
+}

tests/unit/Core/LyricsServiceTest.vala

@@ -0,0 +1,80 @@
+using Lyrics;
+
+public class Unit.Core.LyricsServiceTest : Unit.TestCase {
+    protected override void setup () {
+        //
+    }
+
+    protected override void teardown () {
+        //
+    }
+
+    public LyricsServiceTest() {
+        register ("/test_can_instantiate_lyrics_service", test_can_instantiate_lyrics_service);
+        register ("/test_set_player_null_song", test_set_player_null_song);
+        register ("/test_set_player_with_song", test_set_player_with_song);
+    }
+
+    void test_can_instantiate_lyrics_service() {
+        var mock_repository = new RepositoryMock ();
+        var lyrics_service = new LyricsService(mock_repository);
+
+        assert_cmpstr (
+            lyrics_service.state.to_string (),
+            GLib.CompareOperator.EQ,
+            LyricsService.State.UNKNOWN.to_string ()
+        );
+    }
+
+    void test_set_player_null_song() {
+        var player = new PlayerMock ();
+        var mock_repository = new RepositoryMock ();
+        var lyrics_service = new LyricsService(mock_repository);
+
+        lyrics_service.set_player (player);
+
+        assert_cmpstr (
+            lyrics_service.state.to_string (),
+            GLib.CompareOperator.EQ,
+            LyricsService.State.UNKNOWN.to_string ()
+        );
+    }
+
+    void test_set_player_with_song() {
+        var song = new Metasong ();
+        var player = new PlayerMock ();
+        var mock_repository = new RepositoryMock ();
+        var lyrics_service = new LyricsService(mock_repository);
+
+        mock_repository
+            .should_call ("find_first")
+            .never ();
+
+        player.current_song = song;
+
+        LyricsService.State[] state_change = {
+            LyricsService.State.DOWNLOADING,
+            LyricsService.State.LYRICS_NOT_FOUND,
+        };
+
+        uint state_changed_count = 0;
+
+        lyrics_service.notify["state"].connect ((sender, state) => {
+            assert_cmpstr (
+                lyrics_service.state.to_string (),
+                GLib.CompareOperator.EQ,
+                state_change[state_changed_count].to_string ()
+            );
+
+            state_changed_count = state_changed_count + 1;
+        });
+
+        lyrics_service.set_player (player);
+
+        while (lyrics_service.state != LyricsService.State.LYRICS_NOT_FOUND) {
+            //  wait for the state to change
+        }
+
+        assert_cmpuint(state_changed_count, GLib.CompareOperator.EQ, 2);
+    }
+}

tests/unit/main.vala

@@ -2,6 +2,7 @@ int main (string[] args) {
     Test.init (ref args);
 
     new Unit.Core.LyricTest();
+    new Unit.Core.LyricsServiceTest ();
     new Unit.Parser.LrcParserTest ();
 
     return Test.run ();