From a0e4492a351ebe0077a563e80f33f3f46efd5cb2 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 12 Dec 2012 03:47:04 +0500 Subject: [PATCH] New implementation of IDatabaseFactory, allows setting the DatabaseContext.Current at runtime with a custom IDatabaseFactory (mostly for testing). Changes AuditTrail to internal so the public way is just with the 'Audit' class. Fixed ThreadSafetyServiceTests which was failing with the new AuditTrail stuff because of the Database instances, this is not solved with the new PerThreadDatabaseFactory for the unit test. Created new 'UmbracoDatabase' object which inherits from the PetaPoco one so that we can future proof the implementation as we might want some custom logic on there. Now the IDatabaseFactory returns an UmbracoDatabase instead of just Database. --- src/Umbraco.Core/Auditing/Audit.cs | 3 + src/Umbraco.Core/Auditing/AuditTrail.cs | 2 +- src/Umbraco.Core/DatabaseContext.cs | 63 ++++++--------- .../Persistence/DatabaseProviders.cs | 2 +- .../Persistence/DefaultDatabaseFactory.cs | 61 +++++++++++++++ .../Persistence/IDatabaseFactory.cs | 12 +++ .../Persistence/UmbracoDatabase.cs | 32 ++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 3 + .../Services/ThreadSafetyServiceTest.cs | 77 +++++++++++++------ 9 files changed, 190 insertions(+), 65 deletions(-) create mode 100644 src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs create mode 100644 src/Umbraco.Core/Persistence/IDatabaseFactory.cs create mode 100644 src/Umbraco.Core/Persistence/UmbracoDatabase.cs diff --git a/src/Umbraco.Core/Auditing/Audit.cs b/src/Umbraco.Core/Auditing/Audit.cs index fe1ec955b8..9598982c2d 100644 --- a/src/Umbraco.Core/Auditing/Audit.cs +++ b/src/Umbraco.Core/Auditing/Audit.cs @@ -1,5 +1,8 @@ namespace Umbraco.Core.Auditing { + /// + /// Public method to create new audit trail + /// public static class Audit { public static void Add(AuditTypes type, string comment, int userId, int objectId) diff --git a/src/Umbraco.Core/Auditing/AuditTrail.cs b/src/Umbraco.Core/Auditing/AuditTrail.cs index 321ecb40aa..e4a7dfc7d4 100644 --- a/src/Umbraco.Core/Auditing/AuditTrail.cs +++ b/src/Umbraco.Core/Auditing/AuditTrail.cs @@ -5,7 +5,7 @@ namespace Umbraco.Core.Auditing /// /// Represents the Audit implementation /// - public class AuditTrail + internal class AuditTrail { #region Singleton diff --git a/src/Umbraco.Core/DatabaseContext.cs b/src/Umbraco.Core/DatabaseContext.cs index 6ebfe6079a..bc463d72f4 100644 --- a/src/Umbraco.Core/DatabaseContext.cs +++ b/src/Umbraco.Core/DatabaseContext.cs @@ -22,25 +22,34 @@ namespace Umbraco.Core /// public class DatabaseContext { - private bool _configured; + private readonly IDatabaseFactory _factory; + private bool _configured; private string _connectionString; private string _providerName; - private static volatile Database _globalInstance = null; - private static readonly object Locker = new object(); - #region Singleton - private static readonly Lazy lazy = new Lazy(() => new DatabaseContext()); - - /// - /// Gets the current Database Context. - /// - public static DatabaseContext Current { get { return lazy.Value; } } - private DatabaseContext() + private static DatabaseContext _customContext = null; + private static readonly Lazy lazy = new Lazy(() => new DatabaseContext(new DefaultDatabaseFactory())); + + /// + /// Gets the current Database Context. + /// + public static DatabaseContext Current + { + //return the _custom context if it is set, otherwise the automatic lazy instance + get { return _customContext ?? lazy.Value; } + //Allows setting a custom database context for the 'Current', normally used for unit tests or if the + //default IDatabaseFactory is not sufficient. + internal set { _customContext = value; } + } + + internal DatabaseContext(IDatabaseFactory factory) { + _factory = factory; } - #endregion + + #endregion /// /// Gets the object for doing CRUD operations @@ -49,38 +58,10 @@ namespace Umbraco.Core /// /// This should not be used for CRUD operations or queries against the /// standard Umbraco tables! Use the Public services for that. - /// - /// If we are running in an http context - /// it will create on per context, otherwise it will a global singleton object /// public Database Database { - get - { - //no http context, create the singleton global object - if (HttpContext.Current == null) - { - if (_globalInstance == null) - { - lock (Locker) - { - //double check - if (_globalInstance == null) - { - _globalInstance = new Database(GlobalSettings.UmbracoConnectionName); - } - } - } - return _globalInstance; - } - - //we have an http context, so only create one per request - if (!HttpContext.Current.Items.Contains(typeof(DatabaseContext))) - { - HttpContext.Current.Items.Add(typeof(DatabaseContext), new Database(GlobalSettings.UmbracoConnectionName)); - } - return (Database)HttpContext.Current.Items[typeof(DatabaseContext)]; - } + get { return _factory.CreateDatabase(); } } /// diff --git a/src/Umbraco.Core/Persistence/DatabaseProviders.cs b/src/Umbraco.Core/Persistence/DatabaseProviders.cs index e3c0ba125f..ec0064b8cf 100644 --- a/src/Umbraco.Core/Persistence/DatabaseProviders.cs +++ b/src/Umbraco.Core/Persistence/DatabaseProviders.cs @@ -1,6 +1,6 @@ namespace Umbraco.Core.Persistence { - public enum DatabaseProviders + public enum DatabaseProviders { SqlServer, SqlAzure, diff --git a/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs b/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs new file mode 100644 index 0000000000..257b2017ef --- /dev/null +++ b/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs @@ -0,0 +1,61 @@ +using System.Web; +using Umbraco.Core.Configuration; + +namespace Umbraco.Core.Persistence +{ + /// + /// The default implementation for the IDatabaseFactory + /// + /// + /// If we are running in an http context + /// it will create on per context, otherwise it will a global singleton object which is NOT thread safe + /// since we need (at least) a new instance of the database object per thread. + /// + internal class DefaultDatabaseFactory : DisposableObject, IDatabaseFactory + { + private static volatile UmbracoDatabase _globalInstance = null; + private static readonly object Locker = new object(); + + public UmbracoDatabase CreateDatabase() + { + //no http context, create the singleton global object + if (HttpContext.Current == null) + { + if (_globalInstance == null) + { + lock (Locker) + { + //double check + if (_globalInstance == null) + { + _globalInstance = new UmbracoDatabase(GlobalSettings.UmbracoConnectionName); + } + } + } + return _globalInstance; + } + + //we have an http context, so only create one per request + if (!HttpContext.Current.Items.Contains(typeof(DefaultDatabaseFactory))) + { + HttpContext.Current.Items.Add(typeof(DefaultDatabaseFactory), new Database(GlobalSettings.UmbracoConnectionName)); + } + return (UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]; + } + + protected override void DisposeResources() + { + if (HttpContext.Current == null) + { + _globalInstance.Dispose(); + } + else + { + if (HttpContext.Current.Items.Contains(typeof(DefaultDatabaseFactory))) + { + ((UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]).Dispose(); + } + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/IDatabaseFactory.cs b/src/Umbraco.Core/Persistence/IDatabaseFactory.cs new file mode 100644 index 0000000000..87c146fa0f --- /dev/null +++ b/src/Umbraco.Core/Persistence/IDatabaseFactory.cs @@ -0,0 +1,12 @@ +using System; + +namespace Umbraco.Core.Persistence +{ + /// + /// Used to create the UmbracoDatabase for use in the DatabaseContext + /// + internal interface IDatabaseFactory : IDisposable + { + UmbracoDatabase CreateDatabase(); + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/UmbracoDatabase.cs b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs new file mode 100644 index 0000000000..e48a615f9e --- /dev/null +++ b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs @@ -0,0 +1,32 @@ +using System.Data; +using System.Data.Common; + +namespace Umbraco.Core.Persistence +{ + /// + /// Represents the Umbraco implementation of the PetaPoco Database object + /// + /// + /// Currently this object exists for 'future proofing' our implementation. By having our own inheritied implementation we + /// can then override any additional execution (such as additional loggging, functionality, etc...) that we need to without breaking compatibility since we'll always be exposing + /// this object instead of the base PetaPoco database object. + /// + public class UmbracoDatabase : Database + { + public UmbracoDatabase(IDbConnection connection) : base(connection) + { + } + + public UmbracoDatabase(string connectionString, string providerName) : base(connectionString, providerName) + { + } + + public UmbracoDatabase(string connectionString, DbProviderFactory provider) : base(connectionString, provider) + { + } + + public UmbracoDatabase(string connectionStringName) : base(connectionStringName) + { + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 40d038138b..ec53a45a9a 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -197,6 +197,7 @@ + @@ -213,6 +214,7 @@ + @@ -396,6 +398,7 @@ + diff --git a/src/Umbraco.Tests/Services/ThreadSafetyServiceTest.cs b/src/Umbraco.Tests/Services/ThreadSafetyServiceTest.cs index e99dd7fa5e..6385e4979c 100644 --- a/src/Umbraco.Tests/Services/ThreadSafetyServiceTest.cs +++ b/src/Umbraco.Tests/Services/ThreadSafetyServiceTest.cs @@ -20,16 +20,27 @@ namespace Umbraco.Tests.Services public class ThreadSafetyServiceTest : BaseDatabaseFactoryTest { private PerThreadPetaPocoUnitOfWorkProvider _uowProvider; + private PerThreadDatabaseFactory _dbFactory; [SetUp] public override void Initialize() { base.Initialize(); + //we need to use our own custom IDatabaseFactory for the DatabaseContext because we MUST ensure that + //a Database instance is created per thread, whereas the default implementation which will work in an HttpContext + //threading environment, or a single apartment threading environment will not work for this test because + //it is multi-threaded. + _dbFactory = new PerThreadDatabaseFactory(); + //assign the custom factory to the new context and assign that to 'Current' + DatabaseContext.Current = new DatabaseContext(_dbFactory); + //overwrite the local object + DatabaseContext = DatabaseContext.Current; + //here we are going to override the ServiceContext because normally with our test cases we use a //global Database object but this is NOT how it should work in the web world or in any multi threaded scenario. //we need a new Database object for each thread. - _uowProvider = new PerThreadPetaPocoUnitOfWorkProvider(); + _uowProvider = new PerThreadPetaPocoUnitOfWorkProvider(_dbFactory); ServiceContext = new ServiceContext(_uowProvider, new FileUnitOfWorkProvider(), new PublishingStrategy()); CreateTestData(); @@ -39,9 +50,9 @@ namespace Umbraco.Tests.Services public override void TearDown() { _error = null; - _lastUowIdWithThread = null; //dispose! + _dbFactory.Dispose(); _uowProvider.Dispose(); base.TearDown(); @@ -54,9 +65,7 @@ namespace Umbraco.Tests.Services /// private volatile Exception _error = null; - private int _maxThreadCount = 1; - private object _locker = new object(); - private Tuple _lastUowIdWithThread = null; + private const int MaxThreadCount = 20; [Test] public void Ensure_All_Threads_Execute_Successfully_Content_Service() @@ -68,7 +77,7 @@ namespace Umbraco.Tests.Services Debug.WriteLine("Starting test..."); - for (var i = 0; i < _maxThreadCount; i++) + for (var i = 0; i < MaxThreadCount; i++) { var t = new Thread(() => { @@ -81,14 +90,14 @@ namespace Umbraco.Tests.Services var content1 = contentService.CreateContent(-1, "umbTextpage", 0); content1.Name = "test" + Guid.NewGuid(); Debug.WriteLine("Saving content1 on thread: " + Thread.CurrentThread.ManagedThreadId); - contentService.Save(content1); + contentService.Save(content1); - //Thread.Sleep(100); //quick pause for maximum overlap! + Thread.Sleep(100); //quick pause for maximum overlap! - //var content2 = contentService.CreateContent(-1, "umbTextpage", 0); - //content2.Name = "test" + Guid.NewGuid(); - //Debug.WriteLine("Saving content2 on thread: " + Thread.CurrentThread.ManagedThreadId); - //contentService.Save(content2); + var content2 = contentService.CreateContent(-1, "umbTextpage", 0); + content2.Name = "test" + Guid.NewGuid(); + Debug.WriteLine("Saving content2 on thread: " + Thread.CurrentThread.ManagedThreadId); + contentService.Save(content2); } catch(Exception e) { @@ -130,7 +139,7 @@ namespace Umbraco.Tests.Services Debug.WriteLine("Starting test..."); - for (var i = 0; i < _maxThreadCount; i++) + for (var i = 0; i < MaxThreadCount; i++) { var t = new Thread(() => { @@ -193,18 +202,16 @@ namespace Umbraco.Tests.Services } /// - /// Creates a Database object per thread, this mimics the web context which is per HttpContext + /// Creates a Database object per thread, this mimics the web context which is per HttpContext and is required for the multi-threaded test /// - internal class PerThreadPetaPocoUnitOfWorkProvider : DisposableObject, IDatabaseUnitOfWorkProvider - { - private readonly ConcurrentDictionary _databases = new ConcurrentDictionary(); + internal class PerThreadDatabaseFactory : DisposableObject, IDatabaseFactory + { + private readonly ConcurrentDictionary _databases = new ConcurrentDictionary(); - public IDatabaseUnitOfWork GetUnitOfWork() + public UmbracoDatabase CreateDatabase() { - //Create or get a database instance for this thread. - //var db = _databases.GetOrAdd(Thread.CurrentThread.ManagedThreadId, i => new Database(Umbraco.Core.Configuration.GlobalSettings.UmbracoConnectionName)); - - return new PetaPocoUnitOfWork(new Database(Umbraco.Core.Configuration.GlobalSettings.UmbracoConnectionName)); + var db = _databases.GetOrAdd(Thread.CurrentThread.ManagedThreadId, i => new UmbracoDatabase(Umbraco.Core.Configuration.GlobalSettings.UmbracoConnectionName)); + return db; } protected override void DisposeResources() @@ -214,5 +221,31 @@ namespace Umbraco.Tests.Services } } + /// + /// Creates a UOW with a Database object per thread + /// + internal class PerThreadPetaPocoUnitOfWorkProvider : DisposableObject, IDatabaseUnitOfWorkProvider + { + private readonly PerThreadDatabaseFactory _dbFactory; + + public PerThreadPetaPocoUnitOfWorkProvider(PerThreadDatabaseFactory dbFactory) + { + _dbFactory = dbFactory; + } + + public IDatabaseUnitOfWork GetUnitOfWork() + { + //Create or get a database instance for this thread. + var db = _dbFactory.CreateDatabase(); + return new PetaPocoUnitOfWork(db); + } + + protected override void DisposeResources() + { + //dispose the databases + _dbFactory.Dispose(); + } + } + } } \ No newline at end of file