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.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.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/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.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index 4937d8ecf..d06230850 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; @@ -11,6 +12,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 +61,14 @@ 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); + _sandboxFolder = Mocker.GetMock().Object.GetUpdateSandboxFolder(); } @@ -259,6 +269,47 @@ 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_log_when_install_cannot_be_started() + { + 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_switch_to_branch_specified_in_updatepackage() + { + _updatePackage.Branch = "fake"; + + Subject.Execute(new ApplicationUpdateCommand()); + + 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/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.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..89c70c585 100644 --- a/src/NzbDrone.Core/Download/DownloadClientBase.cs +++ b/src/NzbDrone.Core/Download/DownloadClientBase.cs @@ -98,26 +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)) { - try + _logger.Error("Folder '{0}' is not writable.", folder); + return new NzbDroneValidationFailure(propertyName, "Unable to write to 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); - 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/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..86351922b 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs @@ -29,15 +29,11 @@ namespace NzbDrone.Core.HealthCheck.Checks { if (OsInfo.IsWindows || _configFileProvider.UpdateAutomatically) { - try + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { - var testPath = Path.Combine(_appFolderInfo.StartUpFolder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception) - { - 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), + "Cannot install update because startup folder is not writable by the user"); } } 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); } } } 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) { diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index 04736e507..7af72a5cb 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -853,11 +853,12 @@ - + + 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 bdaedbb3a..e5ffc928e 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; @@ -15,8 +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; @@ -63,58 +64,80 @@ namespace NzbDrone.Core.Update private void InstallUpdate(UpdatePackage updatePackage) { - try - { - EnsureAppDataSafety(); - - var updateSandboxFolder = _appFolderInfo.GetUpdateSandboxFolder(); + EnsureAppDataSafety(); - var packageDestination = Path.Combine(updateSandboxFolder, updatePackage.FileName); - - if (_diskProvider.FolderExists(updateSandboxFolder)) + if (OsInfo.IsWindows || _configFileProvider.UpdateMechanism != UpdateMechanism.Script) + { + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { - _logger.Info("Deleting old update files"); - _diskProvider.DeleteFolder(updateSandboxFolder, true); + throw new UpdateFolderNotWritableException("Cannot install update because startup folder '{0}' is not writable by the user '{1}'.", _appFolderInfo.StartUpFolder, Environment.UserName); } + } - _logger.ProgressInfo("Downloading update {0}", updatePackage.Version); - _logger.Debug("Downloading update package from [{0}] to [{1}]", updatePackage.Url, packageDestination); - _httpClient.DownloadFile(updatePackage.Url, packageDestination); + var updateSandboxFolder = _appFolderInfo.GetUpdateSandboxFolder(); - _logger.ProgressInfo("Verifying update package"); + var packageDestination = Path.Combine(updateSandboxFolder, updatePackage.FileName); - if (!_updateVerifier.Verify(updatePackage, packageDestination)) - { - _logger.Error("Update package is invalid"); - throw new UpdateVerificationFailedException("Update file '{0}' is invalid", packageDestination); - } + if (_diskProvider.FolderExists(updateSandboxFolder)) + { + _logger.Info("Deleting old update files"); + _diskProvider.DeleteFolder(updateSandboxFolder, true); + } - _logger.Info("Update package verified successfully"); + _logger.ProgressInfo("Downloading update {0}", updatePackage.Version); + _logger.Debug("Downloading update package from [{0}] to [{1}]", updatePackage.Url, packageDestination); + _httpClient.DownloadFile(updatePackage.Url, packageDestination); - _logger.ProgressInfo("Extracting Update package"); - _archiveService.Extract(packageDestination, updateSandboxFolder); - _logger.Info("Update package extracted successfully"); + _logger.ProgressInfo("Verifying update package"); - _backupService.Backup(BackupType.Update); + if (!_updateVerifier.Verify(updatePackage, packageDestination)) + { + _logger.Error("Update package is invalid"); + throw new UpdateVerificationFailedException("Update file '{0}' is invalid", packageDestination); + } - if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) - { - InstallUpdateWithScript(updateSandboxFolder); - return; - } + _logger.Info("Update package verified successfully"); + + _logger.ProgressInfo("Extracting Update package"); + _archiveService.Extract(packageDestination, updateSandboxFolder); + _logger.Info("Update package extracted successfully"); - _logger.Info("Preparing client"); - _diskProvider.MoveFolder(_appFolderInfo.GetUpdateClientFolder(), - updateSandboxFolder); + EnsureValidBranch(updatePackage); - _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath()); - _logger.ProgressInfo("NzbDrone will restart shortly."); + _backupService.Backup(BackupType.Update); - _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); + if (OsInfo.IsNotWindows && _configFileProvider.UpdateMechanism == UpdateMechanism.Script) + { + InstallUpdateWithScript(updateSandboxFolder); + return; } - catch (Exception ex) + + _logger.Info("Preparing client"); + _diskProvider.MoveFolder(_appFolderInfo.GetUpdateClientFolder(), + updateSandboxFolder); + + _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath()); + _logger.ProgressInfo("Sonarr will restart shortly."); + + _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(), GetUpdaterArgs(updateSandboxFolder)); + } + + private void EnsureValidBranch(UpdatePackage package) + { + var currentBranch = _configFileProvider.Branch; + if (package.Branch != currentBranch) { - _logger.ErrorException("Update process failed", ex); + 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); + } } } @@ -124,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"); @@ -153,24 +175,49 @@ 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("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); } } public void Execute(ApplicationUpdateCommand message) { _logger.ProgressDebug("Checking for updates"); + var latestAvailable = _checkUpdateService.AvailableUpdate(); - if (latestAvailable != null) + if (latestAvailable == null) { - InstallUpdate(latestAvailable); + _logger.ProgressDebug("No update available."); + return; } - } - public void Execute(InstallUpdateCommand message) - { - InstallUpdate(message.UpdatePackage); + if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically && !message.Manual) + { + _logger.ProgressDebug("Auto-update not enabled, not installing available update."); + return; + } + + try + { + InstallUpdate(latestAvailable); + + 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); + } } } } 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/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/UpdateCheckService.cs b/src/NzbDrone.Core/Update/UpdateCheckService.cs index bbed226ac..ad8ffc247 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,34 +31,7 @@ namespace NzbDrone.Core.Update public UpdatePackage AvailableUpdate() { - if (OsInfo.IsNotWindows && !_configFileProvider.UpdateAutomatically) - { - return null; - } - - var latestAvailable = _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); - - if (latestAvailable == null) - { - _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 = _configFileProvider.GetConfigDictionary(); - config["Branch"] = latestAvailable.Branch; - _configFileProvider.SaveConfigDictionary(config); - } - catch (Exception e) - { - _logger.ErrorException("Couldn't save the branch redirect.", e); - } - - } - - return latestAvailable; + return _updatePackageProvider.GetLatestUpdate(_configFileProvider.Branch, BuildInfo.Version); } } } \ No newline at end of file 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/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); 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/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 130d72e92..ed5c2ed64 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,6 @@ -using NUnit.Framework; +using System; +using Mono.Unix; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; namespace NzbDrone.Mono.Test.DiskProviderTests @@ -11,5 +13,26 @@ namespace NzbDrone.Mono.Test.DiskProviderTests { MonoOnly(); } + + 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); + + 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.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; 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); + } + } } } 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: 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 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}}