EF: Fixing multiple same-entity references

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
 			}
 		}
 
+		/// <summary>
+		/// Return the first resource with the given slug that is currently tracked by this context.
+		/// This allow one to limit redundant calls to <see cref="IRepository{T}.CreateIfNotExists"/> during the
+		/// same transaction and prevent fails from EF when two same entities are being tracked. 
+		/// </summary>
+		/// <param name="slug">The slug of the resource to check</param>
+		/// <typeparam name="T">The type of entity to check</typeparam>
+		/// <returns>The local entity representing the resource with the given slug if it exists or null.</returns>
+		[CanBeNull]
+		public T LocalEntity<T>(string slug)
+			where T : class, IResource
+		{
+			return ChangeTracker.Entries<T>()
+				.FirstOrDefault(x => x.Entity.Slug == slug)
+				?.Entity;
+		}
+
 		/// <summary>
 		/// Check if the exception is a duplicated exception.
 		/// </summary>
@@ -515,14 +533,12 @@ namespace Kyoo
 		/// </summary>
 		private void DiscardChanges()
 		{
-			foreach (EntityEntry entry in ChangeTracker.Entries().Where(x => x.State != EntityState.Unchanged
+			foreach (EntityEntry entry in ChangeTracker.Entries().Where(x => x.State != EntityState.Detached))
-			                                                                 && x.State != EntityState.Detached))
 			{
 				entry.State = EntityState.Detached;
 			}
 		}
 		
-
 		/// <summary>
 		/// Perform a case insensitive like operation.
 		/// </summary>

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<Provider>(id.Provider.Slug)
+						?? await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-					_database.Entry(id.Provider).State = EntityState.Unchanged;
 				}
 				_database.MetadataIds<Collection>().AttachRange(resource.ExternalIDs);
 			}

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<Provider>(id.Provider.Slug)
+						?? await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-					_database.Entry(id.Provider).State = EntityState.Unchanged;
 				}
 				_database.MetadataIds<Episode>().AttachRange(resource.ExternalIDs);
 			}

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<People>().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);
+				foreach (MetadataID id in resource.ExternalIDs)
+				{
+					id.Provider = _database.LocalEntity<Provider>(id.Provider.Slug)
+						?? await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-				_database.Entry(id.Provider).State = EntityState.Detached;
+				}
-			});
+				_database.MetadataIds<People>().AttachRange(resource.ExternalIDs);
-			await resource.Roles.ForEachAsync(async role =>
+			}
+
+			if (resource.Roles != null)
 			{
-				role.Show = await _shows.Value.CreateIfNotExists(role.Show);
+				foreach (PeopleRole role in resource.Roles)
+				{
+					role.Show = _database.LocalEntity<Show>(role.Show.Slug) 
+						?? await _shows.Value.CreateIfNotExists(role.Show);
 					role.ShowID = role.Show.ID;
-				_database.Entry(role.Show).State = EntityState.Detached;
+					_database.Entry(role).State = EntityState.Added;
-			});
+				}
+			}
 		}
 
 		/// <inheritdoc />
 		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);
 		}
 
 		/// <inheritdoc />

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<Provider>(id.Provider.Slug)
+						?? await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-					_database.Entry(id.Provider).State = EntityState.Unchanged;
 				}
 				_database.MetadataIds<Season>().AttachRange(resource.ExternalIDs);
 			}

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<Provider>(id.Provider.Slug)
+						?? await _providers.CreateIfNotExists(id.Provider);
 					id.ProviderID = id.Provider.ID;
-					_database.Entry(id.Provider).State = EntityState.Detached;
 				}
 				_database.MetadataIds<Show>().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<People>(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;
 				}
 			}

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<Studio>().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 => 
+			if (resource.ExternalIDs != null)
 			{
-				x.Provider = await _providers.CreateIfNotExists(x.Provider);
+				foreach (MetadataID id in resource.ExternalIDs)
-				x.ProviderID = x.Provider.ID;
+				{
-				_database.Entry(x.Provider).State = EntityState.Detached;
+					id.Provider = _database.LocalEntity<Provider>(id.Provider.Slug)
-			});
+						?? await _providers.CreateIfNotExists(id.Provider);
+					id.ProviderID = id.Provider.ID;
+				}
+				_database.MetadataIds<Studio>().AttachRange(resource.ExternalIDs);
+			}
 		}
 		
 		/// <inheritdoc />

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<People>();
+			value.ExternalIDs = new[]
+			{
+				new MetadataID
+				{
+					Provider = TestSample.Get<Provider>(),
+					Link = "link",
+					DataID = "id"
+				},
+				new MetadataID
+				{
+					Provider = TestSample.GetNew<Provider>(),
+					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<People>().Slug);
+			value.Name = "New Name";
+			value.Images = new Dictionary<int, string>
+			{
+				[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<People>().Slug);
+			value.ExternalIDs = new[]
+			{
+				new MetadataID
+				{
+					Provider = TestSample.Get<Provider>(),
+					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<People>().Slug);
+			value.ExternalIDs = new List<MetadataID>
+			{
+				new()
+				{
+					Provider = TestSample.Get<Provider>(),
+					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<Provider>(),
+				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<People> ret = await _repository.Search(query);
+			KAssert.DeepEqual(value, ret.First());
+		}
 	}
 }

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<int, string>
+					{
+						[Images.Logo] = "Old Logo",
+						[Images.Poster] = "Old poster"
+					}
+				}
 			}
 		};