Merge pull request chocolatey#2484 from TheCakeIsNaOH/reset-config

(chocolatey#1443) Reset config via action after every package upgrade

Author: corbob

Invoke-Tests.ps1

@@ -93,7 +93,17 @@ try {
             Verbosity = 'Minimal'
         }
         Filter     = @{
-            ExcludeTag = 'Background', 'Licensed', 'CCM', 'WIP', 'NonAdmin', 'Internal'
+            ExcludeTag = @(
+                'Background'
+                'Licensed'
+                'CCM'
+                'WIP'
+                'NonAdmin'
+                'Internal'
+                if (-not $env:VM_RUNNING -and -not $env:TEST_KITCHEN) {
+                    'VMOnly'
+                }
+            )
         }
     }
 

src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs

@@ -30,6 +30,9 @@ namespace chocolatey.infrastructure.app.configuration
     [Serializable]
     public class ChocolateyConfiguration
     {
+        [NonSerialized]
+        private ChocolateyConfiguration _originalConfiguration;
+
         public ChocolateyConfiguration()
         {
             RegularOutput = true;
@@ -58,6 +61,86 @@ public ChocolateyConfiguration()
 #endif
         }
 
+        /// <summary>
+        /// Creates a backup of the current version of the configuration class.
+        /// </summary>
+        /// <exception cref="System.Runtime.Serialization.SerializationException">One or more objects in the class or child classes are not serializable.</exception>
+        public void start_backup()
+        {
+            // We do this the easy way to ensure that we have a clean copy
+            // of the original configuration file.
+            _originalConfiguration = this.deep_copy();
+        }
+
+        /// <summary>
+        /// Restore the backup that has previously been created to the initial
+        /// state, without making the class reference types the same to prevent
+        /// the initial configuration class being updated at the same time if a
+        /// value changes.
+        /// </summary>
+        /// <param name="removeBackup">Whether a backup that was previously made should be removed after resetting the configuration.</param>
+        /// <exception cref="InvalidOperationException">No backup has been created before trying to reset the current configuration, and removal of the backup was not requested.</exception>
+        /// <remarks>
+        /// This call may make quite a lot of allocations on the Gen0 heap, as such
+        /// it is best to keep the calls to this method at a minimum.
+        /// </remarks>
+        public void reset_config(bool removeBackup = false)
+        {
+            if (_originalConfiguration == null)
+            {
+                if (removeBackup)
+                {
+                    // If we will also be removing the backup, we do not care if it is already
+                    // null or not, as that is the intended state when this method returns.
+                    return;
+                }
+
+                throw new InvalidOperationException("No backup has been created before trying to reset the current configuration, and removal of the backup was not requested.");
+            }
+
+            var t = this.GetType();
+
+            foreach (var property in t.GetProperties(BindingFlags.Public | BindingFlags.Instance))
+            {
+                try
+                {
+                    var originalValue = property.GetValue(_originalConfiguration, new object[0]);
+
+                    if (removeBackup || property.DeclaringType.IsPrimitive || property.DeclaringType.IsValueType || property.DeclaringType == typeof(string))
+                    {
+                        // If the property is a primitive, a value type or a string, then a copy of the value
+                        // will be created by the .NET Runtime automatically, and we do not need to create a deep clone of the value.
+                        // Additionally, if we will be removing the backup there is no need to create a deep copy
+                        // for any reference types, as such we also set the reference itself so it is not needed
+                        // to allocate more memory.
+                        property.SetValue(this, originalValue, new object[0]);
+                    }
+                    else if (originalValue != null)
+                    {
+                        // We need to do a deep copy of the value so it won't copy the reference itself,
+                        // but rather the actual values we are interested in.
+                        property.SetValue(this, originalValue.deep_copy(), new object[0]);
+                    }
+                    else
+                    {
+                        property.SetValue(this, null, new object[0]);
+                    }
+                }
+                catch (Exception ex)
+                {
+                    throw new ApplicationException("Unable to restore the value for the property '{0}'.".format_with(property.Name), ex);
+                }
+            }
+
+            if (removeBackup)
+            {
+                // It is enough to set the original configuration to null to
+                // allow GC to clean it up the next time it runs on the stored Generation
+                // Heap Table.
+                _originalConfiguration = null;
+            }
+        }
+
         // overrides
         public override string ToString()
         {
@@ -237,6 +320,7 @@ private void append_output(StringBuilder propertyValues, string append)
 
         [Obsolete("Side by Side installation is deprecated, and is pending removal in v2.0.0")]
         public bool AllowMultipleVersions { get; set; }
+
         public bool AllowDowngrade { get; set; }
         public bool ForceDependencies { get; set; }
         public string DownloadChecksum { get; set; }

src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs

@@ -22,7 +22,6 @@ namespace chocolatey.infrastructure.app.services
     using System.IO;
     using System.Linq;
     using System.Text;
-    using builders;
     using commandline;
     using configuration;
     using domain;
@@ -31,8 +30,8 @@ namespace chocolatey.infrastructure.app.services
     using infrastructure.events;
     using infrastructure.services;
     using logging;
-    using NuGet;
     using nuget;
+    using NuGet;
     using platforms;
     using results;
     using tolerance;
@@ -91,6 +90,7 @@ system admins into something amazing! Singlehandedly solve your
  organization's struggles with software management and save the day!
  https://chocolatey.org/compare"
 };
+
         private const string PRO_BUSINESS_LIST_MESSAGE = @"
 Did you know Pro / Business automatically syncs with Programs and
  Features? Learn more about Package Synchronizer at
@@ -742,7 +742,7 @@ private IEnumerable<ChocolateyConfiguration> get_packages_from_config(string pac
                     if (pkgSettings.IgnoreDependencies) packageConfig.IgnoreDependencies = true;
                     if (pkgSettings.ApplyInstallArgumentsToDependencies) packageConfig.ApplyInstallArgumentsToDependencies = true;
                     if (pkgSettings.ApplyPackageParametersToDependencies) packageConfig.ApplyPackageParametersToDependencies = true;
-                    
+
                     if (!string.IsNullOrWhiteSpace(pkgSettings.Source) && has_source_type(pkgSettings.Source))
                     {
                         packageConfig.SourceType = pkgSettings.Source;

src/chocolatey/infrastructure.app/services/NugetService.cs

@@ -19,25 +19,24 @@ namespace chocolatey.infrastructure.app.services
     using System;
     using System.Collections.Concurrent;
     using System.Collections.Generic;
-    using System.Globalization;
     using System.IO;
     using System.Linq;
     using System.Net;
-    using NuGet;
     using adapters;
+    using chocolatey.infrastructure.app.utility;
     using commandline;
     using configuration;
     using domain;
     using guards;
     using logging;
     using nuget;
+    using NuGet;
     using platforms;
     using results;
     using tolerance;
     using DateTime = adapters.DateTime;
     using Environment = System.Environment;
     using IFileSystem = filesystem.IFileSystem;
-    using chocolatey.infrastructure.app.utility;
 
     //todo: #2575 - this monolith is too large. Refactor once test coverage is up.
 
@@ -436,13 +435,13 @@ public virtual ConcurrentDictionary<string, PackageResult> install_run(Chocolate
                 uninstallSuccessAction: null,
                 addUninstallHandler: true);
 
-            var originalConfig = config.deep_copy();
+            config.start_backup();
 
             foreach (string packageName in packageNames.or_empty_list_if_null())
             {
-                // reset config each time through
-                config = originalConfig.deep_copy();
-
+                // We need to ensure we are using a clean configuration file
+                // before we start reading it.
+                config.reset_config();
                 //todo: #2577 get smarter about realizing multiple versions have been installed before and allowing that
                 IPackage installedPackage = packageManager.LocalRepository.FindPackage(packageName);
 
@@ -555,6 +554,11 @@ Version was specified as '{0}'. It is possible that version
                 }
             }
 
+            // Reset the configuration again once we are completely done with the processing of
+            // configurations, and make sure that we are removing any backup that was created
+            // as part of this run.
+            config.reset_config(removeBackup: true);
+
             return packageInstalls;
         }
 
@@ -621,12 +625,13 @@ public virtual ConcurrentDictionary<string, PackageResult> upgrade_run(Chocolate
             set_package_names_if_all_is_specified(config, () => { config.IgnoreDependencies = true; });
             config.IgnoreDependencies = configIgnoreDependencies;
 
-            var originalConfig = config.deep_copy();
+            config.start_backup();
 
             foreach (string packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null())
             {
-                // reset config each time through
-                config = originalConfig.deep_copy();
+                // We need to ensure we are using a clean configuration file
+                // before we start reading it.
+                config.reset_config();
 
                 IPackage installedPackage = packageManager.LocalRepository.FindPackage(packageName);
 
@@ -878,6 +883,11 @@ public virtual ConcurrentDictionary<string, PackageResult> upgrade_run(Chocolate
                 }
             }
 
+            // Reset the configuration again once we are completely done with the processing of
+            // configurations, and make sure that we are removing any backup that was created
+            // as part of this run.
+            config.reset_config(removeBackup: true);
+
             return packageInstalls;
         }
 
@@ -897,12 +907,13 @@ public virtual ConcurrentDictionary<string, PackageResult> get_outdated(Chocolat
             set_package_names_if_all_is_specified(config, () => { config.IgnoreDependencies = true; });
             var packageNames = config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null().ToList();
 
-            var originalConfig = config.deep_copy();
+            config.start_backup();
 
             foreach (var packageName in packageNames)
             {
-                // reset config each time through
-                config = originalConfig.deep_copy();
+                // We need to ensure we are using a clean configuration file
+                // before we start reading it.
+                config.reset_config();
 
                 var installedPackage = packageManager.LocalRepository.FindPackage(packageName);
                 var pkgInfo = _packageInfoService.get_package_information(installedPackage);
@@ -961,6 +972,11 @@ public virtual ConcurrentDictionary<string, PackageResult> get_outdated(Chocolat
                 }
             }
 
+            // Reset the configuration again once we are completely done with the processing of
+            // configurations, and make sure that we are removing any backup that was created
+            // as part of this run.
+            config.reset_config(removeBackup: true);
+
             return outdatedPackages;
         }
 
@@ -1374,12 +1390,13 @@ public virtual ConcurrentDictionary<string, PackageResult> uninstall_run(Chocola
                     config.ForceDependencies = false;
                 });
 
-            var originalConfig = config.deep_copy();
+            config.start_backup();
 
             foreach (string packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null())
             {
-                // reset config each time through
-                config = originalConfig.deep_copy();
+                // We need to ensure we are using a clean configuration file
+                // before we start reading it.
+                config.reset_config();
 
                 IList<IPackage> installedPackageVersions = new List<IPackage>();
                 if (string.IsNullOrWhiteSpace(config.Version))
@@ -1507,6 +1524,11 @@ public virtual ConcurrentDictionary<string, PackageResult> uninstall_run(Chocola
                 }
             }
 
+            // Reset the configuration again once we are completely done with the processing of
+            // configurations, and make sure that we are removing any backup that was created
+            // as part of this run.
+            config.reset_config(removeBackup: true);
+
             return packageUninstalls;
         }
 

tests/Vagrantfile

@@ -60,6 +60,7 @@ Vagrant.configure("2") do |config|
 
     Write-Host "Build complete. Executing tests."
     # $env:TEST_KITCHEN = 1
+    $env:VM_RUNNING = 1
     ./Invoke-Tests.ps1
   SHELL
 end