Big refactor or PublishedSnapshotService to split up so that there's a service and repository responsible for the data querying and persistence

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text.RegularExpressions;
@@ -767,8 +767,21 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
 
         #region UnitOfWork Events
 
-        // TODO: The reason these events are in the repository is for legacy, the events should exist at the service
-        // level now since we can fire these events within the transaction... so move the events to service level
+        /*
+         * TODO: The reason these events are in the repository is for legacy, the events should exist at the service
+         * level now since we can fire these events within the transaction...
+         * The reason these events 'need' to fire in the transaction is to ensure data consistency with Nucache (currently
+         * the only thing that uses them). For example, if the transaction succeeds and NuCache listened to ContentService.Saved
+         * and then NuCache failed at persisting data after the trans completed, then NuCache would be out of sync. This way 
+         * the entire trans is rolled back if NuCache files. That said, I'm unsure this is really required because there
+         * are other systems that rely on the "ed" (i.e. Saved) events like Examine which would be inconsistent if it failed
+         * too. I'm just not sure this is totally necessary especially.
+         * So these events can be moved to the service level. However, see the notes below, it seems the only event we
+         * really need is the ScopedEntityRefresh. The only tricky part with moving that to the service level is that the
+         * handlers of that event will need to deal with the data a little differently because it seems that the
+         * "Published" flag on the content item matters and this event is raised before that flag is switched. Weird.
+         * We have the ability with IContent to see if something "WasPublished", etc.. so i think we could still use that.
+         */
 
         public class ScopedEntityEventArgs : EventArgs
         {
@@ -784,6 +797,9 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
 
         public class ScopedVersionEventArgs : EventArgs
         {
+            /// <summary>
+            /// Initializes a new instance of the <see cref="ScopedVersionEventArgs"/> class.
+            /// </summary>
             public ScopedVersionEventArgs(IScope scope, int entityId, int versionId)
             {
                 Scope = scope;
@@ -791,13 +807,43 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
                 VersionId = versionId;
             }
 
+            /// <summary>
+            /// Gets the current <see cref="IScope"/>
+            /// </summary>
             public IScope Scope { get; }
+
+            /// <summary>
+            /// Gets the entity id
+            /// </summary>
             public int EntityId { get; }
+
+            /// <summary>
+            /// Gets the version id
+            /// </summary>
             public int VersionId { get; }
         }
 
+        /// <summary>
+        /// Occurs when an <see cref="TEntity"/> is created or updated from within the <see cref="IScope"/> (transaction)
+        /// </summary>
         public static event TypedEventHandler<TRepository, ScopedEntityEventArgs> ScopedEntityRefresh;
+
+        /// <summary>
+        /// Occurs when an <see cref="TEntity"/> is being deleted from within the <see cref="IScope"/> (transaction)
+        /// </summary>
+        /// <remarks>
+        /// TODO: This doesn't seem to be necessary at all, the service "Deleting" events for this would work just fine
+        /// since they are raised before the item is actually deleted just like this event.
+        /// </remarks>
         public static event TypedEventHandler<TRepository, ScopedEntityEventArgs> ScopeEntityRemove;
+
+        /// <summary>
+        /// Occurs when a version for an <see cref="TEntity"/> is being deleted from within the <see cref="IScope"/> (transaction)
+        /// </summary>
+        /// <remarks>
+        /// TODO: This doesn't seem to be necessary at all, the service "DeletingVersions" events for this would work just fine
+        /// since they are raised before the item is actually deleted just like this event.
+        /// </remarks>
         public static event TypedEventHandler<TRepository, ScopedVersionEventArgs> ScopeVersionRemove;
 
         // used by tests to clear events
@@ -808,20 +854,23 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             ScopeVersionRemove = null;
         }
 
+        /// <summary>
+        /// Raises the <see cref="ScopedEntityRefresh"/> event
+        /// </summary>
         protected void OnUowRefreshedEntity(ScopedEntityEventArgs args)
-        {
-            ScopedEntityRefresh.RaiseEvent(args, This);
-        }
+            => ScopedEntityRefresh.RaiseEvent(args, This);
 
+        /// <summary>
+        /// Raises the <see cref="ScopeEntityRemove"/> event
+        /// </summary>
         protected void OnUowRemovingEntity(ScopedEntityEventArgs args)
-        {
-            ScopeEntityRemove.RaiseEvent(args, This);
-        }
+            => ScopeEntityRemove.RaiseEvent(args, This);
 
+        /// <summary>
+        /// Raises the <see cref="ScopeVersionRemove"/> event
+        /// </summary>
         protected void OnUowRemovingVersion(ScopedVersionEventArgs args)
-        {
-            ScopeVersionRemove.RaiseEvent(args, This);
-        }
+            => ScopeVersionRemove.RaiseEvent(args, This);
 
         #endregion