From 541c7e69552db1a22af6f904eba32e44a03afd8f Mon Sep 17 00:00:00 2001
From: Stephan <stephane@umbraco.dk>
Date: Fri, 22 Feb 2019 15:27:22 +0100
Subject: [PATCH] NuCache: cleanup scoped objects

---
 .../Scoping/ScopeContextualBase.cs            | 33 +++++++++++++++--
 .../NuCache/PublishedSnapshotService.cs       | 36 +++++++++----------
 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/src/Umbraco.Core/Scoping/ScopeContextualBase.cs b/src/Umbraco.Core/Scoping/ScopeContextualBase.cs
index 7461142234..1f2b6155e6 100644
--- a/src/Umbraco.Core/Scoping/ScopeContextualBase.cs
+++ b/src/Umbraco.Core/Scoping/ScopeContextualBase.cs
@@ -2,24 +2,43 @@
 
 namespace Umbraco.Core.Scoping
 {
-    // base class for an object that will be enlisted in scope context, if any. it
-    // must be used in a 'using' block, and if not scoped, released when disposed,
-    // else when scope context runs enlisted actions
+    /// <summary>
+    /// Provides a base class for scope contextual objects.
+    /// </summary>
+    /// <remarks>
+    /// <para>A scope contextual object is enlisted in the current scope context,
+    /// if any, and released when the context exists. It must be used in a 'using'
+    /// block, and will be released when disposed, if not part of a scope.</para>
+    /// </remarks>
     public abstract class ScopeContextualBase : IDisposable
     {
         private bool _using, _scoped;
 
+        /// <summary>
+        /// Gets a contextual object.
+        /// </summary>
+        /// <typeparam name="T">The type of the object.</typeparam>
+        /// <param name="scopeProvider">A scope provider.</param>
+        /// <param name="key">A context key for the object.</param>
+        /// <param name="ctor">A function producing the contextual object.</param>
+        /// <returns>The contextual object.</returns>
+        /// <remarks>
+        /// <para></para>
+        /// </remarks>
         public static T Get<T>(IScopeProvider scopeProvider, string key, Func<bool, T> ctor)
             where T : ScopeContextualBase
         {
+            // no scope context = create a non-scoped object
             var scopeContext = scopeProvider.Context;
             if (scopeContext == null)
                 return ctor(false);
 
+            // create & enlist the scoped object
             var w = scopeContext.Enlist("ScopeContextualBase_" + key,
                 () => ctor(true),
                 (completed, item) => { item.Release(completed); });
 
+            // the object can be 'used' only once at a time
             if (w._using) throw new InvalidOperationException("panic: used.");
             w._using = true;
             w._scoped = true;
@@ -27,6 +46,10 @@ namespace Umbraco.Core.Scoping
             return w;
         }
 
+        /// <inheritdoc />
+        /// <remarks>
+        /// <para>If not scoped, then this releases the contextual object.</para>
+        /// </remarks>
         public void Dispose()
         {
             _using = false;
@@ -35,6 +58,10 @@ namespace Umbraco.Core.Scoping
                 Release(true);
         }
 
+        /// <summary>
+        /// Releases the contextual object.
+        /// </summary>
+        /// <param name="completed">A value indicating whether the scoped operation completed.</param>
         public abstract void Release(bool completed);
     }
 }
diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs
index e19531a25b..541ff2ea23 100755
--- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs
+++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs
@@ -330,14 +330,15 @@ namespace Umbraco.Web.PublishedCache.NuCache
 
         private void LockAndLoadContent(Action<IScope> action)
         {
+            // first get a writer, then a scope
+            // if there already is a scope, the writer will attach to it
+            // otherwise, it will only exist here - cheap
             using (_contentStore.GetWriter(_scopeProvider))
+            using (var scope = _scopeProvider.CreateScope())
             {
-                using (var scope = _scopeProvider.CreateScope())
-                {
-                    scope.ReadLock(Constants.Locks.ContentTree);
-                    action(scope);
-                    scope.Complete();
-                }
+                scope.ReadLock(Constants.Locks.ContentTree);
+                action(scope);
+                scope.Complete();
             }
         }
 
@@ -399,14 +400,13 @@ namespace Umbraco.Web.PublishedCache.NuCache
 
         private void LockAndLoadMedia(Action<IScope> action)
         {
+            // see note in LockAndLoadContent
             using (_mediaStore.GetWriter(_scopeProvider))
+            using (var scope = _scopeProvider.CreateScope())
             {
-                using (var scope = _scopeProvider.CreateScope())
-                {
-                    scope.ReadLock(Constants.Locks.MediaTree);
-                    action(scope);
-                    scope.Complete();
-                }
+                scope.ReadLock(Constants.Locks.MediaTree);
+                action(scope);
+                scope.Complete();
             }
         }
 
@@ -528,14 +528,13 @@ namespace Umbraco.Web.PublishedCache.NuCache
 
         private void LockAndLoadDomains()
         {
+            // see note in LockAndLoadContent
             using (_domainStore.GetWriter(_scopeProvider))
+            using (var scope = _scopeProvider.CreateScope())
             {
-                using (var scope = _scopeProvider.CreateScope())
-                {
-                    scope.ReadLock(Constants.Locks.Domains);
-                    LoadDomainsLocked();
-                    scope.Complete();
-                }
+                scope.ReadLock(Constants.Locks.Domains);
+                LoadDomainsLocked();
+                scope.Complete();
             }
         }
 
@@ -858,6 +857,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
             if (_isReady == false)
                 return;
 
+            // see note in LockAndLoadContent
             using (_domainStore.GetWriter(_scopeProvider))
             {
                 foreach (var payload in payloads)