Use TryAdd in AddXxx methods to only add services if not already pres…

…ent. Issue #956 and also see pull request #1129. This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed.
dotnet · Nov 27, 2014 · 4af77c4 · 4af77c4
1 parent 9159da5
commit 4af77c4
Show file tree

Hide file tree

Showing 23 changed files with 287 additions and 101 deletions.
diff --git a/src/EntityFramework.AzureTableStorage/Extensions/EntityServicesBuilderExtensions.cs b/src/EntityFramework.AzureTableStorage/Extensions/EntityServicesBuilderExtensions.cs
@@ -20,7 +20,7 @@ public static EntityServicesBuilder AddAzureTableStorage([NotNull] this EntitySe
         {
             Check.NotNull(builder, "builder");
 
-            builder.ServiceCollection
+            builder.ServiceCollection.TryAdd(new ServiceCollection()
                 .AddSingleton<AtsQueryFactory>()
                 .AddSingleton<TableEntityAdapterFactory>()
                 .AddSingleton<AtsValueReaderFactory>()
@@ -31,7 +31,7 @@ public static EntityServicesBuilder AddAzureTableStorage([NotNull] this EntitySe
                 .AddScoped<AtsDatabase>()
                 .AddScoped<AtsDataStore>()
                 .AddScoped<AtsConnection>()
-                .AddScoped<AtsDataStoreCreator>();
+                .AddScoped<AtsDataStoreCreator>());
 
             return builder;
         }

diff --git a/src/EntityFramework.InMemory/Extensions/InMemoryEntityServicesBuilderExtensions.cs b/src/EntityFramework.InMemory/Extensions/InMemoryEntityServicesBuilderExtensions.cs
@@ -18,7 +18,7 @@ public static EntityServicesBuilder AddInMemoryStore([NotNull] this EntityServic
         {
             Check.NotNull(builder, "builder");
 
-            builder.ServiceCollection
+            builder.ServiceCollection.TryAdd(new ServiceCollection()
                 .AddSingleton<InMemoryValueGeneratorCache>()
                 .AddSingleton<InMemoryValueGeneratorSelector>()
                 .AddSingleton<SimpleValueGeneratorFactory<InMemoryValueGenerator>>()
@@ -28,7 +28,7 @@ public static EntityServicesBuilder AddInMemoryStore([NotNull] this EntityServic
                 .AddScoped<InMemoryDatabaseFacade>()
                 .AddScoped<InMemoryDataStore>()
                 .AddScoped<InMemoryConnection>()
-                .AddScoped<InMemoryDataStoreCreator>();
+                .AddScoped<InMemoryDataStoreCreator>());
 
             return builder;
         }

diff --git a/src/EntityFramework.Migrations/MigrationsEntityServicesBuilderExtensions.cs b/src/EntityFramework.Migrations/MigrationsEntityServicesBuilderExtensions.cs
@@ -17,9 +17,10 @@ public static EntityServicesBuilder AddMigrations([NotNull] this EntityServicesB
 
             builder
                 .AddRelational().ServiceCollection
-                .AddScoped<MigrationAssembly>()
-                .AddScoped<HistoryRepository>()
-                .AddScoped(MigrationsDataStoreServices.MigratorFactory);
+                .TryAdd(new ServiceCollection()
+                    .AddScoped<MigrationAssembly>()
+                    .AddScoped<HistoryRepository>()
+                    .AddScoped(MigrationsDataStoreServices.MigratorFactory));
 
             return builder;
         }

diff --git a/src/EntityFramework.Redis/Extensions/RedisEntityServicesBuilderExtensions.cs b/src/EntityFramework.Redis/Extensions/RedisEntityServicesBuilderExtensions.cs
@@ -15,7 +15,7 @@ public static EntityServicesBuilder AddRedis([NotNull] this EntityServicesBuilde
         {
             Check.NotNull(builder, "builder");
 
-            builder.ServiceCollection
+            builder.ServiceCollection.TryAdd(new ServiceCollection()
                 .AddSingleton<RedisValueGeneratorSelector>()
                 .AddSingleton<RedisValueGeneratorCache>()
                 .AddSingleton<RedisValueGeneratorFactory>()
@@ -25,7 +25,7 @@ public static EntityServicesBuilder AddRedis([NotNull] this EntityServicesBuilde
                 .AddScoped<RedisDataStoreServices>()
                 .AddScoped<RedisConnection>()
                 .AddScoped<RedisDataStoreCreator>()
-                .AddScoped<RedisDatabase>();
+                .AddScoped<RedisDatabase>());
 
             return builder;
         }

diff --git a/src/EntityFramework.Redis/RedisDataStore.cs b/src/EntityFramework.Redis/RedisDataStore.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
@@ -21,6 +22,15 @@ public class RedisDataStore : DataStore
     {
         private readonly DbContextService<RedisDatabase> _database;
 
+        /// <summary>
+        ///     This constructor is intended only for use when creating test doubles that will override members
+        ///     with mocked or faked behavior. Use of this constructor for other purposes may result in unexpected
+        ///     behavior including but not limited to throwing <see cref="NullReferenceException" />.
+        /// </summary>
+        protected RedisDataStore()
+        {
+        }
+
         public RedisDataStore(
              [NotNull] StateManager stateManager,
              [NotNull] DbContextService<IModel> model,

diff --git a/src/EntityFramework.Relational/RelationalEntityServicesBuilderExtensions.cs b/src/EntityFramework.Relational/RelationalEntityServicesBuilderExtensions.cs
@@ -18,12 +18,12 @@ public static EntityServicesBuilder AddRelational([NotNull] this EntityServicesB
         {
             Check.NotNull(builder, "builder");
 
-            builder.ServiceCollection
+            builder.ServiceCollection.TryAdd(new ServiceCollection()
                 .AddSingleton<RelationalObjectArrayValueReaderFactory>()
                 .AddSingleton<RelationalTypedValueReaderFactory>()
                 .AddSingleton<ParameterNameGeneratorFactory>()
                 .AddSingleton<ModificationCommandComparer>()
-                .AddSingleton<GraphFactory, BidirectionalAdjacencyListGraphFactory>();
+                .AddSingleton<GraphFactory, BidirectionalAdjacencyListGraphFactory>());
 
             return builder;
         }

diff --git a/src/EntityFramework.SQLite/Extensions/SQLiteEntityServicesBuilderExtensions.cs b/src/EntityFramework.SQLite/Extensions/SQLiteEntityServicesBuilderExtensions.cs
@@ -7,9 +7,9 @@
 using Microsoft.Data.Entity.Migrations.Infrastructure;
 using Microsoft.Data.Entity.Relational;
 using Microsoft.Data.Entity.Sqlite;
-using Microsoft.Data.Entity.Sqlite.Utilities;
 using Microsoft.Data.Entity.Sqlite.Metadata;
 using Microsoft.Data.Entity.Sqlite.Migrations;
+using Microsoft.Data.Entity.Sqlite.Utilities;
 using Microsoft.Data.Entity.Storage;
 
 // ReSharper disable once CheckNamespace
@@ -24,30 +24,31 @@ public static EntityServicesBuilder AddSqlite([NotNull] this EntityServicesBuild
 
             builder
                 .AddMigrations().ServiceCollection
-                .AddSingleton<SqliteValueGeneratorCache>()
-                .AddSingleton<SqliteValueGeneratorSelector>()
-                .AddSingleton<SqliteSqlGenerator>()
-                .AddSingleton<SqlStatementExecutor>()
-                .AddSingleton<SqliteTypeMapper>()
-                .AddSingleton<SqliteModificationCommandBatchFactory>()
-                .AddSingleton<SqliteCommandBatchPreparer>()
-                .AddSingleton<SqliteMetadataExtensionProvider>()
-                .AddSingleton<SqliteMigrationOperationFactory>()
-                .AddScoped<SqliteBatchExecutor>()
-                .AddScoped<DataStoreSource, SqliteDataStoreSource>()
-                .AddScoped<SqliteDataStoreServices>()
-                .AddScoped<SqliteDataStore>()
-                .AddScoped<SqliteConnection>()
-                .AddScoped<SqliteMigrationOperationSqlGeneratorFactory>()
-                .AddScoped<SqliteDataStoreCreator>()
-                .AddScoped<SqliteMigrator>()
-                .AddScoped<SqliteDatabase>()
-                // TODO: Move to an AddMigrations extension method?
-                // Issue #556                
-                .AddScoped<SqliteMigrationOperationProcessor>()
-                .AddScoped<SqliteModelDiffer>()
-                .AddScoped<MigrationAssembly>()
-                .AddScoped<HistoryRepository>();
+                .TryAdd(new ServiceCollection()
+                    .AddSingleton<SqliteValueGeneratorCache>()
+                    .AddSingleton<SqliteValueGeneratorSelector>()
+                    .AddSingleton<SqliteSqlGenerator>()
+                    .AddSingleton<SqlStatementExecutor>()
+                    .AddSingleton<SqliteTypeMapper>()
+                    .AddSingleton<SqliteModificationCommandBatchFactory>()
+                    .AddSingleton<SqliteCommandBatchPreparer>()
+                    .AddSingleton<SqliteMetadataExtensionProvider>()
+                    .AddSingleton<SqliteMigrationOperationFactory>()
+                    .AddScoped<SqliteBatchExecutor>()
+                    .AddScoped<DataStoreSource, SqliteDataStoreSource>()
+                    .AddScoped<SqliteDataStoreServices>()
+                    .AddScoped<SqliteDataStore>()
+                    .AddScoped<SqliteConnection>()
+                    .AddScoped<SqliteMigrationOperationSqlGeneratorFactory>()
+                    .AddScoped<SqliteDataStoreCreator>()
+                    .AddScoped<SqliteMigrator>()
+                    .AddScoped<SqliteDatabase>()
+                    // TODO: Move to an AddMigrations extension method?
+                    // Issue #556
+                    .AddScoped<SqliteMigrationOperationProcessor>()
+                    .AddScoped<SqliteModelDiffer>()
+                    .AddScoped<MigrationAssembly>()
+                    .AddScoped<HistoryRepository>());
 
             return builder;
         }

diff --git a/src/EntityFramework.SQLite/SQLiteDataStore.cs b/src/EntityFramework.SQLite/SQLiteDataStore.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Open Technologies, Inc. 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.Data.Entity.ChangeTracking;
 using Microsoft.Data.Entity.Infrastructure;
@@ -11,13 +12,21 @@
 using Microsoft.Data.Entity.Relational.Query.Methods;
 using Microsoft.Data.Entity.Sqlite.Query;
 using Microsoft.Data.Entity.Sqlite.Utilities;
-using Microsoft.Data.Entity.Utilities;
 using Microsoft.Framework.Logging;
 
 namespace Microsoft.Data.Entity.Sqlite
 {
     public class SqliteDataStore : RelationalDataStore
     {
+        /// <summary>
+        ///     This constructor is intended only for use when creating test doubles that will override members
+        ///     with mocked or faked behavior. Use of this constructor for other purposes may result in unexpected
+        ///     behavior including but not limited to throwing <see cref="NullReferenceException" />.
+        /// </summary>
+        protected SqliteDataStore()
+        {
+        }
+
         public SqliteDataStore(
             [NotNull] StateManager stateManager,
             [NotNull] DbContextService<IModel> model,

diff --git a/src/EntityFramework.SqlServer/Extensions/SqlServerEntityServicesBuilderExtensions.cs b/src/EntityFramework.SqlServer/Extensions/SqlServerEntityServicesBuilderExtensions.cs
@@ -4,7 +4,7 @@
 using JetBrains.Annotations;
 using Microsoft.Data.Entity;
 using Microsoft.Data.Entity.Identity;
-using Microsoft.Data.Entity.Migrations.Infrastructure;
+using Microsoft.Data.Entity.Migrations;
 using Microsoft.Data.Entity.Relational;
 using Microsoft.Data.Entity.SqlServer;
 using Microsoft.Data.Entity.SqlServer.Metadata;
@@ -22,31 +22,30 @@ public static EntityServicesBuilder AddSqlServer([NotNull] this EntityServicesBu
         {
             Check.NotNull(builder, "builder");
 
-            builder.AddRelational().ServiceCollection
-                .AddSingleton<SqlServerValueGeneratorCache>()
-                .AddSingleton<SqlServerValueGeneratorSelector>()
-                .AddSingleton<SimpleValueGeneratorFactory<SequentialGuidValueGenerator>>()
-                .AddSingleton<SqlServerSequenceValueGeneratorFactory>()
-                .AddSingleton<SqlServerSqlGenerator>()
-                .AddSingleton<SqlStatementExecutor>()
-                .AddSingleton<SqlServerTypeMapper>()
-                .AddSingleton<SqlServerModificationCommandBatchFactory>()
-                .AddSingleton<SqlServerCommandBatchPreparer>()
-                .AddSingleton<SqlServerMetadataExtensionProvider>()
-                .AddSingleton<SqlServerMigrationOperationFactory>()
-                .AddScoped<SqlServerBatchExecutor>()
-                .AddScoped<DataStoreSource, SqlServerDataStoreSource>()
-                .AddScoped<SqlServerDataStoreServices>()
-                .AddScoped<SqlServerDataStore>()
-                .AddScoped<SqlServerConnection>()                
-                .AddScoped<SqlServerMigrationOperationProcessor>()
-                .AddScoped<SqlServerModelDiffer>()
-                .AddScoped<SqlServerDatabase>()
-                .AddScoped<SqlServerMigrationOperationSqlGeneratorFactory>()
-                .AddScoped<SqlServerDataStoreCreator>()
-                .AddScoped<MigrationAssembly>()
-                .AddScoped<HistoryRepository>()
-                .AddScoped<SqlServerMigrator>();
+            builder.AddMigrations().ServiceCollection
+                .TryAdd(new ServiceCollection()
+                    .AddSingleton<SqlServerValueGeneratorCache>()
+                    .AddSingleton<SqlServerValueGeneratorSelector>()
+                    .AddSingleton<SimpleValueGeneratorFactory<SequentialGuidValueGenerator>>()
+                    .AddSingleton<SqlServerSequenceValueGeneratorFactory>()
+                    .AddSingleton<SqlServerSqlGenerator>()
+                    .AddSingleton<SqlStatementExecutor>()
+                    .AddSingleton<SqlServerTypeMapper>()
+                    .AddSingleton<SqlServerModificationCommandBatchFactory>()
+                    .AddSingleton<SqlServerCommandBatchPreparer>()
+                    .AddSingleton<SqlServerMetadataExtensionProvider>()
+                    .AddSingleton<SqlServerMigrationOperationFactory>()
+                    .AddScoped<SqlServerBatchExecutor>()
+                    .AddScoped<DataStoreSource, SqlServerDataStoreSource>()
+                    .AddScoped<SqlServerDataStoreServices>()
+                    .AddScoped<SqlServerDataStore>()
+                    .AddScoped<SqlServerConnection>()
+                    .AddScoped<SqlServerMigrationOperationProcessor>()
+                    .AddScoped<SqlServerModelDiffer>()
+                    .AddScoped<SqlServerDatabase>()
+                    .AddScoped<SqlServerMigrationOperationSqlGeneratorFactory>()
+                    .AddScoped<SqlServerDataStoreCreator>()
+                    .AddScoped<SqlServerMigrator>());
 
             return builder;
         }

diff --git a/src/EntityFramework.SqlServer/SqlServerDataStore.cs b/src/EntityFramework.SqlServer/SqlServerDataStore.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Open Technologies, Inc. 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.Data.Entity.ChangeTracking;
 using Microsoft.Data.Entity.Infrastructure;
@@ -18,6 +19,15 @@ namespace Microsoft.Data.Entity.SqlServer
 {
     public class SqlServerDataStore : RelationalDataStore
     {
+        /// <summary>
+        ///     This constructor is intended only for use when creating test doubles that will override members
+        ///     with mocked or faked behavior. Use of this constructor for other purposes may result in unexpected
+        ///     behavior including but not limited to throwing <see cref="NullReferenceException" />.
+        /// </summary>
+        protected SqlServerDataStore()
+        {
+        }
+
         public SqlServerDataStore(
             [NotNull] StateManager stateManager,
             [NotNull] DbContextService<IModel> model,

diff --git a/src/EntityFramework/Extensions/EntityServiceCollectionExtensions.cs b/src/EntityFramework/Extensions/EntityServiceCollectionExtensions.cs
@@ -2,8 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
-using System.Linq;
 using JetBrains.Annotations;
 using Microsoft.Data.Entity;
 using Microsoft.Data.Entity.ChangeTracking;
@@ -31,7 +29,7 @@ public static EntityServicesBuilder AddEntityFramework(
         {
             Check.NotNull(serviceCollection, "serviceCollection");
 
-            serviceCollection
+            serviceCollection.TryAdd(new ServiceCollection()
                 .AddSingleton<IModelSource, DefaultModelSource>()
                 .AddSingleton<ModelBuilderFactory>()
                 .AddSingleton<SimpleValueGeneratorFactory<TemporaryValueGenerator>>()
@@ -77,45 +75,14 @@ public static EntityServicesBuilder AddEntityFramework(
                 .AddScoped(DataStoreServices.ConnectionFactory)
                 .AddScoped(DataStoreServices.DatabaseFactory)
                 .AddScoped(DataStoreServices.ValueGeneratorCacheFactory)
-                .AddScoped(DataStoreServices.DataStoreCreatorFactory);
-
-            EnsureLowLevelServices(serviceCollection);
+                .AddScoped(DataStoreServices.DataStoreCreatorFactory)
+                .AddSingleton<ILoggerFactory, LoggerFactory>()
+                .AddTypeActivator()
+                .AddOptions());
 
             return new EntityServicesBuilder(serviceCollection, configuration);
         }
 
-        private static void EnsureLowLevelServices(IServiceCollection serviceCollection)
-        {
-            var requiredServices = new List<Tuple<Type, Action<IServiceCollection>>>
-                {
-                    Tuple.Create<Type, Action<IServiceCollection>>(typeof(ILoggerFactory), c => c.AddSingleton<ILoggerFactory, LoggerFactory>()),
-                    Tuple.Create<Type, Action<IServiceCollection>>(typeof(ITypeActivator), c => c.AddTypeActivator()),
-                    Tuple.Create<Type, Action<IServiceCollection>>(typeof(IOptions<>), c => c.AddOptions()),
-                };
-
-            foreach (var descriptor in serviceCollection)
-            {
-                foreach (var serviceTuple in requiredServices)
-                {
-                    if (serviceTuple.Item1 == descriptor.ServiceType)
-                    {
-                        requiredServices.Remove(serviceTuple);
-                        break;
-                    }
-                }
-
-                if (!requiredServices.Any())
-                {
-                    break;
-                }
-            }
-
-            foreach (var serviceTuple in requiredServices)
-            {
-                serviceTuple.Item2(serviceCollection);
-            }
-        }
-
         public static EntityServicesBuilder AddDbContext<TContext>(
             [NotNull] this EntityServicesBuilder builder,
             [CanBeNull] Action<DbContextOptions> optionsAction = null)

diff --git a/test/EntityFramework.InMemory.Tests/InMemoryEntityServicesBuilderExtensionsTest.cs b/test/EntityFramework.InMemory.Tests/InMemoryEntityServicesBuilderExtensionsTest.cs
@@ -6,6 +6,7 @@
 using Microsoft.Data.Entity.Storage;
 using Microsoft.Data.Entity.Tests;
 using Microsoft.Framework.DependencyInjection;
+using Microsoft.Framework.DependencyInjection.Fallback;
 using Xunit;
 
 namespace Microsoft.Data.Entity.InMemory.Tests
@@ -33,5 +34,22 @@ public void Services_wire_up_correctly()
                 Assert.NotNull(scopedProvider.GetRequiredService<DataStoreSource>());
             }
         }
+
+        [Fact]
+        public void AddInMemoryStore_does_not_replace_services_already_registered()
+        {
+            var services = new ServiceCollection()
+                .AddSingleton<InMemoryDataStore, FakeInMemoryDataStore>();
+
+            services.AddEntityFramework().AddInMemoryStore();
+
+            var serviceProvider = services.BuildServiceProvider();
+
+            Assert.IsType<FakeInMemoryDataStore>(serviceProvider.GetRequiredService<InMemoryDataStore>());
+        }
+
+        private class FakeInMemoryDataStore : InMemoryDataStore
+        {
+        }
     }
 }