From 036a30878adcc22f8cfc8b130fb627b045d94553 Mon Sep 17 00:00:00 2001 From: Gary Ewan Park Date: Tue, 29 May 2018 10:44:29 +0100 Subject: [PATCH] (GH-1580) Only log messages when RegularOutput is enabled - This required passing in a ChocolateyConfiguration object to multiple methods - This resulted in changes to tests - All output is now guarded, and only happens when RegularOutput is true --- .../commands/ChocolateyPinCommandSpecs.cs | 8 +++--- .../AutomaticUninstallerServiceSpecs.cs | 18 ++++++------- .../commands/ChocolateyPinCommand.cs | 6 ++--- .../services/AutomaticUninstallerService.cs | 2 +- .../ChocolateyPackageInformationService.cs | 25 ++++++++++++++++--- .../services/ChocolateyPackageService.cs | 12 ++++----- .../IChocolateyPackageInformationService.cs | 4 +++ .../services/NugetService.cs | 10 ++++---- 8 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs b/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs index f72c0c9039..18f6507deb 100644 --- a/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs +++ b/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs @@ -53,7 +53,7 @@ public override void Context() package = new Mock(); package.Setup(p => p.Id).Returns("regular"); package.Setup(p => p.Version).Returns(new SemanticVersion("1.2.0")); - packageInfoService.Setup(s => s.get_package_information(package.Object)).Returns( + packageInfoService.Setup(s => s.get_package_information(package.Object, configuration)).Returns( new ChocolateyPackageInformation(package.Object) { IsPinned = false @@ -61,7 +61,7 @@ public override void Context() pinnedPackage = new Mock(); pinnedPackage.Setup(p => p.Id).Returns("pinned"); pinnedPackage.Setup(p => p.Version).Returns(new SemanticVersion("1.1.0")); - packageInfoService.Setup(s => s.get_package_information(pinnedPackage.Object)).Returns( + packageInfoService.Setup(s => s.get_package_information(pinnedPackage.Object, configuration)).Returns( new ChocolateyPackageInformation(pinnedPackage.Object) { IsPinned = true @@ -439,7 +439,7 @@ public void should_set_pin_when_command_is_add() command.set_pin(packageManager.Object, configuration); - packageInfoService.Verify(s => s.save_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.save_package_information(It.IsAny(), configuration), Times.Once); } [Pending("NuGet is killing me with extension methods. Need to find proper item to mock out to return the package object.")] @@ -453,7 +453,7 @@ public void should_remove_pin_when_command_is_remove() command.set_pin(packageManager.Object, configuration); - packageInfoService.Verify(s => s.save_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.save_package_information(It.IsAny(), configuration), Times.Once); } } } diff --git a/src/chocolatey.tests/infrastructure.app/services/AutomaticUninstallerServiceSpecs.cs b/src/chocolatey.tests/infrastructure.app/services/AutomaticUninstallerServiceSpecs.cs index a275cac595..83737e82f0 100644 --- a/src/chocolatey.tests/infrastructure.app/services/AutomaticUninstallerServiceSpecs.cs +++ b/src/chocolatey.tests/infrastructure.app/services/AutomaticUninstallerServiceSpecs.cs @@ -80,7 +80,7 @@ public override void Context() InstallerType = installerType.InstallerType, }); packageInformation.RegistrySnapshot = new Registry("123", registryKeys); - packageInfoService.Setup(s => s.get_package_information(package.Object)).Returns(packageInformation); + packageInfoService.Setup(s => s.get_package_information(package.Object, config)).Returns(packageInformation); packageResults.GetOrAdd("regular", packageResult); fileSystem.Setup(f => f.directory_exists(registryKeys.FirstOrDefault().InstallLocation)).Returns(true); @@ -115,7 +115,7 @@ public void should_log_why_it_skips_auto_uninstaller() [Fact] public void should_not_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Never); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Never); } [Fact] @@ -299,7 +299,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -454,7 +454,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -502,7 +502,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -551,7 +551,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -601,7 +601,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -693,7 +693,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] @@ -748,7 +748,7 @@ public override void Because() [Fact] public void should_call_get_package_information() { - packageInfoService.Verify(s => s.get_package_information(It.IsAny()), Times.Once); + packageInfoService.Verify(s => s.get_package_information(It.IsAny(), config), Times.Once); } [Fact] diff --git a/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs b/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs index d129e71c82..5c59826426 100644 --- a/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs +++ b/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs @@ -156,7 +156,7 @@ public virtual void list_pins(IPackageManager packageManager, ChocolateyConfigur foreach (var pkg in packages.or_empty_list_if_null()) { - var pkgInfo = _packageInfoService.get_package_information(pkg.Package); + var pkgInfo = _packageInfoService.get_package_information(pkg.Package, config); if (pkgInfo != null && pkgInfo.IsPinned) { this.Log().Info(() => "{0}|{1}".format_with(pkgInfo.Package.Id, pkgInfo.Package.Version)); @@ -186,12 +186,12 @@ public virtual void set_pin(IPackageManager packageManager, ChocolateyConfigurat } } - var pkgInfo = _packageInfoService.get_package_information(installedPackage); + var pkgInfo = _packageInfoService.get_package_information(installedPackage, config); bool changeMessage = pkgInfo.IsPinned != addingAPin; pkgInfo.IsPinned = addingAPin; - _packageInfoService.save_package_information(pkgInfo); + _packageInfoService.save_package_information(pkgInfo, config); if (changeMessage) { diff --git a/src/chocolatey/infrastructure.app/services/AutomaticUninstallerService.cs b/src/chocolatey/infrastructure.app/services/AutomaticUninstallerService.cs index 868e1d95e9..e210d966f0 100644 --- a/src/chocolatey/infrastructure.app/services/AutomaticUninstallerService.cs +++ b/src/chocolatey/infrastructure.app/services/AutomaticUninstallerService.cs @@ -71,7 +71,7 @@ public void run(PackageResult packageResult, ChocolateyConfiguration config) } } - var pkgInfo = _packageInfoService.get_package_information(packageResult.Package); + var pkgInfo = _packageInfoService.get_package_information(packageResult.Package, config); if (pkgInfo.RegistrySnapshot == null) { this.Log().Info(" Skipping auto uninstaller - No registry snapshot."); diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs index 62a46f3b70..d4064260a3 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs @@ -19,6 +19,7 @@ namespace chocolatey.infrastructure.app.services using System; using System.IO; using System.Text; + using configuration; using NuGet; using domain; using tolerance; @@ -47,11 +48,16 @@ public ChocolateyPackageInformationService(IFileSystem fileSystem, IRegistryServ } public ChocolateyPackageInformation get_package_information(IPackage package) + { + return get_package_information(package, new ChocolateyConfiguration()); + } + + public ChocolateyPackageInformation get_package_information(IPackage package, ChocolateyConfiguration config) { var packageInformation = new ChocolateyPackageInformation(package); if (package == null) { - this.Log().Debug("No package information as package is null."); + if (config.RegularOutput) { this.Log().Debug("No package information as package is null."); } return packageInformation; } @@ -79,7 +85,7 @@ focus exclusively in the string values not surrounded by CData. Once { if (_fileSystem.file_exists(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BAD_FILE))) { - this.Log().Warn(deserializationErrorMessage); + if(config.RegularOutput) { this.Log().Warn(deserializationErrorMessage); } } else { @@ -91,7 +97,7 @@ focus exclusively in the string values not surrounded by CData. Once FaultTolerance.try_catch_with_logging_exception( () => { - this.Log().Warn(deserializationErrorMessage); + if(config.RegularOutput) { this.Log().Warn(deserializationErrorMessage); } // rename the bad registry file so that it isn't processed again _fileSystem.move_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE), _fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BAD_FILE)); @@ -141,13 +147,18 @@ focus exclusively in the string values not surrounded by CData. Once } public void save_package_information(ChocolateyPackageInformation packageInformation) + { + save_package_information(packageInformation, new ChocolateyConfiguration()); + } + + public void save_package_information(ChocolateyPackageInformation packageInformation, ChocolateyConfiguration config) { _fileSystem.create_directory_if_not_exists(ApplicationParameters.ChocolateyPackageInfoStoreLocation); _fileSystem.ensure_file_attribute_set(ApplicationParameters.ChocolateyPackageInfoStoreLocation, FileAttributes.Hidden); if (packageInformation.Package == null) { - this.Log().Debug("No package information to save as package is null."); + if(config.RegularOutput) { this.Log().Debug("No package information to save as package is null."); } return; } @@ -229,8 +240,14 @@ public void save_package_information(ChocolateyPackageInformation packageInforma } public void remove_package_information(IPackage package) + { + remove_package_information(package, new ChocolateyConfiguration()); + } + + public void remove_package_information(IPackage package, ChocolateyConfiguration config) { var pkgStorePath = _fileSystem.combine_paths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".format_with(package.Id, package.Version.to_string())); + if (config.RegularOutput) { this.Log().Info("Removing Package Information for {0}".format_with(pkgStorePath)); } _fileSystem.delete_directory_if_exists(pkgStorePath, recursive: true); } } diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs index a6c742fe25..fe3f1af82f 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs @@ -205,7 +205,7 @@ public IEnumerable list_run(ChocolateyConfiguration config) private IEnumerable report_registry_programs(ChocolateyConfiguration config, IEnumerable list) { - var itemsToRemoveFromMachine = list.Select(package => _packageInfoService.get_package_information(package)). + var itemsToRemoveFromMachine = list.Select(package => _packageInfoService.get_package_information(package, config)). Where(p => p.RegistrySnapshot != null). Select(p => p.RegistrySnapshot.RegistryKeys.FirstOrDefault()). Where(p => p != null). @@ -414,7 +414,7 @@ public void handle_package_result(PackageResult packageResult, ChocolateyConfigu if (key != null) Environment.SetEnvironmentVariable(ApplicationParameters.Environment.ChocolateyPackageInstallLocation, key.InstallLocation, EnvironmentVariableTarget.Process); } - update_package_information(pkgInfo); + update_package_information(pkgInfo, config); ensure_bad_package_path_is_clean(config, packageResult); EventManager.publish(new HandlePackageResultCompletedMessage(packageResult, config, commandName)); @@ -476,7 +476,7 @@ private void create_ignore_files_for_executables(string installLocation, bool is protected virtual ChocolateyPackageInformation get_package_information(PackageResult packageResult, ChocolateyConfiguration config) { - var pkgInfo = _packageInfoService.get_package_information(packageResult.Package); + var pkgInfo = _packageInfoService.get_package_information(packageResult.Package, config); if (config.AllowMultipleVersions) { pkgInfo.IsSideBySide = true; @@ -485,9 +485,9 @@ protected virtual ChocolateyPackageInformation get_package_information(PackageRe return pkgInfo; } - protected virtual void update_package_information(ChocolateyPackageInformation pkgInfo) + protected virtual void update_package_information(ChocolateyPackageInformation pkgInfo, ChocolateyConfiguration config) { - _packageInfoService.save_package_information(pkgInfo); + _packageInfoService.save_package_information(pkgInfo, config); } private string capture_arguments(ChocolateyConfiguration config, PackageResult packageResult) @@ -955,7 +955,7 @@ public void handle_package_uninstall(PackageResult packageResult, ChocolateyConf private void uninstall_cleanup(ChocolateyConfiguration config, PackageResult packageResult) { - if (config.Features.RemovePackageInformationOnUninstall) _packageInfoService.remove_package_information(packageResult.Package); + if (config.Features.RemovePackageInformationOnUninstall) _packageInfoService.remove_package_information(packageResult.Package, config); ensure_bad_package_path_is_clean(config, packageResult); remove_rollback_if_exists(packageResult); diff --git a/src/chocolatey/infrastructure.app/services/IChocolateyPackageInformationService.cs b/src/chocolatey/infrastructure.app/services/IChocolateyPackageInformationService.cs index 859cde7726..14888700ac 100644 --- a/src/chocolatey/infrastructure.app/services/IChocolateyPackageInformationService.cs +++ b/src/chocolatey/infrastructure.app/services/IChocolateyPackageInformationService.cs @@ -16,13 +16,17 @@ namespace chocolatey.infrastructure.app.services { + using configuration; using domain; using NuGet; public interface IChocolateyPackageInformationService { ChocolateyPackageInformation get_package_information(IPackage package); + ChocolateyPackageInformation get_package_information(IPackage package, ChocolateyConfiguration config); void save_package_information(ChocolateyPackageInformation packageInformation); + void save_package_information(ChocolateyPackageInformation packageInformation, ChocolateyConfiguration config); void remove_package_information(IPackage package); + void remove_package_information(IPackage package, ChocolateyConfiguration config); } } diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index 835710afd4..b4fd649586 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -137,7 +137,7 @@ public virtual IEnumerable list_run(ChocolateyConfiguration confi if (config.ListCommand.LocalOnly) { - var packageInfo = _packageInfoService.get_package_information(package); + var packageInfo = _packageInfoService.get_package_information(package, config); if (config.ListCommand.IncludeVersionOverrides) { if (packageInfo.VersionOverride != null) @@ -492,7 +492,7 @@ Version was specified as '{0}'. It is possible that version forcedResult.Messages.Add(new ResultMessage(ResultType.Note, "Backing up and removing old version")); remove_rollback_directory_if_exists(packageName); - backup_existing_version(config, installedPackage, _packageInfoService.get_package_information(installedPackage)); + backup_existing_version(config, installedPackage, _packageInfoService.get_package_information(installedPackage, config)); try { @@ -652,7 +652,7 @@ public virtual ConcurrentDictionary upgrade_run(Chocolate continue; } - var pkgInfo = _packageInfoService.get_package_information(installedPackage); + var pkgInfo = _packageInfoService.get_package_information(installedPackage, config); bool isPinned = pkgInfo != null && pkgInfo.IsPinned; if (isPinned && config.OutdatedCommand.IgnorePinned) @@ -1167,7 +1167,7 @@ public virtual ConcurrentDictionary uninstall_run(Chocola // is this the latest version, have you passed --sxs, or is this a side-by-side install? This is the only way you get through to the continue action. var latestVersion = packageManager.LocalRepository.FindPackage(e.Package.Id); - var pkgInfo = _packageInfoService.get_package_information(e.Package); + var pkgInfo = _packageInfoService.get_package_information(e.Package, config); if (latestVersion.Version == pkg.Version || config.AllowMultipleVersions || (pkgInfo != null && pkgInfo.IsSideBySide)) { packageResult.Messages.Add(new ResultMessage(ResultType.Debug, ApplicationParameters.Messages.ContinueChocolateyAction)); @@ -1309,7 +1309,7 @@ public virtual ConcurrentDictionary uninstall_run(Chocola foreach (var packageVersion in packageVersionsToRemove) { - var pkgInfo = _packageInfoService.get_package_information(packageVersion); + var pkgInfo = _packageInfoService.get_package_information(packageVersion, config); if (pkgInfo != null && pkgInfo.IsPinned) { string logMessage = "{0} is pinned. Skipping pinned package.".format_with(packageName);