Skip to content

Commit

Permalink
Throw when AddDbContext called with configuration and only parameterl…
Browse files Browse the repository at this point in the history 
…ess constructor

Issue #6963
  • Loading branch information
@ajcvickers
ajcvickers committed Jan 6, 2017
1 parent b9c8c7f commit 094ea55
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 51 deletions.
80 changes: 51 additions & 29 deletions src/Microsoft.EntityFrameworkCore/EntityFrameworkServiceCollectionExtensions.cs
View file
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore;
Expand All @@ -28,13 +29,13 @@ public static class EntityFrameworkServiceCollectionExtensions
/// </summary>
/// <example>
/// <code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services.AddDbContext&lt;MyContext&gt;(options => options.UseSqlServer(connectionString));
/// }
/// </code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services.AddDbContext&lt;MyContext&gt;(options => options.UseSqlServer(connectionString));
/// }
/// </code>
/// </example>
/// <typeparam name="TContext"> The type of context to be registered. </typeparam>
/// <param name="serviceCollection"> The <see cref="IServiceCollection" /> to add services to. </param>
Expand Down Expand Up @@ -63,7 +64,11 @@ public static class EntityFrameworkServiceCollectionExtensions
[CanBeNull] Action<DbContextOptionsBuilder> optionsAction = null,
ServiceLifetime contextLifetime = ServiceLifetime.Scoped)
where TContext : DbContext
=> AddDbContext<TContext>(serviceCollection, (p, b) => optionsAction?.Invoke(b), contextLifetime);
=> AddDbContext<TContext>(
serviceCollection,
optionsAction == null
? (Action<IServiceProvider, DbContextOptionsBuilder>)null
: (p, b) => optionsAction?.Invoke(b), contextLifetime);

/// <summary>
/// Registers the given context as a service in the <see cref="IServiceCollection" /> and enables DbContext pooling.
Expand All @@ -87,7 +92,7 @@ public static class EntityFrameworkServiceCollectionExtensions
/// <returns>
/// The same service collection so that multiple calls can be chained.
/// </returns>
public static IServiceCollection AddDbContextPool<TContext>(
public static IServiceCollection AddDbContextPool<TContext>(
[NotNull] this IServiceCollection serviceCollection,
[NotNull] Action<DbContextOptionsBuilder> optionsAction,
int poolSize = 128)
Expand Down Expand Up @@ -140,6 +145,8 @@ public static class EntityFrameworkServiceCollectionExtensions
throw new ArgumentOutOfRangeException(nameof(poolSize), CoreStrings.InvalidPoolSize);
}

CheckContextConstructors<TContext>();

AddCoreServices<TContext>(serviceCollection,
(sp, ob) =>
{
Expand Down Expand Up @@ -170,13 +177,13 @@ public static class EntityFrameworkServiceCollectionExtensions
/// </summary>
/// <example>
/// <code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services.AddDbContext&lt;MyContext&gt;(ServiceLifetime.Scoped);
/// }
/// </code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services.AddDbContext&lt;MyContext&gt;(ServiceLifetime.Scoped);
/// }
/// </code>
/// </example>
/// <typeparam name="TContext"> The type of context to be registered. </typeparam>
/// <param name="serviceCollection"> The <see cref="IServiceCollection" /> to add services to. </param>
Expand Down Expand Up @@ -207,17 +214,17 @@ public static class EntityFrameworkServiceCollectionExtensions
/// </summary>
/// <example>
/// <code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services
/// .AddEntityFrameworkSqlServer()
/// .AddDbContext&lt;MyContext&gt;((serviceProvider, options) =>
/// options.UseSqlServer(connectionString)
/// .UseInternalServiceProvider(serviceProvider));
/// }
/// </code>
/// public void ConfigureServices(IServiceCollection services)
/// {
/// var connectionString = "connection string to database";
///
/// services
/// .AddEntityFrameworkSqlServer()
/// .AddDbContext&lt;MyContext&gt;((serviceProvider, options) =>
/// options.UseSqlServer(connectionString)
/// .UseInternalServiceProvider(serviceProvider));
/// }
/// </code>
/// </example>
/// <typeparam name="TContext"> The type of context to be registered. </typeparam>
/// <param name="serviceCollection"> The <see cref="IServiceCollection" /> to add services to. </param>
Expand Down Expand Up @@ -249,6 +256,11 @@ public static class EntityFrameworkServiceCollectionExtensions
{
Check.NotNull(serviceCollection, nameof(serviceCollection));

if (optionsAction != null)
{
CheckContextConstructors<TContext>();
}

AddCoreServices<TContext>(serviceCollection, optionsAction);

serviceCollection.TryAdd(new ServiceDescriptor(typeof(TContext), typeof(TContext), contextLifetime));
Expand All @@ -257,8 +269,8 @@ public static class EntityFrameworkServiceCollectionExtensions
}

private static void AddCoreServices<TContext>(
IServiceCollection serviceCollection,
Action<IServiceProvider, DbContextOptionsBuilder> optionsAction)
IServiceCollection serviceCollection,
Action<IServiceProvider, DbContextOptionsBuilder> optionsAction)
where TContext : DbContext
{
serviceCollection
Expand All @@ -283,5 +295,15 @@ public static class EntityFrameworkServiceCollectionExtensions

return builder.Options;
}

private static void CheckContextConstructors<TContext>() where TContext : DbContext
{
var declaredConstructors = typeof(TContext).GetTypeInfo().DeclaredConstructors.ToList();
if (declaredConstructors.Count == 1
&& declaredConstructors[0].GetParameters().Length == 0)
{
throw new ArgumentException(CoreStrings.DbContextMissingConstructor(typeof(TContext).ShortDisplayName()));
}
}
}
}
8 changes: 8 additions & 0 deletions src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.Designer.cs
View file

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

3 changes: 3 additions & 0 deletions src/Microsoft.EntityFrameworkCore/Properties/CoreStrings.resx
View file
Expand Up @@ -237,6 +237,9 @@
<data name="MultipleProvidersConfigured" xml:space="preserve">
<value>The database providers {storeNames}are configured. A context can only be configured to use a single database provider.</value>
</data>
<data name="DbContextMissingConstructor" xml:space="preserve">
<value>AddDbContext was called with configuration, but the context type '{contextType}' only declares a parameterless constructor. This means that the configuration passed to AddDbContext will never be used. If configuration is passed to AddDbContext, then ‘{contextType}’ should declare a constructor that accepts a DbContextOptions&lt;{contextType}&gt; and must pass it to the base constructor for DbContext.</value>
</data>
<data name="NoProviderConfigured" xml:space="preserve">
<value>No database provider has been configured for this DbContext. A provider can be configured by overriding the DbContext.OnConfiguring method or by using AddDbContext on the application service provider. If AddDbContext is used, then also ensure that your DbContext type accepts a DbContextOptions&lt;TContext&gt; object in its constructor and passes it to the base constructor for DbContext.</value>
</data>
Expand Down
16 changes: 11 additions & 5 deletions test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
View file
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Specification.Tests.TestUtilities.Xunit;
using Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests.TestModels;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -99,14 +100,19 @@ private class BadCtorContext : DbContext
}

[Fact]
public void Constructor_validation()
public void Throws_when_used_with_parameterless_constructor_context()
{
var serviceProvider = BuildServiceProvider<BadCtorContext>();
var serviceCollection = new ServiceCollection();

var serviceScope1 = serviceProvider.CreateScope();
Assert.Equal(CoreStrings.DbContextMissingConstructor(nameof(BadCtorContext)),
Assert.Throws<ArgumentException>(
() => serviceCollection.AddDbContextPool<BadCtorContext>(
_ => { })).Message);

Assert.Throws<InvalidOperationException>(
() => serviceScope1.ServiceProvider.GetService<BadCtorContext>());
Assert.Equal(CoreStrings.DbContextMissingConstructor(nameof(BadCtorContext)),
Assert.Throws<ArgumentException>(
() => serviceCollection.AddDbContextPool<BadCtorContext>(
(_, __) => { })).Message);
}

[Fact]
Expand Down
52 changes: 35 additions & 17 deletions test/Microsoft.EntityFrameworkCore.Tests/DbContextTest.cs
View file
Expand Up @@ -3791,6 +3791,22 @@ public void Can_add_derived_context_as_singleton(bool addSingletonFirst, bool us
Assert.Same(context1.Model, context2.Model);
}

[Fact]
public void Throws_when_used_with_parameterless_constructor_context()
{
var serviceCollection = new ServiceCollection();

Assert.Equal(CoreStrings.DbContextMissingConstructor(nameof(ConstructorTestContextWithOC1A)),
Assert.Throws<ArgumentException>(
() => serviceCollection.AddDbContext<ConstructorTestContextWithOC1A>(
_ => { })).Message);

Assert.Equal(CoreStrings.DbContextMissingConstructor(nameof(ConstructorTestContextWithOC1A)),
Assert.Throws<ArgumentException>(
() => serviceCollection.AddDbContext<ConstructorTestContextWithOC1A>(
(_, __) => { })).Message);
}

[Theory]
[InlineData(true, false)]
[InlineData(false, false)]
Expand All @@ -3799,20 +3815,21 @@ public void Can_add_derived_context_as_singleton_controlling_internal_services(b
{
var appServiceProivder = useDbContext
? new ServiceCollection()
.AddDbContext<ConstructorTestContextWithOC1A>(
(p, b) => b.UseInternalServiceProvider(p),
.AddEntityFrameworkInMemoryDatabase()
.AddDbContext<ConstructorTestContextWithOC3A>(
(p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase(),
ServiceLifetime.Singleton)
.BuildServiceProvider()
: (addSingletonFirst
? new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.AddSingleton<ConstructorTestContextWithOC1A>()
.AddDbContext<ConstructorTestContextWithOC1A>((p, b) => b.UseInternalServiceProvider(p))
.AddSingleton<ConstructorTestContextWithOC3A>()
.AddDbContext<ConstructorTestContextWithOC3A>((p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase())
.BuildServiceProvider()
: new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.AddDbContext<ConstructorTestContextWithOC1A>((p, b) => b.UseInternalServiceProvider(p))
.AddSingleton<ConstructorTestContextWithOC1A>()
.AddDbContext<ConstructorTestContextWithOC3A>((p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase())
.AddSingleton<ConstructorTestContextWithOC3A>()
.BuildServiceProvider());

var singleton = new object[3];
Expand All @@ -3823,7 +3840,7 @@ public void Can_add_derived_context_as_singleton_controlling_internal_services(b
.GetRequiredService<IServiceScopeFactory>()
.CreateScope())
{
context1 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC1A>();
context1 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC3A>();

Assert.NotNull(singleton[0] = context1.GetService<IInMemoryStoreSource>());
Assert.NotNull(singleton[1] = context1.GetService<ILoggerFactory>());
Expand All @@ -3838,7 +3855,7 @@ public void Can_add_derived_context_as_singleton_controlling_internal_services(b
.GetRequiredService<IServiceScopeFactory>()
.CreateScope())
{
context2 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC1A>();
context2 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC3A>();

Assert.Same(singleton[0], context2.GetService<IInMemoryStoreSource>());
Assert.Same(singleton[1], context2.GetService<ILoggerFactory>());
Expand Down Expand Up @@ -3917,20 +3934,21 @@ public void Can_add_derived_context_as_transient_controlling_internal_services(b
{
var appServiceProivder = useDbContext
? new ServiceCollection()
.AddDbContext<ConstructorTestContextWithOC1A>(
(p, b) => b.UseInternalServiceProvider(p),
.AddEntityFrameworkInMemoryDatabase()
.AddDbContext<ConstructorTestContextWithOC3A>(
(p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase(),
ServiceLifetime.Transient)
.BuildServiceProvider()
: (addTransientFirst
? new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.AddTransient<ConstructorTestContextWithOC1A>()
.AddDbContext<ConstructorTestContextWithOC1A>((p, b) => b.UseInternalServiceProvider(p))
.AddTransient<ConstructorTestContextWithOC3A>()
.AddDbContext<ConstructorTestContextWithOC3A>((p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase())
.BuildServiceProvider()
: new ServiceCollection()
.AddEntityFrameworkInMemoryDatabase()
.AddDbContext<ConstructorTestContextWithOC1A>((p, b) => b.UseInternalServiceProvider(p))
.AddTransient<ConstructorTestContextWithOC1A>()
.AddDbContext<ConstructorTestContextWithOC3A>((p, b) => b.UseInternalServiceProvider(p).UseInMemoryDatabase())
.AddTransient<ConstructorTestContextWithOC3A>()
.BuildServiceProvider());

var singleton = new object[3];
Expand All @@ -3939,8 +3957,8 @@ public void Can_add_derived_context_as_transient_controlling_internal_services(b
.GetRequiredService<IServiceScopeFactory>()
.CreateScope())
{
var context1 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC1A>();
var context2 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC1A>();
var context1 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC3A>();
var context2 = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC3A>();

Assert.NotSame(context1, context2);

Expand All @@ -3962,7 +3980,7 @@ public void Can_add_derived_context_as_transient_controlling_internal_services(b
.GetRequiredService<IServiceScopeFactory>()
.CreateScope())
{
var context = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC1A>();
var context = serviceScope.ServiceProvider.GetService<ConstructorTestContextWithOC3A>();

Assert.Same(singleton[0], context.GetService<IInMemoryStoreSource>());
Assert.Same(singleton[1], context.GetService<ILoggerFactory>());
Expand Down

0 comments on commit 094ea55

Please sign in to comment.