Skip to content

Commit

Permalink
(chocolatey#2884) Store config backups as a stack
Browse files Browse the repository at this point in the history
In some cases, code paths may attempt to take or restore multiple config
backups. Previously, this would always result in an error.

With this change, as long as the code paths correctly use start_backup
and reset_config, nested levels of backups will work correctly instead
of unpredictably interfering with each other.
  • Loading branch information
vexx32 committed Nov 23, 2022
1 parent b273d8b commit 43e9faf
Showing 1 changed file with 19 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace chocolatey.infrastructure.app.configuration
public class ChocolateyConfiguration
{
[NonSerialized]
private ChocolateyConfiguration _originalConfiguration;
private Stack<ChocolateyConfiguration> _configurationBackups;

public ChocolateyConfiguration()
{
Expand Down Expand Up @@ -67,9 +67,14 @@ public ChocolateyConfiguration()
/// <exception cref="System.Runtime.Serialization.SerializationException">One or more objects in the class or child classes are not serializable.</exception>
public void start_backup()
{
if (_configurationBackups == null)
{
_configurationBackups = new Stack<ChocolateyConfiguration>();
}

// We do this the easy way to ensure that we have a clean copy
// of the original configuration file.
_originalConfiguration = this.deep_copy();
_configurationBackups.Push(this.deep_copy());
}

/// <summary>
Expand All @@ -86,33 +91,40 @@ public void start_backup()
/// </remarks>
public void reset_config(bool removeBackup = false)
{
if (_originalConfiguration == null)
if (_configurationBackups == null || _configurationBackups.Count == 0)
{
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.
this.Log().Debug("Requested removal of a configuration backup that does not exist: the backup stack is empty.");
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();
var t = typeof(ChocolateyConfiguration);

var backup = removeBackup ? _configurationBackups.Pop() : _configurationBackups.Peek();

foreach (var property in t.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
if (property.Name == nameof(_configurationBackups))
{
continue;
}

try
{
var originalValue = property.GetValue(_originalConfiguration, new object[0]);
var originalValue = property.GetValue(backup, 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.
// for any reference types. We won't have any duplicate references because the backup is being discarded.
property.SetValue(this, originalValue, new object[0]);
}
else if (originalValue != null)
Expand All @@ -131,14 +143,6 @@ public void reset_config(bool removeBackup = false)
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
Expand Down

0 comments on commit 43e9faf

Please sign in to comment.