Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ private void GenerateProperty(IProperty property, bool useDataAnnotations)
}

var dummyLogger = new DiagnosticsLogger<DbLoggerCategory.Model>(
new LoggerFactory(),
new ScopedLoggerFactory(new LoggerFactory(), dispose: true),
new LoggingOptions(),
new DiagnosticListener(""));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ protected virtual KeyBuilder VisitPrimaryKey([NotNull] EntityTypeBuilder builder
if (property != null)
{
var dummyLogger = new DiagnosticsLogger<DbLoggerCategory.Model>(
new LoggerFactory(),
new ScopedLoggerFactory(new LoggerFactory(), dispose: true),
new LoggingOptions(),
new DiagnosticListener(""));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ public RelationalConventionSetBuilderDependencies(

TypeMappingSource = typeMappingSource;
Logger = logger
?? new DiagnosticsLogger<DbLoggerCategory.Model>(new LoggerFactory(), new LoggingOptions(), new DiagnosticListener(""));
?? new DiagnosticsLogger<DbLoggerCategory.Model>(
new ScopedLoggerFactory(new LoggerFactory(), dispose: true),
new LoggingOptions(),
new DiagnosticListener(""));
Context = currentContext;
SetFinder = setFinder;
}
Expand Down
9 changes: 0 additions & 9 deletions src/EFCore/Infrastructure/CoreOptionsExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,6 @@ public virtual CoreOptionsExtension WithCacheServiceProvider(bool cacheServicePr
/// <returns> False since no database provider is registered. </returns>
public virtual bool ApplyServices(IServiceCollection services)
{
var loggerFactory = GetLoggerFactory();
if (loggerFactory != null)
{
services.AddScoped<ILoggerFactory>(_ => new ExternalLoggerFactory(loggerFactory));
}

var memoryCache = GetMemoryCache();
if (memoryCache != null)
{
Expand All @@ -363,9 +357,6 @@ public virtual bool ApplyServices(IServiceCollection services)
private IMemoryCache GetMemoryCache()
=> MemoryCache;

private ILoggerFactory GetLoggerFactory()
=> LoggerFactory ?? ApplicationServiceProvider?.GetService<ILoggerFactory>();

/// <summary>
/// Returns a hash code created from any options that would cause a new <see cref="IServiceProvider" />
/// to be needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public virtual EntityFrameworkServicesBuilder TryAddCoreServices()
TryAdd<ICoreConventionSetBuilder, CoreConventionSetBuilder>();
TryAdd<IModelCustomizer, ModelCustomizer>();
TryAdd<IModelCacheKeyFactory, ModelCacheKeyFactory>();
TryAdd<ILoggerFactory, LoggerFactory>();
TryAdd<ILoggerFactory>(p => ScopedLoggerFactory.Create(p, null));
TryAdd<IModelSource, ModelSource>();
TryAdd<IInternalEntityEntryFactory, InternalEntityEntryFactory>();
TryAdd<IInternalEntityEntrySubscriber, InternalEntityEntrySubscriber>();
Expand Down
50 changes: 0 additions & 50 deletions src/EFCore/Internal/ExternalLoggerFactory.cs

This file was deleted.

86 changes: 86 additions & 0 deletions src/EFCore/Internal/ScopedLoggerFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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 JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Microsoft.EntityFrameworkCore.Internal
{
public class ScopedLoggerFactory : ILoggerFactory
{
private readonly ILoggerFactory _underlyingFactory;
private readonly bool _dispose;

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public ScopedLoggerFactory(
[NotNull] ILoggerFactory loggerFactory,
bool dispose)
{
_underlyingFactory = loggerFactory;
_dispose = dispose;
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public static ScopedLoggerFactory Create(
[NotNull] IServiceProvider internalServiceProvider,
[CanBeNull] IDbContextOptions contextOptions)
{
var coreOptions
= (contextOptions ??
internalServiceProvider.GetService<IDbContextOptions>())
?.FindExtension<CoreOptionsExtension>();

if (coreOptions != null)
{
if (coreOptions.LoggerFactory != null)
{
return new ScopedLoggerFactory(coreOptions.LoggerFactory, dispose: false);
}

var applicationServiceProvider = coreOptions.ApplicationServiceProvider;
if (applicationServiceProvider != null
&& applicationServiceProvider != internalServiceProvider)
{
return new ScopedLoggerFactory(applicationServiceProvider.GetService<ILoggerFactory>(), dispose: false);
}
}

return new ScopedLoggerFactory(new LoggerFactory(), dispose: true);
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual void Dispose()
{
if (_dispose)
{
_underlyingFactory.Dispose();
}
}

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual ILogger CreateLogger(string categoryName)
=> _underlyingFactory.CreateLogger(categoryName);

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual void AddProvider(ILoggerProvider provider)
=> _underlyingFactory.AddProvider(provider);
}
}
10 changes: 8 additions & 2 deletions src/EFCore/Internal/ServiceProviderCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
Expand Down Expand Up @@ -100,8 +101,13 @@ public virtual IServiceProvider GetOrAdd([NotNull] IDbContextOptions options, bo

using (var scope = serviceProvider.CreateScope())
{
var logger =
scope.ServiceProvider.GetRequiredService<IDiagnosticsLogger<DbLoggerCategory.Infrastructure>>();
var scopedProvider = scope.ServiceProvider;

// Because IDbContextOptions cannot yet be resolved from the internal provider
var logger = new DiagnosticsLogger<DbLoggerCategory.Infrastructure>(
ScopedLoggerFactory.Create(scopedProvider, options),
scopedProvider.GetService<ILoggingOptions>(),
scopedProvider.GetService<DiagnosticSource>());

if (_configurations.Count == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace Microsoft.EntityFrameworkCore.TestUtilities
public class ListLoggerFactory : ILoggerFactory
{
private readonly Func<string, bool> _shouldLogCategory;
private bool _disposed;

public ListLoggerFactory()
: this(_ => true)
Expand All @@ -37,16 +38,30 @@ public void SetTestOutputHelper(ITestOutputHelper testOutputHelper)
}

public virtual ILogger CreateLogger(string name)
=> !_shouldLogCategory(name)
{
CheckDisposed();

return !_shouldLogCategory(name)
? (ILogger)NullLogger.Instance
: Logger;
}

private void CheckDisposed()
{
if (_disposed)
{
throw new ObjectDisposedException(nameof(ListLoggerFactory));
}
}

public void AddProvider(ILoggerProvider provider)
{
CheckDisposed();
}

public void Dispose()
{
_disposed = true;
}

protected class ListLogger : ILogger
Expand Down
89 changes: 87 additions & 2 deletions test/EFCore.Tests/DbContextServicesTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,21 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu

private class MyLoggerProvider : ILoggerProvider
{
public ILogger CreateLogger(string categoryName) => new MyListLogger(Log);
private bool _disposed;

public ILogger CreateLogger(string categoryName)
{
if (_disposed)
{
throw new ObjectDisposedException(nameof(MyLoggerProvider));
}

return new MyListLogger(Log);
}

public void Dispose()
{
_disposed = true;
}

private class MyListLogger : ILogger
Expand Down Expand Up @@ -164,6 +175,58 @@ public void Log<TState>(
}
}

[Theory]
[InlineData(ServiceLifetime.Scoped)]
[InlineData(ServiceLifetime.Singleton)]
public void Logger_factory_registered_on_application_service_provider_is_not_disposed(ServiceLifetime optionsLifetime)
{
for (var i = 0; i < 2; i++)
{
ILoggerFactory loggerFactory;

using (var appServiceProvider
= new ServiceCollection()
.AddScoped<Random>()
.AddDbContext<ConstructorTestContext1A>(
b => b.UseInMemoryDatabase("Scratch"), optionsLifetime: optionsLifetime)
.BuildServiceProvider())
{
loggerFactory = appServiceProvider.GetService<ILoggerFactory>();
Random scopedExternalService;

using (var scope = appServiceProvider.CreateScope())
{
var context = scope.ServiceProvider.GetRequiredService<ConstructorTestContext1A>();

// Should not throw
var _ = context.Model;

Assert.Same(loggerFactory, scope.ServiceProvider.GetService<ILoggerFactory>());

scopedExternalService = scope.ServiceProvider.GetService<Random>();
Assert.NotNull(scopedExternalService);
}

using (var scope = appServiceProvider.CreateScope())
{
var context = scope.ServiceProvider.GetRequiredService<ConstructorTestContext1A>();

// Should not throw
var _ = context.Model;

Assert.Same(loggerFactory, scope.ServiceProvider.GetService<ILoggerFactory>());

Assert.NotSame(scopedExternalService, scope.ServiceProvider.GetService<Random>());
}

// Should not throw
loggerFactory.CreateLogger("MyLogger");
}

Assert.Throws<ObjectDisposedException>(() => loggerFactory.CreateLogger("MyLogger"));
}
}

[Fact]
public void Can_use_GetInfrastructure_with_inferred_generic_to_get_service_provider()
{
Expand All @@ -175,6 +238,28 @@ public void Can_use_GetInfrastructure_with_inferred_generic_to_get_service_provi
}
}

[Fact]
public void Logger_factory_registered_on_internal_service_provider_is_not_disposed()
{
var serviceProvider
= new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.BuildServiceProvider();

var appServiceProvider
= new ServiceCollection()
.AddDbContext<ConstructorTestContext1A>(
b => b.UseInMemoryDatabase("Scratch")
.UseInternalServiceProvider(serviceProvider))
.BuildServiceProvider();

var context = appServiceProvider.GetRequiredService<ConstructorTestContext1A>();
var _ = context.Model;

context = appServiceProvider.GetRequiredService<ConstructorTestContext1A>();
_ = context.Model;
}

[Fact]
public void Each_context_gets_new_scoped_services()
{
Expand Down Expand Up @@ -306,7 +391,7 @@ public void Required_low_level_services_are_added_if_needed()
new EntityFrameworkServicesBuilder(serviceCollection).TryAddCoreServices();
var provider = serviceCollection.BuildServiceProvider();

Assert.IsType<LoggerFactory>(provider.GetRequiredService<ILoggerFactory>());
Assert.IsType<ScopedLoggerFactory>(provider.GetRequiredService<ILoggerFactory>());
}

[Fact]
Expand Down