From 928a8d21474f820555f07d05550609d1fec54e95 Mon Sep 17 00:00:00 2001 From: Zoe Roux Date: Mon, 2 Aug 2021 00:41:05 +0200 Subject: [PATCH] EF: Fixing multiple same-entity references --- Kyoo.CommonAPI/DatabaseContext.cs | 24 +++- .../Repositories/CollectionRepository.cs | 4 +- .../Repositories/EpisodeRepository.cs | 4 +- .../Repositories/PeopleRepository.cs | 35 +++-- .../Repositories/SeasonRepository.cs | 4 +- .../Repositories/ShowRepository.cs | 8 +- .../Repositories/StudioRepository.cs | 17 ++- .../Database/SpecificTests/PeopleTests.cs | 133 ++++++++++++++++++ tests/Kyoo.Tests/Database/TestSample.cs | 14 ++ 9 files changed, 209 insertions(+), 34 deletions(-) diff --git a/Kyoo.CommonAPI/DatabaseContext.cs b/Kyoo.CommonAPI/DatabaseContext.cs index fd850d02..ad96ff78 100644 --- a/Kyoo.CommonAPI/DatabaseContext.cs +++ b/Kyoo.CommonAPI/DatabaseContext.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Linq.Expressions; using System.Threading; using System.Threading.Tasks; +using JetBrains.Annotations; using Kyoo.Controllers; using Kyoo.Models; using Kyoo.Models.Exceptions; @@ -503,6 +504,23 @@ namespace Kyoo } } + /// + /// Return the first resource with the given slug that is currently tracked by this context. + /// This allow one to limit redundant calls to during the + /// same transaction and prevent fails from EF when two same entities are being tracked. + /// + /// The slug of the resource to check + /// The type of entity to check + /// The local entity representing the resource with the given slug if it exists or null. + [CanBeNull] + public T LocalEntity(string slug) + where T : class, IResource + { + return ChangeTracker.Entries() + .FirstOrDefault(x => x.Entity.Slug == slug) + ?.Entity; + } + /// /// Check if the exception is a duplicated exception. /// @@ -515,14 +533,12 @@ namespace Kyoo /// private void DiscardChanges() { - foreach (EntityEntry entry in ChangeTracker.Entries().Where(x => x.State != EntityState.Unchanged - && x.State != EntityState.Detached)) + foreach (EntityEntry entry in ChangeTracker.Entries().Where(x => x.State != EntityState.Detached)) { entry.State = EntityState.Detached; } } - - + /// /// Perform a case insensitive like operation. /// diff --git a/Kyoo/Controllers/Repositories/CollectionRepository.cs b/Kyoo/Controllers/Repositories/CollectionRepository.cs index 13273ab3..e4250c8f 100644 --- a/Kyoo/Controllers/Repositories/CollectionRepository.cs +++ b/Kyoo/Controllers/Repositories/CollectionRepository.cs @@ -71,9 +71,9 @@ namespace Kyoo.Controllers { foreach (MetadataID id in resource.ExternalIDs) { - id.Provider = await _providers.CreateIfNotExists(id.Provider); + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); id.ProviderID = id.Provider.ID; - _database.Entry(id.Provider).State = EntityState.Unchanged; } _database.MetadataIds().AttachRange(resource.ExternalIDs); } diff --git a/Kyoo/Controllers/Repositories/EpisodeRepository.cs b/Kyoo/Controllers/Repositories/EpisodeRepository.cs index d50fe8c4..92a573a4 100644 --- a/Kyoo/Controllers/Repositories/EpisodeRepository.cs +++ b/Kyoo/Controllers/Repositories/EpisodeRepository.cs @@ -170,9 +170,9 @@ namespace Kyoo.Controllers { foreach (MetadataID id in resource.ExternalIDs) { - id.Provider = await _providers.CreateIfNotExists(id.Provider); + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); id.ProviderID = id.Provider.ID; - _database.Entry(id.Provider).State = EntityState.Unchanged; } _database.MetadataIds().AttachRange(resource.ExternalIDs); } diff --git a/Kyoo/Controllers/Repositories/PeopleRepository.cs b/Kyoo/Controllers/Repositories/PeopleRepository.cs index a55c8e0e..3b6d3f87 100644 --- a/Kyoo/Controllers/Repositories/PeopleRepository.cs +++ b/Kyoo/Controllers/Repositories/PeopleRepository.cs @@ -62,7 +62,6 @@ namespace Kyoo.Controllers { await base.Create(obj); _database.Entry(obj).State = EntityState.Added; - obj.ExternalIDs.ForEach(x => _database.MetadataIds().Attach(x)); await _database.SaveChangesAsync($"Trying to insert a duplicated people (slug {obj.Slug} already exists)."); return obj; } @@ -71,23 +70,35 @@ namespace Kyoo.Controllers protected override async Task Validate(People resource) { await base.Validate(resource); - await resource.ExternalIDs.ForEachAsync(async id => + + if (resource.ExternalIDs != null) { - id.Provider = await _providers.CreateIfNotExists(id.Provider); - id.ProviderID = id.Provider.ID; - _database.Entry(id.Provider).State = EntityState.Detached; - }); - await resource.Roles.ForEachAsync(async role => + foreach (MetadataID id in resource.ExternalIDs) + { + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); + id.ProviderID = id.Provider.ID; + } + _database.MetadataIds().AttachRange(resource.ExternalIDs); + } + + if (resource.Roles != null) { - role.Show = await _shows.Value.CreateIfNotExists(role.Show); - role.ShowID = role.Show.ID; - _database.Entry(role.Show).State = EntityState.Detached; - }); + foreach (PeopleRole role in resource.Roles) + { + role.Show = _database.LocalEntity(role.Show.Slug) + ?? await _shows.Value.CreateIfNotExists(role.Show); + role.ShowID = role.Show.ID; + _database.Entry(role).State = EntityState.Added; + } + } } /// protected override async Task EditRelations(People resource, People changed, bool resetOld) { + await Validate(changed); + if (changed.Roles != null || resetOld) { await Database.Entry(resource).Collection(x => x.Roles).LoadAsync(); @@ -98,9 +109,7 @@ namespace Kyoo.Controllers { await Database.Entry(resource).Collection(x => x.ExternalIDs).LoadAsync(); resource.ExternalIDs = changed.ExternalIDs; - } - await base.EditRelations(resource, changed, resetOld); } /// diff --git a/Kyoo/Controllers/Repositories/SeasonRepository.cs b/Kyoo/Controllers/Repositories/SeasonRepository.cs index 88abfaea..0ac7e759 100644 --- a/Kyoo/Controllers/Repositories/SeasonRepository.cs +++ b/Kyoo/Controllers/Repositories/SeasonRepository.cs @@ -107,9 +107,9 @@ namespace Kyoo.Controllers { foreach (MetadataID id in resource.ExternalIDs) { - id.Provider = await _providers.CreateIfNotExists(id.Provider); + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); id.ProviderID = id.Provider.ID; - _database.Entry(id.Provider).State = EntityState.Unchanged; } _database.MetadataIds().AttachRange(resource.ExternalIDs); } diff --git a/Kyoo/Controllers/Repositories/ShowRepository.cs b/Kyoo/Controllers/Repositories/ShowRepository.cs index d74c2fe3..3ef8747c 100644 --- a/Kyoo/Controllers/Repositories/ShowRepository.cs +++ b/Kyoo/Controllers/Repositories/ShowRepository.cs @@ -102,9 +102,9 @@ namespace Kyoo.Controllers { foreach (MetadataID id in resource.ExternalIDs) { - id.Provider = await _providers.CreateIfNotExists(id.Provider); + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); id.ProviderID = id.Provider.ID; - _database.Entry(id.Provider).State = EntityState.Detached; } _database.MetadataIds().AttachRange(resource.ExternalIDs); } @@ -113,9 +113,9 @@ namespace Kyoo.Controllers { foreach (PeopleRole role in resource.People) { - role.People = await _people.CreateIfNotExists(role.People); + role.People = _database.LocalEntity(role.People.Slug) + ?? await _people.CreateIfNotExists(role.People); role.PeopleID = role.People.ID; - _database.Entry(role.People).State = EntityState.Detached; _database.Entry(role).State = EntityState.Added; } } diff --git a/Kyoo/Controllers/Repositories/StudioRepository.cs b/Kyoo/Controllers/Repositories/StudioRepository.cs index 744529bd..2c807375 100644 --- a/Kyoo/Controllers/Repositories/StudioRepository.cs +++ b/Kyoo/Controllers/Repositories/StudioRepository.cs @@ -54,7 +54,6 @@ namespace Kyoo.Controllers { await base.Create(obj); _database.Entry(obj).State = EntityState.Added; - obj.ExternalIDs.ForEach(x => _database.MetadataIds().Attach(x)); await _database.SaveChangesAsync($"Trying to insert a duplicated studio (slug {obj.Slug} already exists)."); return obj; } @@ -63,12 +62,16 @@ namespace Kyoo.Controllers protected override async Task Validate(Studio resource) { await base.Validate(resource); - await resource.ExternalIDs.ForEachAsync(async x => - { - x.Provider = await _providers.CreateIfNotExists(x.Provider); - x.ProviderID = x.Provider.ID; - _database.Entry(x.Provider).State = EntityState.Detached; - }); + if (resource.ExternalIDs != null) + { + foreach (MetadataID id in resource.ExternalIDs) + { + id.Provider = _database.LocalEntity(id.Provider.Slug) + ?? await _providers.CreateIfNotExists(id.Provider); + id.ProviderID = id.Provider.ID; + } + _database.MetadataIds().AttachRange(resource.ExternalIDs); + } } /// diff --git a/tests/Kyoo.Tests/Database/SpecificTests/PeopleTests.cs b/tests/Kyoo.Tests/Database/SpecificTests/PeopleTests.cs index 23d40bfe..3de88edf 100644 --- a/tests/Kyoo.Tests/Database/SpecificTests/PeopleTests.cs +++ b/tests/Kyoo.Tests/Database/SpecificTests/PeopleTests.cs @@ -1,5 +1,9 @@ +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; using Kyoo.Controllers; using Kyoo.Models; +using Microsoft.EntityFrameworkCore; using Xunit; using Xunit.Abstractions; @@ -33,5 +37,134 @@ namespace Kyoo.Tests.Database { _repository = Repositories.LibraryManager.PeopleRepository; } + + [Fact] + public async Task CreateWithExternalIdTest() + { + People value = TestSample.GetNew(); + value.ExternalIDs = new[] + { + new MetadataID + { + Provider = TestSample.Get(), + Link = "link", + DataID = "id" + }, + new MetadataID + { + Provider = TestSample.GetNew(), + Link = "new-provider-link", + DataID = "new-id" + } + }; + await _repository.Create(value); + + People retrieved = await _repository.Get(2); + await Repositories.LibraryManager.Load(retrieved, x => x.ExternalIDs); + Assert.Equal(2, retrieved.ExternalIDs.Count); + KAssert.DeepEqual(value.ExternalIDs.First(), retrieved.ExternalIDs.First()); + KAssert.DeepEqual(value.ExternalIDs.Last(), retrieved.ExternalIDs.Last()); + } + + [Fact] + public async Task EditTest() + { + People value = await _repository.Get(TestSample.Get().Slug); + value.Name = "New Name"; + value.Images = new Dictionary + { + [Images.Poster] = "new-poster" + }; + await _repository.Edit(value, false); + + await using DatabaseContext database = Repositories.Context.New(); + People retrieved = await database.People.FirstAsync(); + + KAssert.DeepEqual(value, retrieved); + } + + [Fact] + public async Task EditMetadataTest() + { + People value = await _repository.Get(TestSample.Get().Slug); + value.ExternalIDs = new[] + { + new MetadataID + { + Provider = TestSample.Get(), + Link = "link", + DataID = "id" + }, + }; + await _repository.Edit(value, false); + + await using DatabaseContext database = Repositories.Context.New(); + People retrieved = await database.People + .Include(x => x.ExternalIDs) + .ThenInclude(x => x.Provider) + .FirstAsync(); + + KAssert.DeepEqual(value, retrieved); + } + + [Fact] + public async Task AddMetadataTest() + { + People value = await _repository.Get(TestSample.Get().Slug); + value.ExternalIDs = new List + { + new() + { + Provider = TestSample.Get(), + Link = "link", + DataID = "id" + }, + }; + await _repository.Edit(value, false); + + { + await using DatabaseContext database = Repositories.Context.New(); + People retrieved = await database.People + .Include(x => x.ExternalIDs) + .ThenInclude(x => x.Provider) + .FirstAsync(); + + KAssert.DeepEqual(value, retrieved); + } + + value.ExternalIDs.Add(new MetadataID + { + Provider = TestSample.GetNew(), + Link = "link", + DataID = "id" + }); + await _repository.Edit(value, false); + + { + await using DatabaseContext database = Repositories.Context.New(); + People retrieved = await database.People + .Include(x => x.ExternalIDs) + .ThenInclude(x => x.Provider) + .FirstAsync(); + + KAssert.DeepEqual(value, retrieved); + } + } + + [Theory] + [InlineData("Me")] + [InlineData("me")] + [InlineData("na")] + public async Task SearchTest(string query) + { + People value = new() + { + Slug = "slug", + Name = "name", + }; + await _repository.Create(value); + ICollection ret = await _repository.Search(query); + KAssert.DeepEqual(value, ret.First()); + } } } \ No newline at end of file diff --git a/tests/Kyoo.Tests/Database/TestSample.cs b/tests/Kyoo.Tests/Database/TestSample.cs index 423a5c78..6f9a69ff 100644 --- a/tests/Kyoo.Tests/Database/TestSample.cs +++ b/tests/Kyoo.Tests/Database/TestSample.cs @@ -104,6 +104,20 @@ namespace Kyoo.Tests [Images.Logo] = "logo" } } + }, + { + typeof(People), + () => new People + { + ID = 2, + Slug = "new-person-name", + Name = "New person name", + Images = new Dictionary + { + [Images.Logo] = "Old Logo", + [Images.Poster] = "Old poster" + } + } } };