Diagnostics analyzer for unnecessary Include

[enricobenedos]

Good morning,

after upgrading Entity Framework Core to .NET 6 I faced a breaking change on my production API that was not listed on these docs.

It is related to Microsoft.EntityFrameworkCore.Query.NavigationBaseIncludeIgnored that change the EF Core behaviour when using multiple level properties.
The solution is explained on this question on Stack Overflow.

Old working code:

Tour tour = await _context.Tours.Include(mpt => mpt.MarkersPerTours)
                                                 .ThenInclude(mrk => mrk.Marker)
                                                 .ThenInclude(mrkProp => mrkProp.MarkersTranslations)
                                                 .ThenInclude(mrk => mrk.Marker)
                                                 .ThenInclude(mrkTp => mrkTp.Type)
                                                 .FirstOrDefaultAsync(t => t.Id == tourId);

New working code:

Tour tour = await _context.Tours.Include(t => t.MarkersPerTours)
                                                    .ThenInclude(mpt => mpt.Marker)
                                                    .ThenInclude(mrk => mrk.MarkersTranslations)
                                                 .Include(tour => tour.MarkersPerTours)
                                                    .ThenInclude(mpt => mpt.Marker)
                                                    .ThenInclude(mrk => mrk.Type)
                                                 .FirstOrDefaultAsync(t => t.Id == tourId);

Can it be a good idea to include it in the docs?

Thank you

[ajcvickers]

@enricobenedos The important point here is that:

.ThenInclude(mrk => mrk.Marker)
.ThenInclude(mrkTp => mrkTp.Type)

did not work in EF Core 5.0. It would fail to honor the last Include.

The docs already document the correct way of writing this kind of Include, which is as you show in the "working code'.'

[enricobenedos]

The docs already document the correct way of writing this kind of Include, which is as you show in the "working code'.'

Yes you are right, but why not include this change in the breaking changes list of EF Core 6.0?

Normally in production environments I think that developers will not use transition/not LTS versions as .NET 5.0, so migrating from .Net Core 3.1 to .NET 6.0 can broke production API at runtime due to this change.

Can it be convenient to specify from which version the breaking change is coming?
For example: "The following API and behavior changes have the potential to break existing applications updating to EF Core 6.0.0. from EF Core 5.0.0".

[Prinsn]

@enricobenedos The important point here is that:
did not work in EF Core 5.0. It would fail to honor the last Include.

What about 3?

We had to skip .NET 5 because of Azure Functions, and that's where I'm running into this from.

[JanKotschenreuther]

We just upgraded from .NET 5 to .NET 6 and EF Core 6.
We made sure that the documented Breaking Changes do not affect us.

Now at runtime we run into the following exception:

An error was generated for warning 'Microsoft.EntityFrameworkCore.Query.NavigationBaseIncludeIgnored':
The navigation 'WebShopOrderPosition.Order' was ignored from 'Include' in the query since the fix-up will automatically populate it.
If any further navigations are specified in 'Include' afterwards then they will be ignored.
Walking back include tree is not allowed.
This exception can be suppressed or logged by passing event ID 'CoreEventId.NavigationBaseIncludeIgnored' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.

The causing query works in .NET 5 but throws the shown exception in .NET 6:

var order = await _RetailContext.WebShopOrder
	.Include(x => x.WebShopOrderPosition)
		.ThenInclude(x => x.Order)
	.Include(x => x.WebShopOrderPosition)
		.ThenInclude(x => x.WebShopOrderPositionBooking)
			.ThenInclude(x => x.Sale)
				.ThenInclude(x => x.Booking)
					.ThenInclude(x => x.Item)
	.Include(x => x.WebShopOrderPosition)
		.ThenInclude(x => x.WebShopOrderPositionBooking)
			.ThenInclude(x => x.Sale)
				.ThenInclude(x => x.Booking)
					.ThenInclude(x => x.Season)
	.Where(o => o.OrderId == request.OrderId)

The exception tells me that the navigation was ignored from include because some fix-up automatically populates it.
As far as I know, we do have to include all entities we want to be eagerly loaded.
So I am wondering, why is the exception mentioning some fix-up which automatically populates anything?

Why is a fix-up ignoring all includes afterwards?
Not including the navigations mentioned in the includes will cause unexpected behavior for the code working with the not loaded data.
If we define includes in a query, then we need those navigations, why would a fix-up ignore those and cause unexpected behavior at runtime?

The exception tells us we can suppress the exception, but that will not fix the problem that includes are ignored which are required.
Do we need to load every ignored include separatly?
How can we identify which includes are causing other includes to be ignored?

[JanKotschenreuther]

I configured our DbContext to ignore that exception and checked if all required navigations are loaded.
I was able to find all navigation properties loaded for the specified includes.
Also checked the executed SQL with the profiler, which looks like what we expect.

So I am not sure if I misunderstand the exception message or if the exception message is misleading, but everything seems fine.

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
	optionsBuilder.ConfigureWarnings(builder =>
	{
		builder.Ignore(CoreEventId.NavigationBaseIncludeIgnored);
	});
}

[ajcvickers]

@JanKotschenreuther If you are ignoring the warning and getting the same behavior, then that confirms that you have unneeded Incudes. Specifically, the code to include the relationship between Order and WebShopOrderPosition:

_RetailContext.WebShopOrder
	.Include(x => x.WebShopOrderPosition)

Does not then need to be also specified in the other direction with:

		.ThenInclude(x => x.Order)

[JanKotschenreuther]

Thank you for that hint @ajcvickers .

I did not notice that the Order navigation on WebShopOrderPosition is also a WebShopOrder entity.
And I also did not know that EF Core resolves that without another include.

Is there an analyzer which helps avoiding unnecessary includes?
I think that would be nicer than running into such problems at runtime.
Especially with a lot of EF Core queries in the code base.

[ajcvickers]

Note from triage: putting on the backlog (and adding to #22086) to consider an analyzer, although this is a difficult problem to solve since it requires understanding of the EF model at compile time.

[adurmus]

This definitely should have been mentioned in breaking changes from 5 to 6. Wrong way of including perfectly worked in 5 and breaks in 6.