diff --git a/Kyoo.Common/Utility/Merger.cs b/Kyoo.Common/Utility/Merger.cs index 1918ba41..3c2d6247 100644 --- a/Kyoo.Common/Utility/Merger.cs +++ b/Kyoo.Common/Utility/Merger.cs @@ -44,22 +44,89 @@ namespace Kyoo /// The second dictionary to merge /// The type of the keys in dictionaries /// The type of values in the dictionaries - /// A dictionary containing the result of the merge. + /// The first dictionary with the missing elements of . + /// [ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)] public static IDictionary MergeDictionaries([CanBeNull] IDictionary first, [CanBeNull] IDictionary second) + { + return MergeDictionaries(first, second, out bool _); + } + + /// + /// Merge two dictionary, if the same key is found on both dictionary, the values of the first one is kept. + /// + /// The first dictionary to merge + /// The second dictionary to merge + /// + /// true if a new items has been added to the dictionary, false otherwise. + /// + /// The type of the keys in dictionaries + /// The type of values in the dictionaries + /// The first dictionary with the missing elements of . + [ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)] + public static IDictionary MergeDictionaries([CanBeNull] IDictionary first, + [CanBeNull] IDictionary second, + out bool hasChanged) { if (first == null) + { + hasChanged = true; return second; + } + + hasChanged = false; if (second == null) return first; - Dictionary 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; + } + + /// + /// Merge two dictionary, if the same key is found on both dictionary, the values of the second one is kept. + /// + /// + /// The only difference in this function compared to + /// + /// is the way is calculated and the order of the arguments. + /// + /// MergeDictionaries(first, second); + /// + /// will do the same thing as + /// + /// CompleteDictionaries(second, first, out bool _); + /// + /// + /// The first dictionary to merge + /// The second dictionary to merge + /// + /// true if a new items has been added to the dictionary, false otherwise. + /// + /// The type of the keys in dictionaries + /// The type of values in the dictionaries + /// + /// A dictionary with the missing elements of + /// set to those of . + /// + [ContractAnnotation("first:notnull => notnull; second:notnull => notnull", true)] + public static IDictionary CompleteDictionaries([CanBeNull] IDictionary first, + [CanBeNull] IDictionary 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; } /// @@ -91,7 +158,9 @@ namespace Kyoo /// /// 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 for more details). + /// (see + /// + /// for more details). /// At the end, the OnMerge method of first will be called if first is a /// /// @@ -135,17 +204,24 @@ namespace Kyoo object defaultValue = property.GetCustomAttribute()?.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[] parameters = { + property.GetValue(first), + value, + false + }; + object newDictionary = Utility.RunGenericMethod( 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[] parameters = { + oldValue, + newValue, + false + }; + object newDictionary = Utility.RunGenericMethod( 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)) diff --git a/Kyoo.Common/Utility/Utility.cs b/Kyoo.Common/Utility/Utility.cs index 2f7e14ad..72d25fd5 100644 --- a/Kyoo.Common/Utility/Utility.cs +++ b/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); } /// diff --git a/Kyoo.Tests/Database/SpecificTests/CollectionsTests.cs b/Kyoo.Tests/Database/SpecificTests/CollectionsTests.cs index 3fb9e72f..f1f773ad 100644 --- a/Kyoo.Tests/Database/SpecificTests/CollectionsTests.cs +++ b/Kyoo.Tests/Database/SpecificTests/CollectionsTests.cs @@ -99,7 +99,7 @@ namespace Kyoo.Tests.Database value.Name = "New Title"; value.Images = new Dictionary { - [Images.Poster] = "poster" + [Images.Poster] = "new-poster" }; await _repository.Edit(value, false); diff --git a/Kyoo.Tests/Utility/MergerTests.cs b/Kyoo.Tests/Utility/MergerTests.cs index 4e0ed6e7..6aed3eb2 100644 --- a/Kyoo.Tests/Utility/MergerTests.cs +++ b/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 first = new() + { + [Images.Logo] = "logo", + [Images.Poster] = "poster" + }; + Dictionary second = new() + { + [Images.Poster] = "new-poster", + [Images.Thumbnail] = "thumbnails" + }; + IDictionary 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 first = new() + { + [Images.Poster] = "poster" + }; + Dictionary second = new() + { + [Images.Poster] = "new-poster", + }; + IDictionary 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 Backing; + + [UsedImplicitly] public Dictionary Dictionary + { + get => Backing; + set + { + Backing = value; + KAssert.Fail(); + } + } + } + + [Fact] + public void CompleteDictionaryNoChangeNoSetTest() + { + TestMergeSetter first = new() + { + Backing = new Dictionary + { + [2] = 3 + } + }; + TestMergeSetter second = new() + { + Backing = new Dictionary() + }; + 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 + { + [2] = 3 + } + }; + TestMergeSetter second = new() + { + Backing = new Dictionary() + }; + Merger.Merge(first, second); + // This should no call the setter of first so the test should pass. + } } } \ No newline at end of file diff --git a/Kyoo/Controllers/Repositories/CollectionRepository.cs b/Kyoo/Controllers/Repositories/CollectionRepository.cs index 016028cb..13273ab3 100644 --- a/Kyoo/Controllers/Repositories/CollectionRepository.cs +++ b/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().AttachRange(resource.ExternalIDs); } @@ -82,7 +82,7 @@ namespace Kyoo.Controllers /// protected override async Task EditRelations(Collection resource, Collection changed, bool resetOld) { - await Validate(resource); + await Validate(changed); if (changed.ExternalIDs != null || resetOld) {