U4-9322 - scope and database, getting tests to run

This commit is contained in:
Stephan
2017-01-17 16:32:40 +01:00
parent 1261f6b159
commit 5f68f26d2f
14 changed files with 287 additions and 267 deletions

View File

@@ -62,106 +62,8 @@ namespace Umbraco.Core.Persistence
public UmbracoDatabase CreateDatabase()
{
return ScopeProvider.AmbientOrNoScope.Database;
//var scope = ScopeProvider.AmbientScope;
//return scope != null ? scope.Database : ScopeProvider.CreateNoScope().Database;
/*
UmbracoDatabase database;
// gets or creates a database, using either the call context (if no http context) or
// the current request context (http context) to store it. once done using the database,
// it should be disposed - which will remove it from whatever context it is currently
// stored in. this is automatic with http context because UmbracoDatabase implements
// IDisposeOnRequestEnd, but NOT with call context.
if (HttpContext.Current == null)
{
database = NonContextValue;
if (database == null)
{
lock (Locker)
{
database = NonContextValue;
if (database == null)
{
database = CreateDatabaseInstance(ContextOwner.CallContext);
NonContextValue = database;
}
#if DEBUG_DATABASES
else
{
Log("Get lcc", database);
}
#endif
}
}
#if DEBUG_DATABASES
else
{
Log("Get lcc", database);
}
#endif
return database;
}
if (HttpContext.Current.Items.Contains(typeof (DefaultDatabaseFactory)) == false)
{
database = CreateDatabaseInstance(ContextOwner.HttpContext);
HttpContext.Current.Items.Add(typeof (DefaultDatabaseFactory), database);
}
else
{
database = (UmbracoDatabase) HttpContext.Current.Items[typeof(DefaultDatabaseFactory)];
#if DEBUG_DATABASES
Log("Get ctx", database);
#endif
}
return database;
*/
}
// called by UmbracoDatabase when disposed, so that the factory can de-list it from context
/*
internal void OnDispose(UmbracoDatabase disposing)
{
var value = disposing;
switch (disposing.ContextOwner)
{
case ContextOwner.CallContext:
value = NonContextValue;
break;
case ContextOwner.HttpContext:
value = (UmbracoDatabase) HttpContext.Current.Items[typeof (DefaultDatabaseFactory)];
break;
}
if (value != null && value.InstanceId != disposing.InstanceId) throw new Exception("panic: wrong db.");
switch (disposing.ContextOwner)
{
case ContextOwner.CallContext:
NonContextValue = null;
#if DEBUG_DATABASES
Log("Clr lcc", disposing);
#endif
break;
case ContextOwner.HttpContext:
HttpContext.Current.Items.Remove(typeof(DefaultDatabaseFactory));
#if DEBUG_DATABASES
Log("Clr ctx", disposing);
#endif
break;
}
disposing.ContextOwner = ContextOwner.None;
#if DEBUG_DATABASES
_databases.Remove(value);
#endif
}
*/
#if DEBUG_DATABASES
// helps identifying when non-httpContext databases are created by logging the stack trace
private void LogCallContextStack()
@@ -188,58 +90,23 @@ namespace Umbraco.Core.Persistence
}
#endif
internal enum ContextOwner
{
None,
HttpContext,
CallContext
}
public UmbracoDatabase CreateNewDatabase()
{
return CreateDatabaseInstance(ContextOwner.None);
return CreateDatabaseInstance();
}
internal UmbracoDatabase CreateDatabaseInstance(ContextOwner contextOwner)
internal UmbracoDatabase CreateDatabaseInstance()
{
var database = string.IsNullOrEmpty(ConnectionString) == false && string.IsNullOrEmpty(ProviderName) == false
? new UmbracoDatabase(ConnectionString, ProviderName, _logger)
: new UmbracoDatabase(_connectionStringName, _logger);
database.ContextOwner = contextOwner;
database.DatabaseFactory = this;
//database.EnableSqlTrace = true;
#if DEBUG_DATABASES
Log("Create " + contextOwner, database);
if (contextOwner == ContextOwner.CallContext)
LogCallContextStack();
_databases.Add(database);
#endif
return database;
}
protected override void DisposeResources()
{
/*
UmbracoDatabase database;
if (HttpContext.Current == null)
{
database = NonContextValue;
#if DEBUG_DATABASES
Log("Release lcc", database);
#endif
}
else
{
database = (UmbracoDatabase) HttpContext.Current.Items[typeof (DefaultDatabaseFactory)];
#if DEBUG_DATABASES
Log("Release ctx", database);
#endif
}
if (database != null) database.Dispose(); // removes it from call context
*/
}
}
}

View File

@@ -29,7 +29,6 @@ namespace Umbraco.Core.Persistence
private int _spid = -1;
#endif
internal DefaultDatabaseFactory.ContextOwner ContextOwner = DefaultDatabaseFactory.ContextOwner.None;
internal DefaultDatabaseFactory DatabaseFactory = null;
/// <summary>
@@ -156,6 +155,8 @@ namespace Umbraco.Core.Persistence
// propagate timeout if none yet
#if DEBUG_DATABASES
// determines the database connection SPID for debugging
if (DatabaseType == DBType.MySql)
{
using (var command = connection.CreateCommand())
@@ -220,6 +221,7 @@ namespace Umbraco.Core.Persistence
}
#if DEBUG_DATABASES
// ensures the database does not have an open reader, for debugging
DatabaseDebugHelper.SetCommand(cmd, InstanceSid + " [T" + Thread.CurrentThread.ManagedThreadId + "]");
var refsobj = DatabaseDebugHelper.GetReferencedObjects(cmd.Connection);
if (refsobj != null) _logger.Debug<UmbracoDatabase>("Oops!" + Environment.NewLine + refsobj);
@@ -272,16 +274,5 @@ namespace Umbraco.Core.Persistence
//use the defaults
base.BuildSqlDbSpecificPagingQuery(databaseType, skip, take, sql, sqlSelectRemoved, sqlOrderBy, ref args, out sqlPage);
}
/*
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
#if DEBUG_DATABASES
LogHelper.Debug<UmbracoDatabase>("Dispose (" + InstanceSid + ").");
#endif
if (DatabaseFactory != null) DatabaseFactory.OnDispose(this);
}
*/
}
}

View File

@@ -141,15 +141,14 @@ namespace Umbraco.Core.Persistence.UnitOfWork
get { return _key; }
}
private IScope ThisScope
{
get { return _scope ?? (_scope = _scopeProvider.CreateScope()); }
}
public UmbracoDatabase Database
{
get
{
if (_scope == null)
//throw new InvalidOperationException("Out-of-scope UnitOfWork.");
_scope = _scopeProvider.CreateScope();
return _scope.Database;
}
get { return ThisScope.Database; }
}
#region Operation

View File

@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using Umbraco.Core.Events;
using Umbraco.Core.Persistence;
@@ -7,7 +8,7 @@ namespace Umbraco.Core.Scoping
/// <summary>
/// Represents a scope.
/// </summary>
public interface IScope : IDisposeOnRequestEnd // implies IDisposable
public interface IScope : IDisposable
{
/// <summary>
/// Gets the scope database.

View File

@@ -1,7 +1,7 @@
namespace Umbraco.Core.Scoping
{
/// <summary>
/// Provices scopes.
/// Provides scopes.
/// </summary>
/// <remarks>Extends <see cref="IScopeProvider"/> with internal features.</remarks>
internal interface IScopeProviderInternal : IScopeProvider
@@ -11,18 +11,16 @@
/// </summary>
IScope AmbientScope { get; }
// fixme
/// <summary>
/// Gets the ambient scope if any, else creates and returns a <see cref="NoScope"/>.
/// </summary>
IScope AmbientOrNoScope { get; }
/// <summary>
/// Creates a <see cref="NoScope"/> instance.
/// Resets the ambient scope.
/// </summary>
/// <returns>The created ambient scope.</returns>
/// <remarks>
/// <para>The created scope becomes the ambient scope.</para>
/// <para>If an ambient scope already exists, throws.</para>
/// <para>The <see cref="NoScope"/> instance can be eventually replaced by a real instance.</para>
/// </remarks>
//IScope CreateNoScope();
/// <remarks>Resets the ambient scope (not completed anymore) and disposes the
/// entire scopes chain until there is no more scopes.</remarks>
void Reset();
}
}

View File

@@ -5,6 +5,9 @@ using Umbraco.Core.Persistence;
namespace Umbraco.Core.Scoping
{
/// <summary>
/// Implements <see cref="IScope"/> when there is no scope.
/// </summary>
internal class NoScope : IScope
{
private readonly ScopeProvider _scopeProvider;
@@ -18,8 +21,7 @@ namespace Umbraco.Core.Scoping
_scopeProvider = scopeProvider;
}
//public bool HasDatabase { get { return _database != null; } }
/// <inheritdoc />
public UmbracoDatabase Database
{
get
@@ -38,8 +40,7 @@ namespace Umbraco.Core.Scoping
}
}
//public bool HasMessages { get { return _messages != null; } }
/// <inheritdoc />
public IList<EventMessage> Messages
{
get
@@ -58,6 +59,7 @@ namespace Umbraco.Core.Scoping
}
}
/// <inheritdoc />
public void Complete()
{
throw new NotImplementedException();
@@ -72,7 +74,15 @@ namespace Umbraco.Core.Scoping
public void Dispose()
{
EnsureNotDisposed();
_scopeProvider.Disposing(this);
if (this != _scopeProvider.AmbientScope)
throw new InvalidOperationException("Not the ambient scope.");
if (_database != null)
_database.Dispose();
_scopeProvider.AmbientScope = null;
_disposed = true;
GC.SuppressFinalize(this);
}

View File

@@ -5,8 +5,10 @@ using Umbraco.Core.Persistence;
namespace Umbraco.Core.Scoping
{
// note - scope is not thread-safe obviously
/// <summary>
/// Implements <see cref="IScope"/>.
/// </summary>
/// <remarks>Not thread-safe obviously.</remarks>
internal class Scope : IScope
{
private readonly ScopeProvider _scopeProvider;
@@ -53,11 +55,6 @@ namespace Umbraco.Core.Scoping
// the original scope (when attaching a detachable scope)
public Scope OrigScope { get; set; }
//public bool HasDatabase
//{
// get { return ParentScope == null ? _database != null : ParentScope.HasDatabase; }
//}
/// <inheritdoc />
public UmbracoDatabase Database
{
@@ -88,11 +85,6 @@ namespace Umbraco.Core.Scoping
}
}
//public bool HasMessages
//{
// get { return ParentScope == null ? _messages != null : ParentScope.HasMessages; }
//}
/// <inheritdoc />
public IList<EventMessage> Messages
{
@@ -122,26 +114,16 @@ namespace Umbraco.Core.Scoping
_completed = true;
}
public void CompleteChild(bool? completed)
public void Reset()
{
if (completed.HasValue)
{
if (completed.Value)
{
// child did complete
// nothing to do
}
else
{
// child did not complete, we cannot complete
_completed = false;
}
}
else
{
// child did not complete, we cannot complete
_completed = null;
}
public void ChildCompleted(bool? completed)
{
// if child did not complete we cannot complete
if (completed.HasValue == false || completed.Value == false)
_completed = false;
}
}
private void EnsureNotDisposed()
@@ -153,9 +135,42 @@ namespace Umbraco.Core.Scoping
public void Dispose()
{
EnsureNotDisposed();
_scopeProvider.Disposing(this, _completed);
if (this != _scopeProvider.AmbientScope)
throw new InvalidOperationException("Not the ambient scope.");
var parent = ParentScope;
_scopeProvider.AmbientScope = parent;
if (parent != null)
parent.ChildCompleted(_completed);
else
DisposeLastScope();
_disposed = true;
GC.SuppressFinalize(this);
}
private void DisposeLastScope()
{
// note - messages
// at the moment we are totally not filtering the messages based on completion
// status, so whether the scope is committed or rolled back makes no difference
if (_database == null) return;
try
{
if (_completed.HasValue && _completed.Value)
_database.CompleteTransaction();
else
_database.AbortTransaction();
}
finally
{
_database.Dispose();
_database = null;
}
}
}
}

View File

@@ -5,6 +5,9 @@ using Umbraco.Core.Persistence;
namespace Umbraco.Core.Scoping
{
/// <summary>
/// Implements <see cref="IScopeProvider"/>.
/// </summary>
internal class ScopeProvider : IScopeProviderInternal
{
public ScopeProvider(IDatabaseFactory2 databaseFactory)
@@ -26,13 +29,17 @@ namespace Umbraco.Core.Scoping
var ambient = StaticAmbientScope;
if (ambient != null)
ambient.Dispose();
StaticAmbientScope = (IScope) scope;
StaticAmbientScope = (IScope)scope;
});
}
public IDatabaseFactory2 DatabaseFactory { get; private set; }
private const string ItemKey = "Umbraco.Core.Scoping.IScope";
private const string ItemRefKey = "Umbraco.Core.Scoping.ScopeReference";
// only 1 instance which can be disposed and disposed again
private static readonly ScopeReference StaticScopeReference = new ScopeReference(new ScopeProvider(null));
private static IScope CallContextValue
{
@@ -49,8 +56,17 @@ namespace Umbraco.Core.Scoping
get { return (IScope) HttpContext.Current.Items[ItemKey]; }
set
{
if (value == null) HttpContext.Current.Items.Remove(ItemKey);
else HttpContext.Current.Items[ItemKey] = value;
if (value == null)
{
HttpContext.Current.Items.Remove(ItemKey);
HttpContext.Current.Items.Remove(ItemRefKey);
}
else
{
HttpContext.Current.Items[ItemKey] = value;
if (HttpContext.Current.Items[ItemRefKey] == null)
HttpContext.Current.Items[ItemRefKey] = StaticScopeReference;
}
}
}
@@ -164,54 +180,14 @@ namespace Umbraco.Core.Scoping
return AmbientScope = new Scope(this, scope);
}
public void Disposing(IScope disposing, bool? completed = null)
/// <inheritdoc />
public void Reset()
{
if (disposing != AmbientScope)
throw new InvalidOperationException();
var scope = AmbientScope as Scope;
if (scope != null)
scope.Reset();
var noScope = disposing as NoScope;
if (noScope != null)
{
// fixme - kinda legacy
var noScopeDatabase = noScope.DatabaseOrNull;
if (noScopeDatabase != null)
{
if (noScopeDatabase.InTransaction)
throw new Exception();
noScopeDatabase.Dispose();
}
AmbientScope = null;
return;
}
var scope = disposing as Scope;
if (scope == null)
throw new Exception();
var parent = scope.ParentScope;
AmbientScope = parent;
if (parent != null)
{
parent.CompleteChild(completed);
return;
}
// fixme - a scope is in a transaction only if ... there is a db transaction, or always?
// what shall we do with events if not in a transaction?
// fixme - when completing... the db should be released, no need to dispose the db?
// note - messages
// at the moment we are totally not filtering the messages based on completion
// status, so whether the scope is committed or rolled back makes no difference
var database = scope.DatabaseOrNull;
if (database == null) return;
if (completed.HasValue && completed.Value)
database.CompleteTransaction();
else
database.AbortTransaction();
StaticScopeReference.Dispose();
}
}
}

View File

@@ -0,0 +1,31 @@
namespace Umbraco.Core.Scoping
{
/// <summary>
/// References a scope.
/// </summary>
/// <remarks>Should go into HttpContext to indicate there is also an IScope in context
/// that needs to be disposed at the end of the request (the scope, and the entire scopes
/// chain).</remarks>
internal class ScopeReference : IDisposeOnRequestEnd // implies IDisposable
{
private readonly IScopeProviderInternal _scopeProvider;
public ScopeReference(IScopeProviderInternal scopeProvider)
{
_scopeProvider = scopeProvider;
}
public void Dispose()
{
// dispose the entire chain (if any)
// reset (don't commit by default)
IScope scope;
while ((scope = _scopeProvider.AmbientScope) != null)
{
if (scope is Scope)
((Scope) scope).Reset();
scope.Dispose();
}
}
}
}

View File

@@ -518,6 +518,7 @@
<Compile Include="Scoping\NoScope.cs" />
<Compile Include="Scoping\Scope.cs" />
<Compile Include="Scoping\ScopeProvider.cs" />
<Compile Include="Scoping\ScopeReference.cs" />
<Compile Include="Security\ActiveDirectoryBackOfficeUserPasswordChecker.cs" />
<Compile Include="Security\BackOfficeClaimsIdentityFactory.cs" />
<Compile Include="Security\BackOfficeCookieAuthenticationProvider.cs" />

View File

@@ -0,0 +1,90 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Linq;
using System.Runtime.Remoting.Messaging;
using System.Text;
using System.Threading.Tasks;
using NUnit.Framework;
using Umbraco.Core.Persistence;
using Umbraco.Core.Scoping;
using Umbraco.Tests.TestHelpers;
namespace Umbraco.Tests.Scoping
{
[TestFixture]
[DatabaseTestBehavior(DatabaseBehavior.EmptyDbFilePerTest)]
public class LeakTests : BaseDatabaseFactoryTest
{
private UmbracoDatabase _database;
private IDbConnection _connection;
// setup
public override void Initialize()
{
base.Initialize();
//// initialization leaves a NoScope around, remove it
//var scope = DatabaseContext.ScopeProvider.AmbientScope;
//Assert.IsNotNull(scope);
//Assert.IsInstanceOf<NoScope>(scope);
//scope.Dispose();
Assert.IsNull(DatabaseContext.ScopeProvider.AmbientScope); // gone
}
// note: testing this with a test action is pointless as the
// AfterTest method runs *before* the TearDown methods which
// are the methods that should cleanup the call context...
[Test]
public void LeakTest()
{
_database = DatabaseContext.Database; // creates a database
_database.Execute("CREATE TABLE foo (id INT)"); // opens a connection
Assert.IsNull(_database.Connection); // is immediately closed
_database.BeginTransaction(); // opens and maintains a connection
// the test is leaking a scope with a non-null database
var contextScope = CallContext.LogicalGetData("Umbraco.Core.Scoping.IScope");
Assert.IsNotNull(contextScope);
Assert.IsInstanceOf<NoScope>(CallContext.LogicalGetData("Umbraco.Core.Scoping.IScope"));
Assert.IsNotNull(((NoScope) contextScope).DatabaseOrNull);
Assert.AreSame(_database, ((NoScope)contextScope).DatabaseOrNull);
// save the connection
_connection = _database.Connection;
Assert.IsInstanceOf<StackExchange.Profiling.Data.ProfiledDbConnection>(_connection);
_connection = ((StackExchange.Profiling.Data.ProfiledDbConnection) _connection).InnerConnection;
// the connection is open
Assert.IsNotNull(_connection);
Assert.AreEqual(ConnectionState.Open, _connection.State);
}
// need to explicitely do it in every test which kinda defeats
// the purposes of having an automated check? give me v8!
private static void AssertSafeCallContext()
{
var scope = CallContext.LogicalGetData("Umbraco.Core.Scoping.IScope");
if (scope != null) throw new Exception("Leaked call context scope.");
}
[TearDown]
public override void TearDown()
{
base.TearDown();
// the leaked scope should be gone
AssertSafeCallContext();
// its database should have been disposed
Assert.IsNull(_database.Connection);
// the underlying connection should have been closed
Assert.AreEqual(ConnectionState.Closed, _connection.State);
}
}
}

View File

@@ -1,5 +1,6 @@
using System;
using NUnit.Framework;
using Umbraco.Core;
using Umbraco.Core.Persistence;
using Umbraco.Core.Scoping;
using Umbraco.Tests.TestHelpers;
@@ -15,11 +16,11 @@ namespace Umbraco.Tests.Scoping
{
base.Initialize();
// initialization leaves a NoScope around, remove it
var scope = DatabaseContext.ScopeProvider.AmbientScope;
Assert.IsNotNull(scope);
Assert.IsInstanceOf<NoScope>(scope);
scope.Dispose();
//// initialization leaves a NoScope around, remove it
//var scope = DatabaseContext.ScopeProvider.AmbientScope;
//Assert.IsNotNull(scope);
//Assert.IsInstanceOf<NoScope>(scope);
//scope.Dispose();
Assert.IsNull(DatabaseContext.ScopeProvider.AmbientScope); // gone
}
@@ -102,7 +103,7 @@ namespace Umbraco.Tests.Scoping
Assert.IsInstanceOf<Scope>(nested);
Assert.IsNotNull(scopeProvider.AmbientScope);
Assert.AreSame(nested, scopeProvider.AmbientScope);
Assert.AreSame(scope, ((Scope)nested).ParentScope);
Assert.AreSame(scope, ((Scope) nested).ParentScope);
Assert.AreSame(database, nested.Database);
}
Assert.IsNotNull(database.Connection); // still
@@ -196,7 +197,7 @@ namespace Umbraco.Tests.Scoping
Assert.IsNotNull(database.Connection); // no more
// nested does not have a parent
Assert.IsNull(((Scope)nested).ParentScope);
Assert.IsNull(((Scope) nested).ParentScope);
}
// and when nested is gone, scope is gone
@@ -223,7 +224,8 @@ namespace Umbraco.Tests.Scoping
Assert.Throws<Exception>(() =>
{
// could not steal the database
/*var nested =*/ scopeProvider.CreateScope();
/*var nested =*/
scopeProvider.CreateScope();
});
// cleanup
@@ -387,5 +389,39 @@ namespace Umbraco.Tests.Scoping
Assert.AreEqual("b", n);
}
}
[Test]
public void CallContextScope()
{
var scopeProvider = DatabaseContext.ScopeProvider;
var scope = scopeProvider.CreateScope();
Assert.IsNotNull(scopeProvider.AmbientScope);
using (new SafeCallContext())
{
Assert.IsNull(scopeProvider.AmbientScope);
}
Assert.IsNotNull(scopeProvider.AmbientScope);
Assert.AreSame(scope, scopeProvider.AmbientScope);
}
[Test]
public void ScopeReference()
{
var scopeProvider = DatabaseContext.ScopeProvider;
var scope = scopeProvider.CreateScope();
var nested = scopeProvider.CreateScope();
Assert.IsNotNull(scopeProvider.AmbientScope);
var scopeRef = new ScopeReference(scopeProvider);
scopeRef.Dispose();
Assert.IsNull(scopeProvider.AmbientScope);
Assert.Throws<ObjectDisposedException>(() =>
{
var db = scope.Database;
});
Assert.Throws<ObjectDisposedException>(() =>
{
var db = nested.Database;
});
}
}
}
}

View File

@@ -280,9 +280,9 @@ namespace Umbraco.Tests.TestHelpers
_isFirstTestInFixture = false; //ensure this is false before anything!
if (DatabaseTestBehavior == DatabaseBehavior.NewDbFileAndSchemaPerTest)
{
RemoveDatabaseFile();
}
RemoveDatabaseFile(); // closes connections too
else
CloseDbConnections();
AppDomain.CurrentDomain.SetData("DataDirectory", null);
@@ -294,10 +294,14 @@ namespace Umbraco.Tests.TestHelpers
private void CloseDbConnections()
{
//Ensure that any database connections from a previous test is disposed.
//This is really just double safety as its also done in the TearDown.
if (ApplicationContext != null && DatabaseContext != null && DatabaseContext.Database != null)
DatabaseContext.Database.Dispose();
// just to be sure, although it's also done in TearDown
if (ApplicationContext != null
&& ApplicationContext.DatabaseContext != null
&& ApplicationContext.DatabaseContext.ScopeProvider != null)
{
ApplicationContext.DatabaseContext.ScopeProvider.Reset();
}
SqlCeContextGuardian.CloseBackgroundConnection();
}

View File

@@ -168,6 +168,7 @@
<Compile Include="Persistence\Repositories\RedirectUrlRepositoryTests.cs" />
<Compile Include="Scheduling\DeployTest.cs" />
<Compile Include="Routing\NiceUrlRoutesTests.cs" />
<Compile Include="Scoping\LeakTests.cs" />
<Compile Include="Scoping\ScopeTests.cs" />
<Compile Include="TestHelpers\Entities\MockedPropertyTypes.cs" />
<Compile Include="TryConvertToTests.cs" />