diff --git a/src/Umbraco.Core/Events/PassThroughEventDispatcher.cs b/src/Umbraco.Core/Events/PassThroughEventDispatcher.cs index 1e1a9bc6d1..bc3709ea2a 100644 --- a/src/Umbraco.Core/Events/PassThroughEventDispatcher.cs +++ b/src/Umbraco.Core/Events/PassThroughEventDispatcher.cs @@ -5,8 +5,10 @@ using System.Linq; namespace Umbraco.Core.Events { /// - /// 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. /// + /// This means that events will be raised during the scope transaction, + /// whatever happens, and the transaction could roll back in the end. internal class PassThroughEventDispatcher : IEventDispatcher { public bool DispatchCancelable(EventHandler eventHandler, object sender, CancellableEventArgs args, string eventName = null) diff --git a/src/Umbraco.Core/Events/ScopeEventDispatcher.cs b/src/Umbraco.Core/Events/ScopeEventDispatcher.cs index 1cf6660a40..eb4f8ec34e 100644 --- a/src/Umbraco.Core/Events/ScopeEventDispatcher.cs +++ b/src/Umbraco.Core/Events/ScopeEventDispatcher.cs @@ -3,11 +3,9 @@ using Umbraco.Core.IO; namespace Umbraco.Core.Events { /// - /// 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. /// - /// - /// The outer scope is the only scope that can raise events, the inner scope's will defer to the outer scope - /// internal class ScopeEventDispatcher : ScopeEventDispatcherBase { public ScopeEventDispatcher() diff --git a/src/Umbraco.Core/Events/ScopeEventDispatcherBase.cs b/src/Umbraco.Core/Events/ScopeEventDispatcherBase.cs index e7684f5be8..581f117979 100644 --- a/src/Umbraco.Core/Events/ScopeEventDispatcherBase.cs +++ b/src/Umbraco.Core/Events/ScopeEventDispatcherBase.cs @@ -6,6 +6,15 @@ using Umbraco.Core.Models.EntityBase; namespace Umbraco.Core.Events { + /// + /// An IEventDispatcher that queues events. + /// + /// + /// Can raise, or ignore, cancelable events, depending on option. + /// Implementations must override ScopeExitCompleted to define what + /// to do with the events when the scope exits and has been completed. + /// If the scope exits without being completed, events are ignored. + /// public abstract class ScopeEventDispatcherBase : IEventDispatcher { //events will be enlisted in the order they are raised diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWork.cs b/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWork.cs index e5232092c5..9c655d5d48 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWork.cs +++ b/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(); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs b/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs index 204e7b0bfe..f8d168cdaa 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs +++ b/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; } } diff --git a/src/Umbraco.Core/Scoping/Scope.cs b/src/Umbraco.Core/Scoping/Scope.cs index f08b9f2e6d..bfee772872 100644 --- a/src/Umbraco.Core/Scoping/Scope.cs +++ b/src/Umbraco.Core/Scoping/Scope.cs @@ -465,6 +465,7 @@ namespace Umbraco.Core.Scoping } finally { + // removes the ambient context (ambient scope already gone) _scopeProvider.SetAmbient(null); } } diff --git a/src/Umbraco.Core/Scoping/ScopeContext.cs b/src/Umbraco.Core/Scoping/ScopeContext.cs index cca0be560d..c79f02a0fa 100644 --- a/src/Umbraco.Core/Scoping/ScopeContext.cs +++ b/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 _enlisted; + private bool _exiting; public void ScopeExit(bool completed) { + if (_enlisted == null) + return; + + _exiting = true; + List 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,71 @@ namespace Umbraco.Core.Scoping private interface IEnlistedObject { void Execute(bool completed); + int Priority { get; } } private class EnlistedObject : IEnlistedObject { private readonly Action _action; - public EnlistedObject(T item) - { - Item = item; - } - - public EnlistedObject(T item, Action action) + public EnlistedObject(T item, Action action, int priority) { 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); } } - /// + // todo: replace with optional parameters when we can break things public T Enlist(string key, Func creator) { - IEnlistedObject enlisted; - if (Enlisted.TryGetValue(key, out enlisted)) - { - var enlistedAs = enlisted as EnlistedObject; - 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(creator()); - Enlisted[key] = enlistedOfT; - return enlistedOfT.Item; + return Enlist(key, creator, null, 100); } - /// + // todo: replace with optional parameters when we can break things public T Enlist(string key, Func creator, Action action) { - IEnlistedObject enlisted; - if (Enlisted.TryGetValue(key, out enlisted)) - { - var enlistedAs = enlisted as EnlistedObject; - 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(creator(), action); - Enlisted[key] = enlistedOfT; - return enlistedOfT.Item; + return Enlist(key, creator, action, 100); } - /// + // todo: replace with optional parameters when we can break things public void Enlist(string key, Action action) { + Enlist(key, null, (completed, item) => action(completed), 100); + } + + public void Enlist(string key, Action action, int priority) + { + Enlist(key, null, (completed, item) => action(completed), priority); + } + + public T Enlist(string key, Func creator, Action action, int priority) + { + if (_exiting) + throw new InvalidOperationException("Cannot enlist now, context is exiting."); + + var enlistedObjects = _enlisted ?? (_enlisted = new Dictionary()); + IEnlistedObject enlisted; - if (Enlisted.TryGetValue(key, out enlisted)) + if (enlistedObjects.TryGetValue(key, out enlisted)) { - var enlistedAs = enlisted as EnlistedObject; - if (enlistedAs == null) throw new Exception("An item with a different type has already been enlisted with the same key."); - return; + var enlistedAs = enlisted as EnlistedObject; + 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(null, (completed, item) => action(completed)); + var enlistedOfT = new EnlistedObject(creator == null ? default(T) : creator(), action, priority); Enlisted[key] = enlistedOfT; + return enlistedOfT.Item; } } } diff --git a/src/Umbraco.Tests/Scoping/ScopeTests.cs b/src/Umbraco.Tests/Scoping/ScopeTests.cs index 747e5b1f40..6e24bcf51c 100644 --- a/src/Umbraco.Tests/Scoping/ScopeTests.cs +++ b/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(exception); + } + [Test] public void ScopeContextException() { diff --git a/src/Umbraco.Web/Editors/PackageInstallController.cs b/src/Umbraco.Web/Editors/PackageInstallController.cs index c980a06521..d3771ce82c 100644 --- a/src/Umbraco.Web/Editors/PackageInstallController.cs +++ b/src/Umbraco.Web/Editors/PackageInstallController.cs @@ -58,17 +58,25 @@ namespace Umbraco.Web.Editors [HttpPost] public IHttpActionResult Uninstall(int packageId) { - var pack = InstalledPackage.GetById(packageId); - if (pack == null) return NotFound(); - - PerformUninstall(pack); - - //now get all other packages by this name since we'll uninstall all versions - foreach (var installed in InstalledPackage.GetAllInstalledPackages() - .Where(x => x.Data.Name == pack.Data.Name && x.Data.Id != pack.Data.Id)) + try { - //remove from teh xml - installed.Delete(Security.GetUserId()); + var pack = InstalledPackage.GetById(packageId); + if (pack == null) return NotFound(); + + PerformUninstall(pack); + + //now get all other packages by this name since we'll uninstall all versions + foreach (var installed in InstalledPackage.GetAllInstalledPackages() + .Where(x => x.Data.Name == pack.Data.Name && x.Data.Id != pack.Data.Id)) + { + //remove from teh xml + installed.Delete(Security.GetUserId()); + } + } + catch (Exception e) + { + Logger.Error("Failed to uninstall.", e); + throw; } return Ok(); @@ -176,7 +184,7 @@ namespace Umbraco.Web.Editors pack.Save(); // uninstall actions - //TODO: We should probably report errors to the UI!! + //TODO: We should probably report errors to the UI!! // This never happened before though, but we should do something now if (pack.Data.Actions.IsNullOrWhiteSpace() == false) { @@ -471,7 +479,7 @@ namespace Umbraco.Web.Editors string path = Path.Combine("packages", packageGuid + ".umb"); if (File.Exists(IOHelper.MapPath(Path.Combine(SystemDirectories.Data, path))) == false) { - path = Services.PackagingService.FetchPackageFile(Guid.Parse(packageGuid), UmbracoVersion.Current, Security.GetUserId()); + path = Services.PackagingService.FetchPackageFile(Guid.Parse(packageGuid), UmbracoVersion.Current, Security.GetUserId()); } var model = new LocalPackageInstallModel