Skip to content

Commit

Permalink
Consistent fixup from query when reference navigation explicitly set
Browse files Browse the repository at this point in the history
Fixes #18007

Previously it was possible for the existing instance to be replaced when a query brings back a new instance, but only for the first time the new instance is returned. This change prevents the new instance from being used in all cases, which is the agreed behavior here, and also now matches what EF6 does consistently.
  • Loading branch information
ajcvickers committed May 26, 2020
1 parent 7641cc3 commit f16d6b4
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/EFCore/ChangeTracking/Internal/NavigationFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -626,9 +626,12 @@ private void InitialFixup(
{
foreach (InternalEntityEntry dependentEntry in dependents)
{
if (!foreignKey.IsOwnership
|| (dependentEntry.EntityState != EntityState.Deleted
&& dependentEntry.EntityState != EntityState.Detached))
if ((!foreignKey.IsOwnership
|| (dependentEntry.EntityState != EntityState.Deleted
&& dependentEntry.EntityState != EntityState.Detached))
&& (!fromQuery
|| foreignKey.DependentToPrincipal == null
|| dependentEntry.GetCurrentValue(foreignKey.DependentToPrincipal) == null))
{
// Add to collection on principal indicated by FK and set inverse navigation
AddToCollection(entry, foreignKey.PrincipalToDependent, dependentEntry, fromQuery);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.Cosmos.TestUtilities;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Cosmos
{
public class OverzealousInitializationCosmosTest
: OverzealousInitializationTestBase<OverzealousInitializationCosmosTest.OverzealousInitializationCosmosFixture>
{
public OverzealousInitializationCosmosTest(OverzealousInitializationCosmosFixture fixture)
: base(fixture)
{
}

[ConditionalFact(Skip = "Issue #17246")]
public override void Fixup_does_not_ignore_eagerly_initialized_reference_navs()
{
}

public class OverzealousInitializationCosmosFixture : OverzealousInitializationFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => CosmosTestStoreFactory.Instance;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class OverzealousInitializationInMemoryTest
: OverzealousInitializationTestBase<OverzealousInitializationInMemoryTest.OverzealousInitializationInMemoryFixture>
{
public OverzealousInitializationInMemoryTest(OverzealousInitializationInMemoryFixture fixture)
: base(fixture)
{
}

public class OverzealousInitializationInMemoryFixture : OverzealousInitializationFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => InMemoryTestStoreFactory.Instance;
}
}
}
118 changes: 118 additions & 0 deletions test/EFCore.Specification.Tests/OverzealousInitializationTestBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

namespace Microsoft.EntityFrameworkCore
{
public abstract class OverzealousInitializationTestBase<TFixture> : IClassFixture<TFixture>
where TFixture : OverzealousInitializationTestBase<TFixture>.OverzealousInitializationFixtureBase, new()
{
protected OverzealousInitializationTestBase(TFixture fixture) => Fixture = fixture;

[ConditionalFact]
public virtual void Fixup_does_not_ignore_eagerly_initialized_reference_navs()
{
using var context = CreateContext();

var albums = context.Set<Album>()
.Include(e => e.Tracks)
.Include(e => e.Artist)
.OrderBy(e => e.Artist)
.ToList();

foreach (var album in albums)
{
Assert.Equal(0, album.Artist.Id);
Assert.Null(album.Artist.Name);
}
}

protected class Album
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }
public int ArtistId { get; set; }

public virtual Artist Artist { get; set; }
public virtual IList<Track> Tracks { get; set; }

public Album()
{
Artist = new Artist();
Tracks = new List<Track>();
}
}

protected class Artist
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }
public string Name { get; set; }
}

public class Track
{
[DatabaseGenerated(DatabaseGeneratedOption.None)]
public int Id { get; set; }
public int AlbumId { get; set; }
}

public class AlbumViewerContext : PoolableDbContext
{
public AlbumViewerContext(DbContextOptions<AlbumViewerContext> options)
: base(options)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Album>();
modelBuilder.Entity<Artist>();
modelBuilder.Entity<Track>();
}
}

protected TFixture Fixture { get; }

protected AlbumViewerContext CreateContext() => Fixture.CreateContext();

public abstract class OverzealousInitializationFixtureBase : SharedStoreFixtureBase<AlbumViewerContext>
{
public virtual IDisposable BeginTransaction(DbContext context) => context.Database.BeginTransaction();

protected override string StoreName { get; } = "OverzealousInitialization";

protected override void Seed(AlbumViewerContext context)
{
var artists = new[]
{
new Artist { Id = 1, Name = "Freddie" },
new Artist { Id = 2, Name = "Kendrick"},
new Artist { Id = 3, Name = "Jarvis" }
};

for (var i = 1; i <= 10; i++)
{
context.Add(new Album
{
Id = i,
Artist = artists[i % 3],
Tracks = new List<Track>
{
new Track { Id = i * 2 },
new Track { Id = i * 2 + 1}
}
});
}

context.SaveChanges();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class OverzealousInitializationSqlServerTest
: OverzealousInitializationTestBase<OverzealousInitializationSqlServerTest.OverzealousInitializationSqlServerFixture>
{
public OverzealousInitializationSqlServerTest(OverzealousInitializationSqlServerFixture fixture)
: base(fixture)
{
}

public class OverzealousInitializationSqlServerFixture : OverzealousInitializationFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => SqlServerTestStoreFactory.Instance;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.EntityFrameworkCore.TestUtilities;

namespace Microsoft.EntityFrameworkCore
{
public class OverzealousInitializationSqliteTest
: OverzealousInitializationTestBase<OverzealousInitializationSqliteTest.OverzealousInitializationSqliteFixture>
{
public OverzealousInitializationSqliteTest(OverzealousInitializationSqliteFixture fixture)
: base(fixture)
{
}

public class OverzealousInitializationSqliteFixture : OverzealousInitializationFixtureBase
{
protected override ITestStoreFactory TestStoreFactory => SqliteTestStoreFactory.Instance;
}
}
}

0 comments on commit f16d6b4

Please sign in to comment.