From 74ac275ecea9ea26848d9a44e0bed906a4f8f15e Mon Sep 17 00:00:00 2001 From: Hyzual Date: Tue, 25 Nov 2014 17:38:53 +0100 Subject: [PATCH] Refactors the player service. The idea is that the service should ideally be used directly, so we know who depends on what and don't end up with cycles. To achieve this, we should not add functions to $rootScope but instead provide them from our service. Services can use them directly, and controllers have to create other functions in their own scope. This means that we have a player-controller that does not do much by itself but it enables us to avoid filling up the $rootScope. I have left the $rootScope additions at the end of player-service because it's going to take some time to refactor it, so in the meantime I want things to keep working. --- app/app.js | 108 +++++++++++++++--------------- app/player/player-service.js | 16 +++-- app/player/player-service_test.js | 55 +++++++++++++-- app/player/player.html | 2 +- app/player/player.js | 35 ++-------- app/player/player_test.js | 42 +++++++++++- 6 files changed, 164 insertions(+), 94 deletions(-) diff --git a/app/app.js b/app/app.js index 4059452..b3df69c 100755 --- a/app/app.js +++ b/app/app.js @@ -1,54 +1,54 @@ - -/* Declare app level module */ -angular.module('JamStash', ['ngCookies', 'ngRoute', 'ngSanitize', - 'jamstash.subsonic.ctrl', 'jamstash.archive.ctrl', 'jamstash.player']) - -.config(['$routeProvider',function($routeProvider) { - 'use strict'; - - $routeProvider - .when('/index', { redirectTo: '/library' }) - .when('/settings', { templateUrl: 'settings/settings.html', controller: 'SettingsCtrl' }) - .when('/queue', { templateUrl: 'queue/queue.html', controller: 'QueueCtrl' }) - .when('/library', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl' }) - .when('/library/:artistId', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl', reloadOnSearch: false }) - .when('/library/:artistId/:albumId', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl', reloadOnSearch: false }) - .when('/podcasts', { templateUrl: 'podcasts/podcasts.html', controller: 'PodcastCtrl' }) - .when('/archive', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) - .when('/archive/:artist', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) - .when('/archive/:artist/:album', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) - .otherwise({ redirectTo: '/index' }); -}]) - -.config(['$httpProvider',function($httpProvider) { - 'use strict'; - - $httpProvider.interceptors.push(['$rootScope', '$location', '$q', 'globals', function ($rootScope, $location, $q, globals) { - return { - 'request': function (request) { - // if we're not logged-in to the AngularJS app, redirect to login page - //$rootScope.loggedIn = $rootScope.loggedIn || globals.settings.Username; - $rootScope.loggedIn = false; - if (globals.settings.Username != "" && globals.settings.Password != "" && globals.settings.Server != "") { - $rootScope.loggedIn = true; - } - var path = ''; - path = $location.path(); - if (globals.settings.Debug) { console.log('Logged In: ' + $rootScope.loggedIn); } - if (globals.settings.Debug) { console.log('Current Path: ' + path); } - if (!$rootScope.loggedIn && path != '/settings' && path.search('archive') < 0) { - $location.path('/settings'); - } - return request; - }, - 'responseError': function (rejection) { - // if we're not logged-in to the web service, redirect to login page - if (rejection.status === 401 && $location.path() != '/settings') { - $rootScope.loggedIn = false; - $location.path('/settings'); - } - return $q.reject(rejection); - } - }; - }]); -}]); + +/* Declare app level module */ +angular.module('JamStash', ['ngCookies', 'ngRoute', 'ngSanitize', + 'jamstash.subsonic.ctrl', 'jamstash.archive.ctrl', 'jamstash.player.ctrl']) + +.config(['$routeProvider',function($routeProvider) { + 'use strict'; + + $routeProvider + .when('/index', { redirectTo: '/library' }) + .when('/settings', { templateUrl: 'settings/settings.html', controller: 'SettingsCtrl' }) + .when('/queue', { templateUrl: 'queue/queue.html', controller: 'QueueCtrl' }) + .when('/library', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl' }) + .when('/library/:artistId', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl', reloadOnSearch: false }) + .when('/library/:artistId/:albumId', { templateUrl: 'subsonic/subsonic.html', controller: 'SubsonicCtrl', reloadOnSearch: false }) + .when('/podcasts', { templateUrl: 'podcasts/podcasts.html', controller: 'PodcastCtrl' }) + .when('/archive', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) + .when('/archive/:artist', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) + .when('/archive/:artist/:album', { templateUrl: 'archive/archive.html', controller: 'ArchiveCtrl' }) + .otherwise({ redirectTo: '/index' }); +}]) + +.config(['$httpProvider',function($httpProvider) { + 'use strict'; + + $httpProvider.interceptors.push(['$rootScope', '$location', '$q', 'globals', function ($rootScope, $location, $q, globals) { + return { + 'request': function (request) { + // if we're not logged-in to the AngularJS app, redirect to login page + //$rootScope.loggedIn = $rootScope.loggedIn || globals.settings.Username; + $rootScope.loggedIn = false; + if (globals.settings.Username != "" && globals.settings.Password != "" && globals.settings.Server != "") { + $rootScope.loggedIn = true; + } + var path = ''; + path = $location.path(); + if (globals.settings.Debug) { console.log('Logged In: ' + $rootScope.loggedIn); } + if (globals.settings.Debug) { console.log('Current Path: ' + path); } + if (!$rootScope.loggedIn && path != '/settings' && path.search('archive') < 0) { + $location.path('/settings'); + } + return request; + }, + 'responseError': function (rejection) { + // if we're not logged-in to the web service, redirect to login page + if (rejection.status === 401 && $location.path() != '/settings') { + $rootScope.loggedIn = false; + $location.path('/settings'); + } + return $q.reject(rejection); + } + }; + }]); +}]); diff --git a/app/player/player-service.js b/app/player/player-service.js index 467f6ce..14d70cf 100644 --- a/app/player/player-service.js +++ b/app/player/player-service.js @@ -9,7 +9,7 @@ angular.module('jamstash.player.service', ['jamstash.utils', 'jamstash.settings' var player2 = '#playdeck_2'; var scrobbled = false; var timerid = 0; - $rootScope.defaultPlay = function (data, event) { + this.defaultPlay = function (data, event) { if (typeof $(player1).data("jPlayer") == 'undefined') { $rootScope.nextTrack(); } @@ -27,7 +27,7 @@ angular.module('jamstash.player.service', ['jamstash.utils', 'jamstash.settings' $rootScope.playSong(false, next); } }; - $rootScope.previousTrack = function () { + this.previousTrack = function () { var next = getNextSong(true); if (next) { $rootScope.playSong(false, next); @@ -176,11 +176,11 @@ angular.module('jamstash.player.service', ['jamstash.utils', 'jamstash.settings' if (globals.settings.Debug) { console.log('HTML5::loadStorage not supported on your browser, ' + html.length + ' characters'); } } }; - $rootScope.restartSong = function (loadonly, data) { + this.restartSong = function (loadonly, data) { var audio = $(player1).data("jPlayer"); audio.play(0); }; - $rootScope.playSong = function (loadonly, data) { + this.playSong = function (loadonly, data) { if (globals.settings.Debug) { console.log('Play: ' + JSON.stringify(data, null, 2)); } angular.forEach($rootScope.queue, function(item, key) { item.playing = false; @@ -413,4 +413,12 @@ angular.module('jamstash.player.service', ['jamstash.utils', 'jamstash.settings' } }); }; + + // TODO: Hyz: Those are temporary. Remove them when no one else is calling them. + // The idea is to have them so while refactoring so we don't break anything + $rootScope.defaultPlay = this.defaultPlay; + $rootScope.nextTrack = this.nextTrack; + $rootScope.previousTrack = this.previousTrack; + $rootScope.restartSong = this.restartSong; + $rootScope.playSong = this.playSong; }]); diff --git a/app/player/player-service_test.js b/app/player/player-service_test.js index 0327fd5..0459ed8 100644 --- a/app/player/player-service_test.js +++ b/app/player/player-service_test.js @@ -11,9 +11,10 @@ describe("Player service", function() { }); }); - describe("nextTrack -", function() { + describe("Given that I have 3 songs in my playing queue", function() { - it("Given that I have 3 songs in my playing queue and no song is playing, it plays the first song", function() { + var stubPlaySong, stubRestartSong; + beforeEach(function() { $rootScope.queue = [ { id: 6726, @@ -32,10 +33,56 @@ describe("Player service", function() { album: 'redux' } ]; + stubPlaySong = spyOn($rootScope, "playSong").and.stub(); + stubRestartSong = spyOn($rootScope, "restartSong").and.stub(); + }); - player.nextTrack(); + describe("when I call nextTrack", function() { + it("and no song is playing, it plays the first song", function() { + player.nextTrack(); - expect($rootScope.queue[0].playing).toBeTruthy(); + expect(stubPlaySong).toHaveBeenCalled(); + }); + + it("and the first song is playing, it plays the second song", function() { + $rootScope.queue[0].playing = true; + + player.nextTrack(); + + expect(stubPlaySong).toHaveBeenCalled(); + }); + + it("and the last song is playing, it does nothing", function() { + $rootScope.queue[2].playing = true; + + player.nextTrack(); + + expect(stubPlaySong).not.toHaveBeenCalled(); + }); + }); + + describe("when I call previousTrack", function() { + it("and no song is playing, it plays the first song", function() { + player.previousTrack(); + + expect(stubRestartSong).toHaveBeenCalled(); + }); + + it("and the first song is playing, it restarts the first song", function() { + $rootScope.queue[0].playing = true; + + player.previousTrack(); + + expect(stubRestartSong).toHaveBeenCalled(); + }); + + it("and the last song is playing, it plays the seconde song", function() { + $rootScope.queue[2].playing = true; + + player.previousTrack(); + + expect(stubPlaySong).toHaveBeenCalled(); + }); }); }); }); diff --git a/app/player/player.html b/app/player/player.html index d72ff32..96e50aa 100644 --- a/app/player/player.html +++ b/app/player/player.html @@ -18,7 +18,7 @@