From d45db2a98fd69d3fa381b0c6d9fcadb3eef86c70 Mon Sep 17 00:00:00 2001 From: Hyzual Date: Thu, 14 May 2015 23:01:29 +0200 Subject: [PATCH] Fix a bug that prevented you from changing your server If the new server had an older ApiVersion (e.g. 1.8.0 compared to 1.12.0), when saving the settings, the server would respond an error due to the api version and when dealing with that error, we didn't update the ApiVersion when we should have. --- app/settings/settings.js | 1 + app/settings/settings_test.js | 18 ++++++++++ app/subsonic/subsonic-service.js | 1 + app/subsonic/subsonic-service_test.js | 47 ++++++++++++++++++++------- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/app/settings/settings.js b/app/settings/settings.js index 30c2bec..0075aff 100644 --- a/app/settings/settings.js +++ b/app/settings/settings.js @@ -71,6 +71,7 @@ angular.module('jamstash.settings.controller', ['jamstash.settings.service', 'ja $rootScope.showIndex = true; }, function (error) { //TODO: Hyz: Duplicate from subsonic.js - requestSongs. Find a way to handle this only once. + globals.settings.ApiVersion = error.version; var errorNotif; if (error.subsonicError !== undefined) { errorNotif = error.reason + ' ' + error.subsonicError.message; diff --git a/app/settings/settings_test.js b/app/settings/settings_test.js index 3755ad8..e1839b3 100644 --- a/app/settings/settings_test.js +++ b/app/settings/settings_test.js @@ -90,6 +90,24 @@ describe("Settings controller", function() { expect(subsonic.ping).toHaveBeenCalled(); }); + + it("Given the server and Jamstash had different api versions, when I save the settings and the server responds an error, then the ApiVersion setting will be updated with the one sent from the server", function() { + scope.settings.Server = 'http://gallotannate.com/tetranychus/puzzlement?a=stoically&b=mantuamaker#marianolatrist'; + scope.settings.Username = 'Vandervelden'; + scope.settings.Password = 'PA3DhdfAu0dy'; + scope.ApiVersion = '1.10.2'; + subsonic.ping.and.returnValue(deferred.promise); + + scope.save(); + deferred.reject({ + reason: 'Error when contacting the Subsonic server.', + subsonicError: {code: 30, message: 'Incompatible Subsonic REST protocol version. Server must upgrade.'}, + version: '1.8.0' + }); + scope.$apply(); + + expect(mockGlobals.settings.ApiVersion).toEqual('1.8.0'); + }); }); it("reset() - When I reset the settings, they will be deleted from the persistence service and will be reloaded with default values", function() { diff --git a/app/subsonic/subsonic-service.js b/app/subsonic/subsonic-service.js index d1086a6..cc3b2e3 100644 --- a/app/subsonic/subsonic-service.js +++ b/app/subsonic/subsonic-service.js @@ -106,6 +106,7 @@ angular.module('jamstash.subsonic.service', ['angular-underscore/utils', } else { if(subsonicResponse.status === 'failed' && subsonicResponse.error !== undefined) { exception.subsonicError = subsonicResponse.error; + exception.version = subsonicResponse.version; } deferred.reject(exception); } diff --git a/app/subsonic/subsonic-service_test.js b/app/subsonic/subsonic-service_test.js index 6274d62..c008729 100644 --- a/app/subsonic/subsonic-service_test.js +++ b/app/subsonic/subsonic-service_test.js @@ -67,33 +67,58 @@ describe("Subsonic service -", function() { 'c=Jamstash&callback=JSON_CALLBACK&f=jsonp&p=enc:cGFzc3dvcmQ%3D&u=Hyzual&v=1.10.2'; }); - it("Given that the Subsonic server is not responding, when I make a request to Subsonic it returns an error object with a message", function() { + it("Given that the Subsonic server was not responding, when I make a request to Subsonic, then an error object with a message will be returned", function() { mockBackend.expectJSONP(url).respond(503, 'Service Unavailable'); var promise = subsonic.subsonicRequest(partialUrl); mockBackend.flush(); - expect(promise).toBeRejectedWith({reason: 'Error when contacting the Subsonic server.', httpError: 503}); + expect(promise).toBeRejectedWith({ + reason: 'Error when contacting the Subsonic server.', + httpError: 503 + }); }); - it("Given a missing parameter, when I make a request to Subsonic it returns an error object with a message", function() { + it("Given a missing parameter, when I make a request to Subsonic, then an error object with a message will be returned", function() { delete mockGlobals.settings.Password; var missingPasswordUrl = 'http://demo.subsonic.com/rest/getStarred.view?'+ 'c=Jamstash&callback=JSON_CALLBACK&f=jsonp&u=Hyzual&v=1.10.2'; - var errorResponse = {"subsonic-response" : { - "status" : "failed", - "version" : "1.10.2", - "error" : {"code" : 10,"message" : "Required parameter is missing."} + var errorResponse = {'subsonic-response' : { + status : 'failed', + version : '1.10.2', + error : {code : 10, message : 'Required parameter is missing.'} }}; mockBackend.expectJSONP(missingPasswordUrl).respond(JSON.stringify(errorResponse)); var promise = subsonic.subsonicRequest(partialUrl); mockBackend.flush(); - expect(promise).toBeRejectedWith({reason: 'Error when contacting the Subsonic server.', subsonicError: {code: 10, message:'Required parameter is missing.'}}); + expect(promise).toBeRejectedWith({ + reason: 'Error when contacting the Subsonic server.', + subsonicError: {code: 10, message:'Required parameter is missing.'}, + version: '1.10.2' + }); }); - it("Given a partialUrl that does not start with '/', it adds '/' before it and makes a correct request", function() { + it("Given that server and Jamstash had different api versions, when I make a request to Subsonic and the server responds an error, then it will return the server's api version in the error object", function() { + var errorResponse = {'subsonic-response': { + status: 'failed', + version: '1.8.0', + error: {code: 30, message: 'Incompatible Subsonic REST protocol version. Server must upgrade.'} + }}; + mockBackend.expectJSONP(url).respond(JSON.stringify(errorResponse)); + + var promise = subsonic.subsonicRequest(partialUrl); + mockBackend.flush(); + + expect(promise).toBeRejectedWith({ + reason: 'Error when contacting the Subsonic server.', + subsonicError: {code: 30, message: 'Incompatible Subsonic REST protocol version. Server must upgrade.'}, + version: '1.8.0' + }); + }); + + it("Given a partialUrl that did not start with '/', when I make a request to Subsonic, then a '/' will be added before the partial url", function() { partialUrl = 'getStarred.view'; mockBackend.expectJSONP(url).respond(JSON.stringify(response)); @@ -101,7 +126,7 @@ describe("Subsonic service -", function() { mockBackend.flush(); }); - it("Given $http config params, it does not overwrite them", function() { + it("Given $http config params, when I make a request to Subsonic, then the params won't be overwritten", function() { partialUrl = 'scrobble.view'; url ='http://demo.subsonic.com/rest/scrobble.view?'+ 'c=Jamstash&callback=JSON_CALLBACK&f=jsonp&id=75&p=enc:cGFzc3dvcmQ%3D&submission=false&u=Hyzual&v=1.10.2'; @@ -116,7 +141,7 @@ describe("Subsonic service -", function() { mockBackend.flush(); }); - it("Given that the global protocol setting is 'json', when I make a request to Subsonic it uses GET and does not use the JSON_CALLBACK parameter", function() { + it("Given that the global protocol setting was 'json', when I make a request to Subsonic, then it will use GET and won't use the JSON_CALLBACK parameter", function() { mockGlobals.settings.Protocol = 'json'; var getUrl = 'http://demo.subsonic.com/rest/getStarred.view?'+ 'c=Jamstash&f=json&p=enc:cGFzc3dvcmQ%3D&u=Hyzual&v=1.10.2';