Get rid of IScopeInternal, add IScopeContext

src/Umbraco.Core/Scoping/IScopeContext.cs

@@ -0,0 +1,42 @@
+using System;
+
+namespace Umbraco.Core.Scoping
+{
+    /// <summary>
+    /// Represents a scope context.
+    /// </summary>
+    /// <remarks>A scope context can enlist objects that will be attached to the scope, and available
+    /// for the duration of the scope. In addition, it can enlist actions, that will run when the
+    /// scope is exiting, and after the database transaction has been commited.</remarks>
+    public interface IScopeContext
+    {
+        /// <summary>
+        /// Enlists an action.
+        /// </summary>
+        /// <param name="key">The action unique identifier.</param>
+        /// <param name="action">The action.</param>
+        /// <param name="priority">The optional action priority (default is 100, lower runs first).</param>
+        /// <remarks>
+        /// <para>It is ok to enlist multiple action with the same key but only the first one will run.</para>
+        /// <para>The action boolean parameter indicates whether the scope completed or not.</para>
+        /// </remarks>
+        void Enlist(string key, Action<bool> action, int priority = 100);
+
+        /// <summary>
+        /// Enlists an object and action.
+        /// </summary>
+        /// <typeparam name="T">The type of the object.</typeparam>
+        /// <param name="key">The object unique identifier.</param>
+        /// <param name="creator">A function providing the object.</param>
+        /// <param name="action">The optional action.</param>
+        /// <param name="priority">The optional action priority (default is 100, lower runs first).</param>
+        /// <returns>The object.</returns>
+        /// <remarks>
+        /// <para>On the first time an object is enlisted with a given key, the object is actually
+        /// created. Next calls just return the existing object. It is ok to enlist multiple objects
+        /// and action with the same key but only the first one is used, the others are ignored.</para>
+        /// <para>The action boolean parameter indicates whether the scope completed or not.</para>
+        /// </remarks>
+        T Enlist<T>(string key, Func<T> creator, Action<bool, T> action = null, int priority = 100);
+    }
+}

src/Umbraco.Core/Scoping/IScopeInternal.cs

@@ -1,56 +0,0 @@
-using System.Data;
-using Umbraco.Core.Events;
-using Umbraco.Core.Persistence;
-
-namespace Umbraco.Core.Scoping
-{
-    /// <summary>
-    /// Provides additional, internal scope functionnalities.
-    /// </summary>
-    internal interface IScopeInternal : IScope // fixme - define what's internal and why
-    {
-        /// <summary>
-        /// Gets the parent scope, if any, or null.
-        /// </summary>
-        IScopeInternal ParentScope { get; }
-
-        /// <summary>
-        /// Gets a value indicating whether this scope should be registered in
-        /// call context even when an http context is available.
-        /// </summary>
-        bool CallContext { get; }
-
-        /// <summary>
-        /// Gets the scope transaction isolation level.
-        /// </summary>
-        IsolationLevel IsolationLevel { get; }
-
-        /// <summary>
-        /// Gets the scope database, if any, else null.
-        /// </summary>
-        IUmbracoDatabase DatabaseOrNull { get; }
-
-        /// <summary>
-        /// Get the scope event messages, if any, else null.
-        /// </summary>
-        EventMessages MessagesOrNull { get; }
-
-        /// <summary>
-        /// Gets a value indicating whether filesystems are scoped.
-        /// </summary>
-        bool ScopedFileSystems { get; }
-
-        /// <summary>
-        /// Registers that a child has completed.
-        /// </summary>
-        /// <param name="completed">The child's completion status.</param>
-        /// <remarks>Completion status can be true (completed), false (could not complete), or null (not properly exited).</remarks>
-        void ChildCompleted(bool? completed);
-
-        /// <summary>
-        /// Resets the scope.
-        /// </summary>
-        /// <remarks>Reset completion to "unspecified".</remarks>
-        void Reset();
-    }
-}

src/Umbraco.Core/Scoping/IScopeProvider.cs

@@ -74,7 +74,7 @@ namespace Umbraco.Core.Scoping
         /// <summary>
         /// Gets the scope context.
         /// </summary>
-        ScopeContext Context { get; }
+        IScopeContext Context { get; }
 
 #if DEBUG_SCOPES
         Dictionary<Guid, object> CallContextObjects { get; }

src/Umbraco.Core/Scoping/Scope.cs

@@ -14,7 +14,7 @@ namespace Umbraco.Core.Scoping
     /// Implements <see cref="IScope"/>.
     /// </summary>
     /// <remarks>Not thread-safe obviously.</remarks>
-    internal class Scope : IScopeInternal
+    internal class Scope : IScope
     {
         // fixme
         // considering that a great amount of things here are only useful for the top-level
@@ -182,12 +182,12 @@ namespace Umbraco.Core.Scoping
         public bool Detachable { get; }
 
         // the parent scope (in a nested scopes chain)
-        public IScopeInternal ParentScope { get; set; }
+        public Scope ParentScope { get; set; }
 
         public bool Attached { get; set; }
 
         // the original scope (when attaching a detachable scope)
-        public IScopeInternal OrigScope { get; set; }
+        public Scope OrigScope { get; set; }
 
         // the original context (when attaching a detachable scope)
         public ScopeContext OrigContext { get; set; }

src/Umbraco.Core/Scoping/ScopeContext.cs

@@ -4,10 +4,7 @@ using System.Linq;
 
 namespace Umbraco.Core.Scoping
 {
-    // fixme should we have an IScopeContext?
+    internal class ScopeContext : IScopeContext, IInstanceIdentifiable
-    // fixme document all this properly!
-
-    public class ScopeContext : IInstanceIdentifiable
     {
         private Dictionary<string, IEnlistedObject> _enlisted;
         private bool _exiting;
@@ -62,7 +59,7 @@ namespace Umbraco.Core.Scoping
 
             public T Item { get; }
 
-            public int Priority { get; private set; }
+            public int Priority { get; }
 
             public void Execute(bool completed)
             {
@@ -70,38 +67,19 @@ namespace Umbraco.Core.Scoping
             }
         }
 
-        // todo: replace with optional parameters when we can break things
+        public void Enlist(string key, Action<bool> action, int priority = 100)
-        public T Enlist<T>(string key, Func<T> creator)
-        {
-            return Enlist(key, creator, null, 100);
-        }
-
-        // todo: replace with optional parameters when we can break things
-        public T Enlist<T>(string key, Func<T> creator, Action<bool, T> action)
-        {
-            return Enlist(key, creator, action, 100);
-        }
-
-        // todo: replace with optional parameters when we can break things
-        public void Enlist(string key, Action<bool> action)
-        {
-            Enlist<object>(key, null, (completed, item) => action(completed), 100);
-        }
-
-        public void Enlist(string key, Action<bool> action, int priority)
         {
             Enlist<object>(key, null, (completed, item) => action(completed), priority);
         }
 
-        public T Enlist<T>(string key, Func<T> creator, Action<bool, T> action, int priority)
+        public T Enlist<T>(string key, Func<T> creator, Action<bool, T> action = null, int priority = 100)
         {
             if (_exiting)
                 throw new InvalidOperationException("Cannot enlist now, context is exiting.");
 
             var enlistedObjects = _enlisted ?? (_enlisted = new Dictionary<string, IEnlistedObject>());
 
-            IEnlistedObject enlisted;
+            if (enlistedObjects.TryGetValue(key, out IEnlistedObject enlisted))
-            if (enlistedObjects.TryGetValue(key, out enlisted))
             {
                 var enlistedAs = enlisted as EnlistedObject<T>;
                 if (enlistedAs == null) throw new InvalidOperationException("An item with the key already exists, but with a different type.");

src/Umbraco.Core/Scoping/ScopeProvider.cs

@@ -3,7 +3,6 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Data;
 using System.Runtime.Remoting.Messaging;
-using System.Text;
 using System.Web;
 using Umbraco.Core.Composing;
 using Umbraco.Core.Events;
@@ -39,7 +38,7 @@ namespace Umbraco.Core.Scoping
             SafeCallContext.Register(
                 () =>
                 {
-                    var scope = GetCallContextObject<IScopeInternal>(ScopeItemKey);
+                    var scope = GetCallContextObject<Scope>(ScopeItemKey);
                     var context = GetCallContextObject<ScopeContext>(ContextItemKey);
                     SetCallContextObject(ScopeItemKey, null);
                     SetCallContextObject(ContextItemKey, null);
@@ -48,12 +47,12 @@ namespace Umbraco.Core.Scoping
                 o =>
                 {
                     // cannot re-attached over leaked scope/context
-                    if (GetCallContextObject<IScope>(ScopeItemKey) != null)
+                    if (GetCallContextObject<Scope>(ScopeItemKey) != null)
                             throw new Exception("Found leaked scope when restoring call context.");
                     if (GetCallContextObject<ScopeContext>(ContextItemKey) != null)
                         throw new Exception("Found leaked context when restoring call context.");
 
-                    var t = (Tuple<IScopeInternal, ScopeContext>) o;
+                    var t = (Tuple<Scope, ScopeContext>) o;
                     SetCallContextObject(ScopeItemKey, t.Item1);
                     SetCallContextObject(ContextItemKey, t.Item2);
                 });
@@ -231,7 +230,7 @@ namespace Umbraco.Core.Scoping
 
         internal const string ContextItemKey = "Umbraco.Core.Scoping.ScopeContext";
 
-        internal static ScopeContext AmbientContextInternal
+        internal static ScopeContext AmbientContextStatic
         {
             get
             {
@@ -253,7 +252,7 @@ namespace Umbraco.Core.Scoping
         }
 
         /// <inheritdoc />
-        public ScopeContext AmbientContext => AmbientContextInternal;
+        public ScopeContext AmbientContext => AmbientContextStatic;
 
         #endregion
 
@@ -266,13 +265,13 @@ namespace Umbraco.Core.Scoping
         // fixme - more weird static - we should try to get rid of all static & use an accessor
         private static readonly ScopeReference StaticScopeReference = new ScopeReference(new ScopeProvider(null, null, null));
 
-        internal static IScopeInternal AmbientScopeInternal
+        private static Scope AmbientScopeStatic
         {
             get
             {
                 // try http context, fallback onto call context
-                var value = GetHttpContextObject<IScopeInternal>(ScopeItemKey, false);
+                var value = GetHttpContextObject<Scope>(ScopeItemKey, false);
-                return value ?? GetCallContextObject<IScopeInternal>(ScopeItemKey);
+                return value ?? GetCallContextObject<Scope>(ScopeItemKey);
             }
             set
             {
@@ -291,15 +290,15 @@ namespace Umbraco.Core.Scoping
         }
 
         /// <inheritdoc />
-        public IScopeInternal AmbientScope
+        public Scope AmbientScope
         {
-            get => AmbientScopeInternal;
+            get => AmbientScopeStatic;
-            internal set => AmbientScopeInternal = value;
+            set => AmbientScopeStatic = value;
         }
 
         #endregion
 
-        public void SetAmbient(IScopeInternal scope, ScopeContext context = null)
+        public void SetAmbient(Scope scope, ScopeContext context = null)
         {
             // clear all
             SetHttpContextObject(ScopeItemKey, null, false);
@@ -415,7 +414,7 @@ namespace Umbraco.Core.Scoping
         }
 
         /// <inheritdoc />
-        public ScopeContext Context => AmbientContext;
+        public IScopeContext Context => AmbientContext;
 
 #if DEBUG_SCOPES
         // this code needs TLC

src/Umbraco.Core/Scoping/ScopeReference.cs

@@ -19,7 +19,7 @@
         {
             // dispose the entire chain (if any)
             // reset (don't commit by default)
-            IScopeInternal scope;
+            Scope scope;
             while ((scope = _scopeProvider.AmbientScope) != null)
             {
                 scope.Reset();

src/Umbraco.Core/Umbraco.Core.csproj

@@ -1253,7 +1253,7 @@
     <Compile Include="SafeCallContext.cs" />
     <Compile Include="Scoping\IInstanceIdentifiable.cs" />
     <Compile Include="Scoping\IScope.cs" />
-    <Compile Include="Scoping\IScopeInternal.cs" />
+    <Compile Include="Scoping\IScopeContext.cs" />
     <Compile Include="Scoping\IScopeProvider.cs" />
     <Compile Include="Scoping\RepositoryCacheMode.cs" />
     <Compile Include="Scoping\Scope.cs" />

src/Umbraco.Tests/Scoping/ScopeTests.cs

@@ -104,7 +104,7 @@ namespace Umbraco.Tests.Scoping
         [Test]
         public void NestedMigrateScope()
         {
-            var scopeProvider = ScopeProvider as ScopeProvider;
+            var scopeProvider = ScopeProvider;
             Assert.IsNull(scopeProvider.AmbientScope);
 
             var httpContextItems = new Hashtable();
@@ -162,7 +162,7 @@ namespace Umbraco.Tests.Scoping
                 Assert.AreSame(scope, scopeProvider.AmbientScope);
                 Assert.IsNotNull(scopeProvider.AmbientContext);
 
-                ScopeContext context;
+                IScopeContext context;
                 using (var nested = scopeProvider.CreateScope())
                 {
                     Assert.IsInstanceOf<Scope>(nested);

src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheComponent.cs

@@ -21,7 +21,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
             // register the XML facade service
             composition.SetFacadeService(factory => new FacadeService(
                 factory.GetInstance<ServiceContext>(),
-                (ScopeProvider) factory.GetInstance<IScopeProvider>(),
+                factory.GetInstance<IScopeProvider>(),
                 factory.GetInstance<IScopeUnitOfWorkProvider>(),
                 factory.GetInstance<CacheHelper>().RequestCache,
                 factory.GetInstance<UrlSegmentProviderCollection>(),