From 104d35299bbda41da0b764db5bc26701cde61843 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 20 Jan 2015 22:20:39 +0100 Subject: [PATCH 01/13] Checks for update regardless of settings, but won't install it. --- src/NzbDrone.Core/Update/UpdateCheckService.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/NzbDrone.Core/Update/UpdateCheckService.cs b/src/NzbDrone.Core/Update/UpdateCheckService.cs index bbed226ac..16ffe17da 100644 --- a/src/NzbDrone.Core/Update/UpdateCheckService.cs +++ b/src/NzbDrone.Core/Update/UpdateCheckService.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using NLog; using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Instrumentation.Extensions; @@ -30,13 +31,13 @@ namespace NzbDrone.Core.Update public UpdatePackage AvailableUpdate() { + var latestAvailable = _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); + if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically) { return null; } - var latestAvailable = _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); - if (latestAvailable == null) { _logger.ProgressDebug("No update available."); @@ -46,7 +47,7 @@ namespace NzbDrone.Core.Update try { _logger.Info("Branch [{0}] is being redirected to [{1}]]", _configFileProvider.Branch, latestAvailable.Branch); - var config = _configFileProvider.GetConfigDictionary(); + var config = new Dictionary(); config["Branch"] = latestAvailable.Branch; _configFileProvider.SaveConfigDictionary(config); } From 11803afc3932e5cceef1bd3345f6389c440ab5a7 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 21 Jan 2015 20:59:03 +0100 Subject: [PATCH 02/13] Added FolderWritable to DiskProvider to centralize the check. --- .../DiskTests/DiskProviderFixtureBase.cs | 34 ++++++++++++++++++- src/NzbDrone.Common/Disk/DiskProviderBase.cs | 19 +++++++++++ src/NzbDrone.Common/Disk/IDiskProvider.cs | 1 + .../Download/Clients/Pneumatic/Pneumatic.cs | 26 ++------------ .../Download/DownloadClientBase.cs | 10 ++---- .../HealthCheck/Checks/DroneFactoryCheck.cs | 8 +---- .../HealthCheck/Checks/UpdateCheck.cs | 8 +---- .../DiskProviderTests/DiskProviderFixture.cs | 19 ++++++++++- .../NzbDrone.Mono.Test.csproj | 4 +++ .../DiskProviderTests/DiskProviderFixture.cs | 26 +++++++++++++- 10 files changed, 106 insertions(+), 49 deletions(-) diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs index 3c98cdfe2..fc34df228 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs @@ -8,7 +8,7 @@ using NzbDrone.Test.Common; namespace NzbDrone.Common.Test.DiskTests { - public class DiskProviderFixtureBase : TestBase where TSubject : class, IDiskProvider + public abstract class DiskProviderFixtureBase : TestBase where TSubject : class, IDiskProvider { public DirectoryInfo GetFilledTempFolder() { @@ -46,6 +46,38 @@ namespace NzbDrone.Common.Test.DiskTests Subject.FolderExists(@"C:\ThisBetterNotExist\".AsOsAgnostic()).Should().BeFalse(); } + protected abstract void SetWritePermissions(string path, bool writable); + + [Test] + public void FolderWritable_should_return_true_for_writable_directory() + { + var tempFolder = GetTempFilePath(); + Directory.CreateDirectory(tempFolder); + + var result = Subject.FolderWritable(tempFolder); + + result.Should().BeTrue(); + } + + [Test] + public void FolderWritable_should_return_false_for_unwritable_directory() + { + var tempFolder = GetTempFilePath(); + Directory.CreateDirectory(tempFolder); + + SetWritePermissions(tempFolder, false); + try + { + var result = Subject.FolderWritable(tempFolder); + + result.Should().BeFalse(); + } + finally + { + SetWritePermissions(tempFolder, true); + } + } + [Test] public void MoveFile_should_overwrite_existing_file() { diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index c50801cc9..de3075901 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -109,6 +109,25 @@ namespace NzbDrone.Common.Disk } } + public bool FolderWritable(string path) + { + Ensure.That(path, () => path).IsValidPath(); + + try + { + var testPath = Path.Combine(path, "sonarr_write_test.txt"); + var testContent = string.Format("This file was created to verify if '{0}' is writable. It should've been automatically deleted. Feel free to delete it.", path); + File.WriteAllText(testPath, testContent); + File.Delete(testPath); + return true; + } + catch (Exception e) + { + Logger.Trace("Directory '{0}' isn't writable. {1}", path, e.Message); + return false; + } + } + public string[] GetDirectories(string path) { Ensure.That(path, () => path).IsValidPath(); diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index 445a97fed..7374080e5 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -19,6 +19,7 @@ namespace NzbDrone.Common.Disk bool FolderExists(string path); bool FileExists(string path); bool FileExists(string path, StringComparison stringComparison); + bool FolderWritable(string path); string[] GetDirectories(string path); string[] GetFiles(string path, SearchOption searchOption); long GetFolderSize(string path); diff --git a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs index 12cfaf82a..cbbf5f26b 100644 --- a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs +++ b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs @@ -122,30 +122,8 @@ namespace NzbDrone.Core.Download.Clients.Pneumatic protected override void Test(List failures) { - failures.AddIfNotNull(TestWrite(Settings.NzbFolder, "NzbFolder")); - failures.AddIfNotNull(TestWrite(Settings.StrmFolder, "StrmFolder")); - } - - private ValidationFailure TestWrite(String folder, String propertyName) - { - if (!_diskProvider.FolderExists(folder)) - { - return new ValidationFailure(propertyName, "Folder does not exist"); - } - - try - { - var testPath = Path.Combine(folder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception ex) - { - _logger.ErrorException(ex.Message, ex); - return new ValidationFailure(propertyName, "Unable to write to folder"); - } - - return null; + failures.AddIfNotNull(TestFolder(Settings.NzbFolder, "NzbFolder")); + failures.AddIfNotNull(TestFolder(Settings.StrmFolder, "StrmFolder")); } private String WriteStrmFile(String title, String nzbFile) diff --git a/src/NzbDrone.Core/Download/DownloadClientBase.cs b/src/NzbDrone.Core/Download/DownloadClientBase.cs index e492eed81..61de7769f 100644 --- a/src/NzbDrone.Core/Download/DownloadClientBase.cs +++ b/src/NzbDrone.Core/Download/DownloadClientBase.cs @@ -104,15 +104,9 @@ namespace NzbDrone.Core.Download if (mustBeWritable) { - try + if (!_diskProvider.FolderWritable(folder)) { - var testPath = Path.Combine(folder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception ex) - { - _logger.ErrorException(ex.Message, ex); + _logger.Error("Folder '{0}' is not writable.", folder); return new NzbDroneValidationFailure(propertyName, "Unable to write to folder") { DetailedDescription = "The folder you specified is not writable. Please verify the folder permissions for the user account that is used to execute NzbDrone." diff --git a/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs index 4ba86b8e0..0bc4f7255 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs @@ -31,13 +31,7 @@ namespace NzbDrone.Core.HealthCheck.Checks return new HealthCheck(GetType(), HealthCheckResult.Error, "Drone factory folder does not exist"); } - try - { - var testPath = Path.Combine(droneFactoryFolder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception) + if (!_diskProvider.FolderWritable(droneFactoryFolder)) { return new HealthCheck(GetType(), HealthCheckResult.Error, "Unable to write to drone factory folder"); } diff --git a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs index abeaa0137..c9c968a84 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs @@ -29,13 +29,7 @@ namespace NzbDrone.Core.HealthCheck.Checks { if (OsInfo.IsWindows || _configFileProvider.UpdateAutomatically) { - try - { - var testPath = Path.Combine(_appFolderInfo.StartUpFolder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception) + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { return new HealthCheck(GetType(), HealthCheckResult.Error, "Unable to update, running from write-protected folder"); } diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 130d72e92..5317ad891 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,5 @@ -using NUnit.Framework; +using Mono.Unix; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; namespace NzbDrone.Mono.Test.DiskProviderTests @@ -11,5 +12,21 @@ namespace NzbDrone.Mono.Test.DiskProviderTests { MonoOnly(); } + + protected override void SetWritePermissions(string path, bool writable) + { + // Remove Write permissions, we're still owner so we can clean it up, but we'll have to do that explicitly. + + var entry = UnixFileSystemInfo.GetFileSystemEntry(path); + + if (writable) + { + entry.FileAccessPermissions |= FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite; + } + else + { + entry.FileAccessPermissions &= ~(FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); + } + } } } diff --git a/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj b/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj index 2a4622534..8fa63c167 100644 --- a/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj +++ b/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj @@ -58,6 +58,10 @@ False ..\packages\FluentAssertions.3.2.1\lib\net40\FluentAssertions.Core.dll + + False + ..\Libraries\Mono.Posix.dll + False ..\packages\NUnit.2.6.3\lib\nunit.framework.dll diff --git a/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs index 88def60e3..c60052c72 100644 --- a/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,7 @@ -using NUnit.Framework; +using System.IO; +using System.Security.AccessControl; +using System.Security.Principal; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; namespace NzbDrone.Windows.Test.DiskProviderTests @@ -11,5 +14,26 @@ namespace NzbDrone.Windows.Test.DiskProviderTests { WindowsOnly(); } + + protected override void SetWritePermissions(string path, bool writable) + { + // Remove Write permissions, we're owner and have Delete permissions, so we can still clean it up. + + var owner = WindowsIdentity.GetCurrent().Owner; + var accessControlType = writable ? AccessControlType.Allow : AccessControlType.Deny; + + if (Directory.Exists(path)) + { + var ds = Directory.GetAccessControl(path); + ds.SetAccessRule(new FileSystemAccessRule(owner, FileSystemRights.Write, accessControlType)); + Directory.SetAccessControl(path, ds); + } + else + { + var fs = File.GetAccessControl(path); + fs.SetAccessRule(new FileSystemAccessRule(owner, FileSystemRights.Write, accessControlType)); + File.SetAccessControl(path, fs); + } + } } } From 3a938e18fa33971dad03cd9d7d232017f8bfaa42 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 21 Jan 2015 23:57:35 +0100 Subject: [PATCH 03/13] Fixed: Install Update UI should now report an error if the application folder is not writable instead of failing silently. --- .../Checks/DroneFactoryCheckFixture.cs | 12 +++---- .../HealthCheck/Checks/UpdateCheckFixture.cs | 12 +++---- .../UpdateTests/UpdateServiceFixture.cs | 35 +++++++++++++++++++ .../Download/DownloadClientBase.cs | 15 ++++---- .../HealthCheck/Checks/UpdateCheck.cs | 2 +- .../Update/InstallUpdateService.cs | 23 ++++++++++-- 6 files changed, 74 insertions(+), 25 deletions(-) diff --git a/src/NzbDrone.Core.Test/HealthCheck/Checks/DroneFactoryCheckFixture.cs b/src/NzbDrone.Core.Test/HealthCheck/Checks/DroneFactoryCheckFixture.cs index 3c363c658..06f01333f 100644 --- a/src/NzbDrone.Core.Test/HealthCheck/Checks/DroneFactoryCheckFixture.cs +++ b/src/NzbDrone.Core.Test/HealthCheck/Checks/DroneFactoryCheckFixture.cs @@ -13,7 +13,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks { private const string DRONE_FACTORY_FOLDER = @"C:\Test\Unsorted"; - private void GivenDroneFactoryFolder(bool exists = false) + private void GivenDroneFactoryFolder(bool exists = false, bool writable = true) { Mocker.GetMock() .SetupGet(s => s.DownloadedEpisodesFolder) @@ -22,6 +22,10 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks Mocker.GetMock() .Setup(s => s.FolderExists(DRONE_FACTORY_FOLDER)) .Returns(exists); + + Mocker.GetMock() + .Setup(s => s.FolderWritable(It.IsAny())) + .Returns(exists && writable); } [Test] @@ -35,11 +39,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks [Test] public void should_return_error_when_unable_to_write_to_drone_factory_folder() { - GivenDroneFactoryFolder(true); - - Mocker.GetMock() - .Setup(s => s.WriteAllText(It.IsAny(), It.IsAny())) - .Throws(); + GivenDroneFactoryFolder(true, false); Subject.Check().ShouldBeError(); } diff --git a/src/NzbDrone.Core.Test/HealthCheck/Checks/UpdateCheckFixture.cs b/src/NzbDrone.Core.Test/HealthCheck/Checks/UpdateCheckFixture.cs index 68123676a..5d66d3220 100644 --- a/src/NzbDrone.Core.Test/HealthCheck/Checks/UpdateCheckFixture.cs +++ b/src/NzbDrone.Core.Test/HealthCheck/Checks/UpdateCheckFixture.cs @@ -21,9 +21,9 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks .Setup(s => s.StartUpFolder) .Returns(@"C:\NzbDrone"); - Mocker.GetMock() - .Setup(s => s.WriteAllText(It.IsAny(), It.IsAny())) - .Throws(); + Mocker.GetMock() + .Setup(c => c.FolderWritable(Moq.It.IsAny())) + .Returns(false); Subject.Check().ShouldBeError(); } @@ -41,9 +41,9 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks .Setup(s => s.StartUpFolder) .Returns(@"/opt/nzbdrone"); - Mocker.GetMock() - .Setup(s => s.WriteAllText(It.IsAny(), It.IsAny())) - .Throws(); + Mocker.GetMock() + .Setup(c => c.FolderWritable(Moq.It.IsAny())) + .Returns(false); Subject.Check().ShouldBeError(); } diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index 6fbd83684..a8175265d 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs @@ -11,6 +11,7 @@ using NzbDrone.Common.Http; using NzbDrone.Common.Model; using NzbDrone.Common.Processes; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Exceptions; using NzbDrone.Core.Test.Framework; using NzbDrone.Core.Update; using NzbDrone.Core.Update.Commands; @@ -59,6 +60,10 @@ namespace NzbDrone.Core.Test.UpdateTests Mocker.GetMock().Setup(c => c.GetCurrentProcess()).Returns(new ProcessInfo { Id = 12 }); Mocker.GetMock().Setup(c => c.ExecutingApplication).Returns(@"C:\Test\NzbDrone.exe"); + Mocker.GetMock() + .Setup(c => c.FolderWritable(It.IsAny())) + .Returns(true); + _sandboxFolder = Mocker.GetMock().Object.GetUpdateSandboxFolder(); } @@ -259,6 +264,36 @@ namespace NzbDrone.Core.Test.UpdateTests ExceptionVerification.ExpectedErrors(1); } + [Test] + public void should_log_error_when_startup_folder_is_not_writable() + { + Mocker.GetMock() + .Setup(c => c.FolderWritable(It.IsAny())) + .Returns(false); + + var updateArchive = Path.Combine(_sandboxFolder, _updatePackage.FileName); + + Subject.Execute(new ApplicationUpdateCommand()); + + Mocker.GetMock().Verify(c => c.DownloadFile(_updatePackage.Url, updateArchive), Times.Never()); + ExceptionVerification.ExpectedErrors(1); + } + + [Test] + public void should_throw_when_install_cannot_be_started() + { + Mocker.GetMock() + .Setup(c => c.FolderWritable(It.IsAny())) + .Returns(false); + + var updateArchive = Path.Combine(_sandboxFolder, _updatePackage.FileName); + + Assert.Throws(() => Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage })); + + Mocker.GetMock().Verify(c => c.DownloadFile(_updatePackage.Url, updateArchive), Times.Never()); + ExceptionVerification.ExpectedErrors(1); + } + [TearDown] public void TearDown() { diff --git a/src/NzbDrone.Core/Download/DownloadClientBase.cs b/src/NzbDrone.Core/Download/DownloadClientBase.cs index 61de7769f..89c70c585 100644 --- a/src/NzbDrone.Core/Download/DownloadClientBase.cs +++ b/src/NzbDrone.Core/Download/DownloadClientBase.cs @@ -98,20 +98,17 @@ namespace NzbDrone.Core.Download { return new NzbDroneValidationFailure(propertyName, "Folder does not exist") { - DetailedDescription = "The folder you specified does not exist or is inaccessible. Please verify the folder permissions for the user account that is used to execute NzbDrone." + DetailedDescription = string.Format("The folder you specified does not exist or is inaccessible. Please verify the folder permissions for the user account '{0}', which is used to execute Sonarr.", Environment.UserName) }; } - if (mustBeWritable) + if (mustBeWritable && !_diskProvider.FolderWritable(folder)) { - if (!_diskProvider.FolderWritable(folder)) + _logger.Error("Folder '{0}' is not writable.", folder); + return new NzbDroneValidationFailure(propertyName, "Unable to write to folder") { - _logger.Error("Folder '{0}' is not writable.", folder); - return new NzbDroneValidationFailure(propertyName, "Unable to write to folder") - { - DetailedDescription = "The folder you specified is not writable. Please verify the folder permissions for the user account that is used to execute NzbDrone." - }; - } + DetailedDescription = string.Format("The folder you specified is not writable. Please verify the folder permissions for the user account '{0}', which is used to execute Sonarr.", Environment.UserName) + }; } return null; diff --git a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs index c9c968a84..5dfd56841 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs @@ -31,7 +31,7 @@ namespace NzbDrone.Core.HealthCheck.Checks { if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { - return new HealthCheck(GetType(), HealthCheckResult.Error, "Unable to update, running from write-protected folder"); + return new HealthCheck(GetType(), HealthCheckResult.Error, string.Format("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName)); } } diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index bdaedbb3a..af6ab6c6e 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -10,6 +10,7 @@ using NzbDrone.Common.Instrumentation.Extensions; using NzbDrone.Common.Processes; using NzbDrone.Core.Backup; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Exceptions; using NzbDrone.Core.Messaging.Commands; using NzbDrone.Core.Update.Commands; @@ -61,12 +62,20 @@ namespace NzbDrone.Core.Update _logger = logger; } - private void InstallUpdate(UpdatePackage updatePackage) + private bool InstallUpdate(UpdatePackage updatePackage) { try { EnsureAppDataSafety(); + if (OsInfo.IsWindows || _configFileProvider.UpdateMechanism != UpdateMechanism.Script) + { + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) + { + throw new ApplicationException(string.Format("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName)); + } + } + var updateSandboxFolder = _appFolderInfo.GetUpdateSandboxFolder(); var packageDestination = Path.Combine(updateSandboxFolder, updatePackage.FileName); @@ -100,7 +109,7 @@ namespace NzbDrone.Core.Update if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) { InstallUpdateWithScript(updateSandboxFolder); - return; + return true; } _logger.Info("Preparing client"); @@ -111,10 +120,13 @@ namespace NzbDrone.Core.Update _logger.ProgressInfo("NzbDrone will restart shortly."); _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); + + return true; } catch (Exception ex) { _logger.ErrorException("Update process failed", ex); + return false; } } @@ -170,7 +182,12 @@ namespace NzbDrone.Core.Update public void Execute(InstallUpdateCommand message) { - InstallUpdate(message.UpdatePackage); + var success = InstallUpdate(message.UpdatePackage); + + if (!success) + { + throw new NzbDroneClientException(System.Net.HttpStatusCode.Conflict, "Failed to install update"); + } } } } From 6e179839d9741bdd5a02f4085085ba6296847df6 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Thu, 22 Jan 2015 19:46:55 +0100 Subject: [PATCH 04/13] Added test to check Config behavior. --- .../ConfigFileProviderTest.cs | 47 +++++++++++++++++-- .../Configuration/ConfigFileProvider.cs | 2 +- src/NzbDrone.Test.Common/TestBase.cs | 1 + 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/NzbDrone.Common.Test/ConfigFileProviderTest.cs b/src/NzbDrone.Common.Test/ConfigFileProviderTest.cs index e3a937f4a..76383d88e 100644 --- a/src/NzbDrone.Common.Test/ConfigFileProviderTest.cs +++ b/src/NzbDrone.Common.Test/ConfigFileProviderTest.cs @@ -1,6 +1,9 @@ -using System.IO; +using System.Collections.Generic; +using System.IO; using FluentAssertions; +using Moq; using NUnit.Framework; +using NzbDrone.Common.Disk; using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Extensions; using NzbDrone.Core.Authentication; @@ -13,6 +16,8 @@ namespace NzbDrone.Common.Test public class ConfigFileProviderTest : TestBase { + private string _configFileContents; + [SetUp] public void SetUp() { @@ -20,8 +25,24 @@ namespace NzbDrone.Common.Test var configFile = Mocker.Resolve().GetConfigPath(); - if (File.Exists(configFile)) - File.Delete(configFile); + _configFileContents = null; + + WithMockConfigFile(configFile); + } + + protected void WithMockConfigFile(string configFile) + { + Mocker.GetMock() + .Setup(v => v.FileExists(configFile)) + .Returns(p => _configFileContents != null); + + Mocker.GetMock() + .Setup(v => v.ReadAllText(configFile)) + .Returns(p => _configFileContents); + + Mocker.GetMock() + .Setup(v => v.WriteAllText(configFile, It.IsAny())) + .Callback((p, t) => _configFileContents = t); } [Test] @@ -142,8 +163,28 @@ namespace NzbDrone.Common.Test Subject.SaveConfigDictionary(dic); + Subject.Port.Should().Be(port); + } + + [Test] + public void SaveDictionary_should_only_save_specified_values() + { + int port = 20555; + int origSslPort = 20551; + int sslPort = 20552; + + var dic = Subject.GetConfigDictionary(); + dic["Port"] = port; + dic["SslPort"] = origSslPort; + Subject.SaveConfigDictionary(dic); + + + dic = new Dictionary(); + dic["SslPort"] = sslPort; + Subject.SaveConfigDictionary(dic); Subject.Port.Should().Be(port); + Subject.SslPort.Should().Be(sslPort); } } diff --git a/src/NzbDrone.Core/Configuration/ConfigFileProvider.cs b/src/NzbDrone.Core/Configuration/ConfigFileProvider.cs index 6c10e7d0c..962eaa638 100644 --- a/src/NzbDrone.Core/Configuration/ConfigFileProvider.cs +++ b/src/NzbDrone.Core/Configuration/ConfigFileProvider.cs @@ -339,7 +339,7 @@ namespace NzbDrone.Core.Configuration { if (_diskProvider.FileExists(_configFile)) { - return XDocument.Load(_configFile); + return XDocument.Parse(_diskProvider.ReadAllText(_configFile)); } var xDoc = new XDocument(new XDeclaration("1.0", "utf-8", "yes")); diff --git a/src/NzbDrone.Test.Common/TestBase.cs b/src/NzbDrone.Test.Common/TestBase.cs index dcaedfc98..20109e461 100644 --- a/src/NzbDrone.Test.Common/TestBase.cs +++ b/src/NzbDrone.Test.Common/TestBase.cs @@ -6,6 +6,7 @@ using Moq; using NLog; using NUnit.Framework; using NzbDrone.Common.Cache; +using NzbDrone.Common.Disk; using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Messaging; using NzbDrone.Core.Messaging.Events; From a058333f65503676af2d3dd2750a948b9854a9b0 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sun, 25 Jan 2015 21:10:27 +0100 Subject: [PATCH 05/13] Fixed: Manually triggering Check Health will now also run health checks that normally only run on startup. --- src/NzbDrone.Core/HealthCheck/HealthCheckService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs b/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs index 14369951f..5733c4f0a 100644 --- a/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs +++ b/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs @@ -94,7 +94,7 @@ namespace NzbDrone.Core.HealthCheck public void Execute(CheckHealthCommand message) { - PerformHealthCheck(c => c.CheckOnSchedule); + PerformHealthCheck(c => message.Manual || c.CheckOnSchedule); } } } From 8833f1ad31b9f88ab5733c307a4cf15b2f9716b0 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sun, 25 Jan 2015 21:10:49 +0100 Subject: [PATCH 06/13] Allow failing a Command using a specific message. --- src/NzbDrone.Core/Messaging/Commands/Command.cs | 8 ++++---- src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/NzbDrone.Core/Messaging/Commands/Command.cs b/src/NzbDrone.Core/Messaging/Commands/Command.cs index 699770242..7e7777049 100644 --- a/src/NzbDrone.Core/Messaging/Commands/Command.cs +++ b/src/NzbDrone.Core/Messaging/Commands/Command.cs @@ -60,21 +60,21 @@ namespace NzbDrone.Core.Messaging.Commands SetMessage("Starting"); } - public void Failed(Exception exception) + public void Failed(Exception exception, string message = "Failed") { _stopWatch.Stop(); StateChangeTime = DateTime.UtcNow; State = CommandStatus.Failed; Exception = exception; - SetMessage("Failed"); + SetMessage(message); } - public void Completed() + public void Completed(string message = "Completed") { _stopWatch.Stop(); StateChangeTime = DateTime.UtcNow; State = CommandStatus.Completed; - SetMessage("Completed"); + SetMessage(message); } public void SetMessage(string message) diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs b/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs index 2f3c72ee0..68ae2b1b4 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs @@ -129,7 +129,11 @@ namespace NzbDrone.Core.Messaging.Commands } handler.Execute((TCommand)command); - _trackCommands.Completed(command); + + if (command.State == CommandStatus.Running) + { + _trackCommands.Completed(command); + } } catch (Exception e) { From 7ce9f416d1200420727012f713bd2f2b67e9b3b3 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sun, 25 Jan 2015 21:55:16 +0100 Subject: [PATCH 07/13] InstallUpdate pre-check failures should now show a nice error on the UI. --- .../UpdateTests/UpdateServiceFixture.cs | 4 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 2 + .../Update/InstallUpdateService.cs | 129 ++++++++++-------- .../Update/UpdateAbortedException.cs | 17 +++ .../UpdateFolderNotWritableException.cs | 17 +++ .../UpdateVerificationFailedException.cs | 10 +- src/UI/Commands/CommandMessengerItemView.js | 5 +- 7 files changed, 121 insertions(+), 63 deletions(-) create mode 100644 src/NzbDrone.Core/Update/UpdateAbortedException.cs create mode 100644 src/NzbDrone.Core/Update/UpdateFolderNotWritableException.cs diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index a8175265d..29637d3cc 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs @@ -280,7 +280,7 @@ namespace NzbDrone.Core.Test.UpdateTests } [Test] - public void should_throw_when_install_cannot_be_started() + public void should_log_when_install_cannot_be_started() { Mocker.GetMock() .Setup(c => c.FolderWritable(It.IsAny())) @@ -288,7 +288,7 @@ namespace NzbDrone.Core.Test.UpdateTests var updateArchive = Path.Combine(_sandboxFolder, _updatePackage.FileName); - Assert.Throws(() => Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage })); + Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage }); Mocker.GetMock().Verify(c => c.DownloadFile(_updatePackage.Url, updateArchive), Times.Never()); ExceptionVerification.ExpectedErrors(1); diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 2c4a2f698..ae7c50e7f 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -855,8 +855,10 @@ + + diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index af6ab6c6e..7635fe88a 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; using NLog; using NzbDrone.Common; using NzbDrone.Common.Disk; @@ -10,13 +12,11 @@ using NzbDrone.Common.Instrumentation.Extensions; using NzbDrone.Common.Processes; using NzbDrone.Core.Backup; using NzbDrone.Core.Configuration; -using NzbDrone.Core.Exceptions; using NzbDrone.Core.Messaging.Commands; using NzbDrone.Core.Update.Commands; namespace NzbDrone.Core.Update { - public class InstallUpdateService : IExecute, IExecute { private readonly ICheckUpdateService _checkUpdateService; @@ -62,72 +62,62 @@ namespace NzbDrone.Core.Update _logger = logger; } - private bool InstallUpdate(UpdatePackage updatePackage) + private void InstallUpdate(UpdatePackage updatePackage) { - try - { - EnsureAppDataSafety(); + EnsureAppDataSafety(); - if (OsInfo.IsWindows || _configFileProvider.UpdateMechanism != UpdateMechanism.Script) + if (OsInfo.IsWindows || _configFileProvider.UpdateMechanism != UpdateMechanism.Script) + { + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { - if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) - { - throw new ApplicationException(string.Format("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName)); - } + throw new UpdateFolderNotWritableException("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName); } + } - var updateSandboxFolder = _appFolderInfo.GetUpdateSandboxFolder(); - - var packageDestination = Path.Combine(updateSandboxFolder, updatePackage.FileName); + var updateSandboxFolder = _appFolderInfo.GetUpdateSandboxFolder(); - if (_diskProvider.FolderExists(updateSandboxFolder)) - { - _logger.Info("Deleting old update files"); - _diskProvider.DeleteFolder(updateSandboxFolder, true); - } + var packageDestination = Path.Combine(updateSandboxFolder, updatePackage.FileName); - _logger.ProgressInfo("Downloading update {0}", updatePackage.Version); - _logger.Debug("Downloading update package from [{0}] to [{1}]", updatePackage.Url, packageDestination); - _httpClient.DownloadFile(updatePackage.Url, packageDestination); + if (_diskProvider.FolderExists(updateSandboxFolder)) + { + _logger.Info("Deleting old update files"); + _diskProvider.DeleteFolder(updateSandboxFolder, true); + } - _logger.ProgressInfo("Verifying update package"); + _logger.ProgressInfo("Downloading update {0}", updatePackage.Version); + _logger.Debug("Downloading update package from [{0}] to [{1}]", updatePackage.Url, packageDestination); + _httpClient.DownloadFile(updatePackage.Url, packageDestination); - if (!_updateVerifier.Verify(updatePackage, packageDestination)) - { - _logger.Error("Update package is invalid"); - throw new UpdateVerificationFailedException("Update file '{0}' is invalid", packageDestination); - } + _logger.ProgressInfo("Verifying update package"); - _logger.Info("Update package verified successfully"); + if (!_updateVerifier.Verify(updatePackage, packageDestination)) + { + _logger.Error("Update package is invalid"); + throw new UpdateVerificationFailedException("Update file '{0}' is invalid", packageDestination); + } - _logger.ProgressInfo("Extracting Update package"); - _archiveService.Extract(packageDestination, updateSandboxFolder); - _logger.Info("Update package extracted successfully"); + _logger.Info("Update package verified successfully"); - _backupService.Backup(BackupType.Update); + _logger.ProgressInfo("Extracting Update package"); + _archiveService.Extract(packageDestination, updateSandboxFolder); + _logger.Info("Update package extracted successfully"); - if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) - { - InstallUpdateWithScript(updateSandboxFolder); - return true; - } + _backupService.Backup(BackupType.Update); - _logger.Info("Preparing client"); - _diskProvider.MoveFolder(_appFolderInfo.GetUpdateClientFolder(), - updateSandboxFolder); + if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) + { + InstallUpdateWithScript(updateSandboxFolder); + return; + } - _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath()); - _logger.ProgressInfo("NzbDrone will restart shortly."); + _logger.Info("Preparing client"); + _diskProvider.MoveFolder(_appFolderInfo.GetUpdateClientFolder(), + updateSandboxFolder); - _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); + _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath()); + _logger.ProgressInfo("Sonarr will restart shortly."); - return true; - } - catch (Exception ex) - { - _logger.ErrorException("Update process failed", ex); - return false; - } + _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); } private void InstallUpdateWithScript(String updateSandboxFolder) @@ -165,7 +155,32 @@ namespace NzbDrone.Core.Update if (_appFolderInfo.StartUpFolder.IsParentPath(_appFolderInfo.AppDataFolder) || _appFolderInfo.StartUpFolder.PathEquals(_appFolderInfo.AppDataFolder)) { - throw new NotSupportedException("Update will cause AppData to be deleted, correct you configuration before proceeding"); + throw new UpdateFailedException("You Sonarr configuration ('{0}') is being stored in application folder ('{1}') which will cause data lost during the upgrade. Please remove any symlinks or redirects before trying again.", _appFolderInfo.AppDataFolder, _appFolderInfo.StartUpFolder); + } + } + + private void ExecuteInstallUpdate(Command message, UpdatePackage package) + { + try + { + InstallUpdate(package); + + message.Completed("Restarting Sonarr to apply updates"); + } + catch (UpdateFolderNotWritableException ex) + { + _logger.ErrorException("Update process failed", ex); + message.Failed(ex, string.Format("Startup folder not writable by user '{0}'", Environment.UserName)); + } + catch (UpdateVerificationFailedException ex) + { + _logger.ErrorException("Update process failed", ex); + message.Failed(ex, "Downloaded update package is corrupt"); + } + catch (UpdateFailedException ex) + { + _logger.ErrorException("Update process failed", ex); + message.Failed(ex); } } @@ -176,18 +191,20 @@ namespace NzbDrone.Core.Update if (latestAvailable != null) { - InstallUpdate(latestAvailable); + ExecuteInstallUpdate(message, latestAvailable); } } public void Execute(InstallUpdateCommand message) { - var success = InstallUpdate(message.UpdatePackage); + var latestAvailable = _checkUpdateService.AvailableUpdate(); - if (!success) + if (latestAvailable == null || latestAvailable.Hash != message.UpdatePackage.Hash) { - throw new NzbDroneClientException(System.Net.HttpStatusCode.Conflict, "Failed to install update"); + throw new ApplicationException("Unknown or invalid update specified"); } + + ExecuteInstallUpdate(message, latestAvailable); } } } diff --git a/src/NzbDrone.Core/Update/UpdateAbortedException.cs b/src/NzbDrone.Core/Update/UpdateAbortedException.cs new file mode 100644 index 000000000..f6561dcec --- /dev/null +++ b/src/NzbDrone.Core/Update/UpdateAbortedException.cs @@ -0,0 +1,17 @@ +using NzbDrone.Common.Exceptions; + +namespace NzbDrone.Core.Update +{ + public class UpdateFailedException : NzbDroneException + { + public UpdateFailedException(string message, params object[] args) + : base(message, args) + { + } + + public UpdateFailedException(string message) + : base(message) + { + } + } +} diff --git a/src/NzbDrone.Core/Update/UpdateFolderNotWritableException.cs b/src/NzbDrone.Core/Update/UpdateFolderNotWritableException.cs new file mode 100644 index 000000000..6d7ad9025 --- /dev/null +++ b/src/NzbDrone.Core/Update/UpdateFolderNotWritableException.cs @@ -0,0 +1,17 @@ +using NzbDrone.Common.Exceptions; + +namespace NzbDrone.Core.Update +{ + public class UpdateFolderNotWritableException : UpdateFailedException + { + public UpdateFolderNotWritableException(string message, params object[] args) + : base(message, args) + { + } + + public UpdateFolderNotWritableException(string message) + : base(message) + { + } + } +} diff --git a/src/NzbDrone.Core/Update/UpdateVerificationFailedException.cs b/src/NzbDrone.Core/Update/UpdateVerificationFailedException.cs index 56d9da122..d7116fcb3 100644 --- a/src/NzbDrone.Core/Update/UpdateVerificationFailedException.cs +++ b/src/NzbDrone.Core/Update/UpdateVerificationFailedException.cs @@ -2,14 +2,16 @@ namespace NzbDrone.Core.Update { - public class UpdateVerificationFailedException : NzbDroneException + public class UpdateVerificationFailedException : UpdateFailedException { - public UpdateVerificationFailedException(string message, params object[] args) : base(message, args) + public UpdateVerificationFailedException(string message, params object[] args) + : base(message, args) { } - public UpdateVerificationFailedException(string message) : base(message) + public UpdateVerificationFailedException(string message) + : base(message) { } } -} +} \ No newline at end of file diff --git a/src/UI/Commands/CommandMessengerItemView.js b/src/UI/Commands/CommandMessengerItemView.js index 1b56ebc78..f3f6effde 100644 --- a/src/UI/Commands/CommandMessengerItemView.js +++ b/src/UI/Commands/CommandMessengerItemView.js @@ -15,12 +15,15 @@ module.exports = Marionette.ItemView.extend({ id : this.model.id, hideAfter : 0 }; + + var isManual = this.model.get('manual'); + switch (this.model.get('state')) { case 'completed': message.hideAfter = 4; break; case 'failed': - message.hideAfter = 4; + message.hideAfter = isManual ? 10 : 4; message.type = 'error'; break; default: From 2f06cc6ffa7fdf90aeb692d56c19948df6319712 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sun, 25 Jan 2015 22:21:17 +0100 Subject: [PATCH 08/13] Fixed: Branch redirects will now occur during install of the a new update instead of during an update check. --- .../UpdateTests/UpdateServiceFixture.cs | 12 +++++++++++ .../Update/InstallUpdateService.cs | 21 +++++++++++++++++++ .../Update/UpdateCheckService.cs | 15 ------------- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index 29637d3cc..981fc894b 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using FluentAssertions; using Moq; @@ -294,6 +295,17 @@ namespace NzbDrone.Core.Test.UpdateTests ExceptionVerification.ExpectedErrors(1); } + [Test] + public void should_switch_to_branch_specified_in_updatepackage() + { + _updatePackage.Branch = "fake"; + + Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage }); + + Mocker.GetMock() + .Verify(v => v.SaveConfigDictionary(It.Is>(d => d.ContainsKey("Branch") && (string)d["Branch"] == "fake")), Times.Once()); + } + [TearDown] public void TearDown() { diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index 7635fe88a..300eb6f48 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -102,6 +102,8 @@ namespace NzbDrone.Core.Update _archiveService.Extract(packageDestination, updateSandboxFolder); _logger.Info("Update package extracted successfully"); + EnsureValidBranch(updatePackage); + _backupService.Backup(BackupType.Update); if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) @@ -120,6 +122,25 @@ namespace NzbDrone.Core.Update _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); } + private void EnsureValidBranch(UpdatePackage package) + { + var currentBranch = _configFileProvider.Branch; + if (package.Branch != currentBranch) + { + try + { + _logger.Info("Branch [{0}] is being redirected to [{1}]]", currentBranch, package.Branch); + var config = new Dictionary(); + config["Branch"] = package.Branch; + _configFileProvider.SaveConfigDictionary(config); + } + catch (Exception e) + { + _logger.ErrorException(string.Format("Couldn't change the branch from [{0}] to [{1}].", currentBranch, package.Branch), e); + } + } + } + private void InstallUpdateWithScript(String updateSandboxFolder) { var scriptPath = _configFileProvider.UpdateScriptPath; diff --git a/src/NzbDrone.Core/Update/UpdateCheckService.cs b/src/NzbDrone.Core/Update/UpdateCheckService.cs index 16ffe17da..f17b72dcc 100644 --- a/src/NzbDrone.Core/Update/UpdateCheckService.cs +++ b/src/NzbDrone.Core/Update/UpdateCheckService.cs @@ -42,21 +42,6 @@ namespace NzbDrone.Core.Update { _logger.ProgressDebug("No update available."); } - else if (latestAvailable.Branch != _configFileProvider.Branch) - { - try - { - _logger.Info("Branch [{0}] is being redirected to [{1}]]", _configFileProvider.Branch, latestAvailable.Branch); - var config = new Dictionary(); - config["Branch"] = latestAvailable.Branch; - _configFileProvider.SaveConfigDictionary(config); - } - catch (Exception e) - { - _logger.ErrorException("Couldn't save the branch redirect.", e); - } - - } return latestAvailable; } From 071839fa8656c54f0da41c54805803242e5b1391 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 27 Jan 2015 20:32:48 +0100 Subject: [PATCH 09/13] Removed InstallUpdate, instead manually triggering ApplicationUpdate. --- .../UpdateTests/UpdateServiceFixture.cs | 8 +++- src/NzbDrone.Core/NzbDrone.Core.csproj | 1 - .../Update/Commands/InstallUpdateCommand.cs | 17 ------- .../Update/InstallUpdateService.cs | 47 ++++++++----------- .../Update/UpdateCheckService.cs | 14 +----- src/UI/System/Update/UpdateItemView.js | 2 +- .../System/Update/UpdateItemViewTemplate.hbs | 7 ++- 7 files changed, 33 insertions(+), 63 deletions(-) delete mode 100644 src/NzbDrone.Core/Update/Commands/InstallUpdateCommand.cs diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index 981fc894b..3800751e7 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs @@ -61,6 +61,10 @@ namespace NzbDrone.Core.Test.UpdateTests Mocker.GetMock().Setup(c => c.GetCurrentProcess()).Returns(new ProcessInfo { Id = 12 }); Mocker.GetMock().Setup(c => c.ExecutingApplication).Returns(@"C:\Test\NzbDrone.exe"); + Mocker.GetMock() + .SetupGet(s => s.UpdateAutomatically) + .Returns(true); + Mocker.GetMock() .Setup(c => c.FolderWritable(It.IsAny())) .Returns(true); @@ -289,7 +293,7 @@ namespace NzbDrone.Core.Test.UpdateTests var updateArchive = Path.Combine(_sandboxFolder, _updatePackage.FileName); - Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage }); + Subject.Execute(new ApplicationUpdateCommand()); Mocker.GetMock().Verify(c => c.DownloadFile(_updatePackage.Url, updateArchive), Times.Never()); ExceptionVerification.ExpectedErrors(1); @@ -300,7 +304,7 @@ namespace NzbDrone.Core.Test.UpdateTests { _updatePackage.Branch = "fake"; - Subject.Execute(new InstallUpdateCommand() { UpdatePackage = _updatePackage }); + Subject.Execute(new ApplicationUpdateCommand()); Mocker.GetMock() .Verify(v => v.SaveConfigDictionary(It.Is>(d => d.ContainsKey("Branch") && (string)d["Branch"] == "fake")), Times.Once()); diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index ae7c50e7f..718d27848 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -852,7 +852,6 @@ - diff --git a/src/NzbDrone.Core/Update/Commands/InstallUpdateCommand.cs b/src/NzbDrone.Core/Update/Commands/InstallUpdateCommand.cs deleted file mode 100644 index 890a31199..000000000 --- a/src/NzbDrone.Core/Update/Commands/InstallUpdateCommand.cs +++ /dev/null @@ -1,17 +0,0 @@ -using NzbDrone.Core.Messaging.Commands; - -namespace NzbDrone.Core.Update.Commands -{ - public class InstallUpdateCommand : Command - { - public UpdatePackage UpdatePackage { get; set; } - - public override bool SendUpdatesToClient - { - get - { - return true; - } - } - } -} diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index 300eb6f48..2d0d1c491 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -17,7 +17,7 @@ using NzbDrone.Core.Update.Commands; namespace NzbDrone.Core.Update { - public class InstallUpdateService : IExecute, IExecute + public class InstallUpdateService : IExecute { private readonly ICheckUpdateService _checkUpdateService; private readonly Logger _logger; @@ -176,15 +176,31 @@ namespace NzbDrone.Core.Update if (_appFolderInfo.StartUpFolder.IsParentPath(_appFolderInfo.AppDataFolder) || _appFolderInfo.StartUpFolder.PathEquals(_appFolderInfo.AppDataFolder)) { - throw new UpdateFailedException("You Sonarr configuration ('{0}') is being stored in application folder ('{1}') which will cause data lost during the upgrade. Please remove any symlinks or redirects before trying again.", _appFolderInfo.AppDataFolder, _appFolderInfo.StartUpFolder); + throw new UpdateFailedException("Your Sonarr configuration '{0}' is being stored in application folder '{1}' which will cause data lost during the upgrade. Please remove any symlinks or redirects before trying again.", _appFolderInfo.AppDataFolder, _appFolderInfo.StartUpFolder); } } - private void ExecuteInstallUpdate(Command message, UpdatePackage package) + public void Execute(ApplicationUpdateCommand message) { + _logger.ProgressDebug("Checking for updates"); + + var latestAvailable = _checkUpdateService.AvailableUpdate(); + + if (latestAvailable == null) + { + _logger.ProgressDebug("No update available."); + return; + } + + if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically && !message.Manual) + { + _logger.ProgressDebug("Auto-update not enabled, not installing available update."); + return; + } + try { - InstallUpdate(package); + InstallUpdate(latestAvailable); message.Completed("Restarting Sonarr to apply updates"); } @@ -204,28 +220,5 @@ namespace NzbDrone.Core.Update message.Failed(ex); } } - - public void Execute(ApplicationUpdateCommand message) - { - _logger.ProgressDebug("Checking for updates"); - var latestAvailable = _checkUpdateService.AvailableUpdate(); - - if (latestAvailable != null) - { - ExecuteInstallUpdate(message, latestAvailable); - } - } - - public void Execute(InstallUpdateCommand message) - { - var latestAvailable = _checkUpdateService.AvailableUpdate(); - - if (latestAvailable == null || latestAvailable.Hash != message.UpdatePackage.Hash) - { - throw new ApplicationException("Unknown or invalid update specified"); - } - - ExecuteInstallUpdate(message, latestAvailable); - } } } diff --git a/src/NzbDrone.Core/Update/UpdateCheckService.cs b/src/NzbDrone.Core/Update/UpdateCheckService.cs index f17b72dcc..ad8ffc247 100644 --- a/src/NzbDrone.Core/Update/UpdateCheckService.cs +++ b/src/NzbDrone.Core/Update/UpdateCheckService.cs @@ -31,19 +31,7 @@ namespace NzbDrone.Core.Update public UpdatePackage AvailableUpdate() { - var latestAvailable = _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); - - if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically) - { - return null; - } - - if (latestAvailable == null) - { - _logger.ProgressDebug("No update available."); - } - - return latestAvailable; + return _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); } } } \ No newline at end of file diff --git a/src/UI/System/Update/UpdateItemView.js b/src/UI/System/Update/UpdateItemView.js index 48ee09de4..d21104de6 100644 --- a/src/UI/System/Update/UpdateItemView.js +++ b/src/UI/System/Update/UpdateItemView.js @@ -13,7 +13,7 @@ module.exports = Marionette.ItemView.extend({ } this.updating = true; var self = this; - var promise = CommandController.Execute('installUpdate', {updatePackage : this.model.toJSON()}); + var promise = CommandController.Execute('applicationUpdate'); promise.done(function(){ window.setTimeout(function(){ self.updating = false; diff --git a/src/UI/System/Update/UpdateItemViewTemplate.hbs b/src/UI/System/Update/UpdateItemViewTemplate.hbs index aecefacbd..f9a2dce19 100644 --- a/src/UI/System/Update/UpdateItemViewTemplate.hbs +++ b/src/UI/System/Update/UpdateItemViewTemplate.hbs @@ -5,14 +5,17 @@ - {{ShortDate releaseDate}} + {{#unless_eq branch compare="master"}} + {{branch}} + {{/unless_eq}} {{#if installed}} Installed {{else}} {{#if latest}} {{#if installable}} - Install + Install Latest {{else}} - Install + Install Latest {{/if}} {{/if}} {{/if}} From f7bdf635b32bb8adb6c0747b8105280097d780f8 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 27 Jan 2015 20:33:32 +0100 Subject: [PATCH 10/13] Reordered and renamed tabs in System. --- src/UI/System/SystemLayout.js | 19 ++++++++++--------- src/UI/System/SystemLayoutTemplate.hbs | 12 ++++++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/UI/System/SystemLayout.js b/src/UI/System/SystemLayout.js index cdb4e680a..6232626fd 100644 --- a/src/UI/System/SystemLayout.js +++ b/src/UI/System/SystemLayout.js @@ -12,21 +12,21 @@ var StatusModel = require('./StatusModel'); module.exports = Marionette.Layout.extend({ template : 'System/SystemLayoutTemplate', regions : { - info : '#info', + status : '#status', logs : '#logs', updates : '#updates', backup : '#backup', tasks : '#tasks' }, ui : { - infoTab : '.x-info-tab', + statusTab : '.x-status-tab', logsTab : '.x-logs-tab', updatesTab : '.x-updates-tab', backupTab : '.x-backup-tab', tasksTab : '.x-tasks-tab' }, events : { - 'click .x-info-tab' : '_showInfo', + 'click .x-status-tab' : '_showStatus', 'click .x-logs-tab' : '_showLogs', 'click .x-updates-tab' : '_showUpdates', 'click .x-backup-tab' : '_showBackup', @@ -58,7 +58,7 @@ module.exports = Marionette.Layout.extend({ this._showTasks(); break; default: - this._showInfo(); + this._showStatus(); } }, _navigate : function(route){ @@ -67,13 +67,14 @@ module.exports = Marionette.Layout.extend({ replace : true }); }, - _showInfo : function(e){ - if(e) { + _showStatus : function (e) { + if (e) { e.preventDefault(); } - this.info.show(new SystemInfoLayout()); - this.ui.infoTab.tab('show'); - this._navigate('system/info'); + + this.status.show(new SystemInfoLayout()); + this.ui.statusTab.tab('show'); + this._navigate('system/status'); }, _showLogs : function(e){ if(e) { diff --git a/src/UI/System/SystemLayoutTemplate.hbs b/src/UI/System/SystemLayoutTemplate.hbs index a14338353..8c5882e7e 100644 --- a/src/UI/System/SystemLayoutTemplate.hbs +++ b/src/UI/System/SystemLayoutTemplate.hbs @@ -1,9 +1,9 @@ 
-
-
+
-
+
+
\ No newline at end of file From 52a71a4e969e5ca42f21765f40d711fe1298f76c Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 4 Feb 2015 01:10:16 +0100 Subject: [PATCH 11/13] Fixed some mono specific tests. --- src/NzbDrone.Core/Update/InstallUpdateService.cs | 5 ++--- .../DiskProviderTests/DiskProviderFixture.cs | 8 +++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index 2d0d1c491..e5ffc928e 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -147,13 +147,12 @@ namespace NzbDrone.Core.Update if (scriptPath.IsNullOrWhiteSpace()) { - throw new ArgumentException("Update Script has not been defined"); + throw new UpdateFailedException("Update Script has not been defined"); } if (!_diskProvider.FileExists(scriptPath, StringComparison.Ordinal)) { - var message = String.Format("Update Script: '{0}' does not exist", scriptPath); - throw new FileNotFoundException(message, scriptPath); + throw new UpdateFailedException("Update Script: '{0}' does not exist", scriptPath); } _logger.Info("Removing NzbDrone.Update"); diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 5317ad891..ed5c2ed64 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,5 @@ -using Mono.Unix; +using System; +using Mono.Unix; using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; @@ -15,6 +16,11 @@ namespace NzbDrone.Mono.Test.DiskProviderTests protected override void SetWritePermissions(string path, bool writable) { + if (Environment.UserName == "root") + { + Assert.Inconclusive("Need non-root user to test write permissions."); + } + // Remove Write permissions, we're still owner so we can clean it up, but we'll have to do that explicitly. var entry = UnixFileSystemInfo.GetFileSystemEntry(path); From 955029ec434075bf2e9613a72f27ed3d490d0cab Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Thu, 5 Feb 2015 08:52:58 +0100 Subject: [PATCH 12/13] Fixed: Updated installation HealthCheck warning link to wiki. --- src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs index 5dfd56841..86351922b 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs @@ -31,7 +31,9 @@ namespace NzbDrone.Core.HealthCheck.Checks { if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { - return new HealthCheck(GetType(), HealthCheckResult.Error, string.Format("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName)); + return new HealthCheck(GetType(), HealthCheckResult.Error, + string.Format("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName), + "Cannot install update because startup folder is not writable by the user"); } } From 40987cc335a79552d940c044d1791afadf93f03e Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sat, 7 Feb 2015 15:56:36 +0100 Subject: [PATCH 13/13] Include version in services Changes api call so the server knows how to redirect. --- .../UpdateTests/UpdatePackageProviderFixture.cs | 2 +- src/NzbDrone.Core/Update/RecentUpdateProvider.cs | 3 ++- src/NzbDrone.Core/Update/UpdatePackageProvider.cs | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdatePackageProviderFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdatePackageProviderFixture.cs index d137750e3..ff2ae0699 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdatePackageProviderFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdatePackageProviderFixture.cs @@ -37,7 +37,7 @@ namespace NzbDrone.Core.Test.UpdateTests { const string branch = "master"; UseRealHttp(); - var recent = Subject.GetRecentUpdates(branch); + var recent = Subject.GetRecentUpdates(branch, new Version(2, 0)); recent.Should().NotBeEmpty(); recent.Should().OnlyContain(c => c.Hash.IsNotNullOrWhiteSpace()); diff --git a/src/NzbDrone.Core/Update/RecentUpdateProvider.cs b/src/NzbDrone.Core/Update/RecentUpdateProvider.cs index 96a915694..6fcaf42c2 100644 --- a/src/NzbDrone.Core/Update/RecentUpdateProvider.cs +++ b/src/NzbDrone.Core/Update/RecentUpdateProvider.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Core.Configuration; namespace NzbDrone.Core.Update @@ -23,7 +24,7 @@ namespace NzbDrone.Core.Update public List GetRecentUpdatePackages() { var branch = _configFileProvider.Branch; - return _updatePackageProvider.GetRecentUpdates(branch); + return _updatePackageProvider.GetRecentUpdates(branch, BuildInfo.Version); } } } diff --git a/src/NzbDrone.Core/Update/UpdatePackageProvider.cs b/src/NzbDrone.Core/Update/UpdatePackageProvider.cs index b31b4bb78..86a4f6299 100644 --- a/src/NzbDrone.Core/Update/UpdatePackageProvider.cs +++ b/src/NzbDrone.Core/Update/UpdatePackageProvider.cs @@ -9,7 +9,7 @@ namespace NzbDrone.Core.Update public interface IUpdatePackageProvider { UpdatePackage GetLatestUpdate(string branch, Version currentVersion); - List GetRecentUpdates(string branch); + List GetRecentUpdates(string branch, Version currentVersion); } public class UpdatePackageProvider : IUpdatePackageProvider @@ -37,9 +37,10 @@ namespace NzbDrone.Core.Update return update.UpdatePackage; } - public List GetRecentUpdates(string branch) + public List GetRecentUpdates(string branch, Version currentVersion) { var request = _requestBuilder.Build("/update/{branch}/changes"); + request.UriBuilder.SetQueryParam("version", currentVersion); request.UriBuilder.SetQueryParam("os", OsInfo.Os.ToString().ToLowerInvariant()); request.AddSegment("branch", branch);