From cf306f4aba051c0a8011dc03b047993297cef8e6 Mon Sep 17 00:00:00 2001 From: Marcelo Castagna Date: Sun, 12 Feb 2017 16:20:16 -0300 Subject: [PATCH] Throw exception with error message return by diskstation (#1672) --- .../DownloadStationFixture.cs | 14 ++-- .../DownloadStation/DownloadStation.cs | 51 ++++++------ .../DownloadStation/Proxies/DSMInfoProxy.cs | 10 +-- .../Proxies/DiskStationProxyBase.cs | 83 ++++++++++--------- .../Proxies/DownloadStationProxy.cs | 66 +++++---------- .../Proxies/FileStationProxy.cs | 9 +- 6 files changed, 101 insertions(+), 132 deletions(-) diff --git a/src/NzbDrone.Core.Test/Download/DownloadClientTests/DownloadStationTests/DownloadStationFixture.cs b/src/NzbDrone.Core.Test/Download/DownloadClientTests/DownloadStationTests/DownloadStationFixture.cs index 7a329805c..03887e9f7 100644 --- a/src/NzbDrone.Core.Test/Download/DownloadClientTests/DownloadStationTests/DownloadStationFixture.cs +++ b/src/NzbDrone.Core.Test/Download/DownloadClientTests/DownloadStationTests/DownloadStationFixture.cs @@ -331,13 +331,11 @@ namespace NzbDrone.Core.Test.Download.DownloadClientTests.DownloadStationTests .Returns(r => new HttpResponse(r, new HttpHeader(), new byte[1000])); Mocker.GetMock() - .Setup(s => s.AddTorrentFromUrl(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(true) + .Setup(s => s.AddTorrentFromUrl(It.IsAny(), It.IsAny(), It.IsAny())) .Callback(PrepareClientToReturnQueuedItem); Mocker.GetMock() - .Setup(s => s.AddTorrentFromData(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(true) + .Setup(s => s.AddTorrentFromData(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Callback(PrepareClientToReturnQueuedItem); } @@ -467,7 +465,7 @@ namespace NzbDrone.Core.Test.Download.DownloadClientTests.DownloadStationTests } [Test] - public void GetItems_should_set_outputhPath_to_base_folder_when_single_file_non_finished_torrent() + public void GetItems_should_set_outputPath_to_base_folder_when_single_file_non_finished_torrent() { GivenSerialNumber(); GivenSharedFolder(); @@ -481,7 +479,7 @@ namespace NzbDrone.Core.Test.Download.DownloadClientTests.DownloadStationTests } [Test] - public void GetItems_should_set_outputhPath_to_torrent_folder_when_multiple_files_non_finished_torrent() + public void GetItems_should_set_outputPath_to_torrent_folder_when_multiple_files_non_finished_torrent() { GivenSerialNumber(); GivenSharedFolder(); @@ -495,7 +493,7 @@ namespace NzbDrone.Core.Test.Download.DownloadClientTests.DownloadStationTests } [Test] - public void GetItems_should_set_outputhPath_to_base_folder_when_single_file_finished_torrent() + public void GetItems_should_set_outputPath_to_base_folder_when_single_file_finished_torrent() { GivenSerialNumber(); GivenSharedFolder(); @@ -509,7 +507,7 @@ namespace NzbDrone.Core.Test.Download.DownloadClientTests.DownloadStationTests } [Test] - public void GetItems_should_set_outputhPath_to_torrent_folder_when_multiple_files_finished_torrent() + public void GetItems_should_set_outputPath_to_torrent_folder_when_multiple_files_finished_torrent() { GivenSerialNumber(); GivenSharedFolder(); diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/DownloadStation.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/DownloadStation.cs index dd72b3ccd..7b9c45759 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/DownloadStation.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/DownloadStation.cs @@ -117,13 +117,16 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation public override void RemoveItem(string downloadId, bool deleteData) { - if (_proxy.RemoveTorrent(ParseDownloadId(downloadId), deleteData, Settings)) + try { + _proxy.RemoveTorrent(ParseDownloadId(downloadId), deleteData, Settings); _logger.Debug("{0} removed correctly", downloadId); return; } - - _logger.Error("Failed to remove {0}", downloadId); + catch (DownloadClientException e) + { + _logger.Error(e); + } } protected OsPath GetOutputPath(OsPath outputPath, DownloadStationTorrent torrent, string serialNumber) @@ -141,19 +144,18 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation { var hashedSerialNumber = _serialNumberProvider.GetSerialNumber(Settings); - if (_proxy.AddTorrentFromUrl(magnetLink, GetDownloadDirectory(), Settings)) + _proxy.AddTorrentFromUrl(magnetLink, GetDownloadDirectory(), Settings); + + var item = _proxy.GetTorrents(Settings).SingleOrDefault(t => t.Additional.Detail["uri"] == magnetLink); + + if (item != null) { - var item = _proxy.GetTorrents(Settings).Where(t => t.Additional.Detail["uri"] == magnetLink).SingleOrDefault(); - - if (item != null) - { - _logger.Debug("{0} added correctly", remoteEpisode); - return CreateDownloadId(item.Id, hashedSerialNumber); - } - - _logger.Debug("No such task {0} in Download Station", magnetLink); + _logger.Debug("{0} added correctly", remoteEpisode); + return CreateDownloadId(item.Id, hashedSerialNumber); } + _logger.Debug("No such task {0} in Download Station", magnetLink); + throw new DownloadClientException("Failed to add magnet task to Download Station"); } @@ -161,21 +163,20 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation { var hashedSerialNumber = _serialNumberProvider.GetSerialNumber(Settings); - if (_proxy.AddTorrentFromData(fileContent, filename, GetDownloadDirectory(), Settings)) + _proxy.AddTorrentFromData(fileContent, filename, GetDownloadDirectory(), Settings); + + var items = _proxy.GetTorrents(Settings).Where(t => t.Additional.Detail["uri"] == Path.GetFileNameWithoutExtension(filename)); + + var item = items.SingleOrDefault(); + + if (item != null) { - var items = _proxy.GetTorrents(Settings).Where(t => t.Additional.Detail["uri"] == Path.GetFileNameWithoutExtension(filename)); - - var item = items.SingleOrDefault(); - - if (item != null) - { - _logger.Debug("{0} added correctly", remoteEpisode); - return CreateDownloadId(item.Id, hashedSerialNumber); - } - - _logger.Debug("No such task {0} in Download Station", filename); + _logger.Debug("{0} added correctly", remoteEpisode); + return CreateDownloadId(item.Id, hashedSerialNumber); } + _logger.Debug("No such task {0} in Download Station", filename); + throw new DownloadClientException("Failed to add torrent task to Download Station"); } diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DSMInfoProxy.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DSMInfoProxy.cs index 86a2aaf39..b5687a0e0 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DSMInfoProxy.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DSMInfoProxy.cs @@ -26,14 +26,8 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "method", "getinfo" } }; - var response = ProcessRequest(DiskStationApi.DSMInfo, arguments, settings); - - if (response.Success == true) - { - return response.Data.SerialNumber; - } - _logger.Debug("Failed to get Download Station serial number"); - throw new DownloadClientException("Failed to get Download Station serial number"); + var response = ProcessRequest(DiskStationApi.DSMInfo, arguments, settings, "get serial number"); + return response.Data.SerialNumber; } } } diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DiskStationProxyBase.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DiskStationProxyBase.cs index 3d52bf39c..7a6538970 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DiskStationProxyBase.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DiskStationProxyBase.cs @@ -34,14 +34,16 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies protected DiskStationResponse ProcessRequest(DiskStationApi api, Dictionary arguments, DownloadStationSettings settings, + string operation, HttpMethod method = HttpMethod.GET) { - return ProcessRequest(api, arguments, settings, method); + return ProcessRequest(api, arguments, settings, operation, method); } protected DiskStationResponse ProcessRequest(DiskStationApi api, Dictionary arguments, DownloadStationSettings settings, + string operation, HttpMethod method = HttpMethod.GET, int retries = 0) where T : new() { @@ -58,18 +60,28 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies var request = BuildRequest(settings, api, arguments, method); var response = _httpClient.Execute(request); + _logger.Debug("Trying to {0}", operation); + if (response.StatusCode == HttpStatusCode.OK) { var responseContent = Json.Deserialize>(response.Content); - if (!responseContent.Success && responseContent.Error.SessionError) + if (responseContent.Success) { - _authenticated = false; - return ProcessRequest(api, arguments, settings, method, retries++); + return responseContent; } else { - return responseContent; + if (responseContent.Error.SessionError) + { + _authenticated = false; + return ProcessRequest(api, arguments, settings, operation, method, retries++); + } + + var msg = $"Failed to {operation}. Reason: {responseContent.Error.GetMessage(api)}"; + _logger.Error(msg); + + throw new DownloadClientException(msg); } } else @@ -166,42 +178,35 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "query", "SYNO.API.Auth, SYNO.DownloadStation.Info, SYNO.DownloadStation.Task, SYNO.FileStation.List, SYNO.DSM.Info" }, }; - var infoResponse = ProcessRequest(DiskStationApi.Info, arguments, settings); + var infoResponse = ProcessRequest(DiskStationApi.Info, arguments, settings, "Get api version"); - if (infoResponse.Success == true) + //TODO: Refactor this into more elegant code + var infoResponeDSAuth = infoResponse.Data["SYNO.API.Auth"]; + var infoResponeDSInfo = infoResponse.Data["SYNO.DownloadStation.Info"]; + var infoResponeDSTask = infoResponse.Data["SYNO.DownloadStation.Task"]; + var infoResponseFSList = infoResponse.Data["SYNO.FileStation.List"]; + var infoResponseDSMInfo = infoResponse.Data["SYNO.DSM.Info"]; + + Resources[DiskStationApi.Auth] = infoResponeDSAuth.Path; + Resources[DiskStationApi.DownloadStationInfo] = infoResponeDSInfo.Path; + Resources[DiskStationApi.DownloadStationTask] = infoResponeDSTask.Path; + Resources[DiskStationApi.FileStationList] = infoResponseFSList.Path; + Resources[DiskStationApi.DSMInfo] = infoResponseDSMInfo.Path; + + switch (api) { - //TODO: Refactor this into more elegant code - var infoResponeDSAuth = infoResponse.Data["SYNO.API.Auth"]; - var infoResponeDSInfo = infoResponse.Data["SYNO.DownloadStation.Info"]; - var infoResponeDSTask = infoResponse.Data["SYNO.DownloadStation.Task"]; - var infoResponseFSList = infoResponse.Data["SYNO.FileStation.List"]; - var infoResponseDSMInfo = infoResponse.Data["SYNO.DSM.Info"]; - - Resources[DiskStationApi.Auth] = infoResponeDSAuth.Path; - Resources[DiskStationApi.DownloadStationInfo] = infoResponeDSInfo.Path; - Resources[DiskStationApi.DownloadStationTask] = infoResponeDSTask.Path; - Resources[DiskStationApi.FileStationList] = infoResponseFSList.Path; - Resources[DiskStationApi.DSMInfo] = infoResponseDSMInfo.Path; - - switch (api) - { - case DiskStationApi.Auth: - return Enumerable.Range(infoResponeDSAuth.MinVersion, infoResponeDSAuth.MaxVersion - infoResponeDSAuth.MinVersion + 1); - case DiskStationApi.DownloadStationInfo: - return Enumerable.Range(infoResponeDSInfo.MinVersion, infoResponeDSInfo.MaxVersion - infoResponeDSInfo.MinVersion + 1); - case DiskStationApi.DownloadStationTask: - return Enumerable.Range(infoResponeDSTask.MinVersion, infoResponeDSTask.MaxVersion - infoResponeDSTask.MinVersion + 1); - case DiskStationApi.FileStationList: - return Enumerable.Range(infoResponseFSList.MinVersion, infoResponseFSList.MaxVersion - infoResponseFSList.MinVersion + 1); - case DiskStationApi.DSMInfo: - return Enumerable.Range(infoResponseDSMInfo.MinVersion, infoResponseDSMInfo.MaxVersion - infoResponseDSMInfo.MinVersion + 1); - default: - throw new DownloadClientException("Api not implemented"); - } - } - else - { - throw new DownloadClientException(infoResponse.Error.GetMessage(DiskStationApi.Info)); + case DiskStationApi.Auth: + return Enumerable.Range(infoResponeDSAuth.MinVersion, infoResponeDSAuth.MaxVersion - infoResponeDSAuth.MinVersion + 1); + case DiskStationApi.DownloadStationInfo: + return Enumerable.Range(infoResponeDSInfo.MinVersion, infoResponeDSInfo.MaxVersion - infoResponeDSInfo.MinVersion + 1); + case DiskStationApi.DownloadStationTask: + return Enumerable.Range(infoResponeDSTask.MinVersion, infoResponeDSTask.MaxVersion - infoResponeDSTask.MinVersion + 1); + case DiskStationApi.FileStationList: + return Enumerable.Range(infoResponseFSList.MinVersion, infoResponseFSList.MaxVersion - infoResponseFSList.MinVersion + 1); + case DiskStationApi.DSMInfo: + return Enumerable.Range(infoResponseDSMInfo.MinVersion, infoResponseDSMInfo.MaxVersion - infoResponseDSMInfo.MinVersion + 1); + default: + throw new DownloadClientException("Api not implemented"); } } } diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DownloadStationProxy.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DownloadStationProxy.cs index 1270fcfd0..1a6905d0c 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DownloadStationProxy.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/DownloadStationProxy.cs @@ -12,9 +12,9 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { IEnumerable GetTorrents(DownloadStationSettings settings); Dictionary GetConfig(DownloadStationSettings settings); - bool RemoveTorrent(string downloadId, bool deleteData, DownloadStationSettings settings); - bool AddTorrentFromUrl(string url, string downloadDirectory, DownloadStationSettings settings); - bool AddTorrentFromData(byte[] torrentData, string filename, string downloadDirectory, DownloadStationSettings settings); + void RemoveTorrent(string downloadId, bool deleteData, DownloadStationSettings settings); + void AddTorrentFromUrl(string url, string downloadDirectory, DownloadStationSettings settings); + void AddTorrentFromData(byte[] torrentData, string filename, string downloadDirectory, DownloadStationSettings settings); IEnumerable GetApiVersion(DownloadStationSettings settings); } @@ -25,7 +25,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { } - public bool AddTorrentFromData(byte[] torrentData, string filename, string downloadDirectory, DownloadStationSettings settings) + public void AddTorrentFromData(byte[] torrentData, string filename, string downloadDirectory, DownloadStationSettings settings) { var arguments = new Dictionary { @@ -40,13 +40,11 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies } arguments.Add("file", new Dictionary() { { "name", filename }, { "data", torrentData } }); - - var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, HttpMethod.POST); - - return response.Success; + + var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, $"add torrent from data {filename}", HttpMethod.POST); } - public bool AddTorrentFromUrl(string torrentUrl, string downloadDirectory, DownloadStationSettings settings) + public void AddTorrentFromUrl(string torrentUrl, string downloadDirectory, DownloadStationSettings settings) { var arguments = new Dictionary { @@ -61,9 +59,7 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies arguments.Add("destination", downloadDirectory); } - var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, HttpMethod.GET); - - return response.Success; + var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, $"add torrent from url {torrentUrl}", HttpMethod.GET); } public IEnumerable GetTorrents(DownloadStationSettings settings) @@ -76,14 +72,17 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "additional", "detail,transfer" } }; - var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings); - - if (response.Success) + try { + var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, "get torrents"); + return response.Data.Tasks.Where(t => t.Type == DownloadStationTaskType.BT); } - - return new List(); + catch (DownloadClientException e) + { + _logger.Error(e); + return new List(); + } } public Dictionary GetConfig(DownloadStationSettings settings) @@ -95,20 +94,12 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "method", "getconfig" } }; - try - { - var response = ProcessRequest>(DiskStationApi.DownloadStationInfo, arguments, settings); - return response.Data; - } - catch (Exception ex) - { - _logger.Error(ex, "Failed to get config from Download Station"); + var response = ProcessRequest>(DiskStationApi.DownloadStationInfo, arguments, settings, "get config"); - throw; - } + return response.Data; } - public bool RemoveTorrent(string downloadId, bool deleteData, DownloadStationSettings settings) + public void RemoveTorrent(string downloadId, bool deleteData, DownloadStationSettings settings) { var arguments = new Dictionary { @@ -119,23 +110,8 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "force_complete", false } }; - try - { - var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings); - - if (response.Success) - { - _logger.Trace("Item {0} removed from Download Station", downloadId); - } - - return response.Success; - } - catch (DownloadClientException e) - { - _logger.Debug(e, "Failed to remove item {0} from Download Station", downloadId); - - throw; - } + var response = ProcessRequest(DiskStationApi.DownloadStationTask, arguments, settings, $"remove item {downloadId}"); + _logger.Trace("Item {0} removed from Download Station", downloadId); } public IEnumerable GetApiVersion(DownloadStationSettings settings) diff --git a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/FileStationProxy.cs b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/FileStationProxy.cs index 4df24dedd..4beacd8e9 100644 --- a/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/FileStationProxy.cs +++ b/src/NzbDrone.Core/Download/Clients/DownloadStation/Proxies/FileStationProxy.cs @@ -47,14 +47,9 @@ namespace NzbDrone.Core.Download.Clients.DownloadStation.Proxies { "additional", $"[\"real_path\"]" } }; - var response = ProcessRequest(DiskStationApi.FileStationList, arguments, settings); + var response = ProcessRequest(DiskStationApi.FileStationList, arguments, settings, $"get info of {path}"); - if (response.Success == true) - { - return response.Data.Files.First(); - } - - throw new DownloadClientException($"Failed to get info of {0}", path); + return response.Data.Files.First(); } } }