Skip to content

Commit

Permalink
Warn on redundant AddEntityFramework* calls
Browse files Browse the repository at this point in the history
Fixes #19053
  • Loading branch information
AndriySvyryd committed Dec 3, 2019
1 parent 63971dd commit 0536522
Show file tree
Hide file tree
Showing 9 changed files with 267 additions and 91 deletions.
14 changes: 14 additions & 0 deletions src/EFCore/Diagnostics/CoreEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ private enum Id
ContextDisposed,
DetachedLazyLoadingWarning,
ServiceProviderDebugInfo,
RedundantAddServicesCallWarning,

// Model events
ShadowPropertyCreated = CoreBaseId + 600,
Expand Down Expand Up @@ -382,6 +383,19 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// </summary>
public static readonly EventId DetachedLazyLoadingWarning = MakeInfraId(Id.DetachedLazyLoadingWarning);

/// <summary>
/// <para>
/// 'AddEntityFramework*' was called on the service provider, but 'UseInternalServiceProvider' wasn't.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Infrastructure" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="ServiceProviderEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId RedundantAddServicesCallWarning = MakeInfraId(Id.RedundantAddServicesCallWarning);

private static readonly string _modelPrefix = DbLoggerCategory.Model.Name + ".";
private static EventId MakeModelId(Id id) => new EventId((int)id, _modelPrefix + id);

Expand Down
28 changes: 28 additions & 0 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,34 @@ private static string DetachedLazyLoadingWarning(EventDefinitionBase definition,
return d.GenerateMessage(p.NavigationPropertyName, p.Entity.GetType().ShortDisplayName());
}

/// <summary>
/// Logs for the <see cref="CoreEventId.RedundantAddServicesCallWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="serviceProvider"> The service provider used. </param>
public static void RedundantAddServicesCallWarning(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Infrastructure> diagnostics,
[NotNull] IServiceProvider serviceProvider)
{
var definition = CoreResources.LogRedundantAddServicesCall(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(diagnostics, warningBehavior);
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new ServiceProviderEventData(
definition,
(d, p) => ((EventDefinition)d).GenerateMessage(),
serviceProvider));
}
}

/// <summary>
/// Logs for the <see cref="CoreEventId.ShadowPropertyCreated" /> event.
/// </summary>
Expand Down
9 changes: 9 additions & 0 deletions src/EFCore/Diagnostics/LoggingDefinitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,15 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogDetachedLazyLoading;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
public EventDefinitionBase LogRedundantAddServicesCall;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ private static void AddCoreServices<TContextImplementation>(
serviceCollection.TryAdd(
new ServiceDescriptor(
typeof(DbContextOptions<TContextImplementation>),
p => DbContextOptionsFactory<TContextImplementation>(p, optionsAction),
p => CreateDbContextOptions<TContextImplementation>(p, optionsAction),
optionsLifetime));

serviceCollection.Add(
Expand All @@ -518,7 +518,7 @@ private static void AddCoreServices<TContextImplementation>(
optionsLifetime));
}

private static DbContextOptions<TContext> DbContextOptionsFactory<TContext>(
private static DbContextOptions<TContext> CreateDbContextOptions<TContext>(
[NotNull] IServiceProvider applicationServiceProvider,
[CanBeNull] Action<IServiceProvider, DbContextOptionsBuilder> optionsAction)
where TContext : DbContext
Expand Down
29 changes: 18 additions & 11 deletions src/EFCore/Internal/ServiceProviderCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -61,6 +62,17 @@ public virtual IServiceProvider GetOrAdd([NotNull] IDbContextOptions options, bo
return internalServiceProvider;
}

if (coreOptionsExtension?.ServiceProviderCachingEnabled == false)
{
return BuildServiceProvider().ServiceProvider;
}

var key = options.Extensions
.OrderBy(e => e.GetType().Name)
.Aggregate(0L, (t, e) => (t * 397) ^ ((long)e.GetType().GetHashCode() * 397) ^ e.Info.GetServiceProviderHashCode());

return _configurations.GetOrAdd(key, k => BuildServiceProvider()).ServiceProvider;

(IServiceProvider ServiceProvider, IDictionary<string, string> DebugInfo) BuildServiceProvider()
{
ValidateOptions(options);
Expand Down Expand Up @@ -137,22 +149,17 @@ public virtual IServiceProvider GetOrAdd([NotNull] IDbContextOptions options, bo
_configurations.Values.Select(e => e.ServiceProvider).ToList());
}
}

var applicationServiceProvider = options.FindExtension<CoreOptionsExtension>()?.ApplicationServiceProvider;
if (applicationServiceProvider?.GetService<IRegisteredServices>() != null)
{
logger.RedundantAddServicesCallWarning(serviceProvider);
}
}
}

return (serviceProvider, debugInfo);
}

if (coreOptionsExtension?.ServiceProviderCachingEnabled == false)
{
return BuildServiceProvider().ServiceProvider;
}

var key = options.Extensions
.OrderBy(e => e.GetType().Name)
.Aggregate(0L, (t, e) => (t * 397) ^ ((long)e.GetType().GetHashCode() * 397) ^ e.Info.GetServiceProviderHashCode());

return _configurations.GetOrAdd(key, k => BuildServiceProvider()).ServiceProvider;
}

private static void ValidateOptions(IDbContextOptions options)
Expand Down
24 changes: 24 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 31 additions & 27 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -1223,6 +1223,10 @@
<value>'{principalEntityType}.{principalNavigation}' may still be null at runtime despite being declared as non-nullable since only the navigation to principal can be configured as required.</value>
<comment>Debug CoreEventId.NonNullableReferenceOnDependent string string</comment>
</data>
<data name="LogRedundantAddServicesCall" xml:space="preserve">
<value>'AddEntityFramework*' was called on the service provider, but 'UseInternalServiceProvider' wasn't called in the DbContext options configuration. Remove the 'AddEntityFramework*' call as in most cases it's not needed and might cause conflicts with other products and services registered in the same service provider.</value>
<comment>Warning CoreEventId.RedundantAddServicesCallWarning</comment>
</data>
<data name="ClashingNonOwnedDerivedEntityType" xml:space="preserve">
<value>The type '{entityType}' cannot be marked as owned because the derived entity type - '{derivedType}' has been configured as non-owned.</value>
</data>
Expand Down
4 changes: 1 addition & 3 deletions test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,8 @@ private static readonly IServiceProvider _sensitiveProvider

private static readonly IServiceProvider _poolProvider
= new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.AddDbContextPool<LikeAZooContextPooled>(
p =>
p.UseInMemoryDatabase(nameof(LikeAZooContextPooled))
p => p.UseInMemoryDatabase(nameof(LikeAZooContextPooled))
.UseInternalServiceProvider(InMemoryFixture.BuildServiceProvider(_loggerFactory)))
.BuildServiceProvider();

Expand Down
Loading

0 comments on commit 0536522

Please sign in to comment.