scope - cleanup

src/Umbraco.Core/Events/PassThroughEventDispatcher.cs

@@ -5,8 +5,10 @@ using System.Linq;
 namespace Umbraco.Core.Events
 {
     /// <summary>
-    /// This event manager supports event cancellation and will raise the events as soon as they are tracked, it does not store tracked events
+    /// An IEventDispatcher that immediately raise all events.
     /// </summary>
+    /// <remarks>This means that events will be raised during the scope transaction,
+    /// whatever happens, and the transaction could roll back in the end.</remarks>
     internal class PassThroughEventDispatcher : IEventDispatcher
     {
         public bool DispatchCancelable(EventHandler eventHandler, object sender, CancellableEventArgs args, string eventName = null)

src/Umbraco.Core/Events/QueuingEventDispatcher.cs

@@ -3,14 +3,12 @@ using Umbraco.Core.IO;
 namespace Umbraco.Core.Events
 {
     /// <summary>
-    /// This event manager is created for each scope and is aware of if it is nested in an outer scope
+    /// An IEventDispatcher that queues events, and raise them when the scope
+    /// exits and has been completed.
     /// </summary>
-    /// <remarks>
+    internal class QueuingEventDispatcher : QueuingEventDispatcherBase
-    /// The outer scope is the only scope that can raise events, the inner scope's will defer to the outer scope
-    /// </remarks>
-    internal class ScopeEventDispatcher : ScopeEventDispatcherBase
     {
-        public ScopeEventDispatcher()
+        public QueuingEventDispatcher()
             : base(true)
         { }
 

src/Umbraco.Core/Events/QueuingEventDispatcherBase.cs

@@ -4,12 +4,21 @@ using System.Linq;
 
 namespace Umbraco.Core.Events
 {
-    public abstract class ScopeEventDispatcherBase : IEventDispatcher
+    /// <summary>
+    /// An IEventDispatcher that queues events.
+    /// </summary>
+    /// <remarks>
+    /// <para>Can raise, or ignore, cancelable events, depending on option.</para>
+    /// <para>Implementations must override ScopeExitCompleted to define what
+    /// to do with the events when the scope exits and has been completed.</para>
+    /// <para>If the scope exits without being completed, events are ignored.</para>
+    /// </remarks>
+    public abstract class QueuingEventDispatcherBase : IEventDispatcher
     {
         private List<IEventDefinition> _events;
         private readonly bool _raiseCancelable;
 
-        protected ScopeEventDispatcherBase(bool raiseCancelable)
+        protected QueuingEventDispatcherBase(bool raiseCancelable)
         {
             _raiseCancelable = raiseCancelable;
         }

src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWork.cs

@@ -8,5 +8,6 @@ namespace Umbraco.Core.Persistence.UnitOfWork
         IScope Scope { get; }
         EventMessages Messages { get; }
         IEventDispatcher Events { get; }
+        void Flush();
     }
 }

src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs

@@ -160,7 +160,33 @@ namespace Umbraco.Core.Persistence.UnitOfWork
             _key = Guid.NewGuid();
         }
 
-		public object Key
+        public void Flush()
+        {
+            if (_readOnly)
+                throw new NotSupportedException("This unit of work is read-only.");
+
+            while (_operations.Count > 0)
+            {
+                var operation = _operations.Dequeue();
+                switch (operation.Type)
+                {
+                    case TransactionType.Insert:
+                        operation.Repository.PersistNewItem(operation.Entity);
+                        break;
+                    case TransactionType.Delete:
+                        operation.Repository.PersistDeletedItem(operation.Entity);
+                        break;
+                    case TransactionType.Update:
+                        operation.Repository.PersistUpdatedItem(operation.Entity);
+                        break;
+                }
+            }
+
+            _operations.Clear();
+            _key = Guid.NewGuid();
+        }
+
+        public object Key
 		{
 			get { return _key; }
 		}

src/Umbraco.Core/Scoping/Scope.cs

@@ -321,7 +321,7 @@ namespace Umbraco.Core.Scoping
             {
                 EnsureNotDisposed();
                 if (ParentScope != null) return ParentScope.Events;
-                return _eventDispatcher ?? (_eventDispatcher = new ScopeEventDispatcher());
+                return _eventDispatcher ?? (_eventDispatcher = new QueuingEventDispatcher());
             }
         }
 
@@ -465,6 +465,7 @@ namespace Umbraco.Core.Scoping
                     }
                     finally
                     {
+                        // removes the ambient context (ambient scope already gone)
                         _scopeProvider.SetAmbient(null);
                     }
                 }

src/Umbraco.Core/Scoping/ScopeContext.cs

@@ -1,16 +1,23 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 namespace Umbraco.Core.Scoping
 {
     public class ScopeContext : IInstanceIdentifiable
     {
         private Dictionary<string, IEnlistedObject> _enlisted;
+        private bool _exiting;
 
         public void ScopeExit(bool completed)
         {
+            if (_enlisted == null)
+                return;
+
+            _exiting = true;
+
             List<Exception> exceptions = null;
-            foreach (var enlisted in Enlisted.Values)
+            foreach (var enlisted in _enlisted.Values.OrderBy(x => x.Priority))
             {
                 try
                 {
@@ -23,6 +30,7 @@ namespace Umbraco.Core.Scoping
                     exceptions.Add(e);
                 }
             }
+
             if (exceptions != null)
                 throw new AggregateException("Exceptions were thrown by listed actions.", exceptions);
         }
@@ -42,73 +50,53 @@ namespace Umbraco.Core.Scoping
         private interface IEnlistedObject
         {
             void Execute(bool completed);
+            int Priority { get; }
         }
 
         private class EnlistedObject<T> : IEnlistedObject
         {
             private readonly Action<bool, T> _action;
 
-            public EnlistedObject(T item)
+            public EnlistedObject(T item, Action<bool, T> action, int priority)
-            {
-                Item = item;
-            }
-
-            public EnlistedObject(T item, Action<bool, T> action)
             {
                 Item = item;
+                Priority = priority;
                 _action = action;
             }
 
             public T Item { get; private set; }
 
+            public int Priority { get; private set; }
+
             public void Execute(bool completed)
             {
                 _action(completed, Item);
             }
         }
 
-        /// <inheritdoc />
+        public void Enlist(string key, Action<bool> action, int priority = 100)
-        public T Enlist<T>(string key, Func<T> creator)
         {
-            IEnlistedObject enlisted;
+            Enlist<object>(key, null, (completed, item) => action(completed), priority);
-            if (Enlisted.TryGetValue(key, out enlisted))
-            {
-                var enlistedAs = enlisted as EnlistedObject<T>;
-                if (enlistedAs == null) throw new Exception("An item with a different type has already been enlisted with the same key.");
-                return enlistedAs.Item;
-            }
-            var enlistedOfT = new EnlistedObject<T>(creator());
-            Enlisted[key] = enlistedOfT;
-            return enlistedOfT.Item;
         }
 
-        /// <inheritdoc />
+        public T Enlist<T>(string key, Func<T> creator = null, Action<bool, T> action = null, int priority = 100)
-        public T Enlist<T>(string key, Func<T> creator, Action<bool, T> action)
         {
+            if (_exiting)
+                throw new InvalidOperationException("Cannot enlist now, context is exiting.");
+
+            var enlistedObjects = _enlisted ?? (_enlisted = new Dictionary<string, IEnlistedObject>());
+
             IEnlistedObject enlisted;
-            if (Enlisted.TryGetValue(key, out enlisted))
+            if (enlistedObjects.TryGetValue(key, out enlisted))
             {
                 var enlistedAs = enlisted as EnlistedObject<T>;
-                if (enlistedAs == null) throw new Exception("An item with a different type has already been enlisted with the same key.");
+                if (enlistedAs == null) throw new InvalidOperationException("An item with the key already exists, but with a different type.");
+                if (enlistedAs.Priority != priority) throw new InvalidOperationException("An item with the key already exits, but with a different priority.");
                 return enlistedAs.Item;
             }
-            var enlistedOfT = new EnlistedObject<T>(creator(), action);
+            var enlistedOfT = new EnlistedObject<T>(creator == null ? default(T) : creator(), action, priority);
             Enlisted[key] = enlistedOfT;
             return enlistedOfT.Item;
         }
-
-        /// <inheritdoc />
-        public void Enlist(string key, Action<bool> action)
-        {
-            IEnlistedObject enlisted;
-            if (Enlisted.TryGetValue(key, out enlisted))
-            {
-                var enlistedAs = enlisted as EnlistedObject<object>;
-                if (enlistedAs == null) throw new Exception("An item with a different type has already been enlisted with the same key.");
-                return;
-            }
-            var enlistedOfT = new EnlistedObject<object>(null, (completed, item) => action(completed));
-            Enlisted[key] = enlistedOfT;
-        }
     }
 }

src/Umbraco.Core/Umbraco.Core.csproj

@@ -337,7 +337,7 @@
     <Compile Include="Deploy\IFileTypeCollection.cs" />
     <Compile Include="Events\EventDefinitionFilter.cs" />
     <Compile Include="Events\IDeletingMediaFilesEventArgs.cs" />
-    <Compile Include="Events\ScopeEventDispatcherBase.cs" />
+    <Compile Include="Events\QueuingEventDispatcherBase.cs" />
     <Compile Include="Events\ScopeLifespanMessagesFactory.cs" />
     <Compile Include="Exceptions\ConnectionException.cs" />
     <Compile Include="IHttpContextAccessor.cs" />
@@ -377,7 +377,7 @@
     <Compile Include="Events\EventNameExtractorResult.cs" />
     <Compile Include="Events\IEventDefinition.cs" />
     <Compile Include="Events\PassThroughEventDispatcher.cs" />
-    <Compile Include="Events\ScopeEventDispatcher.cs" />
+    <Compile Include="Events\QueuingEventDispatcher.cs" />
     <Compile Include="Events\EventMessage.cs" />
     <Compile Include="Events\EventMessages.cs" />
     <Compile Include="Events\ExportEventArgs.cs" />

src/Umbraco.Tests/Scoping/ScopeEventDispatcherTests.cs

@@ -183,7 +183,7 @@ namespace Umbraco.Tests.Scoping
 
         public static event TypedEventHandler<ScopeEventDispatcherTests, SaveEventArgs<decimal>> DoThing3;
 
-        public class PassiveEventDispatcher : ScopeEventDispatcherBase
+        public class PassiveEventDispatcher : QueuingEventDispatcherBase
         {
             public PassiveEventDispatcher()
                 : base(false)

src/Umbraco.Tests/Scoping/ScopeTests.cs

@@ -591,7 +591,7 @@ namespace Umbraco.Tests.Scoping
                         Assert.IsNull(scopeProvider.AmbientContext);
 
                         // back to original thread
-                        ScopeProvider.HttpContextItemsGetter = () => httpContextItems; 
+                        ScopeProvider.HttpContextItemsGetter = () => httpContextItems;
                     }
                     Assert.IsNotNull(scopeProvider.AmbientScope);
                     Assert.AreSame(scope, scopeProvider.AmbientScope);
@@ -656,6 +656,44 @@ namespace Umbraco.Tests.Scoping
             Assert.IsNotNull(ambientContext); // the context is still there
         }
 
+        [TestCase(true)]
+        [TestCase(false)]
+        public void ScopeContextEnlistAgain(bool complete)
+        {
+            var scopeProvider = DatabaseContext.ScopeProvider;
+
+            bool? completed = null;
+            Exception exception = null;
+
+            Assert.IsNull(scopeProvider.AmbientScope);
+            using (var scope = scopeProvider.CreateScope())
+            {
+                scopeProvider.Context.Enlist("name", c =>
+                {
+                    completed = c;
+
+                    // at that point the scope is gone, but the context is still there
+                    var ambientContext = scopeProvider.AmbientContext;
+
+                    try
+                    {
+                        ambientContext.Enlist("another", c2 => { });
+                    }
+                    catch (Exception e)
+                    {
+                        exception = e;
+                    }
+                });
+                if (complete)
+                    scope.Complete();
+            }
+            Assert.IsNull(scopeProvider.AmbientScope);
+            Assert.IsNull(scopeProvider.AmbientContext);
+            Assert.IsNotNull(completed);
+            Assert.IsNotNull(exception);
+            Assert.IsInstanceOf<InvalidOperationException>(exception);
+        }
+
         [Test]
         public void ScopeContextException()
         {