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