Skip to content

Commit

Permalink
Fix pass by reference bug when applying changes to all installers (#164)
Browse files Browse the repository at this point in the history
* fix all installer serialization bug

* add generic implementation
  • Loading branch information
ryfu-msft authored Sep 13, 2021
1 parent 7342087 commit a3ef904
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 3 deletions.
46 changes: 43 additions & 3 deletions src/WingetCreateCLI/Commands/NewCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Microsoft.WingetCreateCLI.Commands
{
using System;
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.IO;
Expand Down Expand Up @@ -312,17 +313,56 @@ private static List<string> GenerateInstallerSelectionList(List<Installer> insta

private static void ApplyChangesToIndividualInstallers(Installer installerCopy, List<Installer> installers)
{
// Skip architecture as the default value when instantiated is x86, which we don't want to override other installer archs with.
// Skip architecture as the default value when instantiated is x86.
var modifiedFields = installerCopy.GetType().GetProperties()
.Select(prop => prop)
.Where(pi => pi.GetValue(installerCopy) != null && pi.Name != nameof(Installer.Architecture));
.Where(pi =>
pi.GetValue(installerCopy) != null &&
pi.Name != nameof(Installer.Architecture) &&
pi.Name != nameof(Installer.AdditionalProperties));

foreach (var field in modifiedFields)
{
foreach (Installer installer in installers)
{
var fieldValue = field.GetValue(installerCopy);
installer.GetType().GetProperty(field.Name).SetValue(installer, fieldValue);
var prop = installer.GetType().GetProperty(field.Name);
if (prop.PropertyType.IsValueType)
{
prop.SetValue(installer, fieldValue);
}
else if (fieldValue is IList list)
{
prop.SetValue(installer, list.DeepClone());
}
else if (fieldValue is Dependencies dependencies)
{
ApplyDependencyChangesToInstaller(dependencies, installer);
}
}
}
}

/// <summary>
/// Clones any non-null property values of the dependencies object and assigns them to the provided installer object.
/// </summary>
/// <param name="dependencies">Dependencies object with new values.</param>
/// <param name="installer">Installer object to assign new changes to.</param>
private static void ApplyDependencyChangesToInstaller(Dependencies dependencies, Installer installer)
{
var modifiedFields = dependencies.GetType().GetProperties()
.Select(prop => prop)
.Where(pi => pi.GetValue(dependencies) != null);

foreach (var field in modifiedFields.Where(f => f.Name != nameof(Installer.AdditionalProperties)))
{
var fieldValue = field.GetValue(dependencies);
installer.Dependencies ??= new Dependencies();
var prop = installer.Dependencies.GetType().GetProperty(field.Name);

if (fieldValue is IList list)
{
prop.SetValue(installer.Dependencies, list.DeepClone());
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions src/WingetCreateCore/Common/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.WingetCreateCore.Common
using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.Serialization;

Expand Down Expand Up @@ -103,5 +104,42 @@ public static bool IsEmptyObject(this object o)
{
return !o.GetType().GetProperties().Select(pi => pi.GetValue(o)).Where(value => !value.IsDictionary() && value != null).Any();
}

/// <summary>
/// Creates a new List object that is a deep clone copy of the current list object instance.
/// </summary>
/// <param name="list">List object to be cloned.</param>
/// <returns>New List object that is a copy of this instance.</returns>
public static IList DeepClone(this IList list)
{
if (list == null)
{
return null;
}

Type elementType = list.GetType().GetGenericArguments()[0];
Type listType = typeof(List<>).MakeGenericType(elementType);
var newList = (IList)Activator.CreateInstance(listType);
foreach (var item in list)
{
object toAdd;
if (item.GetType().IsValueType)
{
toAdd = item;
}
else if (item is ICloneable clonableItem)
{
toAdd = clonableItem.Clone();
}
else
{
throw new InvalidDataException();
}

newList.Add(toAdd);
}

return newList;
}
}
}
22 changes: 22 additions & 0 deletions src/WingetCreateCore/Models/Partials.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license.

namespace Microsoft.WingetCreateCore.Models.Installer
{
using System;

/// <summary>
/// Partial class that implements cloning functionality to the PackageDependencies class.
/// </summary>
public partial class PackageDependencies : ICloneable
{
/// <summary>
/// Creates a new PackageDependencies object that is a copy of the current instance.
/// </summary>
/// <returns>A new PackageDependencies object that is a copy of the current instance.</returns>
public object Clone()
{
return new PackageDependencies { MinimumVersion = this.MinimumVersion, PackageIdentifier = this.PackageIdentifier };
}
}
}

0 comments on commit a3ef904

Please sign in to comment.