Merger: Fixing Dictionary complete and merge

Kyoo.Common/Utility/Merger.cs

@@ -44,22 +44,89 @@ namespace Kyoo
 		/// <param name="second">The second dictionary to merge</param>
 		/// <typeparam name="T">The type of the keys in dictionaries</typeparam>
 		/// <typeparam name="T2">The type of values in the dictionaries</typeparam>
-		/// <returns>A dictionary containing the result of the merge.</returns>
+		/// <returns>The first dictionary with the missing elements of <paramref name="second"/>.</returns>
+		/// <seealso cref="MergeDictionaries{T,T2}(System.Collections.Generic.IDictionary{T,T2},System.Collections.Generic.IDictionary{T,T2},out bool)"/>
 		[ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)]
 		public static IDictionary<T, T2> MergeDictionaries<T, T2>([CanBeNull] IDictionary<T, T2> first,
 			[CanBeNull] IDictionary<T, T2> second)
+		{
+			return MergeDictionaries(first, second, out bool _);
+		}
+
+		/// <summary>
+		/// Merge two dictionary, if the same key is found on both dictionary, the values of the first one is kept.
+		/// </summary>
+		/// <param name="first">The first dictionary to merge</param>
+		/// <param name="second">The second dictionary to merge</param>
+		/// <param name="hasChanged">
+		/// <c>true</c> if a new items has been added to the dictionary, <c>false</c> otherwise.
+		/// </param>
+		/// <typeparam name="T">The type of the keys in dictionaries</typeparam>
+		/// <typeparam name="T2">The type of values in the dictionaries</typeparam>
+		/// <returns>The first dictionary with the missing elements of <paramref name="second"/>.</returns>
+		[ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)]
+		public static IDictionary<T, T2> MergeDictionaries<T, T2>([CanBeNull] IDictionary<T, T2> first,
+			[CanBeNull] IDictionary<T, T2> second,
+			out bool hasChanged)
 		{
 			if (first == null)
+			{
+				hasChanged = true;
 				return second;
+			}
+
+			hasChanged = false;
 			if (second == null)
 				return first;
-			Dictionary<T, T2> merged = new();
-			merged.EnsureCapacity(first.Count + second.Count);
-			foreach ((T key, T2 value) in first)
-				merged.Add(key, value);
 			foreach ((T key, T2 value) in second)
-				merged.TryAdd(key, value);
-			return merged;
+				hasChanged |= first.TryAdd(key, value);
+			return first;
+		}
+
+		/// <summary>
+		/// Merge two dictionary, if the same key is found on both dictionary, the values of the second one is kept.
+		/// </summary>
+		/// <remarks>
+		/// The only difference in this function compared to
+		/// <see cref="MergeDictionaries{T,T2}(System.Collections.Generic.IDictionary{T,T2},System.Collections.Generic.IDictionary{T,T2}, out bool)"/>
+		/// is the way <paramref name="hasChanged"/> is calculated and the order of the arguments.
+		/// <code>
+		/// MergeDictionaries(first, second);
+		/// </code>
+		/// will do the same thing as
+		/// <code>
+		/// CompleteDictionaries(second, first, out bool _);
+		/// </code>
+		/// </remarks>
+		/// <param name="first">The first dictionary to merge</param>
+		/// <param name="second">The second dictionary to merge</param>
+		/// <param name="hasChanged">
+		/// <c>true</c> if a new items has been added to the dictionary, <c>false</c> otherwise.
+		/// </param>
+		/// <typeparam name="T">The type of the keys in dictionaries</typeparam>
+		/// <typeparam name="T2">The type of values in the dictionaries</typeparam>
+		/// <returns>
+		/// A dictionary with the missing elements of <paramref name="second"/>
+		/// set to those of <paramref name="first"/>.
+		/// </returns>
+		[ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)]
+		public static IDictionary<T, T2> CompleteDictionaries<T, T2>([CanBeNull] IDictionary<T, T2> first,
+			[CanBeNull] IDictionary<T, T2> second,
+			out bool hasChanged)
+		{
+			if (first == null)
+			{
+				hasChanged = true;
+				return second;
+			}
+
+			hasChanged = false;
+			if (second == null)
+				return first;
+			hasChanged = second.Any(x => !x.Value.Equals(first[x.Key]));
+			foreach ((T key, T2 value) in first)
+				second.TryAdd(key, value);
+			return second;
 		}
 
 		/// <summary>
@@ -91,7 +158,9 @@ namespace Kyoo
 		/// <summary>
 		/// Set every non-default values of seconds to the corresponding property of second.
 		/// Dictionaries are handled like anonymous objects with a property per key/pair value
-		/// (see <see cref="MergeDictionaries{T,T2}"/> for more details).
+		/// (see
+		/// <see cref="MergeDictionaries{T,T2}(System.Collections.Generic.IDictionary{T,T2},System.Collections.Generic.IDictionary{T,T2})"/>
+		/// for more details).
 		/// At the end, the OnMerge method of first will be called if first is a <see cref="IOnMerge"/>
 		/// </summary>
 		/// <remarks>
@@ -135,17 +204,24 @@ namespace Kyoo
 				object defaultValue = property.GetCustomAttribute<DefaultValueAttribute>()?.Value
 					?? property.PropertyType.GetClrDefault();
 
-				if (value?.Equals(defaultValue) != false || value == property.GetValue(first))
+				if (value?.Equals(defaultValue) != false || value.Equals(property.GetValue(first)))
 					continue;
 				if (Utility.IsOfGenericType(property.PropertyType, typeof(IDictionary<,>)))
 				{
 					Type[] dictionaryTypes = Utility.GetGenericDefinition(property.PropertyType, typeof(IDictionary<,>))
 						.GenericTypeArguments;
-					property.SetValue(first, Utility.RunGenericMethod<object>(
+					object[] parameters = {
+						property.GetValue(first),
+						value,
+						false
+					};
+					object newDictionary = Utility.RunGenericMethod<object>(
 						typeof(Merger),
-						nameof(MergeDictionaries),
+						nameof(CompleteDictionaries),
 						dictionaryTypes,
-						value, property.GetValue(first)));
+						parameters);
+					if ((bool)parameters[2])
+						property.SetValue(first, newDictionary);
 				}
 				else
 					property.SetValue(first, value);
@@ -205,11 +281,18 @@ namespace Kyoo
 				{
 					Type[] dictionaryTypes = Utility.GetGenericDefinition(property.PropertyType, typeof(IDictionary<,>))
 						.GenericTypeArguments;
-					property.SetValue(first, Utility.RunGenericMethod<object>(
+					object[] parameters = {
+						oldValue,
+						newValue,
+						false
+					};
+					object newDictionary = Utility.RunGenericMethod<object>(
 						typeof(Merger),
 						nameof(MergeDictionaries),
 						dictionaryTypes,
-						oldValue, newValue));
+						parameters);
+					if ((bool)parameters[2])
+						property.SetValue(first, newDictionary);
 				}
 				else if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)
 				         && property.PropertyType != typeof(string))

Kyoo.Common/Utility/Utility.cs

@@ -325,7 +325,7 @@ namespace Kyoo
 			if (types.Length < 1)
 				throw new ArgumentException($"The {nameof(types)} array is empty. At least one type is needed.");
 			MethodInfo method = GetMethod(owner, BindingFlags.Static, methodName, types, args);
-			return (T)method.MakeGenericMethod(types).Invoke(null, args.ToArray());
+			return (T)method.MakeGenericMethod(types).Invoke(null, args);
 		}
 
 		/// <summary>

Kyoo.Tests/Database/SpecificTests/CollectionsTests.cs

@@ -99,7 +99,7 @@ namespace Kyoo.Tests.Database
 			value.Name = "New Title";
 			value.Images = new Dictionary<int, string>
 			{
-				[Images.Poster] = "poster"
+				[Images.Poster] = "new-poster"
 			};
 			await _repository.Edit(value, false);
 		

Kyoo.Tests/Utility/MergerTests.cs

@@ -2,6 +2,7 @@ using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
+using JetBrains.Annotations;
 using Kyoo.Models;
 using Kyoo.Models.Attributes;
 using Xunit;
@@ -365,5 +366,94 @@ namespace Kyoo.Tests.Utility
 			Assert.Equal("thumbnails", ret.Images[Images.Thumbnail]);
 			Assert.Equal("logo", ret.Images[Images.Logo]);
 		}
+
+		[Fact]
+		public void CompleteDictionaryOutParam()
+		{
+			Dictionary<int, string> first = new()
+			{
+				[Images.Logo] = "logo",
+				[Images.Poster] = "poster"
+			};
+			Dictionary<int, string> second = new()
+			{
+				[Images.Poster] = "new-poster",
+				[Images.Thumbnail] = "thumbnails"
+			};
+			IDictionary<int, string> ret = Merger.CompleteDictionaries(first, second, out bool changed);
+			Assert.True(changed);
+			Assert.Equal(3, ret.Count);
+			Assert.Equal("new-poster", ret[Images.Poster]);
+			Assert.Equal("thumbnails", ret[Images.Thumbnail]);
+			Assert.Equal("logo", ret[Images.Logo]);
+		}
+		
+		[Fact]
+		public void CompleteDictionaryEqualTest()
+		{
+			Dictionary<int, string> first = new()
+			{
+				[Images.Poster] = "poster"
+			};
+			Dictionary<int, string> second = new()
+			{
+				[Images.Poster] = "new-poster",
+			};
+			IDictionary<int, string> ret = Merger.CompleteDictionaries(first, second, out bool changed);
+			Assert.True(changed);
+			Assert.Equal(1, ret.Count);
+			Assert.Equal("new-poster", ret[Images.Poster]);
+		}
+
+		private class TestMergeSetter
+		{
+			public Dictionary<int, int> Backing;
+			
+			[UsedImplicitly] public Dictionary<int, int> Dictionary
+			{
+				get => Backing;
+				set
+				{
+					Backing = value;
+					KAssert.Fail();
+				}
+			}
+		}
+
+		[Fact]
+		public void CompleteDictionaryNoChangeNoSetTest()
+		{
+			TestMergeSetter first = new()
+			{
+				Backing = new Dictionary<int, int>
+				{
+					[2] = 3
+				}
+			};
+			TestMergeSetter second = new()
+			{
+				Backing = new Dictionary<int, int>()
+			};
+			Merger.Complete(first, second);
+			// This should no call the setter of first so the test should pass.
+		}
+		
+		[Fact]
+		public void MergeDictionaryNoChangeNoSetTest()
+		{
+			TestMergeSetter first = new()
+			{
+				Backing = new Dictionary<int, int>
+				{
+					[2] = 3
+				}
+			};
+			TestMergeSetter second = new()
+			{
+				Backing = new Dictionary<int, int>()
+			};
+			Merger.Merge(first, second);
+			// This should no call the setter of first so the test should pass.
+		}
 	}
 }

Kyoo/Controllers/Repositories/CollectionRepository.cs

@@ -73,7 +73,7 @@ namespace Kyoo.Controllers
 				{ 
 					id.Provider = await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-					_database.Entry(id.Provider).State = EntityState.Detached;
+					_database.Entry(id.Provider).State = EntityState.Unchanged;
 				}
 				_database.MetadataIds<Collection>().AttachRange(resource.ExternalIDs);
 			}
@@ -82,7 +82,7 @@ namespace Kyoo.Controllers
 		/// <inheritdoc />
 		protected override async Task EditRelations(Collection resource, Collection changed, bool resetOld)
 		{
-			await Validate(resource);
+			await Validate(changed);
 			
 			if (changed.ExternalIDs != null || resetOld)
 			{