From ba186144a01b442136c4a747fd5e84888d3a124e Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 Oct 2018 18:09:52 +1100 Subject: [PATCH 01/16] Updates models to allow dirty tracking on variants --- .../Collections/CompositeIntStringKey.cs | 3 +- .../Collections/IReadOnlyKeyedCollection.cs | 16 +++ src/Umbraco.Core/ContentExtensions.cs | 2 +- src/Umbraco.Core/Models/Content.cs | 23 +-- src/Umbraco.Core/Models/ContentBase.cs | 49 +++++-- src/Umbraco.Core/Models/CultureName.cs | 82 +++++++++++ .../Models/CultureNameCollection.cs | 136 ++++++++++++++++++ src/Umbraco.Core/Models/IContent.cs | 3 +- src/Umbraco.Core/Models/IContentBase.cs | 3 +- .../Models/PropertyGroupCollection.cs | 3 +- .../Implement/DocumentRepository.cs | 6 +- src/Umbraco.Core/Umbraco.Core.csproj | 3 + src/Umbraco.Tests/Models/ContentTests.cs | 27 ++++ src/Umbraco.Tests/Models/VariationTests.cs | 8 +- .../Repositories/DocumentRepositoryTest.cs | 2 +- .../Models/Mapping/ContentMapperProfile.cs | 2 +- src/Umbraco.Web/umbraco.presentation/page.cs | 2 +- 17 files changed, 332 insertions(+), 38 deletions(-) create mode 100644 src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs create mode 100644 src/Umbraco.Core/Models/CultureName.cs create mode 100644 src/Umbraco.Core/Models/CultureNameCollection.cs diff --git a/src/Umbraco.Core/Collections/CompositeIntStringKey.cs b/src/Umbraco.Core/Collections/CompositeIntStringKey.cs index eb9db80990..74ef4e45e1 100644 --- a/src/Umbraco.Core/Collections/CompositeIntStringKey.cs +++ b/src/Umbraco.Core/Collections/CompositeIntStringKey.cs @@ -2,6 +2,7 @@ namespace Umbraco.Core.Collections { + /// /// Represents a composite key of (int, string) for fast dictionaries. /// @@ -40,4 +41,4 @@ namespace Umbraco.Core.Collections public static bool operator !=(CompositeIntStringKey key1, CompositeIntStringKey key2) => key1._key2 != key2._key2 || key1._key1 != key2._key1; } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs b/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs new file mode 100644 index 0000000000..8d78241275 --- /dev/null +++ b/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; + +namespace Umbraco.Core.Collections +{ + /// + /// A readonly keyed collection + /// + /// + public interface IReadOnlyKeyedCollection : IReadOnlyList + { + IEnumerable Keys { get; } + bool TryGetValue(TKey key, out TVal val); + TVal this[string key] { get; } + bool Contains(TKey key); + } +} diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index e36731a8cb..4f88c2b803 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -133,7 +133,7 @@ namespace Umbraco.Core } #endregion - + /// /// Removes characters that are not valide XML characters from all entity properties /// of type string. See: http://stackoverflow.com/a/961504/5018 diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 238d87b186..2ee6762e61 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Runtime.Serialization; +using Umbraco.Core.Collections; using Umbraco.Core.Exceptions; namespace Umbraco.Core.Models @@ -20,8 +21,8 @@ namespace Umbraco.Core.Models private PublishedState _publishedState; private DateTime? _releaseDate; private DateTime? _expireDate; - private Dictionary _publishInfos; - private Dictionary _publishInfosOrig; + private CultureNameCollection _publishInfos; + private CultureNameCollection _publishInfosOrig; private HashSet _editedCultures; private static readonly Lazy Ps = new Lazy(); @@ -221,13 +222,13 @@ namespace Umbraco.Core.Models public bool IsCulturePublished(string culture) // just check _publishInfos // a non-available culture could not become published anyways - => _publishInfos != null && _publishInfos.ContainsKey(culture); + => _publishInfos != null && _publishInfos.Contains(culture); /// public bool WasCulturePublished(string culture) // just check _publishInfosOrig - a copy of _publishInfos // a non-available culture could not become published anyways - => _publishInfosOrig != null && _publishInfosOrig.ContainsKey(culture); + => _publishInfosOrig != null && _publishInfosOrig.Contains(culture); /// public bool IsCultureEdited(string culture) @@ -237,7 +238,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IReadOnlyDictionary PublishNames => _publishInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; + public IReadOnlyKeyedCollection PublishNames => _publishInfos ?? NoNames; /// public string GetPublishName(string culture) @@ -267,9 +268,11 @@ namespace Umbraco.Core.Models throw new ArgumentNullOrEmptyException(nameof(culture)); if (_publishInfos == null) - _publishInfos = new Dictionary(StringComparer.OrdinalIgnoreCase); + _publishInfos = new CultureNameCollection(); - _publishInfos[culture.ToLowerInvariant()] = (name, date); + //TODO: Track changes? + + _publishInfos.AddOrUpdate(culture, name, date); } private void ClearPublishInfos() @@ -430,7 +433,11 @@ namespace Umbraco.Core.Models // take care of publish infos _publishInfosOrig = _publishInfos == null ? null - : new Dictionary(_publishInfos, StringComparer.OrdinalIgnoreCase); + : new CultureNameCollection(_publishInfos); + + if (_publishInfos != null) + foreach (var cultureName in _publishInfos) + cultureName.ResetDirtyProperties(rememberDirty); } /// diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index bf2fd580d9..2336188c50 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Reflection; using System.Runtime.Serialization; using System.Web; +using Umbraco.Core.Collections; using Umbraco.Core.Exceptions; using Umbraco.Core.Models.Entities; @@ -19,14 +20,14 @@ namespace Umbraco.Core.Models [DebuggerDisplay("Id: {Id}, Name: {Name}, ContentType: {ContentTypeBase.Alias}")] public abstract class ContentBase : TreeEntityBase, IContentBase { - protected static readonly Dictionary NoNames = new Dictionary(); + protected static readonly CultureNameCollection NoNames = new CultureNameCollection(); private static readonly Lazy Ps = new Lazy(); private int _contentTypeId; protected IContentTypeComposition ContentTypeBase; private int _writerId; private PropertyCollection _properties; - private Dictionary _cultureInfos; + private CultureNameCollection _cultureInfos; /// /// Initializes a new instance of the class. @@ -69,7 +70,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo DefaultContentTypeIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ContentTypeId); public readonly PropertyInfo PropertyCollectionSelector = ExpressionHelper.GetPropertyInfo(x => x.Properties); public readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); - public readonly PropertyInfo NamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); + public readonly PropertyInfo CultureNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); } protected void PropertiesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -147,18 +148,18 @@ namespace Umbraco.Core.Models /// public IEnumerable AvailableCultures - => _cultureInfos?.Select(x => x.Key) ?? Enumerable.Empty(); + => _cultureInfos?.Keys ?? Enumerable.Empty(); /// public bool IsCultureAvailable(string culture) - => _cultureInfos != null && _cultureInfos.ContainsKey(culture); + => _cultureInfos != null && _cultureInfos.Contains(culture); /// [DataMember] - public virtual IReadOnlyDictionary CultureNames => _cultureInfos?.ToDictionary(x => x.Key, x => x.Value.Name, StringComparer.OrdinalIgnoreCase) ?? NoNames; - + public virtual IReadOnlyKeyedCollection CultureNames => _cultureInfos ?? NoNames; + /// - public virtual string GetCultureName(string culture) + public string GetCultureName(string culture) { if (culture.IsNullOrWhiteSpace()) return Name; if (!ContentTypeBase.VariesByCulture()) return null; @@ -176,7 +177,7 @@ namespace Umbraco.Core.Models } /// - public virtual void SetCultureName(string name, string culture) + public void SetCultureName(string name, string culture) { if (ContentTypeBase.VariesByCulture()) // set on variant content type { @@ -202,16 +203,18 @@ namespace Umbraco.Core.Models } } + //fixme: this isn't used anywhere internal void TouchCulture(string culture) { if (ContentTypeBase.VariesByCulture() && _cultureInfos != null && _cultureInfos.TryGetValue(culture, out var infos)) - _cultureInfos[culture] = (infos.Name, DateTime.Now); + _cultureInfos.AddOrUpdate(culture, infos.Name, DateTime.Now); } protected void ClearCultureInfos() { + if (_cultureInfos != null) + _cultureInfos.Clear(); _cultureInfos = null; - OnPropertyChanged(Ps.Value.NamesSelector); } protected void ClearCultureInfo(string culture) @@ -223,7 +226,6 @@ namespace Umbraco.Core.Models _cultureInfos.Remove(culture); if (_cultureInfos.Count == 0) _cultureInfos = null; - OnPropertyChanged(Ps.Value.NamesSelector); } // internal for repository @@ -236,10 +238,22 @@ namespace Umbraco.Core.Models throw new ArgumentNullOrEmptyException(nameof(culture)); if (_cultureInfos == null) - _cultureInfos = new Dictionary(StringComparer.OrdinalIgnoreCase); + { + _cultureInfos = new CultureNameCollection(); + _cultureInfos.CollectionChanged += CultureNamesCollectionChanged; + } - _cultureInfos[culture.ToLowerInvariant()] = (name, date); - OnPropertyChanged(Ps.Value.NamesSelector); + _cultureInfos.AddOrUpdate(culture, name, date); + } + + /// + /// Event handler for when the culture names collection is modified + /// + /// + /// + private void CultureNamesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) + { + OnPropertyChanged(Ps.Value.CultureNamesSelector); } #endregion @@ -387,6 +401,11 @@ namespace Umbraco.Core.Models // also reset dirty changes made to user's properties foreach (var prop in Properties) prop.ResetDirtyProperties(rememberDirty); + + // take care of culture names + if (_cultureInfos != null) + foreach (var cultureName in _cultureInfos) + cultureName.ResetDirtyProperties(rememberDirty); } /// diff --git a/src/Umbraco.Core/Models/CultureName.cs b/src/Umbraco.Core/Models/CultureName.cs new file mode 100644 index 0000000000..64db50c36d --- /dev/null +++ b/src/Umbraco.Core/Models/CultureName.cs @@ -0,0 +1,82 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using Umbraco.Core.Models.Entities; + +namespace Umbraco.Core.Models +{ + /// + /// The name of a content variant for a given culture + /// + public class CultureName : BeingDirtyBase, IDeepCloneable, IEquatable + { + private DateTime _date; + private string _name; + private static readonly Lazy Ps = new Lazy(); + + public CultureName(string culture, string name, DateTime date) + { + if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("message", nameof(culture)); + Culture = culture; + _name = name; + _date = date; + } + + public string Culture { get; private set; } + + public string Name + { + get => _name; + set => SetPropertyValueAndDetectChanges(value, ref _name, Ps.Value.NameSelector); + } + + public DateTime Date + { + get => _date; + set => SetPropertyValueAndDetectChanges(value, ref _date, Ps.Value.DateSelector); + } + + public object DeepClone() + { + return new CultureName(Culture, Name, Date); + } + + public override bool Equals(object obj) + { + return obj is CultureName && Equals((CultureName)obj); + } + + public bool Equals(CultureName other) + { + return Culture == other.Culture && + Name == other.Name; + } + + public override int GetHashCode() + { + var hashCode = 479558943; + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(Culture); + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(Name); + return hashCode; + } + + /// + /// Allows deconstructing into culture and name + /// + /// + /// + public void Deconstruct(out string culture, out string name) + { + culture = Culture; + name = Name; + } + + // ReSharper disable once ClassNeverInstantiated.Local + private class PropertySelectors + { + public readonly PropertyInfo CultureSelector = ExpressionHelper.GetPropertyInfo(x => x.Culture); + public readonly PropertyInfo NameSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); + public readonly PropertyInfo DateSelector = ExpressionHelper.GetPropertyInfo(x => x.Date); + } + } +} diff --git a/src/Umbraco.Core/Models/CultureNameCollection.cs b/src/Umbraco.Core/Models/CultureNameCollection.cs new file mode 100644 index 0000000000..3c00603c88 --- /dev/null +++ b/src/Umbraco.Core/Models/CultureNameCollection.cs @@ -0,0 +1,136 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Collections.Specialized; +using System.Linq; +using Umbraco.Core.Collections; + +namespace Umbraco.Core.Models +{ + + + /// + /// The culture names of a content's variants + /// + public class CultureNameCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable, IReadOnlyKeyedCollection + { + /// + /// Creates a new collection from another collection + /// + /// + public CultureNameCollection(IEnumerable names) : base(StringComparer.InvariantCultureIgnoreCase) + { + foreach (var n in names) + Add(n); + } + + /// + /// Creates a new collection + /// + public CultureNameCollection() : base(StringComparer.InvariantCultureIgnoreCase) + { + } + + /// + /// Returns all keys in the collection + /// + public IEnumerable Keys => Dictionary != null ? Dictionary.Keys : this.Select(x => x.Culture); + + public bool TryGetValue(string culture, out CultureName name) + { + name = this.FirstOrDefault(x => x.Culture.InvariantEquals(culture)); + return name != null; + } + + /// + /// Add or update the + /// + /// + public void AddOrUpdate(string culture, string name, DateTime date) + { + culture = culture.ToLowerInvariant(); + if (TryGetValue(culture, out var found)) + { + found.Name = name; + found.Date = date; + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, found, found)); + } + else + Add(new CultureName(culture, name, date)); + } + + /// + /// Gets the index for a specified culture + /// + public int IndexOfKey(string key) + { + for (var i = 0; i < Count; i++) + { + if (this[i].Culture.InvariantEquals(key)) + return i; + } + return -1; + } + + public event NotifyCollectionChangedEventHandler CollectionChanged; + + protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs args) + { + CollectionChanged?.Invoke(this, args); + } + + public object DeepClone() + { + var clone = new CultureNameCollection(); + foreach (var name in this) + { + clone.Add((CultureName)name.DeepClone()); + } + return clone; + } + + protected override string GetKeyForItem(CultureName item) + { + return item.Culture; + } + + /// + /// Resets the collection to only contain the instances referenced in the parameter. + /// + /// The property groups. + /// + internal void Reset(IEnumerable names) + { + Clear(); + foreach (var name in names) + Add(name); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + + protected override void SetItem(int index, CultureName item) + { + base.SetItem(index, item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); + } + + protected override void RemoveItem(int index) + { + var removed = this[index]; + base.RemoveItem(index); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removed)); + } + + protected override void InsertItem(int index, CultureName item) + { + base.InsertItem(index, item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); + } + + protected override void ClearItems() + { + base.ClearItems(); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + + } +} diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index d9bc32aaf0..3ddffe8f75 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Umbraco.Core.Collections; namespace Umbraco.Core.Models { @@ -132,7 +133,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get via the property. /// - IReadOnlyDictionary PublishNames { get; } + IReadOnlyKeyedCollection PublishNames { get; } /// /// Gets the published cultures. diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index 460bd521d4..811cbf74f3 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Umbraco.Core.Collections; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -57,7 +58,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot contain the invariant /// culture name, which must be get or set via the property. /// - IReadOnlyDictionary CultureNames { get; } + IReadOnlyKeyedCollection CultureNames { get; } /// /// Gets the available cultures. diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index d10b375285..80b663fa05 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -8,6 +8,7 @@ using System.Threading; namespace Umbraco.Core.Models { + /// /// Represents a collection of objects /// @@ -168,7 +169,7 @@ namespace Umbraco.Core.Models var clone = new PropertyGroupCollection(); foreach (var group in this) { - clone.Add((PropertyGroup) group.DeepClone()); + clone.Add((PropertyGroup)group.DeepClone()); } return clone; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index bf41cd1ad1..83d9e80f5f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1172,8 +1172,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // using the default culture if it has a name, otherwise anything we can var defaultCulture = LanguageRepository.GetDefaultIsoCode(); content.Name = defaultCulture != null && content.CultureNames.TryGetValue(defaultCulture, out var cultureName) - ? cultureName - : content.CultureNames.First().Value; + ? cultureName.Name + : content.CultureNames[0].Name; } else { @@ -1234,7 +1234,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // update the name, and the publish name if published content.SetCultureName(uniqueName, culture); - if (publishing && content.PublishNames.ContainsKey(culture)) + if (publishing && content.PublishNames.Contains(culture)) content.SetPublishInfo(culture, uniqueName, DateTime.Now); } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index aef18e59db..0eba543e0d 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -109,6 +109,7 @@ + @@ -374,6 +375,8 @@ + + diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 32fbd37d0e..a7d0f2f050 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -42,6 +42,33 @@ namespace Umbraco.Tests.Models Container.Register(_ => Mock.Of()); } + [Test] + public void Variant_Names_Track_Dirty_Changes() + { + var contentType = new ContentType(-1) { Alias = "contentType" }; + var content = new Content("content", -1, contentType) { Id = 1, VersionId = 1 }; + + const string langFr = "fr-FR"; + + contentType.Variations = ContentVariation.Culture; + + Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //hasn't been changed + + content.SetCultureName("name-fr", langFr); + Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //now it will be changed since the collection has changed + var frCultureName = content.CultureNames[langFr]; + Assert.IsFalse(frCultureName.IsPropertyDirty("Date")); //this won't be dirty because it wasn't actually updated, just created + + content.ResetDirtyProperties(); + + Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //it's been reset + Assert.IsTrue(content.WasPropertyDirty("CultureNames")); + + content.SetCultureName("name-fr", langFr); + Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); //this will be dirty because it was already created and now has been updated + Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //it's true now since we've updated a name + } + [Test] public void Get_Non_Grouped_Properties() { diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 8d566e81f2..0e62c41f46 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -237,10 +237,10 @@ namespace Umbraco.Tests.Models // variant dictionary of names work Assert.AreEqual(2, content.CultureNames.Count); - Assert.IsTrue(content.CultureNames.ContainsKey(langFr)); - Assert.AreEqual("name-fr", content.CultureNames[langFr]); - Assert.IsTrue(content.CultureNames.ContainsKey(langUk)); - Assert.AreEqual("name-uk", content.CultureNames[langUk]); + Assert.IsTrue(content.CultureNames.Contains(langFr)); + Assert.AreEqual("name-fr", content.CultureNames[langFr].Name); + Assert.IsTrue(content.CultureNames.Contains(langUk)); + Assert.AreEqual("name-uk", content.CultureNames[langUk].Name); } [Test] diff --git a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs index 68e29c4efe..62c3116ab1 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs @@ -767,7 +767,7 @@ namespace Umbraco.Tests.Persistence.Repositories foreach (var r in result) { var isInvariant = r.ContentType.Alias == "umbInvariantTextpage"; - var name = isInvariant ? r.Name : r.CultureNames["en-US"]; + var name = isInvariant ? r.Name : r.CultureNames["en-US"].Name; var namePrefix = isInvariant ? "INV" : "VAR"; //ensure the correct name (invariant vs variant) is in the result diff --git a/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs index 8b23763763..4557dfb1bd 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs @@ -132,7 +132,7 @@ namespace Umbraco.Web.Models.Mapping // if we don't have a name for a culture, it means the culture is not available, and // hey we should probably not be mapping it, but it's too late, return a fallback name - return source.CultureNames.TryGetValue(culture, out var name) && !name.IsNullOrWhiteSpace() ? name : $"(({source.Name}))"; + return source.CultureNames.TryGetValue(culture, out var name) && !name.Name.IsNullOrWhiteSpace() ? name.Name : $"(({source.Name}))"; } } } diff --git a/src/Umbraco.Web/umbraco.presentation/page.cs b/src/Umbraco.Web/umbraco.presentation/page.cs index ac35c336b2..91cf690996 100644 --- a/src/Umbraco.Web/umbraco.presentation/page.cs +++ b/src/Umbraco.Web/umbraco.presentation/page.cs @@ -396,7 +396,7 @@ namespace umbraco return _cultureInfos; return _cultureInfos = _inner.PublishNames - .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value, _inner.GetPublishDate(x.Key) ?? DateTime.MinValue)); + .ToDictionary(x => x.Culture, x => new PublishedCultureInfo(x.Culture, x.Name, x.Date)); } } From 6cd9102fac2b28d056c7fd5573f78e0c541b9948 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 18 Oct 2018 21:12:55 +1100 Subject: [PATCH 02/16] Gets publish names tracking changes, adds unit tests --- src/Umbraco.Core/Models/Content.cs | 22 ++++- src/Umbraco.Core/Models/CultureName.cs | 21 ++++- .../Models/CultureNameCollection.cs | 6 +- src/Umbraco.Tests/Models/ContentTests.cs | 40 ++++++++- .../Services/ContentServiceTests.cs | 86 +++++++++++++++++++ 5 files changed, 164 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 2ee6762e61..eb95804c2c 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using System.Reflection; using System.Runtime.Serialization; @@ -88,6 +89,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo PublishedSelector = ExpressionHelper.GetPropertyInfo(x => x.Published); public readonly PropertyInfo ReleaseDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ReleaseDate); public readonly PropertyInfo ExpireDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ExpireDate); + public readonly PropertyInfo PublishNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.PublishNames); } /// @@ -222,7 +224,7 @@ namespace Umbraco.Core.Models public bool IsCulturePublished(string culture) // just check _publishInfos // a non-available culture could not become published anyways - => _publishInfos != null && _publishInfos.Contains(culture); + => _publishInfos != null && _publishInfos.Contains(culture); /// public bool WasCulturePublished(string culture) @@ -268,9 +270,10 @@ namespace Umbraco.Core.Models throw new ArgumentNullOrEmptyException(nameof(culture)); if (_publishInfos == null) + { _publishInfos = new CultureNameCollection(); - - //TODO: Track changes? + _publishInfos.CollectionChanged += PublishNamesCollectionChanged; + } _publishInfos.AddOrUpdate(culture, name, date); } @@ -314,6 +317,16 @@ namespace Umbraco.Core.Models } } + /// + /// Event handler for when the culture names collection is modified + /// + /// + /// + private void PublishNamesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) + { + OnPropertyChanged(Ps.Value.PublishNamesSelector); + } + [IgnoreDataMember] public int PublishedVersionId { get; internal set; } @@ -430,7 +443,8 @@ namespace Umbraco.Core.Models // take care of the published state _publishedState = _published ? PublishedState.Published : PublishedState.Unpublished; - // take care of publish infos + // Make a copy of the _publishInfos, this is purely so that we can detect + // if this entity's previous culture publish state (regardless of the rememberDirty flag) _publishInfosOrig = _publishInfos == null ? null : new CultureNameCollection(_publishInfos); diff --git a/src/Umbraco.Core/Models/CultureName.cs b/src/Umbraco.Core/Models/CultureName.cs index 64db50c36d..d683f07b49 100644 --- a/src/Umbraco.Core/Models/CultureName.cs +++ b/src/Umbraco.Core/Models/CultureName.cs @@ -14,12 +14,27 @@ namespace Umbraco.Core.Models private string _name; private static readonly Lazy Ps = new Lazy(); - public CultureName(string culture, string name, DateTime date) + /// + /// Used for cloning without change tracking + /// + /// + /// + /// + private CultureName(string culture, string name, DateTime date) + : this(culture) + { + _name = name; + _date = date; + } + + /// + /// Constructor + /// + /// + public CultureName(string culture) { if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("message", nameof(culture)); Culture = culture; - _name = name; - _date = date; } public string Culture { get; private set; } diff --git a/src/Umbraco.Core/Models/CultureNameCollection.cs b/src/Umbraco.Core/Models/CultureNameCollection.cs index 3c00603c88..be6540c399 100644 --- a/src/Umbraco.Core/Models/CultureNameCollection.cs +++ b/src/Umbraco.Core/Models/CultureNameCollection.cs @@ -56,7 +56,11 @@ namespace Umbraco.Core.Models OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, found, found)); } else - Add(new CultureName(culture, name, date)); + Add(new CultureName(culture) + { + Name = name, + Date = date + }); } /// diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index a7d0f2f050..1c3e2453c3 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.Linq; +using System.Threading; using Moq; using NUnit.Framework; using Umbraco.Core; @@ -43,7 +44,7 @@ namespace Umbraco.Tests.Models } [Test] - public void Variant_Names_Track_Dirty_Changes() + public void Variant_Culture_Names_Track_Dirty_Changes() { var contentType = new ContentType(-1) { Alias = "contentType" }; var content = new Content("content", -1, contentType) { Id = 1, VersionId = 1 }; @@ -54,21 +55,54 @@ namespace Umbraco.Tests.Models Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //hasn't been changed + Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //now it will be changed since the collection has changed var frCultureName = content.CultureNames[langFr]; - Assert.IsFalse(frCultureName.IsPropertyDirty("Date")); //this won't be dirty because it wasn't actually updated, just created + Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); content.ResetDirtyProperties(); Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //it's been reset Assert.IsTrue(content.WasPropertyDirty("CultureNames")); + Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); - Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); //this will be dirty because it was already created and now has been updated + Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //it's true now since we've updated a name } + [Test] + public void Variant_Published_Culture_Names_Track_Dirty_Changes() + { + var contentType = new ContentType(-1) { Alias = "contentType" }; + var content = new Content("content", -1, contentType) { Id = 1, VersionId = 1 }; + + const string langFr = "fr-FR"; + + contentType.Variations = ContentVariation.Culture; + + Assert.IsFalse(content.IsPropertyDirty("PublishNames")); //hasn't been changed + + Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date + content.SetCultureName("name-fr", langFr); + content.PublishCulture(langFr); //we've set the name, now we're publishing it + Assert.IsTrue(content.IsPropertyDirty("PublishNames")); //now it will be changed since the collection has changed + var frCultureName = content.PublishNames[langFr]; + Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); + + content.ResetDirtyProperties(); + + Assert.IsFalse(content.IsPropertyDirty("PublishNames")); //it's been reset + Assert.IsTrue(content.WasPropertyDirty("PublishNames")); + + Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date + content.SetCultureName("name-fr", langFr); + content.PublishCulture(langFr); //we've set the name, now we're publishing it + Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); + Assert.IsTrue(content.IsPropertyDirty("PublishNames")); //it's true now since we've updated a name + } + [Test] public void Get_Non_Grouped_Properties() { diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index f187b8b70d..4d31d8342a 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1234,6 +1234,92 @@ namespace Umbraco.Tests.Services } } + [Test] + public void Can_Unpublish_Content_Variation() + { + // Arrange + + var langUk = new Language("en-UK") { IsDefault = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", -1, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.SetCultureName("content-en", langUk.IsoCode); + content.PublishCulture(langFr.IsoCode); + content.PublishCulture(langUk.IsoCode); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); //not persisted yet, will be false + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsFalse(content.WasCulturePublished(langUk.IsoCode)); //not persisted yet, will be false + + var published = ServiceContext.ContentService.SavePublishing(content); + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.IsTrue(published.Success); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.WasCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(content.WasCulturePublished(langUk.IsoCode)); + + var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); + Assert.IsTrue(unpublished.Success); + + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + //this is slightly confusing but this will be false because this method is used for checking the state of the current model, + //but the state on the model has changed with the above Unpublish call + Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); + //this is slightly confusing but this will be false because this method is used for checking the state of a current model, + //but we've re-fetched from the database + Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + Assert.IsTrue(content.WasCulturePublished(langUk.IsoCode)); + + } + + [Test] + public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() + { + // Arrange + + var langUk = new Language("en-UK") { IsDefault = true }; + var langFr = new Language("fr-FR"); + + ServiceContext.LocalizationService.Save(langFr); + ServiceContext.LocalizationService.Save(langUk); + + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + IContent content = new Content("content", -1, contentType); + content.SetCultureName("content-fr", langFr.IsoCode); + content.PublishCulture(langFr.IsoCode); + var published = ServiceContext.ContentService.SavePublishing(content); + //audit log will only show that french was published + var lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); + Assert.AreEqual($"Published culture fr-fr", lastLog.Comment); + + //re-get + content = ServiceContext.ContentService.GetById(content.Id); + content.SetCultureName("content-en", langUk.IsoCode); + content.PublishCulture(langUk.IsoCode); + published = ServiceContext.ContentService.SavePublishing(content); + //audit log will only show that english was published + lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); + Assert.AreEqual($"Published culture en-uk", lastLog.Comment); + } + [Test] public void Can_Publish_Content_1() { From f1ca32ea5406301dd72fbef19ae7067ea56c5ab4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 18 Oct 2018 22:47:12 +1100 Subject: [PATCH 03/16] Cleaning up the log/audit trail usage, removing duplicate and unecessary entries and adding new cols --- .../Migrations/Upgrade/UmbracoPlan.cs | 1 + .../Upgrade/V_8_0_0/AddLogTableColumns.cs | 24 +++++++ src/Umbraco.Core/Models/AuditItem.cs | 10 ++- src/Umbraco.Core/Persistence/Dtos/LogDto.cs | 18 +++++ .../Repositories/Implement/AuditRepository.cs | 18 +++-- src/Umbraco.Core/Services/IAuditService.cs | 2 +- .../Services/Implement/AuditService.cs | 4 +- .../Services/Implement/ContentService.cs | 72 +++++++++++++------ ...peServiceBaseOfTRepositoryTItemTService.cs | 13 ++-- .../Services/Implement/DataTypeService.cs | 10 +-- .../Services/Implement/FileService.cs | 26 +++---- .../Services/Implement/LocalizationService.cs | 12 ++-- .../Services/Implement/MacroService.cs | 8 +-- .../Services/Implement/MediaService.cs | 28 ++++---- .../Services/Implement/MemberService.cs | 12 ++-- .../Services/Implement/PackagingService.cs | 2 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + 17 files changed, 174 insertions(+), 87 deletions(-) create mode 100644 src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index eeaf7533a9..4761f2a1fc 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -137,6 +137,7 @@ namespace Umbraco.Core.Migrations.Upgrade // resume at {290C18EE-B3DE-4769-84F1-1F467F3F76DA}... Chain("{6A2C7C1B-A9DB-4EA9-B6AB-78E7D5B722A7}"); + Chain("{8804D8E8-FE62-4E3A-B8A2-C047C2118C38}"); //FINAL diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs new file mode 100644 index 0000000000..d038da2573 --- /dev/null +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs @@ -0,0 +1,24 @@ +using System.Linq; +using Umbraco.Core.Persistence.Dtos; + +namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 +{ + public class AddLogTableColumns : MigrationBase + { + public AddLogTableColumns(IMigrationContext context) + : base(context) + { } + + public override void Migrate() + { + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); + + if (columns.Any(x => x.TableName.InvariantEquals(Constants.DatabaseSchema.Tables.Log) && !x.ColumnName.InvariantEquals("entityType"))) + AddColumn("entityType"); + + if (columns.Any(x => x.TableName.InvariantEquals(Constants.DatabaseSchema.Tables.Log) && !x.ColumnName.InvariantEquals("parameters"))) + AddColumn("parameters"); + + } + } +} diff --git a/src/Umbraco.Core/Models/AuditItem.cs b/src/Umbraco.Core/Models/AuditItem.cs index 6bfe32bd77..483548f558 100644 --- a/src/Umbraco.Core/Models/AuditItem.cs +++ b/src/Umbraco.Core/Models/AuditItem.cs @@ -11,7 +11,7 @@ namespace Umbraco.Core.Models /// /// /// - public AuditItem(int objectId, string comment, AuditType type, int userId) + public AuditItem(int objectId, AuditType type, int userId, string entityType, string comment = null, string parameters = null) { DisableChangeTracking(); @@ -19,11 +19,19 @@ namespace Umbraco.Core.Models Comment = comment; AuditType = type; UserId = userId; + EntityType = entityType; + Parameters = parameters; EnableChangeTracking(); } public string Comment { get; } + + /// + public string EntityType { get; } + /// + public string Parameters { get; } + public AuditType AuditType { get; } public int UserId { get; } } diff --git a/src/Umbraco.Core/Persistence/Dtos/LogDto.cs b/src/Umbraco.Core/Persistence/Dtos/LogDto.cs index 2ecf85e87c..a362d5d50c 100644 --- a/src/Umbraco.Core/Persistence/Dtos/LogDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/LogDto.cs @@ -25,10 +25,20 @@ namespace Umbraco.Core.Persistence.Dtos [Index(IndexTypes.NonClustered, Name = "IX_umbracoLog")] public int NodeId { get; set; } + /// + /// This is the entity type associated with the log + /// + [Column("entityType")] + [Length(50)] + [NullSetting(NullSetting = NullSettings.Null)] + public string EntityType { get; set; } + + //TODO: Should we have an index on this since we allow searching on it? [Column("Datestamp")] [Constraint(Default = SystemMethods.CurrentDateTime)] public DateTime Datestamp { get; set; } + //TODO: Should we have an index on this since we allow searching on it? [Column("logHeader")] [Length(50)] public string Header { get; set; } @@ -37,5 +47,13 @@ namespace Umbraco.Core.Persistence.Dtos [NullSetting(NullSetting = NullSettings.Null)] [Length(4000)] public string Comment { get; set; } + + /// + /// Used to store additional data parameters for the log + /// + [Column("parameters")] + [NullSetting(NullSetting = NullSettings.Null)] + [Length(500)] + public string Parameters { get; set; } } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/AuditRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/AuditRepository.cs index 5d386d9cb4..6c61fe7ad5 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/AuditRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/AuditRepository.cs @@ -28,20 +28,24 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Datestamp = DateTime.Now, Header = entity.AuditType.ToString(), NodeId = entity.Id, - UserId = entity.UserId + UserId = entity.UserId, + EntityType = entity.EntityType, + Parameters = entity.Parameters }); } protected override void PersistUpdatedItem(IAuditItem entity) { - // wtf?! inserting when updating?! + // inserting when updating because we never update a log entry, perhaps this should throw? Database.Insert(new LogDto { Comment = entity.Comment, Datestamp = DateTime.Now, Header = entity.AuditType.ToString(), NodeId = entity.Id, - UserId = entity.UserId + UserId = entity.UserId, + EntityType = entity.EntityType, + Parameters = entity.Parameters }); } @@ -53,7 +57,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var dto = Database.First(sql); return dto == null ? null - : new AuditItem(dto.NodeId, dto.Comment, Enum.Parse(dto.Header), dto.UserId ?? Constants.Security.UnknownUserId); + : new AuditItem(dto.NodeId, Enum.Parse(dto.Header), dto.UserId ?? Constants.Security.UnknownUserId, dto.EntityType, dto.Comment, dto.Parameters); } protected override IEnumerable PerformGetAll(params int[] ids) @@ -69,7 +73,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var dtos = Database.Fetch(sql); - return dtos.Select(x => new AuditItem(x.NodeId, x.Comment, Enum.Parse(x.Header), x.UserId ?? Constants.Security.UnknownUserId)).ToArray(); + return dtos.Select(x => new AuditItem(x.NodeId, Enum.Parse(x.Header), x.UserId ?? Constants.Security.UnknownUserId, x.EntityType, x.Comment, x.Parameters)).ToList(); } protected override Sql GetBaseQuery(bool isCount) @@ -160,10 +164,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement totalRecords = page.TotalItems; var items = page.Items.Select( - dto => new AuditItem(dto.Id, dto.Comment, Enum.ParseOrNull(dto.Header) ?? AuditType.Custom, dto.UserId ?? Constants.Security.UnknownUserId)).ToArray(); + dto => new AuditItem(dto.Id, Enum.ParseOrNull(dto.Header) ?? AuditType.Custom, dto.UserId ?? Constants.Security.UnknownUserId, dto.EntityType, dto.Comment, dto.Parameters)).ToList(); // map the DateStamp - for (var i = 0; i < items.Length; i++) + for (var i = 0; i < items.Count; i++) items[i].CreateDate = page.Items[i].Datestamp; return items; diff --git a/src/Umbraco.Core/Services/IAuditService.cs b/src/Umbraco.Core/Services/IAuditService.cs index 13d84f802e..f9b5aa2d87 100644 --- a/src/Umbraco.Core/Services/IAuditService.cs +++ b/src/Umbraco.Core/Services/IAuditService.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Services /// public interface IAuditService : IService { - void Add(AuditType type, string comment, int userId, int objectId); + void Add(AuditType type, int userId, int objectId, string entityType, string comment, string parameters = null); IEnumerable GetLogs(int objectId); IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null); diff --git a/src/Umbraco.Core/Services/Implement/AuditService.cs b/src/Umbraco.Core/Services/Implement/AuditService.cs index 389a2337d1..d02d7f541b 100644 --- a/src/Umbraco.Core/Services/Implement/AuditService.cs +++ b/src/Umbraco.Core/Services/Implement/AuditService.cs @@ -27,11 +27,11 @@ namespace Umbraco.Core.Services.Implement _isAvailable = new Lazy(DetermineIsAvailable); } - public void Add(AuditType type, string comment, int userId, int objectId) + public void Add(AuditType type, int userId, int objectId, string entityType, string comment, string parameters = null) { using (var scope = ScopeProvider.CreateScope()) { - _auditRepository.Save(new AuditItem(objectId, comment, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, comment, parameters)); scope.Complete(); } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index a849813b13..c4b17e1350 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -328,7 +328,7 @@ namespace Umbraco.Core.Services.Implement if (withIdentity == false) return; - Audit(AuditType.New, $"Content '{content.Name}' was created with Id {content.Id}", content.CreatorId, content.Id); + Audit(AuditType.New, content.CreatorId, content.Id, $"Content '{content.Name}' was created with Id {content.Id}"); } #endregion @@ -843,7 +843,16 @@ namespace Umbraco.Core.Services.Implement } var changeType = TreeChangeTypes.RefreshNode; scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - Audit(AuditType.Save, "Saved by user", userId, content.Id); + + var culturesChanging = content.ContentType.VariesByCulture() + ? content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() + : null; + + if (culturesChanging != null && culturesChanging.Count > 0) + Audit(AuditType.Save, userId, content.Id, $"Saved culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); + else + Audit(AuditType.Save, userId, content.Id); + scope.Complete(); } @@ -883,7 +892,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); } scope.Events.Dispatch(TreeChanged, this, treeChanges.ToEventArgs()); - Audit(AuditType.Save, "Bulk-saved by user", userId == -1 ? 0 : userId, Constants.System.Root); + Audit(AuditType.Save, userId == -1 ? 0 : userId, Constants.System.Root, "Saved multiple content"); scope.Complete(); } @@ -988,14 +997,14 @@ namespace Umbraco.Core.Services.Implement UnpublishResultType result; if (culture == "*" || culture == null) { - Audit(AuditType.Unpublish, "Unpublished by user", userId, content.Id); + Audit(AuditType.Unpublish, userId, content.Id); result = UnpublishResultType.Success; } else { - Audit(AuditType.Unpublish, $"Culture \"{culture}\" unpublished by user", userId, content.Id); + Audit(AuditType.Unpublish, userId, content.Id, $"Culture \"{culture}\" unpublished"); if (!content.Published) - Audit(AuditType.Unpublish, $"Unpublished (culture \"{culture}\" is mandatory) by user", userId, content.Id); + Audit(AuditType.Unpublish, userId, content.Id, $"Unpublished (culture \"{culture}\" is mandatory)"); result = content.Published ? UnpublishResultType.SuccessCulture : UnpublishResultType.SuccessMandatoryCulture; } scope.Complete(); @@ -1025,6 +1034,8 @@ namespace Umbraco.Core.Services.Implement var publishing = content.PublishedState == PublishedState.Publishing; var unpublishing = content.PublishedState == PublishedState.Unpublishing; + List culturesChanging = null; + using (var scope = ScopeProvider.CreateScope()) { // is the content going to end up published, or unpublished? @@ -1046,6 +1057,10 @@ namespace Umbraco.Core.Services.Implement // we may end up in a state where we won't publish nor unpublish // keep going, though, as we want to save anways } + else + { + culturesChanging = content.PublishNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList(); + } } var isNew = !content.HasIdentity; @@ -1085,6 +1100,7 @@ namespace Umbraco.Core.Services.Implement // ensure that the document can be unpublished, and unpublish // handling events, business rules, etc // note: StrategyUnpublish flips the PublishedState to Unpublishing! + // note: This unpublishes the entire document (not different variants) unpublishResult = StrategyCanUnpublish(scope, content, userId, evtMsgs); if (unpublishResult.Success) unpublishResult = StrategyUnpublish(scope, content, true, userId, evtMsgs); @@ -1122,7 +1138,7 @@ namespace Umbraco.Core.Services.Implement // events and audit scope.Events.Dispatch(Unpublished, this, new PublishEventArgs(content, false, false), "Unpublished"); scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshBranch).ToEventArgs()); - Audit(AuditType.Unpublish, "Unpublished by user", userId, content.Id); + Audit(AuditType.Unpublish, userId, content.Id); scope.Complete(); return new PublishResult(PublishResultType.Success, evtMsgs, content); } @@ -1153,7 +1169,11 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Published, this, new PublishEventArgs(descendants, false, false), "Published"); } - Audit(AuditType.Publish, "Published by user", userId, content.Id); + if (culturesChanging != null && culturesChanging.Count > 0) + Audit(AuditType.Publish, userId, content.Id, $"Published culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); + else + Audit(AuditType.Publish, userId, content.Id); + scope.Complete(); return publishResult; } @@ -1264,6 +1284,8 @@ namespace Umbraco.Core.Services.Implement // deal with descendants // if one fails, abort its branch var exclude = new HashSet(); + + //fixme: should be paged to not overwhelm the database (timeouts) foreach (var d in GetDescendants(document)) { // if parent is excluded, exclude document and ignore @@ -1287,7 +1309,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(TreeChanged, this, new TreeChange(document, TreeChangeTypes.RefreshBranch).ToEventArgs()); scope.Events.Dispatch(Published, this, new PublishEventArgs(publishedDocuments, false, false), "Published"); - Audit(AuditType.Publish, "Branch published by user", userId, document.Id); + Audit(AuditType.Publish, userId, document.Id, "Branch published"); scope.Complete(); } @@ -1356,7 +1378,7 @@ namespace Umbraco.Core.Services.Implement DeleteLocked(scope, content); scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.Remove).ToEventArgs()); - Audit(AuditType.Delete, "Deleted by user", userId, content.Id); + Audit(AuditType.Delete, userId, content.Id); scope.Complete(); } @@ -1424,7 +1446,7 @@ namespace Umbraco.Core.Services.Implement deleteRevisionsEventArgs.CanCancel = false; scope.Events.Dispatch(DeletedVersions, this, deleteRevisionsEventArgs); - Audit(AuditType.Delete, "Delete (by version date) by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, "Delete (by version date)"); scope.Complete(); } @@ -1461,7 +1483,7 @@ namespace Umbraco.Core.Services.Implement _documentRepository.DeleteVersion(versionId); scope.Events.Dispatch(DeletedVersions, this, new DeleteRevisionsEventArgs(id, false,/* specificVersion:*/ versionId)); - Audit(AuditType.Delete, "Delete (by version) by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, "Delete (by version)"); scope.Complete(); } @@ -1506,7 +1528,7 @@ namespace Umbraco.Core.Services.Implement moveEventArgs.CanCancel = false; moveEventArgs.MoveInfoCollection = moveInfo; scope.Events.Dispatch(Trashed, this, moveEventArgs, nameof(Trashed)); - Audit(AuditType.Move, "Moved to Recycle Bin by user", userId, content.Id); + Audit(AuditType.Move, userId, content.Id, "Moved to recycle bin"); scope.Complete(); } @@ -1578,7 +1600,7 @@ namespace Umbraco.Core.Services.Implement moveEventArgs.MoveInfoCollection = moveInfo; moveEventArgs.CanCancel = false; scope.Events.Dispatch(Moved, this, moveEventArgs, nameof(Moved)); - Audit(AuditType.Move, "Moved by user", userId, content.Id); + Audit(AuditType.Move, userId, content.Id); scope.Complete(); } @@ -1675,7 +1697,7 @@ namespace Umbraco.Core.Services.Implement recycleBinEventArgs.RecycleBinEmptiedSuccessfully = true; // oh my?! scope.Events.Dispatch(EmptiedRecycleBin, this, recycleBinEventArgs); scope.Events.Dispatch(TreeChanged, this, deleted.Select(x => new TreeChange(x, TreeChangeTypes.Remove)).ToEventArgs()); - Audit(AuditType.Delete, "Recycle Bin emptied by user", 0, Constants.System.RecycleBinContent); + Audit(AuditType.Delete, 0, Constants.System.RecycleBinContent, "Recycle bin emptied"); scope.Complete(); } @@ -1793,7 +1815,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(TreeChanged, this, new TreeChange(copy, TreeChangeTypes.RefreshBranch).ToEventArgs()); foreach (var x in copies) scope.Events.Dispatch(Copied, this, new CopyEventArgs(x.Item1, x.Item2, false, x.Item2.ParentId, relateToOriginal)); - Audit(AuditType.Copy, "Copy Content performed by user", userId, content.Id); + Audit(AuditType.Copy, userId, content.Id); scope.Complete(); } @@ -1824,7 +1846,15 @@ namespace Umbraco.Core.Services.Implement sendToPublishEventArgs.CanCancel = false; scope.Events.Dispatch(SentToPublish, this, sendToPublishEventArgs); - Audit(AuditType.SendToPublish, "Send to Publish performed by user", content.WriterId, content.Id); + + var culturesChanging = content.ContentType.VariesByCulture() + ? content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() + : null; + + if (culturesChanging != null && culturesChanging.Count > 0) + Audit(AuditType.SendToPublish, userId, content.Id, $"Send To Publish for culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); + else + Audit(AuditType.SendToPublish, content.WriterId, content.Id); } return true; @@ -1930,7 +1960,7 @@ namespace Umbraco.Core.Services.Implement if (raiseEvents && published.Any()) scope.Events.Dispatch(Published, this, new PublishEventArgs(published, false, false), "Published"); - Audit(AuditType.Sort, "Sorting content performed by user", userId, 0); + Audit(AuditType.Sort, userId, 0); return true; } @@ -1977,9 +2007,9 @@ namespace Umbraco.Core.Services.Implement #region Private Methods - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId, string message = null) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, Constants.ObjectTypes.Strings.Document, message)); } #endregion @@ -2311,7 +2341,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos), nameof(Trashed)); scope.Events.Dispatch(TreeChanged, this, changes.ToEventArgs()); - Audit(AuditType.Delete, $"Delete Content of Type {string.Join(",", contentTypeIdsA)} performed by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, $"Delete content of type {string.Join(",", contentTypeIdsA)}"); scope.Complete(); } diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index a114f415cc..60677cfd81 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -423,7 +423,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; OnSaved(scope, saveEventArgs); - Audit(AuditType.Save, $"Save {typeof(TItem).Name} performed by user", userId, item.Id); + Audit(AuditType.Save, userId, item.Id); scope.Complete(); } } @@ -465,7 +465,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; OnSaved(scope, saveEventArgs); - Audit(AuditType.Save, $"Save {typeof(TItem).Name} performed by user", userId, -1); + Audit(AuditType.Save, userId, -1); scope.Complete(); } } @@ -523,7 +523,7 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; OnDeleted(scope, deleteEventArgs); - Audit(AuditType.Delete, $"Delete {typeof(TItem).Name} performed by user", userId, item.Id); + Audit(AuditType.Delete, userId, item.Id); scope.Complete(); } } @@ -576,7 +576,7 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; OnDeleted(scope, deleteEventArgs); - Audit(AuditType.Delete, $"Delete {typeof(TItem).Name} performed by user", userId, -1); + Audit(AuditType.Delete, userId, -1); scope.Complete(); } } @@ -963,9 +963,10 @@ namespace Umbraco.Core.Services.Implement #region Audit - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, + ObjectTypes.GetUmbracoObjectType(ContainedObjectType).GetName())); } #endregion diff --git a/src/Umbraco.Core/Services/Implement/DataTypeService.cs b/src/Umbraco.Core/Services/Implement/DataTypeService.cs index c105b6cfe6..79ca851de9 100644 --- a/src/Umbraco.Core/Services/Implement/DataTypeService.cs +++ b/src/Umbraco.Core/Services/Implement/DataTypeService.cs @@ -353,7 +353,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(Saved, this, saveEventArgs); - Audit(AuditType.Save, "Save DataTypeDefinition performed by user", userId, dataType.Id); + Audit(AuditType.Save, userId, dataType.Id); scope.Complete(); } } @@ -398,7 +398,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(Saved, this, saveEventArgs); } - Audit(AuditType.Save, "Save DataTypeDefinition performed by user", userId, -1); + Audit(AuditType.Save, userId, -1); scope.Complete(); } @@ -456,15 +456,15 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; scope.Events.Dispatch(Deleted, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete DataTypeDefinition performed by user", userId, dataType.Id); + Audit(AuditType.Delete, userId, dataType.Id); scope.Complete(); } } - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.DataType))); } #region Event Handlers diff --git a/src/Umbraco.Core/Services/Implement/FileService.cs b/src/Umbraco.Core/Services/Implement/FileService.cs index c3a8b790cc..f15f0d7d47 100644 --- a/src/Umbraco.Core/Services/Implement/FileService.cs +++ b/src/Umbraco.Core/Services/Implement/FileService.cs @@ -91,7 +91,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(SavedStylesheet, this, saveEventArgs); - Audit(AuditType.Save, "Save Stylesheet performed by user", userId, -1); + Audit(AuditType.Save, userId, -1, ObjectTypes.GetName(UmbracoObjectTypes.Stylesheet)); scope.Complete(); } } @@ -123,7 +123,7 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; scope.Events.Dispatch(DeletedStylesheet, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete Stylesheet performed by user", userId, -1); + Audit(AuditType.Delete, userId, -1, ObjectTypes.GetName(UmbracoObjectTypes.Stylesheet)); scope.Complete(); } } @@ -215,7 +215,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(SavedScript, this, saveEventArgs); - Audit(AuditType.Save, "Save Script performed by user", userId, -1); + Audit(AuditType.Save, userId, -1, "Script"); scope.Complete(); } } @@ -247,7 +247,7 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; scope.Events.Dispatch(DeletedScript, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete Script performed by user", userId, -1); + Audit(AuditType.Delete, userId, -1, "Script"); scope.Complete(); } } @@ -362,7 +362,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(SavedTemplate, this, saveEventArgs); - Audit(AuditType.Save, "Save Template performed by user", userId, template.Id); + Audit(AuditType.Save, userId, template.Id, ObjectTypes.GetName(UmbracoObjectTypes.Template)); scope.Complete(); } @@ -525,7 +525,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(SavedTemplate, this, new SaveEventArgs(template, false)); - Audit(AuditType.Save, "Save Template performed by user", userId, template.Id); + Audit(AuditType.Save, userId, template.Id, ObjectTypes.GetName(UmbracoObjectTypes.Template)); scope.Complete(); } } @@ -551,7 +551,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(SavedTemplate, this, new SaveEventArgs(templatesA, false)); - Audit(AuditType.Save, "Save Template performed by user", userId, -1); + Audit(AuditType.Save, userId, -1, ObjectTypes.GetName(UmbracoObjectTypes.Template)); scope.Complete(); } } @@ -605,7 +605,7 @@ namespace Umbraco.Core.Services.Implement args.CanCancel = false; scope.Events.Dispatch(DeletedTemplate, this, args); - Audit(AuditType.Delete, "Delete Template performed by user", userId, template.Id); + Audit(AuditType.Delete, userId, template.Id, ObjectTypes.GetName(UmbracoObjectTypes.Template)); scope.Complete(); } } @@ -788,7 +788,7 @@ namespace Umbraco.Core.Services.Implement newEventArgs.CanCancel = false; scope.Events.Dispatch(CreatedPartialView, this, newEventArgs); - Audit(AuditType.Save, $"Save {partialViewType} performed by user", userId, -1); + Audit(AuditType.Save, userId, -1, partialViewType.ToString()); scope.Complete(); } @@ -828,7 +828,7 @@ namespace Umbraco.Core.Services.Implement repository.Delete(partialView); deleteEventArgs.CanCancel = false; scope.Events.Dispatch(DeletedPartialView, this, deleteEventArgs); - Audit(AuditType.Delete, $"Delete {partialViewType} performed by user", userId, -1); + Audit(AuditType.Delete, userId, -1, partialViewType.ToString()); scope.Complete(); } @@ -860,7 +860,7 @@ namespace Umbraco.Core.Services.Implement var repository = GetPartialViewRepository(partialViewType); repository.Save(partialView); saveEventArgs.CanCancel = false; - Audit(AuditType.Save, $"Save {partialViewType} performed by user", userId, -1); + Audit(AuditType.Save, userId, -1, partialViewType.ToString()); scope.Events.Dispatch(SavedPartialView, this, saveEventArgs); scope.Complete(); @@ -1038,9 +1038,9 @@ namespace Umbraco.Core.Services.Implement #endregion - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId, string entityType) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, entityType)); } //TODO Method to change name and/or alias of view/masterpage template diff --git a/src/Umbraco.Core/Services/Implement/LocalizationService.cs b/src/Umbraco.Core/Services/Implement/LocalizationService.cs index 49a764b533..c972b949d6 100644 --- a/src/Umbraco.Core/Services/Implement/LocalizationService.cs +++ b/src/Umbraco.Core/Services/Implement/LocalizationService.cs @@ -245,7 +245,7 @@ namespace Umbraco.Core.Services.Implement EnsureDictionaryItemLanguageCallback(dictionaryItem); scope.Events.Dispatch(SavedDictionaryItem, this, new SaveEventArgs(dictionaryItem, false)); - Audit(AuditType.Save, "Save DictionaryItem performed by user", userId, dictionaryItem.Id); + Audit(AuditType.Save, "Save DictionaryItem", userId, dictionaryItem.Id, "DictionaryItem"); scope.Complete(); } } @@ -271,7 +271,7 @@ namespace Umbraco.Core.Services.Implement deleteEventArgs.CanCancel = false; scope.Events.Dispatch(DeletedDictionaryItem, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete DictionaryItem performed by user", userId, dictionaryItem.Id); + Audit(AuditType.Delete, "Delete DictionaryItem", userId, dictionaryItem.Id, "DictionaryItem"); scope.Complete(); } @@ -384,7 +384,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(SavedLanguage, this, saveEventArgs); - Audit(AuditType.Save, "Save Language performed by user", userId, language.Id); + Audit(AuditType.Save, "Save Language", userId, language.Id, ObjectTypes.GetName(UmbracoObjectTypes.Language)); scope.Complete(); } @@ -429,14 +429,14 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(DeletedLanguage, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete Language performed by user", userId, language.Id); + Audit(AuditType.Delete, "Delete Language", userId, language.Id, ObjectTypes.GetName(UmbracoObjectTypes.Language)); scope.Complete(); } } - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, string message, int userId, int objectId, string entityType) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, message)); } /// diff --git a/src/Umbraco.Core/Services/Implement/MacroService.cs b/src/Umbraco.Core/Services/Implement/MacroService.cs index fdcc8e2ee0..5176e2eb22 100644 --- a/src/Umbraco.Core/Services/Implement/MacroService.cs +++ b/src/Umbraco.Core/Services/Implement/MacroService.cs @@ -95,7 +95,7 @@ namespace Umbraco.Core.Services.Implement _macroRepository.Delete(macro); deleteEventArgs.CanCancel = false; scope.Events.Dispatch(Deleted, this, deleteEventArgs); - Audit(AuditType.Delete, "Delete Macro performed by user", userId, -1); + Audit(AuditType.Delete, userId, -1); scope.Complete(); } @@ -125,7 +125,7 @@ namespace Umbraco.Core.Services.Implement _macroRepository.Save(macro); saveEventArgs.CanCancel = false; scope.Events.Dispatch(Saved, this, saveEventArgs); - Audit(AuditType.Save, "Save Macro performed by user", userId, -1); + Audit(AuditType.Save, userId, -1); scope.Complete(); } @@ -150,9 +150,9 @@ namespace Umbraco.Core.Services.Implement // return MacroPropertyTypeResolver.Current.MacroPropertyTypes.FirstOrDefault(x => x.Alias == alias); //} - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, "Macro")); } #region Event Handlers diff --git a/src/Umbraco.Core/Services/Implement/MediaService.cs b/src/Umbraco.Core/Services/Implement/MediaService.cs index 431e20044c..da04f41e18 100644 --- a/src/Umbraco.Core/Services/Implement/MediaService.cs +++ b/src/Umbraco.Core/Services/Implement/MediaService.cs @@ -295,7 +295,7 @@ namespace Umbraco.Core.Services.Implement if (withIdentity == false) return; - Audit(AuditType.New, $"Media '{media.Name}' was created with Id {media.Id}", media.CreatorId, media.Id); + Audit(AuditType.New, media.CreatorId, media.Id, $"Media '{media.Name}' was created with Id {media.Id}"); } #endregion @@ -778,7 +778,7 @@ namespace Umbraco.Core.Services.Implement var changeType = TreeChangeTypes.RefreshNode; scope.Events.Dispatch(TreeChanged, this, new TreeChange(media, changeType).ToEventArgs()); - Audit(AuditType.Save, "Save Media performed by user", userId, media.Id); + Audit(AuditType.Save, userId, media.Id); scope.Complete(); } @@ -821,7 +821,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Saved, this, saveEventArgs); } scope.Events.Dispatch(TreeChanged, this, treeChanges.ToEventArgs()); - Audit(AuditType.Save, "Bulk Save media performed by user", userId == -1 ? 0 : userId, Constants.System.Root); + Audit(AuditType.Save, userId == -1 ? 0 : userId, Constants.System.Root, "Bulk save media"); scope.Complete(); } @@ -855,7 +855,7 @@ namespace Umbraco.Core.Services.Implement DeleteLocked(scope, media); scope.Events.Dispatch(TreeChanged, this, new TreeChange(media, TreeChangeTypes.Remove).ToEventArgs()); - Audit(AuditType.Delete, "Delete Media performed by user", userId, media.Id); + Audit(AuditType.Delete, userId, media.Id); scope.Complete(); } @@ -924,7 +924,7 @@ namespace Umbraco.Core.Services.Implement //repository.DeleteVersions(id, versionDate); //uow.Events.Dispatch(DeletedVersions, this, new DeleteRevisionsEventArgs(id, false, dateToRetain: versionDate)); - //Audit(uow, AuditType.Delete, "Delete Media by version date performed by user", userId, Constants.System.Root); + //Audit(uow, AuditType.Delete, "Delete Media by version date, userId, Constants.System.Root); //uow.Complete(); } @@ -942,7 +942,7 @@ namespace Umbraco.Core.Services.Implement args.CanCancel = false; scope.Events.Dispatch(DeletedVersions, this, args); - Audit(AuditType.Delete, "Delete Media by version date performed by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, "Delete Media by version date"); } /// @@ -978,7 +978,7 @@ namespace Umbraco.Core.Services.Implement args.CanCancel = false; scope.Events.Dispatch(DeletedVersions, this, args); - Audit(AuditType.Delete, "Delete Media by version performed by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, "Delete Media by version"); scope.Complete(); } @@ -1020,7 +1020,7 @@ namespace Umbraco.Core.Services.Implement .ToArray(); scope.Events.Dispatch(Trashed, this, new MoveEventArgs(false, evtMsgs, moveInfo), nameof(Trashed)); - Audit(AuditType.Move, "Move Media to Recycle Bin performed by user", userId, media.Id); + Audit(AuditType.Move, userId, media.Id, "Move Media to recycle bin"); scope.Complete(); } @@ -1080,7 +1080,7 @@ namespace Umbraco.Core.Services.Implement moveEventArgs.MoveInfoCollection = moveInfo; moveEventArgs.CanCancel = false; scope.Events.Dispatch(Moved, this, moveEventArgs, nameof(Moved)); - Audit(AuditType.Move, "Move Media performed by user", userId, media.Id); + Audit(AuditType.Move, userId, media.Id); scope.Complete(); } } @@ -1173,7 +1173,7 @@ namespace Umbraco.Core.Services.Implement args.CanCancel = false; scope.Events.Dispatch(EmptiedRecycleBin, this, args); scope.Events.Dispatch(TreeChanged, this, deleted.Select(x => new TreeChange(x, TreeChangeTypes.Remove)).ToEventArgs()); - Audit(AuditType.Delete, "Empty Media Recycle Bin performed by user", 0, Constants.System.RecycleBinMedia); + Audit(AuditType.Delete, 0, Constants.System.RecycleBinMedia, "Empty Media recycle bin"); scope.Complete(); } @@ -1238,7 +1238,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Saved, this, args); } scope.Events.Dispatch(TreeChanged, this, saved.Select(x => new TreeChange(x, TreeChangeTypes.RefreshNode)).ToEventArgs()); - Audit(AuditType.Sort, "Sorting Media performed by user", userId, 0); + Audit(AuditType.Sort, userId, 0); scope.Complete(); } @@ -1250,9 +1250,9 @@ namespace Umbraco.Core.Services.Implement #region Private Methods - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId, string message = null) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Media), message)); } #endregion @@ -1434,7 +1434,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos), nameof(Trashed)); scope.Events.Dispatch(TreeChanged, this, changes.ToEventArgs()); - Audit(AuditType.Delete, $"Delete Media of types {string.Join(",", mediaTypeIdsA)} performed by user", userId, Constants.System.Root); + Audit(AuditType.Delete, userId, Constants.System.Root, $"Delete Media of types {string.Join(",", mediaTypeIdsA)}"); scope.Complete(); } diff --git a/src/Umbraco.Core/Services/Implement/MemberService.cs b/src/Umbraco.Core/Services/Implement/MemberService.cs index 211e30d01c..3fd714f974 100644 --- a/src/Umbraco.Core/Services/Implement/MemberService.cs +++ b/src/Umbraco.Core/Services/Implement/MemberService.cs @@ -337,7 +337,7 @@ namespace Umbraco.Core.Services.Implement if (withIdentity == false) return; - Audit(AuditType.New, $"Member '{member.Name}' was created with Id {member.Id}", member.CreatorId, member.Id); + Audit(AuditType.New, member.CreatorId, member.Id, $"Member '{member.Name}' was created with Id {member.Id}"); } #endregion @@ -843,7 +843,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(Saved, this, saveEventArgs); } - Audit(AuditType.Save, "Save Member performed by user", 0, member.Id); + Audit(AuditType.Save, 0, member.Id); scope.Complete(); } @@ -884,7 +884,7 @@ namespace Umbraco.Core.Services.Implement saveEventArgs.CanCancel = false; scope.Events.Dispatch(Saved, this, saveEventArgs); } - Audit(AuditType.Save, "Save Member items performed by user", 0, -1); + Audit(AuditType.Save, 0, -1, "Save multiple Members"); scope.Complete(); } @@ -912,7 +912,7 @@ namespace Umbraco.Core.Services.Implement scope.WriteLock(Constants.Locks.MemberTree); DeleteLocked(scope, member, deleteEventArgs); - Audit(AuditType.Delete, "Delete Member performed by user", 0, member.Id); + Audit(AuditType.Delete, 0, member.Id); scope.Complete(); } } @@ -1089,9 +1089,9 @@ namespace Umbraco.Core.Services.Implement #region Private Methods - private void Audit(AuditType type, string message, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId, string message = null) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Member), message)); } #endregion diff --git a/src/Umbraco.Core/Services/Implement/PackagingService.cs b/src/Umbraco.Core/Services/Implement/PackagingService.cs index 67e07e63b6..c0e8f80337 100644 --- a/src/Umbraco.Core/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Core/Services/Implement/PackagingService.cs @@ -1485,7 +1485,7 @@ namespace Umbraco.Core.Services.Implement private void Audit(AuditType type, string message, int userId, int objectId) { - _auditRepository.Save(new AuditItem(objectId, message, type, userId)); + _auditRepository.Save(new AuditItem(objectId, type, userId, "Package", message)); } #endregion diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 0eba543e0d..dc5ca5e2aa 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -353,6 +353,7 @@ + From c3e2d2a821dfc5aa7163adec415bfbf9d8e46ca1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 18 Oct 2018 22:52:49 +1100 Subject: [PATCH 04/16] Cleaning up the log/audit trail usage, removing duplicate and unecessary entries and adding new cols --- .../Components/RelateOnCopyComponent.cs | 5 +-- .../Components/RelateOnTrashComponent.cs | 16 ++++++---- src/Umbraco.Core/Models/IAuditItem.cs | 10 ++++++ .../Cache/DefaultCachePolicyTests.cs | 16 +++++----- .../Cache/FullDataSetCachePolicyTests.cs | 32 +++++++++---------- .../Cache/SingleItemsOnlyCachePolicyTests.cs | 6 ++-- .../Repositories/AuditRepositoryTest.cs | 18 +++++------ src/Umbraco.Web.UI.Client/package-lock.json | 15 +++------ src/Umbraco.Web/_Legacy/Packager/Installer.cs | 4 +-- .../PackageInstance/InstalledPackage.cs | 2 +- 10 files changed, 65 insertions(+), 59 deletions(-) diff --git a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs index 4ebd309e9f..bc66dccd31 100644 --- a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs +++ b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs @@ -37,8 +37,9 @@ namespace Umbraco.Core.Components Current.Services.AuditService.Add( AuditType.Copy, - $"Copied content with Id: '{e.Copy.Id}' related to original content with Id: '{e.Original.Id}'", - e.Copy.WriterId, e.Copy.Id); + e.Copy.WriterId, + e.Copy.Id, ObjectTypes.GetName(UmbracoObjectTypes.Document), + $"Copied content with Id: '{e.Copy.Id}' related to original content with Id: '{e.Original.Id}'"); } } } diff --git a/src/Umbraco.Core/Components/RelateOnTrashComponent.cs b/src/Umbraco.Core/Components/RelateOnTrashComponent.cs index fffae85501..8bcce50c68 100644 --- a/src/Umbraco.Core/Components/RelateOnTrashComponent.cs +++ b/src/Umbraco.Core/Components/RelateOnTrashComponent.cs @@ -82,11 +82,12 @@ namespace Umbraco.Core.Components relationService.Save(relation); Current.Services.AuditService.Add(AuditType.Delete, + item.Entity.WriterId, + item.Entity.Id, + ObjectTypes.GetName(UmbracoObjectTypes.Document), string.Format(textService.Localize( "recycleBin/contentTrashed"), - item.Entity.Id, originalParentId), - item.Entity.WriterId, - item.Entity.Id); + item.Entity.Id, originalParentId)); } } } @@ -120,11 +121,12 @@ namespace Umbraco.Core.Components var relation = new Relation(originalParentId, item.Entity.Id, relationType); relationService.Save(relation); Current.Services.AuditService.Add(AuditType.Delete, - string.Format(textService.Localize( + item.Entity.CreatorId, + item.Entity.Id, + ObjectTypes.GetName(UmbracoObjectTypes.Media), + string.Format(textService.Localize( "recycleBin/mediaTrashed"), - item.Entity.Id, originalParentId), - item.Entity.CreatorId, - item.Entity.Id); + item.Entity.Id, originalParentId)); } } } diff --git a/src/Umbraco.Core/Models/IAuditItem.cs b/src/Umbraco.Core/Models/IAuditItem.cs index 9416e2a055..c1dfd99dbb 100644 --- a/src/Umbraco.Core/Models/IAuditItem.cs +++ b/src/Umbraco.Core/Models/IAuditItem.cs @@ -6,6 +6,16 @@ namespace Umbraco.Core.Models public interface IAuditItem : IEntity { string Comment { get; } + + /// + /// The entity type for the log entry + /// + string EntityType { get; } + + /// + /// Optional additional data parameters for the log + /// + string Parameters { get; } AuditType AuditType { get; } int UserId { get; } } diff --git a/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs b/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs index a8021055a9..37488600c7 100644 --- a/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs @@ -38,7 +38,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); - var unused = defaultPolicy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), o => null); + var unused = defaultPolicy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), o => null); Assert.IsTrue(isCached); } @@ -46,7 +46,7 @@ namespace Umbraco.Tests.Cache public void Get_Single_From_Cache() { var cache = new Mock(); - cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, "blah", AuditType.Copy, 123)); + cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, AuditType.Copy, 123, "test", "blah")); var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); @@ -71,8 +71,8 @@ namespace Umbraco.Tests.Cache var unused = defaultPolicy.GetAll(new object[] {}, ids => new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); Assert.AreEqual(2, cached.Count); @@ -84,8 +84,8 @@ namespace Umbraco.Tests.Cache var cache = new Mock(); cache.Setup(x => x.GetCacheItemsByKeySearch(It.IsAny())).Returns(new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); @@ -108,7 +108,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); try { - defaultPolicy.Update(new AuditItem(1, "blah", AuditType.Copy, 123), item => throw new Exception("blah!")); + defaultPolicy.Update(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => throw new Exception("blah!")); } catch { @@ -134,7 +134,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); try { - defaultPolicy.Delete(new AuditItem(1, "blah", AuditType.Copy, 123), item => throw new Exception("blah!")); + defaultPolicy.Delete(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => throw new Exception("blah!")); } catch { diff --git a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs index a275a44964..404587bcfa 100644 --- a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs @@ -32,8 +32,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var isCached = false; @@ -47,7 +47,7 @@ namespace Umbraco.Tests.Cache var policy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); - var unused = policy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), ids => getAll); + var unused = policy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), ids => getAll); Assert.IsTrue(isCached); } @@ -56,12 +56,12 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cache = new Mock(); - cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, "blah", AuditType.Copy, 123)); + cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, AuditType.Copy, 123, "test", "blah")); var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); @@ -114,8 +114,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cached = new List(); @@ -149,8 +149,8 @@ namespace Umbraco.Tests.Cache cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(() => new DeepCloneableList(ListCloneBehavior.CloneOnce) { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); @@ -164,8 +164,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cacheCleared = false; @@ -179,7 +179,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); try { - defaultPolicy.Update(new AuditItem(1, "blah", AuditType.Copy, 123), item => { throw new Exception("blah!"); }); + defaultPolicy.Update(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => { throw new Exception("blah!"); }); } catch { @@ -196,8 +196,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cacheCleared = false; @@ -211,7 +211,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); try { - defaultPolicy.Delete(new AuditItem(1, "blah", AuditType.Copy, 123), item => { throw new Exception("blah!"); }); + defaultPolicy.Delete(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => { throw new Exception("blah!"); }); } catch { diff --git a/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs b/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs index 9ab98bda7e..1c2227f79b 100644 --- a/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs @@ -41,8 +41,8 @@ namespace Umbraco.Tests.Cache var unused = defaultPolicy.GetAll(new object[] { }, ids => new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); Assert.AreEqual(0, cached.Count); @@ -62,7 +62,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new SingleItemsOnlyRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); - var unused = defaultPolicy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), ids => null); + var unused = defaultPolicy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), ids => null); Assert.IsTrue(isCached); } } diff --git a/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs index 6953634a31..eb85656ee4 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs @@ -27,7 +27,7 @@ namespace Umbraco.Tests.Persistence.Repositories using (var scope = sp.CreateScope()) { var repo = new AuditRepository((IScopeAccessor) sp, CacheHelper, Logger); - repo.Save(new AuditItem(-1, "This is a System audit trail", AuditType.System, -1)); + repo.Save(new AuditItem(-1, AuditType.System, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "This is a System audit trail")); var dtos = scope.Database.Fetch("WHERE id > -1"); @@ -46,8 +46,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -74,8 +74,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -117,8 +117,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -148,8 +148,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, "Content created", AuditType.New, -1)); - repo.Save(new AuditItem(i, "Content published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "Content created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "Content published")); } scope.Complete(); diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index 4b8cc3e6c2..6cce6d7708 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -5505,8 +5505,7 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", @@ -5516,8 +5515,7 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -5634,8 +5632,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -5647,7 +5644,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -5773,8 +5769,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -5786,7 +5781,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -5908,7 +5902,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", diff --git a/src/Umbraco.Web/_Legacy/Packager/Installer.cs b/src/Umbraco.Web/_Legacy/Packager/Installer.cs index 2581e3ad48..ae6b3e0a11 100644 --- a/src/Umbraco.Web/_Legacy/Packager/Installer.cs +++ b/src/Umbraco.Web/_Legacy/Packager/Installer.cs @@ -323,8 +323,8 @@ namespace umbraco.cms.businesslogic.packager if (_currentUserId > -1) { Current.Services.AuditService.Add(AuditType.PackagerInstall, - string.Format("Package '{0}' installed. Package guid: {1}", insPack.Data.Name, insPack.Data.PackageGuid), - _currentUserId, -1); + _currentUserId, + -1, "Package", string.Format("Package '{0}' installed. Package guid: {1}", insPack.Data.Name, insPack.Data.PackageGuid)); } insPack.Save(); diff --git a/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs b/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs index c16afa0b84..ae4d23aa9a 100644 --- a/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs +++ b/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs @@ -67,7 +67,7 @@ namespace umbraco.cms.businesslogic.packager { public void Delete(int userId) { - Current.Services.AuditService.Add(AuditType.PackagerUninstall, string.Format("Package '{0}' uninstalled. Package guid: {1}", Data.Name, Data.PackageGuid), userId, -1); + Current.Services.AuditService.Add(AuditType.PackagerUninstall, userId, -1, "Package", string.Format("Package '{0}' uninstalled. Package guid: {1}", Data.Name, Data.PackageGuid)); Delete(); } From b30f55ea30646184b8454e4bdb358f31033c39af Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 08:26:17 +1100 Subject: [PATCH 05/16] Fixes issue of observable collections, event binding and deep cloning --- src/Umbraco.Core/Models/Content.cs | 14 ++++++- src/Umbraco.Core/Models/ContentBase.cs | 38 +++++++++++++++++++ .../Models/Entities/EntityBase.cs | 9 +++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index fefbdfbef7..92b0488a82 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -438,6 +438,7 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsurePropertyTypes(PropertyTypes); + //TODO: Shouldn't we remove this event handler first before re-adding it in case the handler already exists Properties.CollectionChanged += PropertiesChanged; } @@ -455,6 +456,7 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsureCleanPropertyTypes(PropertyTypes); + //TODO: Shouldn't we remove this event handler first before re-adding it in case the handler already exists Properties.CollectionChanged += PropertiesChanged; return; } @@ -500,10 +502,18 @@ namespace Umbraco.Core.Models public override object DeepClone() { var clone = (Content) base.DeepClone(); + //turn off change tracking clone.DisableChangeTracking(); - //need to manually clone this since it's not settable - clone._contentType = (IContentType)ContentType.DeepClone(); + + //if culture infos exist then deal with event bindings + if (clone._publishInfos != null) + { + clone._publishInfos.CollectionChanged -= this.PublishNamesCollectionChanged; //clear this event handler if any + clone._publishInfos = (CultureNameCollection)_publishInfos.DeepClone(); //manually deep clone + clone._publishInfos.CollectionChanged += clone.PublishNamesCollectionChanged; //re-assign correct event handler + } + //this shouldn't really be needed since we're not tracking clone.ResetDirtyProperties(false); //re-enable tracking diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 2336188c50..1c5ee18be6 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -113,7 +113,11 @@ namespace Umbraco.Core.Models /// /// Gets or sets the collection of properties for the entity. /// + /// + /// Marked DoNotClone since we'll manually clone the underlying field to deal with the event handling + /// [DataMember] + [DoNotClone] public virtual PropertyCollection Properties { get => _properties; @@ -477,5 +481,39 @@ namespace Umbraco.Core.Models } #endregion + + /// + /// Override to deal with specific object instances + /// + /// + public override object DeepClone() + { + var clone = (ContentBase)base.DeepClone(); + //turn off change tracking + clone.DisableChangeTracking(); + + //if culture infos exist then deal with event bindings + if (clone._cultureInfos != null) + { + clone._cultureInfos.CollectionChanged -= this.CultureNamesCollectionChanged; //clear this event handler if any + clone._cultureInfos = (CultureNameCollection)_cultureInfos.DeepClone(); //manually deep clone + clone._cultureInfos.CollectionChanged += clone.CultureNamesCollectionChanged; //re-assign correct event handler + } + + //if properties exist then deal with event bindings + if (clone._properties != null) + { + clone._properties.CollectionChanged -= this.PropertiesChanged; //clear this event handler if any + clone._properties = (PropertyCollection)_properties.DeepClone(); //manually deep clone + clone._properties.CollectionChanged += clone.PropertiesChanged; //re-assign correct event handler + } + + //this shouldn't really be needed since we're not tracking + clone.ResetDirtyProperties(false); + //re-enable tracking + clone.EnableChangeTracking(); + + return clone; + } } } diff --git a/src/Umbraco.Core/Models/Entities/EntityBase.cs b/src/Umbraco.Core/Models/Entities/EntityBase.cs index ab57d57ab6..9cd6d1edad 100644 --- a/src/Umbraco.Core/Models/Entities/EntityBase.cs +++ b/src/Umbraco.Core/Models/Entities/EntityBase.cs @@ -13,6 +13,10 @@ namespace Umbraco.Core.Models.Entities [DebuggerDisplay("Id: {" + nameof(Id) + "}")] public abstract class EntityBase : BeingDirtyBase, IEntity { +#if DEBUG + public Guid InstanceId = Guid.NewGuid(); +#endif + private static readonly Lazy Ps = new Lazy(); private bool _hasIdentity; @@ -161,6 +165,11 @@ namespace Umbraco.Core.Models.Entities var unused = Key; // ensure that 'this' has a key, before cloning var clone = (EntityBase) MemberwiseClone(); +#if DEBUG + clone.InstanceId = Guid.NewGuid(); +#endif + + // clear changes (ensures the clone has its own dictionaries) // then disable change tracking clone.ResetDirtyProperties(false); From 71fd47f452038e24ab2436694b2304e370b39041 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 08:45:53 +1100 Subject: [PATCH 06/16] Fixes issue of observable collections, event binding and deep cloning --- src/Umbraco.Core/Models/Content.cs | 6 ++- src/Umbraco.Core/Models/ContentTypeBase.cs | 51 ++++++++++++++------ src/Umbraco.Core/Models/Media.cs | 5 ++ src/Umbraco.Core/Models/PropertyGroup.cs | 25 ++++++++++ src/Umbraco.Core/Models/PublicAccessEntry.cs | 20 ++++++++ 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 92b0488a82..abe28522ba 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -438,7 +438,8 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsurePropertyTypes(PropertyTypes); - //TODO: Shouldn't we remove this event handler first before re-adding it in case the handler already exists + + Properties.CollectionChanged -= PropertiesChanged; // be sure not to double add Properties.CollectionChanged += PropertiesChanged; } @@ -456,7 +457,8 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsureCleanPropertyTypes(PropertyTypes); - //TODO: Shouldn't we remove this event handler first before re-adding it in case the handler already exists + + Properties.CollectionChanged -= PropertiesChanged; // be sure not to double add Properties.CollectionChanged += PropertiesChanged; return; } diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 9e73205c36..b2ac8610d1 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -119,7 +119,7 @@ namespace Umbraco.Core.Models /// The Alias of the ContentType /// [DataMember] - public virtual string Alias + public string Alias { get => _alias; set => SetPropertyValueAndDetectChanges( @@ -132,7 +132,7 @@ namespace Umbraco.Core.Models /// Description for the ContentType /// [DataMember] - public virtual string Description + public string Description { get => _description; set => SetPropertyValueAndDetectChanges(value, ref _description, Ps.Value.DescriptionSelector); @@ -142,7 +142,7 @@ namespace Umbraco.Core.Models /// Name of the icon (sprite class) used to identify the ContentType /// [DataMember] - public virtual string Icon + public string Icon { get => _icon; set => SetPropertyValueAndDetectChanges(value, ref _icon, Ps.Value.IconSelector); @@ -152,7 +152,7 @@ namespace Umbraco.Core.Models /// Name of the thumbnail used to identify the ContentType /// [DataMember] - public virtual string Thumbnail + public string Thumbnail { get => _thumbnail; set => SetPropertyValueAndDetectChanges(value, ref _thumbnail, Ps.Value.ThumbnailSelector); @@ -162,7 +162,7 @@ namespace Umbraco.Core.Models /// Gets or Sets a boolean indicating whether this ContentType is allowed at the root /// [DataMember] - public virtual bool AllowedAsRoot + public bool AllowedAsRoot { get => _allowedAsRoot; set => SetPropertyValueAndDetectChanges(value, ref _allowedAsRoot, Ps.Value.AllowedAsRootSelector); @@ -175,7 +175,7 @@ namespace Umbraco.Core.Models /// ContentType Containers doesn't show children in the tree, but rather in grid-type view. /// [DataMember] - public virtual bool IsContainer + public bool IsContainer { get => _isContainer; set => SetPropertyValueAndDetectChanges(value, ref _isContainer, Ps.Value.IsContainerSelector); @@ -185,7 +185,7 @@ namespace Umbraco.Core.Models /// Gets or sets a list of integer Ids for allowed ContentTypes /// [DataMember] - public virtual IEnumerable AllowedContentTypes + public IEnumerable AllowedContentTypes { get => _allowedContentTypes; set => SetPropertyValueAndDetectChanges(value, ref _allowedContentTypes, Ps.Value.AllowedContentTypesSelector, @@ -195,7 +195,7 @@ namespace Umbraco.Core.Models /// /// Gets or sets the content variation of the content type. /// - public virtual ContentVariation Variations + public ContentVariation Variations { get => _variations; set => SetPropertyValueAndDetectChanges(value, ref _variations, Ps.Value.VaryBy); @@ -223,10 +223,12 @@ namespace Umbraco.Core.Models /// List of PropertyGroups available on this ContentType /// /// - /// A PropertyGroup corresponds to a Tab in the UI + /// A PropertyGroup corresponds to a Tab in the UI + /// Marked DoNotClone because we will manually deal with cloning and the event handlers /// [DataMember] - public virtual PropertyGroupCollection PropertyGroups + [DoNotClone] + public PropertyGroupCollection PropertyGroups { get => _propertyGroups; set @@ -242,7 +244,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] [DoNotClone] - public virtual IEnumerable PropertyTypes + public IEnumerable PropertyTypes { get { @@ -253,6 +255,10 @@ namespace Umbraco.Core.Models /// /// Gets or sets the property types that are not in a group. /// + /// + /// Marked DoNotClone because we will manually deal with cloning and the event handlers + /// + [DoNotClone] public IEnumerable NoGroupPropertyTypes { get => _propertyTypes; @@ -467,12 +473,25 @@ namespace Umbraco.Core.Models var clone = (ContentTypeBase)base.DeepClone(); //turn off change tracking clone.DisableChangeTracking(); - //need to manually wire up the event handlers for the property type collections - we've ensured - // its ignored from the auto-clone process because its return values are unions, not raw and - // we end up with duplicates, see: http://issues.umbraco.org/issue/U4-4842 - clone._propertyTypes = (PropertyTypeCollection)_propertyTypes.DeepClone(); - clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; + if (clone._propertyTypes != null) + { + //need to manually wire up the event handlers for the property type collections - we've ensured + // its ignored from the auto-clone process because its return values are unions, not raw and + // we end up with duplicates, see: http://issues.umbraco.org/issue/U4-4842 + + clone._propertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any + clone._propertyTypes = (PropertyTypeCollection)_propertyTypes.DeepClone(); //manually deep clone + clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + } + + if (clone._propertyGroups != null) + { + clone._propertyGroups.CollectionChanged -= this.PropertyGroupsChanged; //clear this event handler if any + clone._propertyGroups = (PropertyGroupCollection)_propertyGroups.DeepClone(); //manually deep clone + clone._propertyGroups.CollectionChanged += clone.PropertyGroupsChanged; //re-assign correct event handler + } + //this shouldn't really be needed since we're not tracking clone.ResetDirtyProperties(false); //re-enable tracking diff --git a/src/Umbraco.Core/Models/Media.cs b/src/Umbraco.Core/Models/Media.cs index c3f7cb6dd5..097fc46351 100644 --- a/src/Umbraco.Core/Models/Media.cs +++ b/src/Umbraco.Core/Models/Media.cs @@ -78,6 +78,8 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsurePropertyTypes(PropertyTypes); + + Properties.CollectionChanged -= PropertiesChanged; // be sure not to double add Properties.CollectionChanged += PropertiesChanged; } @@ -95,6 +97,8 @@ namespace Umbraco.Core.Models _contentType = contentType; ContentTypeBase = contentType; Properties.EnsureCleanPropertyTypes(PropertyTypes); + + Properties.CollectionChanged -= PropertiesChanged; // be sure not to double add Properties.CollectionChanged += PropertiesChanged; return; } @@ -113,5 +117,6 @@ namespace Umbraco.Core.Models //The Media Recycle Bin Id is -21 so we correct that here ParentId = parentId == -20 ? -21 : parentId; } + } } diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 286e165764..90f3ef2b55 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -66,7 +66,11 @@ namespace Umbraco.Core.Models /// /// Gets or sets a collection of PropertyTypes for this PropertyGroup /// + /// + /// Marked DoNotClone because we will manually deal with cloning and the event handlers + /// [DataMember] + [DoNotClone] public PropertyTypeCollection PropertyTypes { get => _propertyTypes; @@ -95,5 +99,26 @@ namespace Umbraco.Core.Models var nameHash = Name.ToLowerInvariant().GetHashCode(); return baseHash ^ nameHash; } + + public override object DeepClone() + { + var clone = (PropertyGroup)base.DeepClone(); + //turn off change tracking + clone.DisableChangeTracking(); + + if (clone._propertyTypes != null) + { + clone._propertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any + clone._propertyTypes = (PropertyTypeCollection)_propertyTypes.DeepClone(); //manually deep clone + clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + } + + //this shouldn't really be needed since we're not tracking + clone.ResetDirtyProperties(false); + //re-enable tracking + clone.EnableChangeTracking(); + + return clone; + } } } diff --git a/src/Umbraco.Core/Models/PublicAccessEntry.cs b/src/Umbraco.Core/Models/PublicAccessEntry.cs index a9f568e43a..26e249a35a 100644 --- a/src/Umbraco.Core/Models/PublicAccessEntry.cs +++ b/src/Umbraco.Core/Models/PublicAccessEntry.cs @@ -152,5 +152,25 @@ namespace Umbraco.Core.Models publicAccessRule.ResetDirtyProperties(rememberDirty); } } + + public override object DeepClone() + { + var clone = (PublicAccessEntry)base.DeepClone(); + //turn off change tracking + clone.DisableChangeTracking(); + + if (clone._ruleCollection != null) + { + clone._ruleCollection.CollectionChanged -= this._ruleCollection_CollectionChanged; //clear this event handler if any + clone._ruleCollection.CollectionChanged += clone._ruleCollection_CollectionChanged; //re-assign correct event handler + } + + //this shouldn't really be needed since we're not tracking + clone.ResetDirtyProperties(false); + //re-enable tracking + clone.EnableChangeTracking(); + + return clone; + } } } From cc4ee6527d83b12d25c8e53e95de96c6af1f9580 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 13:24:36 +1100 Subject: [PATCH 07/16] Gets the correct variant logging put in to the audit trail, cleans up audit trail messages to make sense, makes sure audit trail is reloaded when changing variants --- src/Umbraco.Core/Models/AuditType.cs | 24 +++++-- src/Umbraco.Core/Models/ContentTypeBase.cs | 4 +- .../Models/Entities/EntityBase.cs | 4 +- .../Services/Implement/ContentService.cs | 68 ++++++++++++------- .../content/umbcontentnodeinfo.directive.js | 26 ++++++- .../content/umb-content-node-info.html | 18 ++--- src/Umbraco.Web.UI/Umbraco/config/lang/en.xml | 24 ++++--- .../Umbraco/config/lang/en_us.xml | 24 ++++--- src/Umbraco.Web/Editors/LogController.cs | 2 + .../Models/ContentEditing/AuditLog.cs | 6 ++ src/Umbraco.Web/Security/WebSecurity.cs | 6 +- 11 files changed, 136 insertions(+), 70 deletions(-) diff --git a/src/Umbraco.Core/Models/AuditType.cs b/src/Umbraco.Core/Models/AuditType.cs index a5ae34a89d..759aac3bfc 100644 --- a/src/Umbraco.Core/Models/AuditType.cs +++ b/src/Umbraco.Core/Models/AuditType.cs @@ -14,6 +14,10 @@ /// Save, /// + /// Used when variant(s) are saved + /// + SaveVariant, + /// /// Used when nodes are opened /// Open, @@ -26,14 +30,26 @@ /// Publish, /// - /// Used when nodes are send to publishing + /// Used when variant(s) are published + /// + PublishVariant, + /// + /// Used when nodes are sent for publishing /// SendToPublish, /// + /// Used when variant(s) are sent for publishing + /// + SendToPublishVariant, + /// /// Used when nodes are unpublished /// Unpublish, /// + /// Used when variant(s) are unpublished + /// + UnpublishVariant, + /// /// Used when nodes are moved /// Move, @@ -72,11 +88,7 @@ /// /// Used when a package is uninstalled /// - PackagerUninstall, - /// - /// Used when a node is send to translation - /// - SendToTranslate, + PackagerUninstall, /// /// Use this log action for custom log messages that should be shown in the audit trail /// diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index b2ac8610d1..2a57420e9e 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -119,7 +119,7 @@ namespace Umbraco.Core.Models /// The Alias of the ContentType /// [DataMember] - public string Alias + public virtual string Alias { get => _alias; set => SetPropertyValueAndDetectChanges( @@ -195,7 +195,7 @@ namespace Umbraco.Core.Models /// /// Gets or sets the content variation of the content type. /// - public ContentVariation Variations + public virtual ContentVariation Variations { get => _variations; set => SetPropertyValueAndDetectChanges(value, ref _variations, Ps.Value.VaryBy); diff --git a/src/Umbraco.Core/Models/Entities/EntityBase.cs b/src/Umbraco.Core/Models/Entities/EntityBase.cs index 9cd6d1edad..f1ff5f3bf5 100644 --- a/src/Umbraco.Core/Models/Entities/EntityBase.cs +++ b/src/Umbraco.Core/Models/Entities/EntityBase.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Models.Entities [DebuggerDisplay("Id: {" + nameof(Id) + "}")] public abstract class EntityBase : BeingDirtyBase, IEntity { -#if DEBUG +#if ModelDebug public Guid InstanceId = Guid.NewGuid(); #endif @@ -165,7 +165,7 @@ namespace Umbraco.Core.Models.Entities var unused = Key; // ensure that 'this' has a key, before cloning var clone = (EntityBase) MemberwiseClone(); -#if DEBUG +#if ModelDebug clone.InstanceId = Guid.NewGuid(); #endif diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index c4b17e1350..9deadfa5af 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -834,6 +834,15 @@ namespace Umbraco.Core.Services.Implement content.CreatorId = userId; content.WriterId = userId; + //track the cultures that have changed + var culturesChanging = content.ContentType.VariesByCulture() + ? string.Join(",", content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture)) + : null; + //TODO: Currently there's no way to change track which variant properties have changed, we only have change + // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. + // in this particular case, determining which cultures have changed works with the above with names since it will + // have always changed if it's been saved in the back office but that's not really fail safe. + _documentRepository.Save(content); if (raiseEvents) @@ -844,12 +853,8 @@ namespace Umbraco.Core.Services.Implement var changeType = TreeChangeTypes.RefreshNode; scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - var culturesChanging = content.ContentType.VariesByCulture() - ? content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() - : null; - - if (culturesChanging != null && culturesChanging.Count > 0) - Audit(AuditType.Save, userId, content.Id, $"Saved culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); + if (culturesChanging != null) + Audit(AuditType.SaveVariant, userId, content.Id, $"Saved cultures: {culturesChanging}", culturesChanging); else Audit(AuditType.Save, userId, content.Id); @@ -1002,9 +1007,14 @@ namespace Umbraco.Core.Services.Implement } else { - Audit(AuditType.Unpublish, userId, content.Id, $"Culture \"{culture}\" unpublished"); + //unpublishing a specific culture + Audit(AuditType.UnpublishVariant, userId, content.Id, $"Culture \"{culture}\" unpublished", culture); if (!content.Published) + { + //log that the whole content item has been unpublished due to mandatory culture unpublished Audit(AuditType.Unpublish, userId, content.Id, $"Unpublished (culture \"{culture}\" is mandatory)"); + } + result = content.Published ? UnpublishResultType.SuccessCulture : UnpublishResultType.SuccessMandatoryCulture; } scope.Complete(); @@ -1034,7 +1044,7 @@ namespace Umbraco.Core.Services.Implement var publishing = content.PublishedState == PublishedState.Publishing; var unpublishing = content.PublishedState == PublishedState.Unpublishing; - List culturesChanging = null; + string culturesChanging = null; using (var scope = ScopeProvider.CreateScope()) { @@ -1059,7 +1069,7 @@ namespace Umbraco.Core.Services.Implement } else { - culturesChanging = content.PublishNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList(); + culturesChanging = string.Join(",", content.PublishNames.Where(x => x.IsDirty()).Select(x => x.Culture)); } } @@ -1169,8 +1179,8 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Published, this, new PublishEventArgs(descendants, false, false), "Published"); } - if (culturesChanging != null && culturesChanging.Count > 0) - Audit(AuditType.Publish, userId, content.Id, $"Published culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); + if (culturesChanging != null) + Audit(AuditType.PublishVariant, userId, content.Id, $"Published cultures: {culturesChanging}", culturesChanging); else Audit(AuditType.Publish, userId, content.Id); @@ -1840,24 +1850,32 @@ namespace Umbraco.Core.Services.Implement return false; } + //track the cultures changing for auditing + var culturesChanging = content.ContentType.VariesByCulture() + ? string.Join(",", content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture)) + : null; + //TODO: Currently there's no way to change track which variant properties have changed, we only have change + // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. + // in this particular case, determining which cultures have changed works with the above with names since it will + // have always changed if it's been saved in the back office but that's not really fail safe. + //Save before raising event // fixme - nesting uow? - Save(content, userId); + var saveResult = Save(content, userId); - sendToPublishEventArgs.CanCancel = false; - scope.Events.Dispatch(SentToPublish, this, sendToPublishEventArgs); + if (saveResult.Success) + { + sendToPublishEventArgs.CanCancel = false; + scope.Events.Dispatch(SentToPublish, this, sendToPublishEventArgs); - var culturesChanging = content.ContentType.VariesByCulture() - ? content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() - : null; + if (culturesChanging != null) + Audit(AuditType.SendToPublishVariant, userId, content.Id, $"Send To Publish for cultures: {culturesChanging}", culturesChanging); + else + Audit(AuditType.SendToPublish, content.WriterId, content.Id); + } - if (culturesChanging != null && culturesChanging.Count > 0) - Audit(AuditType.SendToPublish, userId, content.Id, $"Send To Publish for culture{(culturesChanging.Count > 1 ? "s" : string.Empty)} {string.Join(",", culturesChanging)}"); - else - Audit(AuditType.SendToPublish, content.WriterId, content.Id); + return saveResult.Success; } - - return true; } /// @@ -2007,9 +2025,9 @@ namespace Umbraco.Core.Services.Implement #region Private Methods - private void Audit(AuditType type, int userId, int objectId, string message = null) + private void Audit(AuditType type, int userId, int objectId, string message = null, string parameters = null) { - _auditRepository.Save(new AuditItem(objectId, type, userId, Constants.ObjectTypes.Strings.Document, message)); + _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Document), message, parameters)); } #endregion diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js index b2e64983d6..f3dc6c7aa7 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbcontentnodeinfo.directive.js @@ -1,12 +1,13 @@ (function () { 'use strict'; - function ContentNodeInfoDirective($timeout, $location, logResource, eventsService, userService, localizationService, dateHelper, editorService, redirectUrlsResource) { + function ContentNodeInfoDirective($timeout, $routeParams, logResource, eventsService, userService, localizationService, dateHelper, editorService, redirectUrlsResource) { - function link(scope, element, attrs, ctrl) { + function link(scope, element, attrs, umbVariantContentCtrl) { var evts = []; var isInfoTab = false; + var auditTrailLoaded = false; var labels = {}; scope.publishStatus = []; @@ -63,10 +64,22 @@ if (scope.documentType !== null) { scope.previewOpenUrl = '#/settings/documenttypes/edit/' + scope.documentType.id; } + + //load in the audit trail if we are currently looking at the INFO tab + if (umbVariantContentCtrl) { + var activeApp = _.find(umbVariantContentCtrl.editor.content.apps, a => a.active); + if (activeApp.alias === "umbInfo") { + isInfoTab = true; + loadAuditTrail(); + loadRedirectUrls(); + } + } + } scope.auditTrailPageChange = function (pageNumber) { scope.auditTrailOptions.pageNumber = pageNumber; + auditTrailLoaded = false; loadAuditTrail(); }; @@ -103,6 +116,9 @@ function loadAuditTrail() { + //don't load this if it's already done + if (auditTrailLoaded) { return; }; + scope.loadingAuditTrail = true; logResource.getPagedEntityLog(scope.auditTrailOptions) @@ -124,6 +140,8 @@ setAuditTrailLogTypeColor(scope.auditTrail); scope.loadingAuditTrail = false; + + auditTrailLoaded = true; }); } @@ -230,7 +248,8 @@ if (!newValue) { return; } if (newValue === oldValue) { return; } - if(isInfoTab) { + if (isInfoTab) { + auditTrailLoaded = false; loadAuditTrail(); loadRedirectUrls(); setNodePublishStatus(scope.node); @@ -249,6 +268,7 @@ } var directive = { + require: '^^umbVariantContent', restrict: 'E', replace: true, templateUrl: 'views/components/content/umb-content-node-info.html', diff --git a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html index 03d4318ba2..bdc4ad6c7f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/content/umb-content-node-info.html @@ -46,10 +46,10 @@
-
- +
+ -
+
@@ -58,7 +58,7 @@
-
+
@@ -88,7 +88,7 @@ {{ item.logType }} - {{ item.comment }} + {{ item.comment }} @@ -119,14 +119,14 @@
- {{status.culture}}: + {{status.culture}}: {{status.label}}
- + {{node.createDateFormatted}} by {{ node.owner.name }} @@ -150,13 +150,13 @@ ng-change="updateTemplate(node.template)"> - + Open
- +
{{ node.id }}
{{ node.key }} diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml index 891e04d3a2..28376bf593 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml @@ -139,24 +139,28 @@ Viewing for - Delete Content performed by user - Unpublish performed by user - Save and Publish performed by user - Save Content performed by user - Move Content performed by user - Copy Content performed by user - Content rollback performed by user - Content Send To Publish performed by user - Content Send To Translation performed by user + Content deleted + Content unpublished + Content saved and Published + Content cultures %0% saved and published + Content saved + Content cultures %0% saved + Content moved + Content copied + Content rolled back + Content sent for publishing + Content cultures %0% sent for publishing Copy Publish + Publish Move Save + Save Delete Unpublish Rollback Send To Publish - Send To Translation + Send To Publish To change the document type for the selected content, first select from the list of valid types for this location. diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index f9138f3baf..f466ff699a 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -144,24 +144,28 @@ Viewing for - Delete Content performed by user - Unpublish performed by user - Save and Publish performed by user - Save Content performed by user - Move Content performed by user - Copy Content performed by user - Content rollback performed by user - Content Send To Publish performed by user - Content Send To Translation performed by user + Content deleted + Content unpublished + Content saved and Published + Content cultures %0% saved and published + Content saved + Content cultures %0% saved + Content moved + Content copied + Content rolled back + Content sent for publishing + Content cultures %0% sent for publishing Copy Publish + Publish Move Save + Save Delete Unpublish Rollback Send To Publish - Send To Translation + Send To Publish To change the document type for the selected content, first select from the list of valid types for this location. diff --git a/src/Umbraco.Web/Editors/LogController.cs b/src/Umbraco.Web/Editors/LogController.cs index 1205226b8f..dcd69d10b7 100644 --- a/src/Umbraco.Web/Editors/LogController.cs +++ b/src/Umbraco.Web/Editors/LogController.cs @@ -7,6 +7,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Mvc; +using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.Editors { @@ -16,6 +17,7 @@ namespace Umbraco.Web.Editors [PluginController("UmbracoApi")] public class LogController : UmbracoAuthorizedJsonController { + [UmbracoApplicationAuthorize(Core.Constants.Applications.Content, Core.Constants.Applications.Media)] public PagedResult GetPagedEntityLog(int id, int pageNumber = 1, int pageSize = 0, diff --git a/src/Umbraco.Web/Models/ContentEditing/AuditLog.cs b/src/Umbraco.Web/Models/ContentEditing/AuditLog.cs index 2e0dca3fbb..9074accdfe 100644 --- a/src/Umbraco.Web/Models/ContentEditing/AuditLog.cs +++ b/src/Umbraco.Web/Models/ContentEditing/AuditLog.cs @@ -24,7 +24,13 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "logType")] public string LogType { get; set; } + [DataMember(Name = "entityType")] + public string EntityType { get; set; } + [DataMember(Name = "comment")] public string Comment { get; set; } + + [DataMember(Name = "parameters")] + public string Parameters { get; set; } } } diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 709f0d719a..85d7128629 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -33,9 +33,9 @@ namespace Umbraco.Web.Security public WebSecurity(HttpContextBase httpContext, IUserService userService, IGlobalSettings globalSettings) { - _httpContext = httpContext; - _userService = userService; - _globalSettings = globalSettings; + _httpContext = httpContext ?? throw new ArgumentNullException(nameof(httpContext)); + _userService = userService ?? throw new ArgumentNullException(nameof(userService)); + _globalSettings = globalSettings ?? throw new ArgumentNullException(nameof(globalSettings)); } /// From 8dfb0dc35c705b46867c2a3b7d2ce43df778bbdb Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 13:25:06 +1100 Subject: [PATCH 08/16] more send to translate cleanup --- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 1 - .../Umbraco/dialogs/sendToTranslation.aspx | 36 -------- .../Trees/ContentTreeController.cs | 1 - src/Umbraco.Web/Umbraco.Web.csproj | 1 - .../_Legacy/Actions/ActionSendToTranslate.cs | 83 ------------------- 5 files changed, 122 deletions(-) delete mode 100644 src/Umbraco.Web.UI/Umbraco/dialogs/sendToTranslation.aspx delete mode 100644 src/Umbraco.Web/_Legacy/Actions/ActionSendToTranslate.cs diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index de52021220..391b0f43e1 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -349,7 +349,6 @@ - Designer diff --git a/src/Umbraco.Web.UI/Umbraco/dialogs/sendToTranslation.aspx b/src/Umbraco.Web.UI/Umbraco/dialogs/sendToTranslation.aspx deleted file mode 100644 index b40993e748..0000000000 --- a/src/Umbraco.Web.UI/Umbraco/dialogs/sendToTranslation.aspx +++ /dev/null @@ -1,36 +0,0 @@ -<%@ Page Language="C#" MasterPageFile="../masterpages/umbracoDialog.Master" AutoEventWireup="true" Codebehind="sendToTranslation.aspx.cs" Inherits="umbraco.presentation.dialogs.sendToTranslation" %> -<%@ Register TagPrefix="cc1" Namespace="Umbraco.Web._Legacy.Controls" Assembly="Umbraco.Web" %> - - - - - - - - - - - - - - - - - - - - - - - - - - -

- -  <%= Services.TextService.Localize("or") %>  <%=Services.TextService.Localize("cancel")%> -

- - diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 3a990a7741..e414e69607 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -245,7 +245,6 @@ namespace Umbraco.Web.Trees AddActionNode(item, menu, true, true); AddActionNode(item, menu, true); - AddActionNode(item, menu, convert: true); AddActionNode(item, menu, true); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index faf08849ce..dbdfce132a 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -555,7 +555,6 @@ - diff --git a/src/Umbraco.Web/_Legacy/Actions/ActionSendToTranslate.cs b/src/Umbraco.Web/_Legacy/Actions/ActionSendToTranslate.cs deleted file mode 100644 index e324d579e7..0000000000 --- a/src/Umbraco.Web/_Legacy/Actions/ActionSendToTranslate.cs +++ /dev/null @@ -1,83 +0,0 @@ -using System; -using Umbraco.Web.UI.Pages; -using Umbraco.Core; -using Umbraco.Core.CodeAnnotations; - -namespace Umbraco.Web._Legacy.Actions -{ - /// - /// This action is invoked when a send to translate request occurs - /// - [ActionMetadata(Constants.Conventions.PermissionCategories.ContentCategory)] - public class ActionSendToTranslate : IAction - { - //create singleton -#pragma warning disable 612,618 - private static readonly ActionSendToTranslate m_instance = new ActionSendToTranslate(); -#pragma warning restore 612,618 - - public static ActionSendToTranslate Instance - { - get { return m_instance; } - } - - #region IAction Members - - public char Letter - { - get - { - return '5'; - } - } - - public string JsFunctionName - { - get - { - return string.Format("{0}.actionSendToTranslate()", ClientTools.Scripts.GetAppActions); - } - } - - public string JsSource - { - get - { - return null; - } - } - - public string Alias - { - get - { - return "sendToTranslate"; - } - } - - public string Icon - { - get - { - return "chat"; - } - } - - public bool ShowInNotifier - { - get - { - return true; - } - } - public bool CanBePermissionAssigned - { - get - { - return true; - } - } - - #endregion - } -} From abeb4e04e56970a8c885a2031bed5d90fd635c57 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 15:41:28 +1100 Subject: [PATCH 09/16] Enhance implementation of ObservableDictionary so we can re-use this in more places, changes CultureNameCollection to ObservableDictionary, fixes issue with dirty tracking changed property type collections ... this was working by pure fluke before! Fixes more tests. --- .../Collections/IReadOnlyKeyedCollection.cs | 16 ---- .../Collections/ObservableDictionary.cs | 88 ++++++++++++++++-- src/Umbraco.Core/Models/Content.cs | 11 ++- src/Umbraco.Core/Models/ContentBase.cs | 8 +- src/Umbraco.Core/Models/ContentTypeBase.cs | 36 ++++---- .../Models/CultureNameCollection.cs | 91 +++---------------- src/Umbraco.Core/Models/IContent.cs | 2 +- src/Umbraco.Core/Models/IContentBase.cs | 2 +- .../Models/PropertyGroupCollection.cs | 2 + .../Models/PropertyTypeCollection.cs | 2 + .../Implement/ContentTypeRepositoryBase.cs | 15 +-- .../Implement/DocumentRepository.cs | 14 +-- .../Repositories/Implement/MacroRepository.cs | 6 +- .../Services/Implement/ContentService.cs | 6 +- .../Services/Implement/PackagingService.cs | 2 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 - .../Composing/TypeLoaderTests.cs | 2 +- src/Umbraco.Tests/Models/VariationTests.cs | 4 +- .../Repositories/MacroRepositoryTest.cs | 35 ++++--- .../Services/ContentServiceTests.cs | 4 +- .../Services/Importing/PackageImportTests.cs | 2 +- .../Services/MacroServiceTests.cs | 8 +- .../Web/TemplateUtilitiesTests.cs | 5 +- .../developer/Macros/EditMacro.aspx.cs | 8 +- .../Models/Mapping/MacroMapperProfile.cs | 2 +- .../NuCache/PublishedSnapshotService.cs | 2 +- src/Umbraco.Web/Security/WebSecurity.cs | 6 +- src/Umbraco.Web/umbraco.presentation/page.cs | 2 +- 28 files changed, 190 insertions(+), 192 deletions(-) delete mode 100644 src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs diff --git a/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs b/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs deleted file mode 100644 index 8d78241275..0000000000 --- a/src/Umbraco.Core/Collections/IReadOnlyKeyedCollection.cs +++ /dev/null @@ -1,16 +0,0 @@ -using System.Collections.Generic; - -namespace Umbraco.Core.Collections -{ - /// - /// A readonly keyed collection - /// - /// - public interface IReadOnlyKeyedCollection : IReadOnlyList - { - IEnumerable Keys { get; } - bool TryGetValue(TKey key, out TVal val); - TVal this[string key] { get; } - bool Contains(TKey key); - } -} diff --git a/src/Umbraco.Core/Collections/ObservableDictionary.cs b/src/Umbraco.Core/Collections/ObservableDictionary.cs index caa2be92a8..ded87c30a6 100644 --- a/src/Umbraco.Core/Collections/ObservableDictionary.cs +++ b/src/Umbraco.Core/Collections/ObservableDictionary.cs @@ -14,20 +14,21 @@ namespace Umbraco.Core.Collections /// /// The type of elements contained in the BindableCollection /// The type of the indexing key - public class ObservableDictionary : ObservableCollection + public class ObservableDictionary : ObservableCollection, IReadOnlyDictionary, IDictionary { - protected Dictionary Indecies = new Dictionary(); - protected Func KeySelector; + protected Dictionary Indecies { get; } + protected Func KeySelector { get; } /// /// Create new ObservableDictionary /// /// Selector function to create key from value - public ObservableDictionary(Func keySelector) + public ObservableDictionary(Func keySelector, IEqualityComparer equalityComparer = null) : base() { if (keySelector == null) throw new ArgumentException("keySelector"); KeySelector = keySelector; + Indecies = new Dictionary(equalityComparer); } #region Protected Methods @@ -73,7 +74,7 @@ namespace Umbraco.Core.Collections } #endregion - public virtual bool ContainsKey(TKey key) + public bool ContainsKey(TKey key) { return Indecies.ContainsKey(key); } @@ -83,7 +84,7 @@ namespace Umbraco.Core.Collections /// /// Key of element to replace /// - public virtual TValue this[TKey key] + public TValue this[TKey key] { get { return this[Indecies[key]]; } @@ -112,7 +113,7 @@ namespace Umbraco.Core.Collections /// /// /// False if key not found - public virtual bool Replace(TKey key, TValue value) + public bool Replace(TKey key, TValue value) { if (!Indecies.ContainsKey(key)) return false; //confirm key matches @@ -124,7 +125,7 @@ namespace Umbraco.Core.Collections } - public virtual bool Remove(TKey key) + public bool Remove(TKey key) { if (!Indecies.ContainsKey(key)) return false; @@ -138,7 +139,7 @@ namespace Umbraco.Core.Collections ///
/// /// - public virtual void ChangeKey(TKey currentKey, TKey newKey) + public void ChangeKey(TKey currentKey, TKey newKey) { if (!Indecies.ContainsKey(currentKey)) { @@ -155,6 +156,75 @@ namespace Umbraco.Core.Collections Indecies.Add(newKey, currentIndex); } + #region IDictionary and IReadOnlyDictionary implementation + + public bool TryGetValue(TKey key, out TValue val) + { + if (Indecies.TryGetValue(key, out var index)) + { + val = this[index]; + return true; + } + val = default; + return false; + } + + /// + /// Returns all keys + /// + public IEnumerable Keys => Indecies.Keys; + + /// + /// Returns all values + /// + public IEnumerable Values => base.Items; + + ICollection IDictionary.Keys => Indecies.Keys; + + //this will never be used + ICollection IDictionary.Values => Values.ToList(); + + bool ICollection>.IsReadOnly + { + get { return false; } + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + foreach (var i in Values) + { + var key = KeySelector(i); + yield return new KeyValuePair(key, i); + } + } + + void IDictionary.Add(TKey key, TValue value) + { + Add(value); + } + + void ICollection>.Add(KeyValuePair item) + { + Add(item.Value); + } + + bool ICollection>.Contains(KeyValuePair item) + { + return ContainsKey(item.Key); + } + + void ICollection>.CopyTo(KeyValuePair[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + bool ICollection>.Remove(KeyValuePair item) + { + return Remove(item.Key); + } + + #endregion + internal class DuplicateKeyException : Exception { diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index abe28522ba..25a30da6db 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -89,7 +89,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo PublishedSelector = ExpressionHelper.GetPropertyInfo(x => x.Published); public readonly PropertyInfo ReleaseDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ReleaseDate); public readonly PropertyInfo ExpireDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ExpireDate); - public readonly PropertyInfo PublishNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.PublishNames); + public readonly PropertyInfo PublishNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.PublishNames); } /// @@ -224,13 +224,13 @@ namespace Umbraco.Core.Models public bool IsCulturePublished(string culture) // just check _publishInfos // a non-available culture could not become published anyways - => _publishInfos != null && _publishInfos.Contains(culture); + => _publishInfos != null && _publishInfos.ContainsKey(culture); /// public bool WasCulturePublished(string culture) // just check _publishInfosOrig - a copy of _publishInfos // a non-available culture could not become published anyways - => _publishInfosOrig != null && _publishInfosOrig.Contains(culture); + => _publishInfosOrig != null && _publishInfosOrig.ContainsKey(culture); // adjust dates to sync between version, cultures etc // used by the repo when persisting @@ -260,7 +260,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IReadOnlyKeyedCollection PublishNames => _publishInfos ?? NoNames; + public IReadOnlyDictionary PublishNames => _publishInfos ?? NoNames; /// public string GetPublishName(string culture) @@ -508,6 +508,9 @@ namespace Umbraco.Core.Models //turn off change tracking clone.DisableChangeTracking(); + //need to manually clone this since it's not settable + clone._contentType = (IContentType)ContentType.DeepClone(); + //if culture infos exist then deal with event bindings if (clone._publishInfos != null) { diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 1c5ee18be6..a288f7fac9 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -70,7 +70,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo DefaultContentTypeIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ContentTypeId); public readonly PropertyInfo PropertyCollectionSelector = ExpressionHelper.GetPropertyInfo(x => x.Properties); public readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); - public readonly PropertyInfo CultureNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); + public readonly PropertyInfo CultureNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); } protected void PropertiesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -156,11 +156,11 @@ namespace Umbraco.Core.Models /// public bool IsCultureAvailable(string culture) - => _cultureInfos != null && _cultureInfos.Contains(culture); + => _cultureInfos != null && _cultureInfos.ContainsKey(culture); /// [DataMember] - public virtual IReadOnlyKeyedCollection CultureNames => _cultureInfos ?? NoNames; + public virtual IReadOnlyDictionary CultureNames => _cultureInfos ?? NoNames; /// public string GetCultureName(string culture) @@ -373,7 +373,7 @@ namespace Umbraco.Core.Models foreach (var (otherCulture, otherName) in other.CultureNames) { if (culture == "*" || culture == otherCulture) - SetCultureName(otherName, otherCulture); + SetCultureName(otherName.Name, otherCulture); } } diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 2a57420e9e..0a86f4cf09 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -28,7 +28,7 @@ namespace Umbraco.Core.Models private bool _allowedAsRoot; // note: only one that's not 'pure element type' private bool _isContainer; private PropertyGroupCollection _propertyGroups; - private PropertyTypeCollection _propertyTypes; + private PropertyTypeCollection _noGroupPropertyTypes; private IEnumerable _allowedContentTypes; private bool _hasPropertyTypeBeenRemoved; private ContentVariation _variations; @@ -43,8 +43,8 @@ namespace Umbraco.Core.Models // actually OK as IsPublishing is constant // ReSharper disable once VirtualMemberCallInConstructor - _propertyTypes = new PropertyTypeCollection(IsPublishing); - _propertyTypes.CollectionChanged += PropertyTypesChanged; + _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing); + _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; _variations = ContentVariation.Nothing; } @@ -64,8 +64,8 @@ namespace Umbraco.Core.Models // actually OK as IsPublishing is constant // ReSharper disable once VirtualMemberCallInConstructor - _propertyTypes = new PropertyTypeCollection(IsPublishing); - _propertyTypes.CollectionChanged += PropertyTypesChanged; + _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing); + _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; _variations = ContentVariation.Nothing; } @@ -248,7 +248,7 @@ namespace Umbraco.Core.Models { get { - return _propertyTypes.Union(PropertyGroups.SelectMany(x => x.PropertyTypes)); + return _noGroupPropertyTypes.Union(PropertyGroups.SelectMany(x => x.PropertyTypes)); } } @@ -261,12 +261,12 @@ namespace Umbraco.Core.Models [DoNotClone] public IEnumerable NoGroupPropertyTypes { - get => _propertyTypes; + get => _noGroupPropertyTypes; set { - _propertyTypes = new PropertyTypeCollection(IsPublishing, value); - _propertyTypes.CollectionChanged += PropertyTypesChanged; - PropertyTypesChanged(_propertyTypes, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + _noGroupPropertyTypes = new PropertyTypeCollection(IsPublishing, value); + _noGroupPropertyTypes.CollectionChanged += PropertyTypesChanged; + PropertyTypesChanged(_noGroupPropertyTypes, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } } @@ -320,7 +320,7 @@ namespace Umbraco.Core.Models { if (PropertyTypeExists(propertyType.Alias) == false) { - _propertyTypes.Add(propertyType); + _noGroupPropertyTypes.Add(propertyType); return true; } @@ -384,7 +384,7 @@ namespace Umbraco.Core.Models } //check through each local property type collection (not assigned to a tab) - if (_propertyTypes.RemoveItem(propertyTypeAlias)) + if (_noGroupPropertyTypes.RemoveItem(propertyTypeAlias)) { if (!HasPropertyTypeBeenRemoved) { @@ -408,7 +408,7 @@ namespace Umbraco.Core.Models foreach (var property in group.PropertyTypes) { property.PropertyGroupId = null; - _propertyTypes.Add(property); + _noGroupPropertyTypes.Add(property); } // actually remove the group @@ -421,7 +421,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] //fixme should we mark this as EditorBrowsable hidden since it really isn't ever used? - internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; + internal PropertyTypeCollection PropertyTypeCollection => _noGroupPropertyTypes; /// /// Indicates whether the current entity is dirty. @@ -474,15 +474,15 @@ namespace Umbraco.Core.Models //turn off change tracking clone.DisableChangeTracking(); - if (clone._propertyTypes != null) + if (clone._noGroupPropertyTypes != null) { //need to manually wire up the event handlers for the property type collections - we've ensured // its ignored from the auto-clone process because its return values are unions, not raw and // we end up with duplicates, see: http://issues.umbraco.org/issue/U4-4842 - clone._propertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any - clone._propertyTypes = (PropertyTypeCollection)_propertyTypes.DeepClone(); //manually deep clone - clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + clone._noGroupPropertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any + clone._noGroupPropertyTypes = (PropertyTypeCollection)_noGroupPropertyTypes.DeepClone(); //manually deep clone + clone._noGroupPropertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler } if (clone._propertyGroups != null) diff --git a/src/Umbraco.Core/Models/CultureNameCollection.cs b/src/Umbraco.Core/Models/CultureNameCollection.cs index be6540c399..406abc7c0f 100644 --- a/src/Umbraco.Core/Models/CultureNameCollection.cs +++ b/src/Umbraco.Core/Models/CultureNameCollection.cs @@ -12,13 +12,14 @@ namespace Umbraco.Core.Models /// /// The culture names of a content's variants /// - public class CultureNameCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable, IReadOnlyKeyedCollection + public class CultureNameCollection : ObservableDictionary, IDeepCloneable { /// /// Creates a new collection from another collection /// /// - public CultureNameCollection(IEnumerable names) : base(StringComparer.InvariantCultureIgnoreCase) + public CultureNameCollection(IEnumerable names) + : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) { foreach (var n in names) Add(n); @@ -27,21 +28,11 @@ namespace Umbraco.Core.Models /// /// Creates a new collection /// - public CultureNameCollection() : base(StringComparer.InvariantCultureIgnoreCase) + public CultureNameCollection() + : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) { } - - /// - /// Returns all keys in the collection - /// - public IEnumerable Keys => Dictionary != null ? Dictionary.Keys : this.Select(x => x.Culture); - - public bool TryGetValue(string culture, out CultureName name) - { - name = this.FirstOrDefault(x => x.Culture.InvariantEquals(culture)); - return name != null; - } - + /// /// Add or update the /// @@ -63,78 +54,20 @@ namespace Umbraco.Core.Models }); } - /// - /// Gets the index for a specified culture - /// - public int IndexOfKey(string key) - { - for (var i = 0; i < Count; i++) - { - if (this[i].Culture.InvariantEquals(key)) - return i; - } - return -1; - } - - public event NotifyCollectionChangedEventHandler CollectionChanged; - - protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs args) - { - CollectionChanged?.Invoke(this, args); - } - public object DeepClone() { var clone = new CultureNameCollection(); foreach (var name in this) { - clone.Add((CultureName)name.DeepClone()); + name.DisableChangeTracking(); + var copy = (CultureName)name.DeepClone(); + copy.ResetDirtyProperties(false); + clone.Add(copy); + name.EnableChangeTracking(); } return clone; } - - protected override string GetKeyForItem(CultureName item) - { - return item.Culture; - } - - /// - /// Resets the collection to only contain the instances referenced in the parameter. - /// - /// The property groups. - /// - internal void Reset(IEnumerable names) - { - Clear(); - foreach (var name in names) - Add(name); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); - } - - protected override void SetItem(int index, CultureName item) - { - base.SetItem(index, item); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); - } - - protected override void RemoveItem(int index) - { - var removed = this[index]; - base.RemoveItem(index); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removed)); - } - - protected override void InsertItem(int index, CultureName item) - { - base.InsertItem(index, item); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); - } - - protected override void ClearItems() - { - base.ClearItems(); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); - } + } } diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 3ddffe8f75..125c1a0f55 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -133,7 +133,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get via the property. /// - IReadOnlyKeyedCollection PublishNames { get; } + IReadOnlyDictionary PublishNames { get; } /// /// Gets the published cultures. diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index 811cbf74f3..c84c768e9c 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -58,7 +58,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot contain the invariant /// culture name, which must be get or set via the property. /// - IReadOnlyKeyedCollection CultureNames { get; } + IReadOnlyDictionary CultureNames { get; } /// /// Gets the available cultures. diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 80b663fa05..c5768c66db 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -14,10 +14,12 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract] + //TODO: Change this to ObservableDictionary so we can reduce the INotifyCollectionChanged implementation details public class PropertyGroupCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable { private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); + //fixme: this doesn't seem to be used anywhere internal Action OnAdd; internal PropertyGroupCollection() diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index 47710e04cb..6053a6a5bf 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -13,11 +13,13 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract] + //TODO: Change this to ObservableDictionary so we can reduce the INotifyCollectionChanged implementation details public class PropertyTypeCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable { [IgnoreDataMember] private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); + //fixme: This doesn't seem to be used [IgnoreDataMember] internal Action OnAdd; diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 3f1ea3116e..bea6eb9bce 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -326,9 +326,11 @@ AND umbracoNode.id <> @id", }); } - // delete property types - // ... by excepting entries from db with entries from collections - if (entity.IsPropertyDirty("PropertyTypes") || entity.PropertyTypes.Any(x => x.IsDirty())) + // Delete property types ... by excepting entries from db with entries from collections. + // We check if the entity's own PropertyTypes has been modified and then also check + // any of the property groups PropertyTypes has been modified. + // This specifically tells us if any property type collections have changed. + if (entity.IsPropertyDirty("PropertyTypes") || entity.PropertyGroups.Any(x => x.IsPropertyDirty("PropertyTypes"))) { var dbPropertyTypes = Database.Fetch("WHERE contentTypeId = @Id", new { entity.Id }); var dbPropertyTypeAlias = dbPropertyTypes.Select(x => x.Id); @@ -338,10 +340,11 @@ AND umbracoNode.id <> @id", DeletePropertyType(entity.Id, item); } - // delete tabs - // ... by excepting entries from db with entries from collections + // Delete tabs ... by excepting entries from db with entries from collections. + // We check if the entity's own PropertyGroups has been modified. + // This specifically tells us if the property group collections have changed. List orphanPropertyTypeIds = null; - if (entity.IsPropertyDirty("PropertyGroups") || entity.PropertyGroups.Any(x => x.IsDirty())) + if (entity.IsPropertyDirty("PropertyGroups")) { // todo // we used to try to propagate tabs renaming downstream, relying on ParentId, but diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 5cbc987ad0..1659ca6427 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -359,7 +359,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // names also impact 'edited' foreach (var (culture, name) in content.CultureNames) - if (name != content.GetPublishName(culture)) + if (name.Name != content.GetPublishName(culture)) (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); // insert content variations @@ -521,7 +521,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // names also impact 'edited' foreach (var (culture, name) in content.CultureNames) - if (name != content.GetPublishName(culture)) + if (name.Name != content.GetPublishName(culture)) { edited = true; (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); @@ -1120,7 +1120,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement VersionId = content.VersionId, LanguageId = LanguageRepository.GetIdByIsoCode(culture) ?? throw new InvalidOperationException("Not a valid culture."), Culture = culture, - Name = name, + Name = name.Name, UpdateDate = content.GetUpdateDate(culture) ?? DateTime.MinValue // we *know* there is a value }; @@ -1135,7 +1135,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement VersionId = content.PublishedVersionId, LanguageId = LanguageRepository.GetIdByIsoCode(culture) ?? throw new InvalidOperationException("Not a valid culture."), Culture = culture, - Name = name, + Name = name.Name, UpdateDate = content.GetPublishDate(culture) ?? DateTime.MinValue // we *know* there is a value }; } @@ -1210,7 +1210,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var defaultCulture = LanguageRepository.GetDefaultIsoCode(); content.Name = defaultCulture != null && content.CultureNames.TryGetValue(defaultCulture, out var cultureName) ? cultureName.Name - : content.CultureNames[0].Name; + : content.CultureNames.First().Value.Name; } else { @@ -1265,13 +1265,13 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // get a unique name var otherNames = cultureNames.Select(x => new SimilarNodeName { Id = x.Id, Name = x.Name }); - var uniqueName = SimilarNodeName.GetUniqueName(otherNames, 0, name); + var uniqueName = SimilarNodeName.GetUniqueName(otherNames, 0, name.Name); if (uniqueName == content.GetCultureName(culture)) continue; // update the name, and the publish name if published content.SetCultureName(uniqueName, culture); - if (publishing && content.PublishNames.Contains(culture)) + if (publishing && content.PublishNames.ContainsKey(culture)) content.SetPublishInfo(culture, uniqueName, DateTime.Now); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs index 546be0b4a8..594f26fa72 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs @@ -160,7 +160,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement //update the properties if they've changed var macro = (Macro)entity; - if (macro.IsPropertyDirty("Properties") || macro.Properties.Any(x => x.IsDirty())) + if (macro.IsPropertyDirty("Properties") || macro.Properties.Values.Any(x => x.IsDirty())) { var ids = dto.MacroPropertyDtos.Where(x => x.Id > 0).Select(x => x.Id).ToArray(); if (ids.Length > 0) @@ -173,7 +173,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var aliases = new Dictionary(); foreach (var propDto in dto.MacroPropertyDtos) { - var prop = macro.Properties.FirstOrDefault(x => x.Id == propDto.Id); + var prop = macro.Properties.Values.FirstOrDefault(x => x.Id == propDto.Id); if (prop == null) throw new Exception("oops: property."); if (propDto.Id == 0 || prop.IsPropertyDirty("Alias")) { @@ -195,7 +195,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement else { // update - var property = macro.Properties.FirstOrDefault(x => x.Id == propDto.Id); + var property = macro.Properties.Values.FirstOrDefault(x => x.Id == propDto.Id); if (property == null) throw new Exception("oops: property."); if (property.IsDirty()) Database.Update(propDto); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 9deadfa5af..a4fb535f04 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -836,7 +836,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures that have changed var culturesChanging = content.ContentType.VariesByCulture() - ? string.Join(",", content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture)) + ? string.Join(",", content.CultureNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)) : null; //TODO: Currently there's no way to change track which variant properties have changed, we only have change // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. @@ -1069,7 +1069,7 @@ namespace Umbraco.Core.Services.Implement } else { - culturesChanging = string.Join(",", content.PublishNames.Where(x => x.IsDirty()).Select(x => x.Culture)); + culturesChanging = string.Join(",", content.PublishNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)); } } @@ -1852,7 +1852,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures changing for auditing var culturesChanging = content.ContentType.VariesByCulture() - ? string.Join(",", content.CultureNames.Where(x => x.IsDirty()).Select(x => x.Culture)) + ? string.Join(",", content.CultureNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)) : null; //TODO: Currently there's no way to change track which variant properties have changed, we only have change // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. diff --git a/src/Umbraco.Core/Services/Implement/PackagingService.cs b/src/Umbraco.Core/Services/Implement/PackagingService.cs index c0e8f80337..fff865e097 100644 --- a/src/Umbraco.Core/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Core/Services/Implement/PackagingService.cs @@ -1318,7 +1318,7 @@ namespace Umbraco.Core.Services.Implement sortOrder = int.Parse(sortOrderAttribute.Value); } - if (macro.Properties.Any(x => string.Equals(x.Alias, propertyAlias, StringComparison.OrdinalIgnoreCase))) continue; + if (macro.Properties.Values.Any(x => string.Equals(x.Alias, propertyAlias, StringComparison.OrdinalIgnoreCase))) continue; macro.Properties.Add(new MacroProperty(propertyAlias, propertyName, sortOrder, editorAlias)); sortOrder++; } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 8990b6d326..bc6c8fee54 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -109,7 +109,6 @@ - diff --git a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs index 9b23ec3d6b..a7b3d0f446 100644 --- a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs +++ b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs @@ -272,7 +272,7 @@ AnotherContentFinder public void Resolves_Actions() { var actions = _typeLoader.GetActions(); - Assert.AreEqual(34, actions.Count()); + Assert.AreEqual(33, actions.Count()); } [Test] diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 0e62c41f46..8a996f9782 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -237,9 +237,9 @@ namespace Umbraco.Tests.Models // variant dictionary of names work Assert.AreEqual(2, content.CultureNames.Count); - Assert.IsTrue(content.CultureNames.Contains(langFr)); + Assert.IsTrue(content.CultureNames.ContainsKey(langFr)); Assert.AreEqual("name-fr", content.CultureNames[langFr].Name); - Assert.IsTrue(content.CultureNames.Contains(langUk)); + Assert.IsTrue(content.CultureNames.ContainsKey(langUk)); Assert.AreEqual("name-uk", content.CultureNames[langUk].Name); } diff --git a/src/Umbraco.Tests/Persistence/Repositories/MacroRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MacroRepositoryTest.cs index a3b9035c8d..5ae25d629f 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MacroRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MacroRepositoryTest.cs @@ -175,7 +175,7 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert Assert.That(macro.HasIdentity, Is.True); Assert.That(macro.Id, Is.EqualTo(4));//With 3 existing entries the Id should be 4 - Assert.Greater(macro.Properties.Single().Id, 0); + Assert.Greater(macro.Properties.Values.Single().Id, 0); } } @@ -268,15 +268,14 @@ namespace Umbraco.Tests.Persistence.Repositories repository.Save(macro); - // Assert - Assert.Greater(macro.Properties.First().Id, 0); //ensure id is returned + Assert.Greater(macro.Properties.Values.First().Id, 0); //ensure id is returned var result = repository.Get(1); - Assert.Greater(result.Properties.First().Id, 0); - Assert.AreEqual(1, result.Properties.Count()); - Assert.AreEqual("new1", result.Properties.First().Alias); - Assert.AreEqual("New1", result.Properties.First().Name); - Assert.AreEqual(3, result.Properties.First().SortOrder); + Assert.Greater(result.Properties.Values.First().Id, 0); + Assert.AreEqual(1, result.Properties.Values.Count()); + Assert.AreEqual("new1", result.Properties.Values.First().Alias); + Assert.AreEqual("New1", result.Properties.Values.First().Name); + Assert.AreEqual(3, result.Properties.Values.First().SortOrder); } } @@ -298,10 +297,10 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert var result = repository.Get(macro.Id); - Assert.AreEqual(1, result.Properties.Count()); - Assert.AreEqual("blah1", result.Properties.First().Alias); - Assert.AreEqual("New1", result.Properties.First().Name); - Assert.AreEqual(4, result.Properties.First().SortOrder); + Assert.AreEqual(1, result.Properties.Values.Count()); + Assert.AreEqual("blah1", result.Properties.Values.First().Alias); + Assert.AreEqual("New1", result.Properties.Values.First().Name); + Assert.AreEqual(4, result.Properties.Values.First().SortOrder); } } @@ -325,7 +324,7 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert result = repository.Get(macro.Id); - Assert.AreEqual(0, result.Properties.Count()); + Assert.AreEqual(0, result.Properties.Values.Count()); } } @@ -355,8 +354,8 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert var result = repository.Get(macro.Id); - Assert.AreEqual(1, result.Properties.Count()); - Assert.AreEqual("blah2", result.Properties.Single().Alias); + Assert.AreEqual(1, result.Properties.Values.Count()); + Assert.AreEqual("blah2", result.Properties.Values.Single().Alias); } } @@ -382,8 +381,8 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert var result = repository.Get(1); - Assert.AreEqual("new1", result.Properties.First().Alias); - Assert.AreEqual("this is a new name", result.Properties.First().Name); + Assert.AreEqual("new1", result.Properties.Values.First().Alias); + Assert.AreEqual("this is a new name", result.Properties.Values.First().Name); } } @@ -408,7 +407,7 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert var result = repository.Get(1); - Assert.AreEqual("newAlias", result.Properties.First().Alias); + Assert.AreEqual("newAlias", result.Properties.Values.First().Alias); } } diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index d5660e708f..99aa9b788a 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1308,7 +1308,7 @@ namespace Umbraco.Tests.Services var published = ServiceContext.ContentService.SavePublishing(content); //audit log will only show that french was published var lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); - Assert.AreEqual($"Published culture fr-fr", lastLog.Comment); + Assert.AreEqual($"Published cultures: fr-fr", lastLog.Comment); //re-get content = ServiceContext.ContentService.GetById(content.Id); @@ -1317,7 +1317,7 @@ namespace Umbraco.Tests.Services published = ServiceContext.ContentService.SavePublishing(content); //audit log will only show that english was published lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); - Assert.AreEqual($"Published culture en-uk", lastLog.Comment); + Assert.AreEqual($"Published cultures: en-uk", lastLog.Comment); } [Test] diff --git a/src/Umbraco.Tests/Services/Importing/PackageImportTests.cs b/src/Umbraco.Tests/Services/Importing/PackageImportTests.cs index 767ffd4fc2..b33ff83c4a 100644 --- a/src/Umbraco.Tests/Services/Importing/PackageImportTests.cs +++ b/src/Umbraco.Tests/Services/Importing/PackageImportTests.cs @@ -628,7 +628,7 @@ namespace Umbraco.Tests.Services.Importing // Assert Assert.That(macros.Any(), Is.True); - Assert.That(macros.First().Properties.Any(), Is.True); + Assert.That(macros.First().Properties.Values.Any(), Is.True); var allMacros = ServiceContext.MacroService.GetAll().ToList(); foreach (var macro in macros) diff --git a/src/Umbraco.Tests/Services/MacroServiceTests.cs b/src/Umbraco.Tests/Services/MacroServiceTests.cs index fa86f4baab..6539a37114 100644 --- a/src/Umbraco.Tests/Services/MacroServiceTests.cs +++ b/src/Umbraco.Tests/Services/MacroServiceTests.cs @@ -195,7 +195,7 @@ namespace Umbraco.Tests.Services macro.Properties["blah1"].EditorAlias = "new"; macro.Properties.Remove("blah3"); - var allPropKeys = macro.Properties.Select(x => new { x.Alias, x.Key }).ToArray(); + var allPropKeys = macro.Properties.Values.Select(x => new { x.Alias, x.Key }).ToArray(); macroService.Save(macro); @@ -228,10 +228,10 @@ namespace Umbraco.Tests.Services macroService.Save(macro); var result1 = macroService.GetById(macro.Id); - Assert.AreEqual(4, result1.Properties.Count()); + Assert.AreEqual(4, result1.Properties.Values.Count()); //simulate clearing the sections - foreach (var s in result1.Properties.ToArray()) + foreach (var s in result1.Properties.Values.ToArray()) { result1.Properties.Remove(s.Alias); } @@ -244,7 +244,7 @@ namespace Umbraco.Tests.Services //re-get result1 = macroService.GetById(result1.Id); - Assert.AreEqual(2, result1.Properties.Count()); + Assert.AreEqual(2, result1.Properties.Values.Count()); } diff --git a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs index df21739c0b..f05e9525c2 100644 --- a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs +++ b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs @@ -22,6 +22,7 @@ using Umbraco.Web.Security; using Umbraco.Web.Templates; using System.Linq; using Umbraco.Core.Services; +using Umbraco.Core.Configuration; namespace Umbraco.Tests.Web { @@ -49,6 +50,8 @@ namespace Umbraco.Tests.Web Umbraco.Web.Composing.Current.UmbracoContextAccessor = new TestUmbracoContextAccessor(); Udi.ResetUdiTypes(); + + UmbracoConfig.For.SetUmbracoSettings(SettingsForTests.GetDefaultUmbracoSettings()); } [TearDown] @@ -90,7 +93,7 @@ namespace Umbraco.Tests.Web .Returns((UmbracoContext umbCtx, IPublishedContent content, UrlProviderMode mode, string culture, Uri url) => "/my-test-url"); var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); - + var contentType = new PublishedContentType(666, "alias", PublishedItemType.Content, Enumerable.Empty(), Enumerable.Empty(), ContentVariation.Nothing); var publishedContent = Mock.Of(); Mock.Get(publishedContent).Setup(x => x.Id).Returns(1234); diff --git a/src/Umbraco.Web.UI/Umbraco/developer/Macros/EditMacro.aspx.cs b/src/Umbraco.Web.UI/Umbraco/developer/Macros/EditMacro.aspx.cs index 96433be9cc..f2d2d5dcd3 100644 --- a/src/Umbraco.Web.UI/Umbraco/developer/Macros/EditMacro.aspx.cs +++ b/src/Umbraco.Web.UI/Umbraco/developer/Macros/EditMacro.aspx.cs @@ -94,7 +94,7 @@ namespace Umbraco.Web.UI.Umbraco.Developer.Macros { var macroPropertyId = (HtmlInputHidden)((Control)sender).Parent.FindControl("macroPropertyID"); - var property = _macro.Properties.Single(x => x.Id == int.Parse(macroPropertyId.Value)); + var property = _macro.Properties.Values.Single(x => x.Id == int.Parse(macroPropertyId.Value)); _macro.Properties.Remove(property); Services.MacroService.Save(_macro); @@ -104,7 +104,7 @@ namespace Umbraco.Web.UI.Umbraco.Developer.Macros public void MacroPropertyBind() { - macroProperties.DataSource = _macro.Properties.OrderBy(x => x.SortOrder); + macroProperties.DataSource = _macro.Properties.Values.OrderBy(x => x.SortOrder); macroProperties.DataBind(); } @@ -152,7 +152,7 @@ namespace Umbraco.Web.UI.Umbraco.Developer.Macros _macro.Properties.Add(new MacroProperty( macroPropertyAliasNew.Text.Trim(), macroPropertyNameNew.Text.Trim(), - _macro.Properties.Any() ? _macro.Properties.Max(x => x.SortOrder) + 1 : 0, + _macro.Properties.Values.Any() ? _macro.Properties.Values.Max(x => x.SortOrder) + 1 : 0, macroPropertyTypeNew.SelectedValue)); Services.MacroService.Save(_macro); @@ -246,7 +246,7 @@ namespace Umbraco.Web.UI.Umbraco.Developer.Macros var macroElementSortOrder = (TextBox)item.FindControl("macroPropertySortOrder"); var macroElementType = (DropDownList)item.FindControl("macroPropertyType"); - var prop = _macro.Properties.Single(x => x.Id == int.Parse(macroPropertyId.Value)); + var prop = _macro.Properties.Values.Single(x => x.Id == int.Parse(macroPropertyId.Value)); var sortOrder = 0; int.TryParse(macroElementSortOrder.Text, out sortOrder); diff --git a/src/Umbraco.Web/Models/Mapping/MacroMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/MacroMapperProfile.cs index 9977b1cfb1..7bede52021 100644 --- a/src/Umbraco.Web/Models/Mapping/MacroMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/MacroMapperProfile.cs @@ -26,7 +26,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(dto => dto.AdditionalData, expression => expression.Ignore()); CreateMap>() - .ConvertUsing(macro => macro.Properties.Select(Mapper.Map).ToList()); + .ConvertUsing(macro => macro.Properties.Values.Select(Mapper.Map).ToList()); CreateMap() .ForMember(x => x.View, expression => expression.Ignore()) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index a9903669b9..355a4e7644 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -1199,7 +1199,7 @@ namespace Umbraco.Web.PublishedCache.NuCache foreach (var (culture, name) in names) { - cultureData[culture] = new CultureVariation { Name = name, Date = content.GetUpdateDate(culture) ?? DateTime.MinValue }; + cultureData[culture] = new CultureVariation { Name = name.Name, Date = content.GetUpdateDate(culture) ?? DateTime.MinValue }; } //the dictionary that will be serialized diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 85d7128629..709f0d719a 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -33,9 +33,9 @@ namespace Umbraco.Web.Security public WebSecurity(HttpContextBase httpContext, IUserService userService, IGlobalSettings globalSettings) { - _httpContext = httpContext ?? throw new ArgumentNullException(nameof(httpContext)); - _userService = userService ?? throw new ArgumentNullException(nameof(userService)); - _globalSettings = globalSettings ?? throw new ArgumentNullException(nameof(globalSettings)); + _httpContext = httpContext; + _userService = userService; + _globalSettings = globalSettings; } /// diff --git a/src/Umbraco.Web/umbraco.presentation/page.cs b/src/Umbraco.Web/umbraco.presentation/page.cs index 91cf690996..ea1a563f9c 100644 --- a/src/Umbraco.Web/umbraco.presentation/page.cs +++ b/src/Umbraco.Web/umbraco.presentation/page.cs @@ -396,7 +396,7 @@ namespace umbraco return _cultureInfos; return _cultureInfos = _inner.PublishNames - .ToDictionary(x => x.Culture, x => new PublishedCultureInfo(x.Culture, x.Name, x.Date)); + .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value.Name, x.Value.Date)); } } From 239112a1de0b1a98df56b5cb84138c959e5addc6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 19 Oct 2018 15:45:05 +1100 Subject: [PATCH 10/16] notes --- src/Umbraco.Core/Models/Property.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index bb922a740b..8a97dc2cfc 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -55,6 +55,9 @@ namespace Umbraco.Core.Models /// public class PropertyValue { + //TODO: Either we allow change tracking at this class level, or we add some special change tracking collections to the Property + // class to deal with change tracking which variants have changed + private string _culture; private string _segment; @@ -100,6 +103,7 @@ namespace Umbraco.Core.Models // ReSharper disable once ClassNeverInstantiated.Local private class PropertySelectors { + //TODO: This allows us to track changes for an entire Property, but doesn't allow us to track changes at the variant level public readonly PropertyInfo ValuesSelector = ExpressionHelper.GetPropertyInfo(x => x.Values); public readonly DelegateEqualityComparer PropertyValueComparer = new DelegateEqualityComparer( From 127ecdf48ef53b3f7b181c1893ac8c08086279c9 Mon Sep 17 00:00:00 2001 From: elitsa Date: Tue, 23 Oct 2018 09:34:02 +0200 Subject: [PATCH 11/16] Right arrow appears only when there are child nodes. --- src/Umbraco.Web/Trees/MacrosTreeController.cs | 9 +++++++++ src/Umbraco.Web/Trees/MemberGroupTreeController.cs | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/src/Umbraco.Web/Trees/MacrosTreeController.cs b/src/Umbraco.Web/Trees/MacrosTreeController.cs index 66f92ffdc0..11197fffb5 100644 --- a/src/Umbraco.Web/Trees/MacrosTreeController.cs +++ b/src/Umbraco.Web/Trees/MacrosTreeController.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Net.Http.Formatting; using Umbraco.Core; using Umbraco.Core.Models; @@ -19,6 +20,14 @@ namespace Umbraco.Web.Trees public class MacrosTreeController : TreeController { + protected override TreeNode CreateRootNode(FormDataCollection queryStrings) + { + var root = base.CreateRootNode(queryStrings); + //check if there are any macros + root.HasChildren = Services.MacroService.GetAll().Any(); + return root; + } + protected override TreeNodeCollection GetTreeNodes(string id, FormDataCollection queryStrings) { var nodes = new TreeNodeCollection(); diff --git a/src/Umbraco.Web/Trees/MemberGroupTreeController.cs b/src/Umbraco.Web/Trees/MemberGroupTreeController.cs index b9910c7b31..9c8c8ea4e0 100644 --- a/src/Umbraco.Web/Trees/MemberGroupTreeController.cs +++ b/src/Umbraco.Web/Trees/MemberGroupTreeController.cs @@ -19,5 +19,13 @@ namespace Umbraco.Web.Trees .OrderBy(x => x.Name) .Select(dt => CreateTreeNode(dt.Id.ToString(), id, queryStrings, dt.Name, "icon-item-arrangement", false)); } + + protected override TreeNode CreateRootNode(FormDataCollection queryStrings) + { + var root = base.CreateRootNode(queryStrings); + //check if there are any groups + root.HasChildren = Services.MemberGroupService.GetAll().Any(); + return root; + } } } From 9320c1a061183bfaeb5842b7a39ae23b65b0bed1 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 23 Oct 2018 15:04:41 +0200 Subject: [PATCH 12/16] Cleanup variants dirty tracking --- .../Collections/CompositeIntStringKey.cs | 1 - .../Collections/ObservableDictionary.cs | 31 ++++---- .../Migrations/MigrationBase_Extra.cs | 14 ++++ .../Upgrade/V_8_0_0/AddLogTableColumns.cs | 8 +- src/Umbraco.Core/Models/AuditItem.cs | 19 ++--- src/Umbraco.Core/Models/AuditType.cs | 71 +++++++++++------- src/Umbraco.Core/Models/Content.cs | 44 +++++------ src/Umbraco.Core/Models/ContentBase.cs | 68 +++++++---------- ...{CultureName.cs => ContentCultureInfos.cs} | 68 ++++++++++------- .../Models/ContentCultureInfosCollection.cs | 70 ++++++++++++++++++ src/Umbraco.Core/Models/ContentTypeBase.cs | 18 ++--- .../Models/CultureNameCollection.cs | 73 ------------------- .../Models/Entities/EntityBase.cs | 5 +- src/Umbraco.Core/Models/IAuditItem.cs | 27 +++++-- src/Umbraco.Core/Models/IContent.cs | 5 +- src/Umbraco.Core/Models/IContentBase.cs | 5 +- src/Umbraco.Core/Models/Media.cs | 8 +- src/Umbraco.Core/Models/PropertyGroup.cs | 9 +-- src/Umbraco.Core/Models/PublicAccessEntry.cs | 9 +-- src/Umbraco.Core/Persistence/Dtos/LogDto.cs | 4 +- .../Implement/DocumentRepository.cs | 24 +++--- .../Services/Implement/ContentService.cs | 8 +- src/Umbraco.Core/Umbraco.Core.csproj | 4 +- src/Umbraco.Tests/Models/ContentTests.cs | 24 +++--- src/Umbraco.Tests/Models/VariationTests.cs | 10 +-- .../Repositories/DocumentRepositoryTest.cs | 2 +- .../Models/Mapping/ContentMapperProfile.cs | 2 +- .../Models/Mapping/MemberMapperProfile.cs | 2 +- .../NuCache/PublishedSnapshotService.cs | 6 +- src/Umbraco.Web/umbraco.presentation/page.cs | 2 +- 30 files changed, 335 insertions(+), 306 deletions(-) rename src/Umbraco.Core/Models/{CultureName.cs => ContentCultureInfos.cs} (52%) create mode 100644 src/Umbraco.Core/Models/ContentCultureInfosCollection.cs delete mode 100644 src/Umbraco.Core/Models/CultureNameCollection.cs diff --git a/src/Umbraco.Core/Collections/CompositeIntStringKey.cs b/src/Umbraco.Core/Collections/CompositeIntStringKey.cs index 74ef4e45e1..cafc209e08 100644 --- a/src/Umbraco.Core/Collections/CompositeIntStringKey.cs +++ b/src/Umbraco.Core/Collections/CompositeIntStringKey.cs @@ -2,7 +2,6 @@ namespace Umbraco.Core.Collections { - /// /// Represents a composite key of (int, string) for fast dictionaries. /// diff --git a/src/Umbraco.Core/Collections/ObservableDictionary.cs b/src/Umbraco.Core/Collections/ObservableDictionary.cs index ded87c30a6..6518533476 100644 --- a/src/Umbraco.Core/Collections/ObservableDictionary.cs +++ b/src/Umbraco.Core/Collections/ObservableDictionary.cs @@ -23,22 +23,22 @@ namespace Umbraco.Core.Collections /// Create new ObservableDictionary /// /// Selector function to create key from value + /// The equality comparer to use when comparing keys, or null to use the default comparer. public ObservableDictionary(Func keySelector, IEqualityComparer equalityComparer = null) - : base() { - if (keySelector == null) throw new ArgumentException("keySelector"); - KeySelector = keySelector; + KeySelector = keySelector ?? throw new ArgumentException("keySelector"); Indecies = new Dictionary(equalityComparer); } #region Protected Methods + protected override void InsertItem(int index, TValue item) { var key = KeySelector(item); if (Indecies.ContainsKey(key)) throw new DuplicateKeyException(key.ToString()); - if (index != this.Count) + if (index != Count) { foreach (var k in Indecies.Keys.Where(k => Indecies[k] >= index).ToList()) { @@ -48,7 +48,6 @@ namespace Umbraco.Core.Collections base.InsertItem(index, item); Indecies[key] = index; - } protected override void ClearItems() @@ -57,7 +56,6 @@ namespace Umbraco.Core.Collections Indecies.Clear(); } - protected override void RemoveItem(int index) { var item = this[index]; @@ -72,6 +70,7 @@ namespace Umbraco.Core.Collections Indecies[k]--; } } + #endregion public bool ContainsKey(TKey key) @@ -87,7 +86,7 @@ namespace Umbraco.Core.Collections public TValue this[TKey key] { - get { return this[Indecies[key]]; } + get => this[Indecies[key]]; set { //confirm key matches @@ -96,7 +95,7 @@ namespace Umbraco.Core.Collections if (!Indecies.ContainsKey(key)) { - this.Add(value); + Add(value); } else { @@ -116,6 +115,7 @@ namespace Umbraco.Core.Collections public bool Replace(TKey key, TValue value) { if (!Indecies.ContainsKey(key)) return false; + //confirm key matches if (!KeySelector(value).Equals(key)) throw new InvalidOperationException("Key of new value does not match"); @@ -129,7 +129,7 @@ namespace Umbraco.Core.Collections { if (!Indecies.ContainsKey(key)) return false; - this.RemoveAt(Indecies[key]); + RemoveAt(Indecies[key]); return true; } @@ -145,6 +145,7 @@ namespace Umbraco.Core.Collections { throw new InvalidOperationException("No item with the key " + currentKey + "was found in the collection"); } + if (ContainsKey(newKey)) { throw new DuplicateKeyException(newKey.ToString()); @@ -184,10 +185,7 @@ namespace Umbraco.Core.Collections //this will never be used ICollection IDictionary.Values => Values.ToList(); - bool ICollection>.IsReadOnly - { - get { return false; } - } + bool ICollection>.IsReadOnly => false; IEnumerator> IEnumerable>.GetEnumerator() { @@ -227,14 +225,13 @@ namespace Umbraco.Core.Collections internal class DuplicateKeyException : Exception { - - public string Key { get; private set; } public DuplicateKeyException(string key) - : base("Attempted to insert duplicate key " + key + " in collection") + : base("Attempted to insert duplicate key \"" + key + "\" in collection.") { Key = key; } - } + public string Key { get; } + } } } diff --git a/src/Umbraco.Core/Migrations/MigrationBase_Extra.cs b/src/Umbraco.Core/Migrations/MigrationBase_Extra.cs index b1b405bcf4..9e13badacf 100644 --- a/src/Umbraco.Core/Migrations/MigrationBase_Extra.cs +++ b/src/Umbraco.Core/Migrations/MigrationBase_Extra.cs @@ -18,12 +18,26 @@ namespace Umbraco.Core.Migrations AddColumn(table, table.Name, columnName); } + protected void AddColumnIfNotExists(IEnumerable columns, string columnName) + { + var table = DefinitionFactory.GetTableDefinition(typeof(T), SqlSyntax); + if (columns.Any(x => x.TableName.InvariantEquals(table.Name) && !x.ColumnName.InvariantEquals(columnName))) + AddColumn(table, table.Name, columnName); + } + protected void AddColumn(string tableName, string columnName) { var table = DefinitionFactory.GetTableDefinition(typeof(T), SqlSyntax); AddColumn(table, tableName, columnName); } + protected void AddColumnIfNotExists(IEnumerable columns, string tableName, string columnName) + { + var table = DefinitionFactory.GetTableDefinition(typeof(T), SqlSyntax); + if (columns.Any(x => x.TableName.InvariantEquals(tableName) && !x.ColumnName.InvariantEquals(columnName))) + AddColumn(table, tableName, columnName); + } + private void AddColumn(TableDefinition table, string tableName, string columnName) { if (ColumnExists(tableName, columnName)) return; diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs index d038da2573..c8a6e38dad 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLogTableColumns.cs @@ -13,12 +13,8 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 { var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); - if (columns.Any(x => x.TableName.InvariantEquals(Constants.DatabaseSchema.Tables.Log) && !x.ColumnName.InvariantEquals("entityType"))) - AddColumn("entityType"); - - if (columns.Any(x => x.TableName.InvariantEquals(Constants.DatabaseSchema.Tables.Log) && !x.ColumnName.InvariantEquals("parameters"))) - AddColumn("parameters"); - + AddColumnIfNotExists(columns, "entityType"); + AddColumnIfNotExists(columns, "parameters"); } } } diff --git a/src/Umbraco.Core/Models/AuditItem.cs b/src/Umbraco.Core/Models/AuditItem.cs index 483548f558..5fbde7f362 100644 --- a/src/Umbraco.Core/Models/AuditItem.cs +++ b/src/Umbraco.Core/Models/AuditItem.cs @@ -5,12 +5,8 @@ namespace Umbraco.Core.Models public sealed class AuditItem : EntityBase, IAuditItem { /// - /// Constructor for creating an item to be created + /// Initializes a new instance of the class. /// - /// - /// - /// - /// public AuditItem(int objectId, AuditType type, int userId, string entityType, string comment = null, string parameters = null) { DisableChangeTracking(); @@ -25,14 +21,19 @@ namespace Umbraco.Core.Models EnableChangeTracking(); } - public string Comment { get; } + /// + public AuditType AuditType { get; } /// public string EntityType { get; } + + /// + public int UserId { get; } + + /// + public string Comment { get; } + /// public string Parameters { get; } - - public AuditType AuditType { get; } - public int UserId { get; } } } diff --git a/src/Umbraco.Core/Models/AuditType.cs b/src/Umbraco.Core/Models/AuditType.cs index 759aac3bfc..8a57948805 100644 --- a/src/Umbraco.Core/Models/AuditType.cs +++ b/src/Umbraco.Core/Models/AuditType.cs @@ -1,96 +1,117 @@ namespace Umbraco.Core.Models { /// - /// Enums for vailable types of auditing + /// Defines audit types. /// public enum AuditType { /// - /// Used when new nodes are added + /// New node(s) being added. /// New, + /// - /// Used when nodes are saved + /// Node(s) being saved. /// Save, + /// - /// Used when variant(s) are saved + /// Variant(s) being saved. /// SaveVariant, + /// - /// Used when nodes are opened + /// Node(s) being opened. /// Open, + /// - /// Used when nodes are deleted + /// Node(s) being deleted. /// Delete, + /// - /// Used when nodes are published + /// Node(s) being published. /// Publish, + /// - /// Used when variant(s) are published + /// Variant(s) being published. /// PublishVariant, + /// - /// Used when nodes are sent for publishing + /// Node(s) being sent to publishing. /// SendToPublish, + /// - /// Used when variant(s) are sent for publishing - /// + /// Variant(s) being sent to publishing. + /// SendToPublishVariant, + /// - /// Used when nodes are unpublished + /// Node(s) being unpublished. /// Unpublish, + /// - /// Used when variant(s) are unpublished + /// Variant(s) being unpublished. /// UnpublishVariant, + /// - /// Used when nodes are moved + /// Node(s) being moved. /// Move, + /// - /// Used when nodes are copied + /// Node(s) being copied. /// Copy, + /// - /// Used when nodes are assígned a domain + /// Node(s) being assigned domains. /// AssignDomain, + /// - /// Used when public access are changed for a node + /// Node(s) public access changing. /// PublicAccess, + /// - /// Used when nodes are sorted + /// Node(s) being sorted. /// Sort, + /// - /// Used when a notification are send to a user + /// Notification(s) being sent to user. /// Notify, + /// - /// General system notification + /// General system audit message. /// System, + /// - /// Used when a node's content is rolled back to a previous version + /// Node's content being rolled back to a previous version. /// RollBack, + /// - /// Used when a package is installed + /// Package being installed. /// PackagerInstall, + /// - /// Used when a package is uninstalled + /// Package being uninstalled. /// - PackagerUninstall, + PackagerUninstall, + /// - /// Use this log action for custom log messages that should be shown in the audit trail + /// Custom audit message. /// Custom } diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 25a30da6db..3e5becf021 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -4,7 +4,6 @@ using System.Collections.Specialized; using System.Linq; using System.Reflection; using System.Runtime.Serialization; -using Umbraco.Core.Collections; using Umbraco.Core.Exceptions; namespace Umbraco.Core.Models @@ -22,8 +21,8 @@ namespace Umbraco.Core.Models private PublishedState _publishedState; private DateTime? _releaseDate; private DateTime? _expireDate; - private CultureNameCollection _publishInfos; - private CultureNameCollection _publishInfosOrig; + private ContentCultureInfosCollection _publishInfos; + private ContentCultureInfosCollection _publishInfosOrig; private HashSet _editedCultures; private static readonly Lazy Ps = new Lazy(); @@ -89,7 +88,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo PublishedSelector = ExpressionHelper.GetPropertyInfo(x => x.Published); public readonly PropertyInfo ReleaseDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ReleaseDate); public readonly PropertyInfo ExpireDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ExpireDate); - public readonly PropertyInfo PublishNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.PublishNames); + public readonly PropertyInfo PublishCultureInfosSelector = ExpressionHelper.GetPropertyInfo>(x => x.PublishCultureInfos); } /// @@ -214,7 +213,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IEnumerable EditedCultures => CultureNames.Keys.Where(IsCultureEdited); + public IEnumerable EditedCultures => CultureInfos.Keys.Where(IsCultureEdited); /// [IgnoreDataMember] @@ -224,7 +223,7 @@ namespace Umbraco.Core.Models public bool IsCulturePublished(string culture) // just check _publishInfos // a non-available culture could not become published anyways - => _publishInfos != null && _publishInfos.ContainsKey(culture); + => _publishInfos != null && _publishInfos.ContainsKey(culture); /// public bool WasCulturePublished(string culture) @@ -247,8 +246,8 @@ namespace Umbraco.Core.Models _publishInfos.AddOrUpdate(culture, publishInfos.Name, date); - if (CultureNames.TryGetValue(culture, out var name)) - SetCultureInfo(culture, name.Name, date); + if (CultureInfos.TryGetValue(culture, out var infos)) + SetCultureInfo(culture, infos.Name, date); } } @@ -260,7 +259,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IReadOnlyDictionary PublishNames => _publishInfos ?? NoNames; + public IReadOnlyDictionary PublishCultureInfos => _publishInfos ?? NoInfos; /// public string GetPublishName(string culture) @@ -291,7 +290,7 @@ namespace Umbraco.Core.Models if (_publishInfos == null) { - _publishInfos = new CultureNameCollection(); + _publishInfos = new ContentCultureInfosCollection(); _publishInfos.CollectionChanged += PublishNamesCollectionChanged; } @@ -338,13 +337,11 @@ namespace Umbraco.Core.Models } /// - /// Event handler for when the culture names collection is modified + /// Handles culture infos collection changes. /// - /// - /// private void PublishNamesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { - OnPropertyChanged(Ps.Value.PublishNamesSelector); + OnPropertyChanged(Ps.Value.PublishCultureInfosSelector); } [IgnoreDataMember] @@ -477,11 +474,12 @@ namespace Umbraco.Core.Models // if this entity's previous culture publish state (regardless of the rememberDirty flag) _publishInfosOrig = _publishInfos == null ? null - : new CultureNameCollection(_publishInfos); + : new ContentCultureInfosCollection(_publishInfos); - if (_publishInfos != null) - foreach (var cultureName in _publishInfos) - cultureName.ResetDirtyProperties(rememberDirty); + if (_publishInfos == null) return; + + foreach (var infos in _publishInfos) + infos.ResetDirtyProperties(rememberDirty); } /// @@ -509,18 +507,16 @@ namespace Umbraco.Core.Models clone.DisableChangeTracking(); //need to manually clone this since it's not settable - clone._contentType = (IContentType)ContentType.DeepClone(); + clone._contentType = (IContentType) ContentType.DeepClone(); //if culture infos exist then deal with event bindings if (clone._publishInfos != null) { - clone._publishInfos.CollectionChanged -= this.PublishNamesCollectionChanged; //clear this event handler if any - clone._publishInfos = (CultureNameCollection)_publishInfos.DeepClone(); //manually deep clone - clone._publishInfos.CollectionChanged += clone.PublishNamesCollectionChanged; //re-assign correct event handler + clone._publishInfos.CollectionChanged -= PublishNamesCollectionChanged; //clear this event handler if any + clone._publishInfos = (ContentCultureInfosCollection) _publishInfos.DeepClone(); //manually deep clone + clone._publishInfos.CollectionChanged += clone.PublishNamesCollectionChanged; //re-assign correct event handler } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); //re-enable tracking clone.EnableChangeTracking(); diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index a288f7fac9..863374726d 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -5,8 +5,6 @@ using System.Diagnostics; using System.Linq; using System.Reflection; using System.Runtime.Serialization; -using System.Web; -using Umbraco.Core.Collections; using Umbraco.Core.Exceptions; using Umbraco.Core.Models.Entities; @@ -20,14 +18,14 @@ namespace Umbraco.Core.Models [DebuggerDisplay("Id: {Id}, Name: {Name}, ContentType: {ContentTypeBase.Alias}")] public abstract class ContentBase : TreeEntityBase, IContentBase { - protected static readonly CultureNameCollection NoNames = new CultureNameCollection(); + protected static readonly ContentCultureInfosCollection NoInfos = new ContentCultureInfosCollection(); private static readonly Lazy Ps = new Lazy(); private int _contentTypeId; protected IContentTypeComposition ContentTypeBase; private int _writerId; private PropertyCollection _properties; - private CultureNameCollection _cultureInfos; + private ContentCultureInfosCollection _cultureInfos; /// /// Initializes a new instance of the class. @@ -70,7 +68,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo DefaultContentTypeIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ContentTypeId); public readonly PropertyInfo PropertyCollectionSelector = ExpressionHelper.GetPropertyInfo(x => x.Properties); public readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); - public readonly PropertyInfo CultureNamesSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureNames); + public readonly PropertyInfo CultureInfosSelector = ExpressionHelper.GetPropertyInfo>(x => x.CultureInfos); } protected void PropertiesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -160,7 +158,7 @@ namespace Umbraco.Core.Models /// [DataMember] - public virtual IReadOnlyDictionary CultureNames => _cultureInfos ?? NoNames; + public virtual IReadOnlyDictionary CultureInfos => _cultureInfos ?? NoInfos; /// public string GetCultureName(string culture) @@ -207,17 +205,9 @@ namespace Umbraco.Core.Models } } - //fixme: this isn't used anywhere - internal void TouchCulture(string culture) - { - if (ContentTypeBase.VariesByCulture() && _cultureInfos != null && _cultureInfos.TryGetValue(culture, out var infos)) - _cultureInfos.AddOrUpdate(culture, infos.Name, DateTime.Now); - } - protected void ClearCultureInfos() { - if (_cultureInfos != null) - _cultureInfos.Clear(); + _cultureInfos?.Clear(); _cultureInfos = null; } @@ -243,21 +233,19 @@ namespace Umbraco.Core.Models if (_cultureInfos == null) { - _cultureInfos = new CultureNameCollection(); - _cultureInfos.CollectionChanged += CultureNamesCollectionChanged; + _cultureInfos = new ContentCultureInfosCollection(); + _cultureInfos.CollectionChanged += CultureInfosCollectionChanged; } _cultureInfos.AddOrUpdate(culture, name, date); } /// - /// Event handler for when the culture names collection is modified + /// Handles culture infos collection changes. /// - /// - /// - private void CultureNamesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) + private void CultureInfosCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { - OnPropertyChanged(Ps.Value.CultureNamesSelector); + OnPropertyChanged(Ps.Value.CultureInfosSelector); } #endregion @@ -370,10 +358,10 @@ namespace Umbraco.Core.Models if (culture == null || culture == "*") Name = other.Name; - foreach (var (otherCulture, otherName) in other.CultureNames) + foreach (var (otherCulture, otherInfos) in other.CultureInfos) { if (culture == "*" || culture == otherCulture) - SetCultureName(otherName.Name, otherCulture); + SetCultureName(otherInfos.Name, otherCulture); } } @@ -406,10 +394,11 @@ namespace Umbraco.Core.Models foreach (var prop in Properties) prop.ResetDirtyProperties(rememberDirty); - // take care of culture names - if (_cultureInfos != null) - foreach (var cultureName in _cultureInfos) - cultureName.ResetDirtyProperties(rememberDirty); + // take care of culture infos + if (_cultureInfos == null) return; + + foreach (var cultureInfo in _cultureInfos) + cultureInfo.ResetDirtyProperties(rememberDirty); } /// @@ -482,34 +471,33 @@ namespace Umbraco.Core.Models #endregion - /// - /// Override to deal with specific object instances - /// - /// + /// + /// + /// Overriden to deal with specific object instances + /// public override object DeepClone() { - var clone = (ContentBase)base.DeepClone(); + var clone = (ContentBase) base.DeepClone(); + //turn off change tracking clone.DisableChangeTracking(); //if culture infos exist then deal with event bindings if (clone._cultureInfos != null) { - clone._cultureInfos.CollectionChanged -= this.CultureNamesCollectionChanged; //clear this event handler if any - clone._cultureInfos = (CultureNameCollection)_cultureInfos.DeepClone(); //manually deep clone - clone._cultureInfos.CollectionChanged += clone.CultureNamesCollectionChanged; //re-assign correct event handler + clone._cultureInfos.CollectionChanged -= CultureInfosCollectionChanged; //clear this event handler if any + clone._cultureInfos = (ContentCultureInfosCollection) _cultureInfos.DeepClone(); //manually deep clone + clone._cultureInfos.CollectionChanged += clone.CultureInfosCollectionChanged; //re-assign correct event handler } //if properties exist then deal with event bindings if (clone._properties != null) { - clone._properties.CollectionChanged -= this.PropertiesChanged; //clear this event handler if any - clone._properties = (PropertyCollection)_properties.DeepClone(); //manually deep clone + clone._properties.CollectionChanged -= PropertiesChanged; //clear this event handler if any + clone._properties = (PropertyCollection) _properties.DeepClone(); //manually deep clone clone._properties.CollectionChanged += clone.PropertiesChanged; //re-assign correct event handler } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); //re-enable tracking clone.EnableChangeTracking(); diff --git a/src/Umbraco.Core/Models/CultureName.cs b/src/Umbraco.Core/Models/ContentCultureInfos.cs similarity index 52% rename from src/Umbraco.Core/Models/CultureName.cs rename to src/Umbraco.Core/Models/ContentCultureInfos.cs index d683f07b49..bcf1dbb1b1 100644 --- a/src/Umbraco.Core/Models/CultureName.cs +++ b/src/Umbraco.Core/Models/ContentCultureInfos.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Reflection; +using Umbraco.Core.Exceptions; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -8,19 +9,26 @@ namespace Umbraco.Core.Models /// /// The name of a content variant for a given culture /// - public class CultureName : BeingDirtyBase, IDeepCloneable, IEquatable + public class ContentCultureInfos : BeingDirtyBase, IDeepCloneable, IEquatable { private DateTime _date; private string _name; private static readonly Lazy Ps = new Lazy(); /// - /// Used for cloning without change tracking + /// Initializes a new instance of the class. /// - /// - /// - /// - private CultureName(string culture, string name, DateTime date) + public ContentCultureInfos(string culture) + { + if (culture.IsNullOrWhiteSpace()) throw new ArgumentNullOrEmptyException(nameof(culture)); + Culture = culture; + } + + /// + /// Initializes a new instance of the class. + /// + /// Used for cloning, without change tracking. + private ContentCultureInfos(string culture, string name, DateTime date) : this(culture) { _name = name; @@ -28,45 +36,47 @@ namespace Umbraco.Core.Models } /// - /// Constructor + /// Gets the culture. /// - /// - public CultureName(string culture) - { - if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("message", nameof(culture)); - Culture = culture; - } - - public string Culture { get; private set; } + public string Culture { get; } + /// + /// Gets the name. + /// public string Name { get => _name; set => SetPropertyValueAndDetectChanges(value, ref _name, Ps.Value.NameSelector); } + /// + /// Gets the date. + /// public DateTime Date { get => _date; set => SetPropertyValueAndDetectChanges(value, ref _date, Ps.Value.DateSelector); } + /// public object DeepClone() { - return new CultureName(Culture, Name, Date); + return new ContentCultureInfos(Culture, Name, Date); } + /// public override bool Equals(object obj) { - return obj is CultureName && Equals((CultureName)obj); + return obj is ContentCultureInfos other && Equals(other); } - public bool Equals(CultureName other) + /// + public bool Equals(ContentCultureInfos other) { - return Culture == other.Culture && - Name == other.Name; + return other != null && Culture == other.Culture && Name == other.Name; } + /// public override int GetHashCode() { var hashCode = 479558943; @@ -76,22 +86,28 @@ namespace Umbraco.Core.Models } /// - /// Allows deconstructing into culture and name + /// Deconstructs into culture and name. /// - /// - /// public void Deconstruct(out string culture, out string name) { culture = Culture; name = Name; } + /// + /// Deconstructs into culture, name and date. + /// + public void Deconstruct(out string culture, out string name, out DateTime date) + { + Deconstruct(out culture, out name); + date = Date; + } + // ReSharper disable once ClassNeverInstantiated.Local private class PropertySelectors { - public readonly PropertyInfo CultureSelector = ExpressionHelper.GetPropertyInfo(x => x.Culture); - public readonly PropertyInfo NameSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); - public readonly PropertyInfo DateSelector = ExpressionHelper.GetPropertyInfo(x => x.Date); + public readonly PropertyInfo NameSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); + public readonly PropertyInfo DateSelector = ExpressionHelper.GetPropertyInfo(x => x.Date); } } } diff --git a/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs b/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs new file mode 100644 index 0000000000..5238e65631 --- /dev/null +++ b/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs @@ -0,0 +1,70 @@ +using System; +using System.Collections.Generic; +using System.Collections.Specialized; +using Umbraco.Core.Collections; +using Umbraco.Core.Exceptions; + +namespace Umbraco.Core.Models +{ + /// + /// The culture names of a content's variants + /// + public class ContentCultureInfosCollection : ObservableDictionary, IDeepCloneable + { + /// + /// Initializes a new instance of the class. + /// + public ContentCultureInfosCollection() + : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) + { } + + /// + /// Initializes a new instance of the class with items. + /// + public ContentCultureInfosCollection(IEnumerable items) + : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) + { + foreach (var item in items) + Add(item); + } + + /// + /// Adds or updates a instance. + /// + public void AddOrUpdate(string culture, string name, DateTime date) + { + if (culture.IsNullOrWhiteSpace()) throw new ArgumentNullOrEmptyException(nameof(culture)); + culture = culture.ToLowerInvariant(); + + if (TryGetValue(culture, out var item)) + { + item.Name = name; + item.Date = date; + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, item)); + } + else + { + Add(new ContentCultureInfos(culture) + { + Name = name, + Date = date + }); + } + } + + /// + public object DeepClone() + { + var clone = new ContentCultureInfosCollection(); + + foreach (var item in this) + { + var itemClone = (ContentCultureInfos) item.DeepClone(); + itemClone.ResetDirtyProperties(false); + clone.Add(itemClone); + } + + return clone; + } + } +} diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 0a86f4cf09..9f848c6d14 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; -using System.ComponentModel; using System.Diagnostics; using System.Linq; using System.Reflection; @@ -470,7 +469,8 @@ namespace Umbraco.Core.Models public override object DeepClone() { - var clone = (ContentTypeBase)base.DeepClone(); + var clone = (ContentTypeBase) base.DeepClone(); + //turn off change tracking clone.DisableChangeTracking(); @@ -480,20 +480,18 @@ namespace Umbraco.Core.Models // its ignored from the auto-clone process because its return values are unions, not raw and // we end up with duplicates, see: http://issues.umbraco.org/issue/U4-4842 - clone._noGroupPropertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any - clone._noGroupPropertyTypes = (PropertyTypeCollection)_noGroupPropertyTypes.DeepClone(); //manually deep clone - clone._noGroupPropertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + clone._noGroupPropertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any + clone._noGroupPropertyTypes = (PropertyTypeCollection) _noGroupPropertyTypes.DeepClone(); //manually deep clone + clone._noGroupPropertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler } if (clone._propertyGroups != null) { - clone._propertyGroups.CollectionChanged -= this.PropertyGroupsChanged; //clear this event handler if any - clone._propertyGroups = (PropertyGroupCollection)_propertyGroups.DeepClone(); //manually deep clone - clone._propertyGroups.CollectionChanged += clone.PropertyGroupsChanged; //re-assign correct event handler + clone._propertyGroups.CollectionChanged -= PropertyGroupsChanged; //clear this event handler if any + clone._propertyGroups = (PropertyGroupCollection) _propertyGroups.DeepClone(); //manually deep clone + clone._propertyGroups.CollectionChanged += clone.PropertyGroupsChanged; //re-assign correct event handler } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); //re-enable tracking clone.EnableChangeTracking(); diff --git a/src/Umbraco.Core/Models/CultureNameCollection.cs b/src/Umbraco.Core/Models/CultureNameCollection.cs deleted file mode 100644 index 406abc7c0f..0000000000 --- a/src/Umbraco.Core/Models/CultureNameCollection.cs +++ /dev/null @@ -1,73 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Collections.Specialized; -using System.Linq; -using Umbraco.Core.Collections; - -namespace Umbraco.Core.Models -{ - - - /// - /// The culture names of a content's variants - /// - public class CultureNameCollection : ObservableDictionary, IDeepCloneable - { - /// - /// Creates a new collection from another collection - /// - /// - public CultureNameCollection(IEnumerable names) - : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) - { - foreach (var n in names) - Add(n); - } - - /// - /// Creates a new collection - /// - public CultureNameCollection() - : base(x => x.Culture, StringComparer.InvariantCultureIgnoreCase) - { - } - - /// - /// Add or update the - /// - /// - public void AddOrUpdate(string culture, string name, DateTime date) - { - culture = culture.ToLowerInvariant(); - if (TryGetValue(culture, out var found)) - { - found.Name = name; - found.Date = date; - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, found, found)); - } - else - Add(new CultureName(culture) - { - Name = name, - Date = date - }); - } - - public object DeepClone() - { - var clone = new CultureNameCollection(); - foreach (var name in this) - { - name.DisableChangeTracking(); - var copy = (CultureName)name.DeepClone(); - copy.ResetDirtyProperties(false); - clone.Add(copy); - name.EnableChangeTracking(); - } - return clone; - } - - - } -} diff --git a/src/Umbraco.Core/Models/Entities/EntityBase.cs b/src/Umbraco.Core/Models/Entities/EntityBase.cs index f1ff5f3bf5..0b69586abf 100644 --- a/src/Umbraco.Core/Models/Entities/EntityBase.cs +++ b/src/Umbraco.Core/Models/Entities/EntityBase.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Models.Entities [DebuggerDisplay("Id: {" + nameof(Id) + "}")] public abstract class EntityBase : BeingDirtyBase, IEntity { -#if ModelDebug +#if DEBUG_MODEL public Guid InstanceId = Guid.NewGuid(); #endif @@ -165,11 +165,10 @@ namespace Umbraco.Core.Models.Entities var unused = Key; // ensure that 'this' has a key, before cloning var clone = (EntityBase) MemberwiseClone(); -#if ModelDebug +#if DEBUG_MODEL clone.InstanceId = Guid.NewGuid(); #endif - // clear changes (ensures the clone has its own dictionaries) // then disable change tracking clone.ResetDirtyProperties(false); diff --git a/src/Umbraco.Core/Models/IAuditItem.cs b/src/Umbraco.Core/Models/IAuditItem.cs index c1dfd99dbb..ed70ada8ad 100644 --- a/src/Umbraco.Core/Models/IAuditItem.cs +++ b/src/Umbraco.Core/Models/IAuditItem.cs @@ -1,22 +1,35 @@ -using System; -using Umbraco.Core.Models.Entities; +using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models { + /// + /// Represents an audit item. + /// public interface IAuditItem : IEntity { - string Comment { get; } + /// + /// Gets the audit type. + /// + AuditType AuditType { get; } /// - /// The entity type for the log entry + /// Gets the audited entity type. /// string EntityType { get; } /// - /// Optional additional data parameters for the log + /// Gets the audit user identifier. + /// + int UserId { get; } + + /// + /// Gets the audit comments. + /// + string Comment { get; } + + /// + /// Gets optional additional data parameters. /// string Parameters { get; } - AuditType AuditType { get; } - int UserId { get; } } } diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 125c1a0f55..0c0d9449e0 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using Umbraco.Core.Collections; namespace Umbraco.Core.Models { @@ -127,13 +126,13 @@ namespace Umbraco.Core.Models string GetPublishName(string culture); /// - /// Gets the published names of the content. + /// Gets the published culture infos of the content. /// /// /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get via the property. /// - IReadOnlyDictionary PublishNames { get; } + IReadOnlyDictionary PublishCultureInfos { get; } /// /// Gets the published cultures. diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index c84c768e9c..cef8086207 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using Umbraco.Core.Collections; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -52,13 +51,13 @@ namespace Umbraco.Core.Models string GetCultureName(string culture); /// - /// Gets the names of the content item. + /// Gets culture infos of the content item. /// /// /// Because a dictionary key cannot be null this cannot contain the invariant /// culture name, which must be get or set via the property. /// - IReadOnlyDictionary CultureNames { get; } + IReadOnlyDictionary CultureInfos { get; } /// /// Gets the available cultures. diff --git a/src/Umbraco.Core/Models/Media.cs b/src/Umbraco.Core/Models/Media.cs index 097fc46351..9c13a22caa 100644 --- a/src/Umbraco.Core/Models/Media.cs +++ b/src/Umbraco.Core/Models/Media.cs @@ -1,6 +1,5 @@ using System; using System.Runtime.Serialization; -using Umbraco.Core.Persistence.Mappers; namespace Umbraco.Core.Models { @@ -21,8 +20,7 @@ namespace Umbraco.Core.Models /// MediaType for the current Media object public Media(string name, IMedia parent, IMediaType contentType) : this(name, parent, contentType, new PropertyCollection()) - { - } + { } /// /// Constructor for creating a Media object @@ -45,8 +43,7 @@ namespace Umbraco.Core.Models /// MediaType for the current Media object public Media(string name, int parentId, IMediaType contentType) : this(name, parentId, contentType, new PropertyCollection()) - { - } + { } /// /// Constructor for creating a Media object @@ -117,6 +114,5 @@ namespace Umbraco.Core.Models //The Media Recycle Bin Id is -21 so we correct that here ParentId = parentId == -20 ? -21 : parentId; } - } } diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 90f3ef2b55..6c1f2e5c61 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -103,18 +103,17 @@ namespace Umbraco.Core.Models public override object DeepClone() { var clone = (PropertyGroup)base.DeepClone(); + //turn off change tracking clone.DisableChangeTracking(); if (clone._propertyTypes != null) { - clone._propertyTypes.CollectionChanged -= this.PropertyTypesChanged; //clear this event handler if any - clone._propertyTypes = (PropertyTypeCollection)_propertyTypes.DeepClone(); //manually deep clone - clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler + clone._propertyTypes.CollectionChanged -= PropertyTypesChanged; //clear this event handler if any + clone._propertyTypes = (PropertyTypeCollection) _propertyTypes.DeepClone(); //manually deep clone + clone._propertyTypes.CollectionChanged += clone.PropertyTypesChanged; //re-assign correct event handler } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); //re-enable tracking clone.EnableChangeTracking(); diff --git a/src/Umbraco.Core/Models/PublicAccessEntry.cs b/src/Umbraco.Core/Models/PublicAccessEntry.cs index 26e249a35a..7fd0849e27 100644 --- a/src/Umbraco.Core/Models/PublicAccessEntry.cs +++ b/src/Umbraco.Core/Models/PublicAccessEntry.cs @@ -155,18 +155,17 @@ namespace Umbraco.Core.Models public override object DeepClone() { - var clone = (PublicAccessEntry)base.DeepClone(); + var clone = (PublicAccessEntry) base.DeepClone(); + //turn off change tracking clone.DisableChangeTracking(); if (clone._ruleCollection != null) { - clone._ruleCollection.CollectionChanged -= this._ruleCollection_CollectionChanged; //clear this event handler if any - clone._ruleCollection.CollectionChanged += clone._ruleCollection_CollectionChanged; //re-assign correct event handler + clone._ruleCollection.CollectionChanged -= _ruleCollection_CollectionChanged; //clear this event handler if any + clone._ruleCollection.CollectionChanged += clone._ruleCollection_CollectionChanged; //re-assign correct event handler } - //this shouldn't really be needed since we're not tracking - clone.ResetDirtyProperties(false); //re-enable tracking clone.EnableChangeTracking(); diff --git a/src/Umbraco.Core/Persistence/Dtos/LogDto.cs b/src/Umbraco.Core/Persistence/Dtos/LogDto.cs index a362d5d50c..9a710c1fec 100644 --- a/src/Umbraco.Core/Persistence/Dtos/LogDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/LogDto.cs @@ -5,11 +5,13 @@ using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Core.Persistence.Dtos { - [TableName(Constants.DatabaseSchema.Tables.Log)] + [TableName(TableName)] [PrimaryKey("id")] [ExplicitColumns] internal class LogDto { + public const string TableName = Constants.DatabaseSchema.Tables.Log; + private int? _userId; [Column("id")] diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index f5de3dc1d7..adb3b8c6b3 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -372,8 +372,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement content.AdjustDates(contentVersionDto.VersionDate); // names also impact 'edited' - foreach (var (culture, name) in content.CultureNames) - if (name.Name != content.GetPublishName(culture)) + foreach (var (culture, infos) in content.CultureInfos) + if (infos.Name != content.GetPublishName(culture)) (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); // insert content variations @@ -534,8 +534,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement content.AdjustDates(contentVersionDto.VersionDate); // names also impact 'edited' - foreach (var (culture, name) in content.CultureNames) - if (name.Name != content.GetPublishName(culture)) + foreach (var (culture, infos) in content.CultureInfos) + if (infos.Name != content.GetPublishName(culture)) { edited = true; (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); @@ -1131,7 +1131,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private IEnumerable GetContentVariationDtos(IContent content, bool publishing) { // create dtos for the 'current' (non-published) version, all cultures - foreach (var (culture, name) in content.CultureNames) + foreach (var (culture, name) in content.CultureInfos) yield return new ContentVersionCultureVariationDto { VersionId = content.VersionId, @@ -1146,7 +1146,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (!publishing) yield break; // create dtos for the 'published' version, for published cultures (those having a name) - foreach (var (culture, name) in content.PublishNames) + foreach (var (culture, name) in content.PublishCultureInfos) yield return new ContentVersionCultureVariationDto { VersionId = content.PublishedVersionId, @@ -1219,15 +1219,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { // content varies by culture // then it must have at least a variant name, else it makes no sense - if (content.CultureNames.Count == 0) + if (content.CultureInfos.Count == 0) throw new InvalidOperationException("Cannot save content with an empty name."); // and then, we need to set the invariant name implicitely, // using the default culture if it has a name, otherwise anything we can var defaultCulture = LanguageRepository.GetDefaultIsoCode(); - content.Name = defaultCulture != null && content.CultureNames.TryGetValue(defaultCulture, out var cultureName) + content.Name = defaultCulture != null && content.CultureInfos.TryGetValue(defaultCulture, out var cultureName) ? cultureName.Name - : content.CultureNames.First().Value.Name; + : content.CultureInfos.First().Value.Name; } else { @@ -1260,7 +1260,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private void EnsureVariantNamesAreUnique(Content content, bool publishing) { - if (!EnsureUniqueNaming || !content.ContentType.VariesByCulture() || content.CultureNames.Count == 0) return; + if (!EnsureUniqueNaming || !content.ContentType.VariesByCulture() || content.CultureInfos.Count == 0) return; // get names per culture, at same level (ie all siblings) var sql = SqlEnsureVariantNamesAreUnique.Sql(true, NodeObjectTypeId, content.ParentId, content.Id); @@ -1274,7 +1274,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // of whether the name has changed (ie the culture has been updated) - some saving culture // fr-FR could cause culture en-UK name to change - not sure that is clean - foreach (var (culture, name) in content.CultureNames) + foreach (var (culture, name) in content.CultureInfos) { var langId = LanguageRepository.GetIdByIsoCode(culture); if (!langId.HasValue) continue; @@ -1288,7 +1288,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // update the name, and the publish name if published content.SetCultureName(uniqueName, culture); - if (publishing && content.PublishNames.ContainsKey(culture)) + if (publishing && content.PublishCultureInfos.ContainsKey(culture)) content.SetPublishInfo(culture, uniqueName, DateTime.Now); } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 2af6de24c7..ebdbe8d83e 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -849,7 +849,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures that have changed var culturesChanging = content.ContentType.VariesByCulture() - ? string.Join(",", content.CultureNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)) + ? string.Join(",", content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key)) : null; //TODO: Currently there's no way to change track which variant properties have changed, we only have change // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. @@ -1082,7 +1082,7 @@ namespace Umbraco.Core.Services.Implement } else { - culturesChanging = string.Join(",", content.PublishNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)); + culturesChanging = string.Join(",", content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key)); } } @@ -1865,7 +1865,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures changing for auditing var culturesChanging = content.ContentType.VariesByCulture() - ? string.Join(",", content.CultureNames.Where(x => x.Value.IsDirty()).Select(x => x.Key)) + ? string.Join(",", content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key)) : null; //TODO: Currently there's no way to change track which variant properties have changed, we only have change // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. @@ -2604,7 +2604,7 @@ namespace Umbraco.Core.Services.Implement //Logging & Audit message Logger.Info("User '{UserId}' rolled back content '{ContentId}' to version '{VersionId}'", userId, id, versionId); - Audit(AuditType.RollBack, $"Content '{content.Name}' was rolled back to version '{versionId}'", userId, id); + Audit(AuditType.RollBack, userId, id, $"Content '{content.Name}' was rolled back to version '{versionId}'"); } scope.Complete(); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index bc6c8fee54..e0aa93efec 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -376,8 +376,8 @@ - - + + diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 1c3e2453c3..807231730b 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -53,23 +53,23 @@ namespace Umbraco.Tests.Models contentType.Variations = ContentVariation.Culture; - Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //hasn't been changed + Assert.IsFalse(content.IsPropertyDirty("CultureInfos")); //hasn't been changed Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); - Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //now it will be changed since the collection has changed - var frCultureName = content.CultureNames[langFr]; + Assert.IsTrue(content.IsPropertyDirty("CultureInfos")); //now it will be changed since the collection has changed + var frCultureName = content.CultureInfos[langFr]; Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); content.ResetDirtyProperties(); - Assert.IsFalse(content.IsPropertyDirty("CultureNames")); //it's been reset - Assert.IsTrue(content.WasPropertyDirty("CultureNames")); + Assert.IsFalse(content.IsPropertyDirty("CultureInfos")); //it's been reset + Assert.IsTrue(content.WasPropertyDirty("CultureInfos")); Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); - Assert.IsTrue(content.IsPropertyDirty("CultureNames")); //it's true now since we've updated a name + Assert.IsTrue(content.IsPropertyDirty("CultureInfos")); //it's true now since we've updated a name } [Test] @@ -82,25 +82,25 @@ namespace Umbraco.Tests.Models contentType.Variations = ContentVariation.Culture; - Assert.IsFalse(content.IsPropertyDirty("PublishNames")); //hasn't been changed + Assert.IsFalse(content.IsPropertyDirty("PublishCultureInfos")); //hasn't been changed Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); content.PublishCulture(langFr); //we've set the name, now we're publishing it - Assert.IsTrue(content.IsPropertyDirty("PublishNames")); //now it will be changed since the collection has changed - var frCultureName = content.PublishNames[langFr]; + Assert.IsTrue(content.IsPropertyDirty("PublishCultureInfos")); //now it will be changed since the collection has changed + var frCultureName = content.PublishCultureInfos[langFr]; Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); content.ResetDirtyProperties(); - Assert.IsFalse(content.IsPropertyDirty("PublishNames")); //it's been reset - Assert.IsTrue(content.WasPropertyDirty("PublishNames")); + Assert.IsFalse(content.IsPropertyDirty("PublishCultureInfos")); //it's been reset + Assert.IsTrue(content.WasPropertyDirty("PublishCultureInfos")); Thread.Sleep(500); //The "Date" wont be dirty if the test runs too fast since it will be the same date content.SetCultureName("name-fr", langFr); content.PublishCulture(langFr); //we've set the name, now we're publishing it Assert.IsTrue(frCultureName.IsPropertyDirty("Date")); - Assert.IsTrue(content.IsPropertyDirty("PublishNames")); //it's true now since we've updated a name + Assert.IsTrue(content.IsPropertyDirty("PublishCultureInfos")); //it's true now since we've updated a name } [Test] diff --git a/src/Umbraco.Tests/Models/VariationTests.cs b/src/Umbraco.Tests/Models/VariationTests.cs index 8a996f9782..e6f4e53d26 100644 --- a/src/Umbraco.Tests/Models/VariationTests.cs +++ b/src/Umbraco.Tests/Models/VariationTests.cs @@ -236,11 +236,11 @@ namespace Umbraco.Tests.Models Assert.AreEqual("name-uk", content.GetCultureName(langUk)); // variant dictionary of names work - Assert.AreEqual(2, content.CultureNames.Count); - Assert.IsTrue(content.CultureNames.ContainsKey(langFr)); - Assert.AreEqual("name-fr", content.CultureNames[langFr].Name); - Assert.IsTrue(content.CultureNames.ContainsKey(langUk)); - Assert.AreEqual("name-uk", content.CultureNames[langUk].Name); + Assert.AreEqual(2, content.CultureInfos.Count); + Assert.IsTrue(content.CultureInfos.ContainsKey(langFr)); + Assert.AreEqual("name-fr", content.CultureInfos[langFr].Name); + Assert.IsTrue(content.CultureInfos.ContainsKey(langUk)); + Assert.AreEqual("name-uk", content.CultureInfos[langUk].Name); } [Test] diff --git a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs index 62c3116ab1..9f84d9faf5 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs @@ -767,7 +767,7 @@ namespace Umbraco.Tests.Persistence.Repositories foreach (var r in result) { var isInvariant = r.ContentType.Alias == "umbInvariantTextpage"; - var name = isInvariant ? r.Name : r.CultureNames["en-US"].Name; + var name = isInvariant ? r.Name : r.CultureInfos["en-US"].Name; var namePrefix = isInvariant ? "INV" : "VAR"; //ensure the correct name (invariant vs variant) is in the result diff --git a/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs index 4557dfb1bd..d5da9ecb51 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentMapperProfile.cs @@ -132,7 +132,7 @@ namespace Umbraco.Web.Models.Mapping // if we don't have a name for a culture, it means the culture is not available, and // hey we should probably not be mapping it, but it's too late, return a fallback name - return source.CultureNames.TryGetValue(culture, out var name) && !name.Name.IsNullOrWhiteSpace() ? name.Name : $"(({source.Name}))"; + return source.CultureInfos.TryGetValue(culture, out var name) && !name.Name.IsNullOrWhiteSpace() ? name.Name : $"(({source.Name}))"; } } } diff --git a/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs b/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs index f64d8dc529..0cfa3cf154 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberMapperProfile.cs @@ -48,7 +48,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(dest => dest.CreatorId, opt => opt.Ignore()) .ForMember(dest => dest.Level, opt => opt.Ignore()) .ForMember(dest => dest.Name, opt => opt.Ignore()) - .ForMember(dest => dest.CultureNames, opt => opt.Ignore()) + .ForMember(dest => dest.CultureInfos, opt => opt.Ignore()) .ForMember(dest => dest.ParentId, opt => opt.Ignore()) .ForMember(dest => dest.Path, opt => opt.Ignore()) .ForMember(dest => dest.SortOrder, opt => opt.Ignore()) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 355a4e7644..f414702824 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -1193,9 +1193,9 @@ namespace Umbraco.Web.PublishedCache.NuCache var names = content is IContent document ? (published - ? document.PublishNames - : document.CultureNames) - : content.CultureNames; + ? document.PublishCultureInfos + : document.CultureInfos) + : content.CultureInfos; foreach (var (culture, name) in names) { diff --git a/src/Umbraco.Web/umbraco.presentation/page.cs b/src/Umbraco.Web/umbraco.presentation/page.cs index ea1a563f9c..e8d395881c 100644 --- a/src/Umbraco.Web/umbraco.presentation/page.cs +++ b/src/Umbraco.Web/umbraco.presentation/page.cs @@ -395,7 +395,7 @@ namespace umbraco if (_cultureInfos != null) return _cultureInfos; - return _cultureInfos = _inner.PublishNames + return _cultureInfos = _inner.PublishCultureInfos .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value.Name, x.Value.Date)); } } From c7cb8b52cc176e419c9261001864c70bd517aa24 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 24 Oct 2018 15:23:21 +1100 Subject: [PATCH 13/16] Fixes AuthorizeUpgrade page since it was still referencing the old UmbracoClient folder --- src/Umbraco.Web.UI/Umbraco/Views/AuthorizeUpgrade.cshtml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI/Umbraco/Views/AuthorizeUpgrade.cshtml b/src/Umbraco.Web.UI/Umbraco/Views/AuthorizeUpgrade.cshtml index 3c79d5458c..0eba571daa 100644 --- a/src/Umbraco.Web.UI/Umbraco/Views/AuthorizeUpgrade.cshtml +++ b/src/Umbraco.Web.UI/Umbraco/Views/AuthorizeUpgrade.cshtml @@ -33,8 +33,7 @@ Umbraco @Html.RenderCssHere( - new BasicPath("Umbraco", IOHelper.ResolveUrl(SystemDirectories.Umbraco)), - new BasicPath("UmbracoClient", IOHelper.ResolveUrl(SystemDirectories.UmbracoClient))) + new BasicPath("Umbraco", IOHelper.ResolveUrl(SystemDirectories.Umbraco))) @*Because we're lazy loading angular js, the embedded cloak style will not be loaded initially, but we need it*@