Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to turn off automatic setting of navigation properties #11564

Open
Tracked by #22954
dgxhubbard opened this issue Apr 5, 2018 · 116 comments
Open
Tracked by #22954

Option to turn off automatic setting of navigation properties #11564

dgxhubbard opened this issue Apr 5, 2018 · 116 comments

Comments

@dgxhubbard
Copy link

dgxhubbard commented Apr 5, 2018

From loading related data EF Core page is the following tip:

Entity Framework Core will automatically fix-up navigation properties to any other entities that were previously loaded into the context instance. So even if you don't explicitly include the data for a navigation property, the property may still be populated if some or all of the related entities were previously loaded.

I would like an option to turn this property off so that if I choose not to load a navigation property it is not loaded for me. The cost of transporting data that is not needed is slowing response time. If there are 70 gages and each gage has a status then that status has 70 gages. There is a reference looping option mentioned it the loading related data article which I did try but the status object still contained the list of gages which I verified through postman.

Below is an example showing the loading of data and clearing the navigation properties that are automatically "fixed up" by EF Core. I am going to need this code in all of my gets in order to drastically reduce the JSON payload size,.

When we use an Include we know the data that is needed for an object. "Fixing up" is something that is not needed. Please remove this feature or please give us a way to disable it.

Code to load data:

try
{
  using ( var context = new MyContext() )
  {
    res =
        ( from c in
            context.Gages.
                 Include ( "Status" )
             select c ).ToList ();

    // code to clear nav property loaded by EF Core
                res.ForEach ( 
                    g =>
                    {
                        if ( g != null && g.Status != null && g.Status.Gages != null )
                            g.Status.Gages.Clear ();
                    } );


  }

}
catch ( Exception ex )
{
	throw ex;
}

Entities

[Table("Gages", Schema = "MYSCHEMA" )]
class Gage
{
#region Properties

	[Key]
    [System.Runtime.Serialization.DataMember]
	
    public virtual int Gage_RID
    { get; set; }

	[Required]
    [StringLength(255)]
    [System.Runtime.Serialization.DataMember]
	
    public virtual string Gage_ID
    { get; set; }


    [System.Runtime.Serialization.DataMember]
	
    public virtual int Status_RID
    { get; set; }
	
	#endregion
	
    #region Navigation Properties
	
    [System.Runtime.Serialization.DataMember]
    public virtual Status Status
    { get; set; }
	
    #endregion
	
}

[Table("Status", Schema = "MYSCHEMA" )]
public class Status
{
    #region Properties

    [Key]
    [DatabaseGenerated ( DatabaseGeneratedOption.Identity )]
    [System.Runtime.Serialization.DataMember]
	
    public virtual int Status_RID
    { get; set; }
    
    
    [Required]
    [StringLength(255)]
    [System.Runtime.Serialization.DataMember]
	
    public virtual string StatusName
    { get; set; }

    #endregion
	
    #region Navigation Properties
	
    [System.Runtime.Serialization.DataMember]

    public virtual ICollection<Gage> Gages
    { get; set; }
	
    #endregion
}

public partial class MyContext : DbContext
{
    public virtual DbSet<Gage> Gages
    { get; set; }

    public virtual DbSet<Status> Status
    { get; set; }

}
@smitpatel
Copy link
Contributor

Or you could just use ViewModels...

@dgxhubbard
Copy link
Author

We do use view model in both wpf and angular. what would a view model do to reduce the size of the data that if being set by ef core

@dgxhubbard
Copy link
Author

In the wpf case you have references sitting around so they don't take up that much space. but sending the data over json increases the size drastically

@smitpatel
Copy link
Contributor

var res =
( from c in
context.Gages
select new GageModel {
    .... // Assign scalar properties
    Status = c.Status
} ).ToList();

No references will be fixed up.

@dgxhubbard
Copy link
Author

you referenced a view model rather that the getting of data. status is an object that is a nav property that has not been retrieved by ef so would this not set a null to status

@dgxhubbard
Copy link
Author

we have our scalar props, these would be retrieved by the select, but if status had not been queried by an include then it would be null.

@smitpatel
Copy link
Contributor

smitpatel commented Apr 5, 2018

If you are referencing c.Status in your DTO then it will be populated by EF Core without needing any include.

@dgxhubbard
Copy link
Author

ok but wouldn't ef core still fix up the Gages nav property on Status, getting back to the same point?

@smitpatel
Copy link
Contributor

Where is the Gage being loaded in the query? Query is creating objects of GageModel & Status only.

@dgxhubbard
Copy link
Author

I just tried your suggestion and looked at the json in postman and it works. But I would still want to have the option to turn this off ""Fixing things up". Its not a desirable feature for us. Yes this is a legitimate work around, but in our case Gage and Status are the DTO. So the objects have been created by the EF query why recreate more objects?

@dgxhubbard
Copy link
Author

I have worked with EF prior to EF Core and maybe it is me being used to explicitly saying this is what I want using multiple includes and "." notation such as
.Include( "mynav1.sub1" ).

@ajcvickers
Copy link
Contributor

@dgxhubbard Few things to note here:

  • The part of the docs highlighted is not really relevant in this case. Fixup is not happening to entities that have already been queried and are being tracked. It is happening between the entities returned by the query. The same behavior will happen for no-tracking queries.
  • Only the entities requested by the query are being returned. EF is not creating any additional objects, only fixing up the references between the objects already returned by the query.
  • This is the same behavior that every version of EF from the first release in 2008 until now has had. There is nothing different here in the behavior of EF Core.
  • If the navigation property should never be populated, then consider removing it from the model. Unidirectional relationships with only one navigation property are perfectly valid.
  • If the navigation property is sometimes used and sometimes not, then a view model seems appropriate, as @smitpatel suggested.

All that being said, we will discuss this in triage and consider if there if a different mode for Include would be useful.

@Tarig0
Copy link

Tarig0 commented Apr 10, 2018

Isn't there a way to ignore a property when serializing to json?

@ajcvickers
Copy link
Contributor

@Tarig0 Of course: see aspnet/Mvc#4160

@dgxhubbard
Copy link
Author

I know in EF 6 that the properties that are not included are not loaded, and EF6 honors what should be and not be loaded. There is no request for loading the Gages property of Status, but the property is being loaded. There are cases when these properties will be loaded so turning it off by ignoring in json is not the answer.

When serializing to return to front end we want a list of gages with their status. Suppose there are a 100 gages returned to the front end, and each has a status, with the behavior or EF Core the size of the payload will be increased by 100 fold.

If include is honored this would be a great solution. I proposed a boolean to turn off the fixup if there if include code cannot be made to behave like EF 6.

The doc says:

Entity Framework Core will automatically fix-up navigation properties to any other entities that were previously loaded into the context instance. So even if you don't explicitly include the data for a navigation property, the property may still be populated if some or all of the related entities were previously loaded.

My interprtation of this is the first query, for gages, the list returned would be just the gages and their status (no Gages would be set on the status). However, on the second query and every query after, for gages, the Gages property will be "fixed up". So, except on the first query, the json payload in the 100 gages case will always be 100 times larger than the original request.

@ajcvickers
Copy link
Contributor

@dgxhubbard EF6 and EF Core behave in the same way here. Also, you mention a first and a second query, but your code above only shows a single query.

@dgxhubbard
Copy link
Author

Code is part of a web api, so there will be multiple queries executed against it. On the first query the connection will store the gages for the next request. Then every request after it will Fixup the object gragh so that Gages of Status will be filled with items. I will double check my EF6 code.

@dgxhubbard
Copy link
Author

I am sorry you are right it does have this behavior. I would not have thought that because of serialization over the wire

I will go back to my original comment on that. In one case I have a WPF app that makes calls against the database, using EF6. In this case there are references to the gages set in the Gages propety and these are in memory, so the size increases but not as much as the serialization in json. Using json, the gages are sent out over the wire so they are not sent as a reference but as a gage and its properties, so the json payload will be roughly increased in size by the number of gages that are returned.

I added the code mentioned in the article

            .AddJsonOptions (
                options =>
                {
                    options.SerializerSettings.ReferenceLoopHandling = 
                            Newtonsoft.Json.ReferenceLoopHandling.Ignore;
                    options.SerializerSettings.ContractResolver = new DefaultContractResolver ();
                } );

and checked the result in postman, and each gage, in Gages property of Status, the properties of the gage are listed.

@dgxhubbard
Copy link
Author

Also in the mock code here I have only listed minimal properties there are more than properties on Gage and Status.

@ajcvickers
Copy link
Contributor

@dgxhubbard In the code above, a new context is created for each query, so the context will not be tracking anything each time the query executes.

@Tarig0
Copy link

Tarig0 commented Apr 10, 2018

Unless you're setting the context as a singleton.

@ajcvickers
Copy link
Contributor

@Tarig0 The code above uses "new".

@Tarig0
Copy link

Tarig0 commented Apr 10, 2018

Ah yes lost the context

@dgxhubbard
Copy link
Author

Tracking anything? I don't understand. A new context is created, but doesn't the context use results that have already been cached? If that is where you are going

@dgxhubbard
Copy link
Author

I do know the Gages property on Status is filled in on the first query

@ajcvickers
Copy link
Contributor

@dgxhubbard Nope.

@dgxhubbard
Copy link
Author

dgxhubbard commented Apr 10, 2018

My test with the EF 6 code and EF core both showed the same thing. The Gages property is set.
This test was start the web api project and hitting with one request from postman. There were no other requests. If there is a problem with what I am doing or interpreting the result please correct me. Sorry this uses full code and there is json in the txt file.

Gages.txt

@ajcvickers
Copy link
Contributor

@dgxhubbard context.Gages.Include("Status") means for every Gage, also load the Status relationship. The relationship consists of one foreign key property and two navigation properties--Gage.Status and its inverse Status.Gages. When loading the relationship both navigation properties are set.

@dgxhubbard
Copy link
Author

dgxhubbard commented Apr 10, 2018

We are running EF Core 2.1 and according to the doc for lazy loading if you call UseLazyLoadingProxies, which we do, then any virtual nav property will be lazy loaded. If I am wrong please correct me. My example above is pseudo and leaves out the important face that we are calling UseLazyLoadingProxies. I apologize for that.

@dgxhubbard
Copy link
Author

dgxhubbard commented Apr 10, 2018

Here is our factory method to create a context.

public static MyContext Create ( bool useLogger = false )
        {

            var optionsBuilder = new DbContextOptionsBuilder<GtContext> ();
            if ( optionsBuilder == null )
                throw new NullReferenceException ( "Failed to create db context options builder" );


           optionsBuilder.UseSqlServer ( MsSqlConnectionString );
            optionsBuilder.UseLazyLoadingProxies ();

            if ( useLogger )
                optionsBuilder.UseLoggerFactory ( DbContextLoggingExtensions.LoggerFactory );


            var context = new MyContext ( optionsBuilder.Options );
            if ( context == null )
                throw new NullReferenceException ( "Failed to create context" );

            // disable change tracking
            context.ChangeTracker.AutoDetectChangesEnabled = false;
            context.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;

            return context;
        }

@Shinjoku
Copy link

Shinjoku commented Mar 14, 2021

Any news about this problem? This behavior is affecting (and breaking) even a sample from the official doc, if ported to a WebAPI instead of a View response!

Turning this into an optional behavior would be extremely useful. With a simple relationship like the above, the JSON serializer is already complaining about a 32-depth JSON...

@stevendarby
Copy link
Contributor

stevendarby commented Apr 17, 2021

In my mind the solution to this is very clearly what was suggested early on by @smitpatel, to use view models or DTOs.

Your entity models have back-navigations. You find this useful in various scenarios, but don’t want need it in a specific representation of the data that you need in a specific scenario. So you can use a DTO which is similar to but not the same as your entity, which doesn’t have those properties.

If using the entity, even if you cleared navigations, you would have an empty or null property; if sending this as JSON this could still serialise depending on your settings. If you just don’t have the property then it wouldn’t be there at all. Much less confusion.

Tools like AutoMapper can make mapping from entity to DTO easy. You can do the projection in SQL too, to minimise data transfer from the DB of you have scalar properties you don’t need too.

@ComptonAlvaro
Copy link

Well, the last answer here was in the year 2020 so I would like to know if it would possible to comment which is the state of this issue.

I asked here (#29152) a doubt about the fix up entities resolution, and I would like to know if there is some work in this area, because i have seen that there are another persons that has the same doubts, for example here: #24871, and the aswered was that he could use AsNoTracking(). However, my problem is that with AsNoTracking() some navigation properties are set although I didn't tell explicitly with an Include() or ThenInclude().

In my original qestion in (#29152), I added a project with a test and in the last post from me I explain what i would expected.

In sumary, I would like to can avoid that EF perform includes by itself with AsNoTracking(), just only to do it if I use Inlcude() or ThenInlcude(), to avoid circle references that makes it hard to serialize, because i would like to can send to client (wth gRPC for exmaple) the result of the repository. Now the only solution I have found it is to modify the navigtaion properties to set null to break the cirlce relationships and then serialize them.

Thanks so much.

@ajcvickers
Copy link
Contributor

I would like to know if it would possible to comment which is the state of this issue.

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 7.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

I asked here (#29152) a doubt about the fix up entities resolution, and I would like to know if there is some work in this area,

I don't think there is anymore more to add over what was already discussed on #29152.

@ComptonAlvaro
Copy link

Thanks for the update.

And which issue should I to vote?

@ajcvickers
Copy link
Contributor

This issue.

@panjonsen
Copy link

I also encountered the problem of infinite i nesting. How to solve it? I don't think it should automatically load entities without include.

@puschie286
Copy link

it even include properties that are not explicit defined for navigation - please disable this feature by default

@StrangeWill
Copy link

StrangeWill commented Mar 17, 2023

Alright, there's a very long discussion on this and I've read a lot points, counterpoints and whatnot with relation to performance claims and I got one I so far have not seen in the thread:

It's really unintuitive, just had a developer that couldn't figure out why a random unincluded property on a model was suddenly populated! There isn't really any clear documentation on this, links I found to EF Core fixup seem dead, so there is a very low visibility on this behavior (it really should be explicitly defined as part of https://learn.microsoft.com/en-us/ef/core/querying/related-data/ ), and while performance hits have been claimed to be significant and there have been claims against that -- it isn't free as far as I can tell.

I do understand long default behavior, but it's also very unintuitive from the coding perspective to have a query that does not reference an object being included, and then using it later down the line assuming some other shared context state with EF is holding onto that data based on some other query, it encourages fragile code.

Definitely not like, huge priority as some have claimed here from what I've experienced, but seems to cause more trouble than benefits, and anyone relying on this functionality should typically be shunned for writing hard to understand and possibly fragile to refactor code.

If there's some internal optimization that causes this to happen, then maybe it's a weird side-effect we just live with and that's okay, but if it could be removed I could see a big argument for such.

@Xriuk
Copy link

Xriuk commented Jun 14, 2023

My two cents: fix-up at least for collections should happen only if the navigation property is already loaded.
Otherwise it is just confusing: you had a NULL collection which gets partially populated with entities loaded elsewhere.

@StrangeWill
Copy link

StrangeWill commented Aug 16, 2023

OK, me dropping in here again, we just got bit by this on a test, where we populate data and then some part of business logic does NOT include it as part of a query, but the test passes because we staged data, and fixup will automatically wire up the missing data!

Our query in the actual working software depends on the data coming from another request, so the lack of include causes a null ref and we have a bug!

We're doubly annoyed because we have a test that's covering this! The only solution is to use factories in our test so we can dispose of a context entirely before working with the business logic we're testing, but this is overly cumbersome for a feature that still isn't documented properly.

Even just being able to turn this off would be a huge bonus for us (which... we'd do across all of our projects).

@Alexandria-Davis-Ametek

StrangeWill's comment is relevant to my use case as well. A motivation for moving to EF7 was the in-memory database support, which has been key to testing a large number of our methods.

I also build up the database components that I need at the start of the test, then write tests to make sure the methods I'm writing are pulling the data I expect, but not pulling the data I don't expect.

This blows a massive hole in the confidence I have in my tests, and to make matters worse - those calls to Entity Framework are an area where I particularly need the tests.

@stevendarby
Copy link
Contributor

stevendarby commented Aug 18, 2023

@StrangeWill

The only solution is to use factories in our test so we can dispose of a context entirely before working with the business logic we're testing, but this is overly cumbersome

I think there could be other solutions, for example, have you considered clearing the context's change tracker? That might be simpler (it's one line). Still just a workaround, I know.

@brb3
Copy link

brb3 commented May 8, 2024

To add an example here:

var store = await Context.Stores
    .Include(s => s.Subscriptions.Where(s => s.Status == Subscription.SubscriptionStatuses.Approved))
    .FirstOrDefaultAsync(s => s.Id == storeId);

store.Subscriptions contains Subscription objects with Status property of Pending. Which I'm trying to remove with the .Include -> .Where specifically.

While debugging, I was also logging the resulting SQL from my context (optionsBuilder.LogTo(Console.WriteLine);) and confusingly it included the WHERE clause which was ultimately being ignored since navigation fixup was was pulling in the Pending Subscriptions.

I resolved this by changing my original query to include .AsNoTracking(), but this also has drawbacks that in this scenario aren't an issue for my use case.

var store = await Context.Stores
    .AsNoTracking()
    .Include(s => s.Subscriptions.Where(s => s.Status == Subscription.SubscriptionStatuses.Approved))
    .FirstOrDefaultAsync(s => s.Id == storeId);

@guillermosaez
Copy link

This happens to me as well.
.AsNoTracking() solves it.
But it doesn't make any sense, given I have set QueryTrackingBehavior.NoTrackingWithIdentityResolution in the whole application, when adding the DbContext to the DI.
Is it a bug?

@ajcvickers ajcvickers removed their assignment Aug 31, 2024
@brianjlacy
Copy link

brianjlacy commented Sep 8, 2024

@guillermosaez

This happens to me as well. .AsNoTracking() solves it. But it doesn't make any sense, given I have set QueryTrackingBehavior.NoTrackingWithIdentityResolution in the whole application, when adding the DbContext to the DI. Is it a bug?

It is not a bug, it's a fundamental design choice within Entity Framework Core, and there is, as far as I know, nothing that can be done to prevent it right now, aside from changing the design of your application to Select into DTOs. If you include circular navigation properties in your query, the corresponding entities will always be loaded -- on BOTH sides of the relationship.


I'm going to add my voice to the chorus of those expressing frustration that there's no way to turn off these "automatic fix-ups". It is highly non-intuitive behavior.

Just because something makes sense to the core designers, and to people who've worked with Entity Framework for a long time, does not mean that it's an intuitive design or even, necessarily, a good one.

Furthermore, there generally only two ways two discover this behavior: 1) happen to notice it early on in the "tips" describing it briefly in the documentation; or 2) discover it after you've developed invested significant development effort, and discovered that your code doesn't work the way you expect. In some cases with the former and in nearly all cases with the latter, you've likely progressed far enough in development that it's (potentially very) costly to change your code to handle it correctly.

Strongly advise/request that this be something that can be turned off, and in the meantime, that a BIG DEAL is made of this in the documentation so that people realize it early enough to account for it in their design.

@Alexandria-Davis-Ametek
Copy link

There is a third way to discover it:

After someone else does work that integrates it and does something odd that relies on the behavior, only for you to discover it while removing seemingly unused code.

@brianjlacy
Copy link

There is a third way to discover it:

After someone else does work that integrates it and does something odd that relies on the behavior, only for you to discover it while removing seemingly unused code.

My friend, you have seen things, and you have my sympathies.

@Alexandria-Davis-Ametek
Copy link

My friend, you have seen things, and you have my sympathies.

If I remember correctly, the entire function "did nothing".

No return value, no use of the data it pulled, nothing.

Turns out, it did not, in fact, do nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests