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 · 110 comments
Open
Tracked by #22954

Option to turn off automatic setting of navigation properties #11564

dgxhubbard opened this issue Apr 5, 2018 · 110 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
Member

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
Member

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
Member

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
Member

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
Member

@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
Member

@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
Member

@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
Member

@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
Member

@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
Member

@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
Member

@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;
        }

@roji
Copy link
Member

roji commented Jul 27, 2020

@JaimeStill if your JSON returns only one item (Nirvana - Nevermind), as opposed to all items (as seems to be suggested by the GetItems method), then yeah, that makes sense - the only loop is with that specific item. If your web service really does return only one item, you may want to refactor your code to make EF not load all items - though I'm lacking a lot of context about your application to know what's going on. In any case, non-tracking queries are indeed also a solution - thanks for sharing that.

@JaimeStill
Copy link

JaimeStill commented Jul 27, 2020

@JaimeStill if your JSON returns only one item (Nirvana - Nevermind), as opposed to all items (as seems to be suggested by the GetItems method)

It returns all items, I only included a screenshot of the first item to illustrate what is happening. If you were to look at the entry for Jimi Hendrix - Electric Ladyland, the array of items contained by it's Category navigation property would contain every item it's related to except for Jimi Hendrix - Electric Ladyland (which indicates ReferenceLoopHandling.Ignore is working).

Here are the results of the same query, with ReferenceLoopHandling.Ignore set, the only difference being the setting for QueryTrackingBehavior.

@roji
Copy link
Member

roji commented Jul 27, 2020

@JaimeStill makes sense, now I understand your example more fully - thanks.

@windhandel
Copy link

@roji, maybe I was unclear in my question. It was on-topic as I previously saw @AndriySvyryd 's request and your sample.

If you don't want me to contribute to the discussion any longer, that's fine, just give me the word.

To clarify: Since a memory dump not only has to dump the contents of the memory, but also the instance-relationships of the, now circular dependency object graph, how does the memory dump handle dumping such a circular object graph? Does it have a built-in mechanism for "trimming" the circular references? I suppose it must for such an issue.

i.e. Will the circular dependency object graph being dumped be large?

@roji
Copy link
Member

roji commented Jul 28, 2020

@windhandel I definitely don't want you to feel you shouldn't contribute to the discussion - I'm only trying to make sure this (already complex) discussion stays focused, and that we don't talk about multiple, unrelated things at once.

In general, memory structures with circular references (such as what EF tracking queries sometimes produce) are a completely normal thing and shouldn't necessarily be avoided as such. All memory dump tools know how to handle these, and generally dump objects by their memory address; so if you have 10 objects holding pointers to the same memory area, your dump file will only contain one copy of that memory area.

So to answer your question: a circular dependency object graph, dumped by a memory profiler/tool (e.g. perfview) will not be large. The best way to help you understand what's going on in your particular case, is for you to come up with a small console program that creates a situation which you consider abnormal, and post that so that it can be investigated - but that's better done in #21766.

@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
Member

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
Member

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.

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