diff --git a/NuGet.Config b/NuGet.Config index 7d786702f4..31fd84e456 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -7,6 +7,5 @@ --> - diff --git a/src/Umbraco.Core/Models/RelationType.cs b/src/Umbraco.Core/Models/RelationType.cs index 1085ecdcdd..25156cdb8a 100644 --- a/src/Umbraco.Core/Models/RelationType.cs +++ b/src/Umbraco.Core/Models/RelationType.cs @@ -1,5 +1,6 @@ using System; using System.Runtime.Serialization; +using Umbraco.Core.Exceptions; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -13,27 +14,41 @@ namespace Umbraco.Core.Models { private string _name; private string _alias; - private bool _isBidrectional; + private bool _isBidirectional; private Guid? _parentObjectType; private Guid? _childObjectType; - //TODO: Should we put back the broken ctors with obsolete attributes? - + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] public RelationType(string alias, string name) - : this(name, alias, false, null, null) + : this(name: name, alias: alias, false, null, null) { } public RelationType(string name, string alias, bool isBidrectional, Guid? parentObjectType, Guid? childObjectType) { + if (name == null) throw new ArgumentNullException(nameof(name)); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(name)); + if (alias == null) throw new ArgumentNullException(nameof(alias)); + if (string.IsNullOrWhiteSpace(alias)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(alias)); + _name = name; _alias = alias; - _isBidrectional = isBidrectional; + _isBidirectional = isBidrectional; _parentObjectType = parentObjectType; _childObjectType = childObjectType; } + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] + public RelationType(Guid childObjectType, Guid parentObjectType, string alias) + : this(name: alias, alias: alias, isBidrectional: false, parentObjectType: parentObjectType, childObjectType: childObjectType) + { + } + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] + public RelationType(Guid childObjectType, Guid parentObjectType, string alias, string name) + : this(name: name, alias: alias, isBidrectional: false, parentObjectType: parentObjectType, childObjectType: childObjectType) + { + } /// /// Gets or sets the Name of the RelationType @@ -61,8 +76,8 @@ namespace Umbraco.Core.Models [DataMember] public bool IsBidirectional { - get => _isBidrectional; - set => SetPropertyValueAndDetectChanges(value, ref _isBidrectional, nameof(IsBidirectional)); + get => _isBidirectional; + set => SetPropertyValueAndDetectChanges(value, ref _isBidirectional, nameof(IsBidirectional)); } /// diff --git a/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs b/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs index edd87fec68..177a0494a2 100644 --- a/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs @@ -9,7 +9,7 @@ namespace Umbraco.Core.Persistence.Factories public static IRelationType BuildEntity(RelationTypeDto dto) { - var entity = new RelationType(dto.Name, dto.Alias, dto.Dual, dto.ChildObjectType, dto.ParentObjectType); + var entity = new RelationType(dto.Name, dto.Alias, dto.Dual, dto.ParentObjectType, dto.ChildObjectType); try { diff --git a/src/Umbraco.Core/PropertyEditors/DataEditor.cs b/src/Umbraco.Core/PropertyEditors/DataEditor.cs index c749c61300..add523ecf6 100644 --- a/src/Umbraco.Core/PropertyEditors/DataEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataEditor.cs @@ -19,7 +19,6 @@ namespace Umbraco.Core.PropertyEditors public class DataEditor : IDataEditor { private IDictionary _defaultConfiguration; - private IDataValueEditor _dataValueEditor; /// /// Initializes a new instance of the class. @@ -91,7 +90,7 @@ namespace Umbraco.Core.PropertyEditors /// simple enough for now. /// // TODO: point of that one? shouldn't we always configure? - public IDataValueEditor GetValueEditor() => ExplicitValueEditor ?? (_dataValueEditor ?? (_dataValueEditor = CreateValueEditor())); + public IDataValueEditor GetValueEditor() => ExplicitValueEditor ?? CreateValueEditor(); /// /// diff --git a/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs index 962737e1dc..edde8e8f81 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs @@ -107,13 +107,16 @@ namespace Umbraco.Tests.Persistence.Repositories var repository = CreateRepository(provider); // Act - var relationType = repository.Get(RelationTypeDto.NodeIdSeed); + var relationType = repository.Get(RelationTypeDto.NodeIdSeed + 2); // Assert Assert.That(relationType, Is.Not.Null); Assert.That(relationType.HasIdentity, Is.True); - Assert.That(relationType.Alias, Is.EqualTo("relateContentOnCopy")); - Assert.That(relationType.Name, Is.EqualTo("Relate Content on Copy")); + Assert.That(relationType.IsBidirectional, Is.True); + Assert.That(relationType.Alias, Is.EqualTo("relateContentToMedia")); + Assert.That(relationType.Name, Is.EqualTo("Relate Content to Media")); + Assert.That(relationType.ChildObjectType, Is.EqualTo(Constants.ObjectTypes.Media)); + Assert.That(relationType.ParentObjectType, Is.EqualTo(Constants.ObjectTypes.Document)); } } @@ -133,7 +136,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(relationTypes, Is.Not.Null); Assert.That(relationTypes.Any(), Is.True); Assert.That(relationTypes.Any(x => x == null), Is.False); - Assert.That(relationTypes.Count(), Is.EqualTo(7)); + Assert.That(relationTypes.Count(), Is.EqualTo(8)); } } @@ -190,7 +193,7 @@ namespace Umbraco.Tests.Persistence.Repositories int count = repository.Count(query); // Assert - Assert.That(count, Is.EqualTo(5)); + Assert.That(count, Is.EqualTo(6)); } } @@ -224,8 +227,9 @@ namespace Umbraco.Tests.Persistence.Repositories public void CreateTestData() { - var relateContent = new RelationType("Relate Content on Copy", "relateContentOnCopy", true, Constants.ObjectTypes.Document, new Guid("C66BA18E-EAF3-4CFF-8A22-41B16D66A972")); - var relateContentType = new RelationType("Relate ContentType on Copy", "relateContentTypeOnCopy", true, Constants.ObjectTypes.DocumentType, new Guid("A2CB7800-F571-4787-9638-BC48539A0EFB")); + var relateContent = new RelationType("Relate Content on Copy", "relateContentOnCopy", true, Constants.ObjectTypes.Document, Constants.ObjectTypes.Document); + var relateContentType = new RelationType("Relate ContentType on Copy", "relateContentTypeOnCopy", true, Constants.ObjectTypes.DocumentType, Constants.ObjectTypes.DocumentType); + var relateContentMedia = new RelationType("Relate Content to Media", "relateContentToMedia", true, Constants.ObjectTypes.Document, Constants.ObjectTypes.Media); var provider = TestObjects.GetScopeProvider(Logger); using (var scope = ScopeProvider.CreateScope()) @@ -234,6 +238,7 @@ namespace Umbraco.Tests.Persistence.Repositories repository.Save(relateContent);//Id 2 repository.Save(relateContentType);//Id 3 + repository.Save(relateContentMedia);//Id 4 scope.Complete(); } } diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs index 4221968429..834d211994 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs @@ -44,7 +44,7 @@ namespace Umbraco.Tests.PublishedContent _snapshotService?.Dispose(); } - private void Init(IEnumerable kits) + private void Init(Func> kits) { Current.Reset(); @@ -133,7 +133,7 @@ namespace Umbraco.Tests.PublishedContent _snapshotAccessor = new TestPublishedSnapshotAccessor(); // create a data source for NuCache - _source = new TestDataSource(kits); + _source = new TestDataSource(kits()); // at last, create the complete NuCache snapshot service! var options = new PublishedSnapshotServiceOptions { IgnoreLocalDb = true }; @@ -371,7 +371,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void EmptyTest() { - Init(Enumerable.Empty()); + Init(() => Enumerable.Empty()); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -383,7 +383,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void ChildrenTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -410,7 +410,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void ParentTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -436,7 +436,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void MoveToRootTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); // get snapshot var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); @@ -478,7 +478,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void MoveFromRootTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); // get snapshot var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); @@ -520,7 +520,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void ReOrderTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); // get snapshot var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); @@ -595,7 +595,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void MoveTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); // get snapshot var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); @@ -696,11 +696,61 @@ namespace Umbraco.Tests.PublishedContent Assert.AreEqual(1, snapshot.Content.GetById(7).Parent?.Id); } + [Test] + public void Clear_Branch_Locked() + { + // This test replicates an issue we saw here https://github.com/umbraco/Umbraco-CMS/pull/7907#issuecomment-610259393 + // The data was sent to me and this replicates it's structure + + var paths = new Dictionary { { -1, "-1" } }; + + Init(() => new List + { + CreateInvariantKit(1, -1, 1, paths), // first level + CreateInvariantKit(2, 1, 1, paths), // second level + CreateInvariantKit(3, 2, 1, paths), // third level + + CreateInvariantKit(4, 3, 1, paths), // fourth level (we'll copy this one to the same level) + + CreateInvariantKit(5, 4, 1, paths), // 6th level + + CreateInvariantKit(6, 5, 2, paths), // 7th level + CreateInvariantKit(7, 5, 3, paths), + CreateInvariantKit(8, 5, 4, paths), + CreateInvariantKit(9, 5, 5, paths), + CreateInvariantKit(10, 5, 6, paths) + }); + + // get snapshot + var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); + _snapshotAccessor.PublishedSnapshot = snapshot; + + var snapshotService = (PublishedSnapshotService)_snapshotService; + var contentStore = snapshotService.GetContentStore(); + //This will set a flag to force creating a new Gen next time the store is locked (i.e. In Notify) + contentStore.CreateSnapshot(); + + // notify - which ensures there are 2 generations in the cache meaning each LinkedNode has a Next value. + _snapshotService.Notify(new[] + { + new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) + }, out _, out _); + + // refresh the branch again, this used to show the issue where a null ref exception would occur + // because in the ClearBranchLocked logic, when SetValueLocked was called within a recursive call + // to a child, we null out the .Value of the LinkedNode within the while loop because we didn't capture + // this value before recursing. + Assert.DoesNotThrow(() => + _snapshotService.Notify(new[] + { + new ContentCacheRefresher.JsonPayload(4, Guid.Empty, TreeChangeTypes.RefreshBranch) + }, out _, out _)); + } + [Test] public void NestedVariationChildrenTest() { - var mixedKits = GetNestedVariantKits(); - Init(mixedKits); + Init(GetNestedVariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -789,7 +839,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void VariantChildrenTest() { - Init(GetVariantKits()); + Init(GetVariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -861,7 +911,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void RemoveTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -910,7 +960,7 @@ namespace Umbraco.Tests.PublishedContent [Test] public void UpdateTest() { - Init(GetInvariantKits()); + Init(GetInvariantKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -957,13 +1007,13 @@ namespace Umbraco.Tests.PublishedContent documents = snapshot.Content.GetById(2).Children().ToArray(); AssertDocuments(documents, "N9", "N8", "N7"); - + } [Test] public void AtRootTest() { - Init(GetVariantWithDraftKits()); + Init(GetVariantWithDraftKits); var snapshot = _snapshotService.CreatePublishedSnapshot(previewToken: null); _snapshotAccessor.PublishedSnapshot = snapshot; @@ -992,7 +1042,7 @@ namespace Umbraco.Tests.PublishedContent yield return CreateInvariantKit(2, 1, 1, paths); } - Init(GetKits()); + Init(GetKits); var snapshotService = (PublishedSnapshotService)_snapshotService; var contentStore = snapshotService.GetContentStore(); @@ -1031,7 +1081,7 @@ namespace Umbraco.Tests.PublishedContent yield return CreateInvariantKit(4, 1, 3, paths); } - Init(GetKits()); + Init(GetKits); var snapshotService = (PublishedSnapshotService)_snapshotService; var contentStore = snapshotService.GetContentStore(); @@ -1111,7 +1161,7 @@ namespace Umbraco.Tests.PublishedContent yield return CreateInvariantKit(40, 1, 3, paths); } - Init(GetKits()); + Init(GetKits); var snapshotService = (PublishedSnapshotService)_snapshotService; var contentStore = snapshotService.GetContentStore(); @@ -1130,7 +1180,7 @@ namespace Umbraco.Tests.PublishedContent _snapshotService.Notify(new[] { - new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshNode) + new ContentCacheRefresher.JsonPayload(1, Guid.Empty, TreeChangeTypes.RefreshNode) }, out _, out _); Assert.AreEqual(2, contentStore.Test.LiveGen); @@ -1150,7 +1200,7 @@ namespace Umbraco.Tests.PublishedContent /// 2) Save and publish it /// 3) Publish it with descendants /// 4) Repeat steps 2 and 3 - /// + /// /// Which has caused an exception. To replicate this test: /// 1) RefreshBranch with kits for a branch where the top most node is unpublished /// 2) RefreshBranch with kits for the branch where the top most node is published @@ -1178,12 +1228,12 @@ namespace Umbraco.Tests.PublishedContent //children of 1 yield return CreateInvariantKit(20, 1, 1, paths); - yield return CreateInvariantKit(30, 1, 2, paths); + yield return CreateInvariantKit(30, 1, 2, paths); yield return CreateInvariantKit(40, 1, 3, paths); } //init with all published - Init(GetKits()); + Init(GetKits); var snapshotService = (PublishedSnapshotService)_snapshotService; var contentStore = snapshotService.GetContentStore(); @@ -1200,7 +1250,7 @@ namespace Umbraco.Tests.PublishedContent //Change the root publish flag var kit = rootKit.Clone(); kit.DraftData = published ? null : kit.PublishedData; - kit.PublishedData = published? kit.PublishedData : null; + kit.PublishedData = published ? kit.PublishedData : null; _source.Kits[1] = kit; _snapshotService.Notify(new[] @@ -1215,12 +1265,12 @@ namespace Umbraco.Tests.PublishedContent var (gen, contentNode) = contentStore.Test.GetValues(1)[0]; Assert.AreEqual(assertGen, gen); //even when unpublishing/re-publishing/etc... the linked list is always maintained - AssertLinkedNode(contentNode, 100, 2, 3, 20, 40); + AssertLinkedNode(contentNode, 100, 2, 3, 20, 40); } //unpublish the root ChangePublishFlagOfRoot(false, 2, TreeChangeTypes.RefreshBranch); - + //publish the root (since it's not published, it will cause a RefreshBranch) ChangePublishFlagOfRoot(true, 3, TreeChangeTypes.RefreshBranch); @@ -1253,7 +1303,7 @@ namespace Umbraco.Tests.PublishedContent yield return CreateInvariantKit(4, 1, 3, paths); } - Init(GetKits()); + Init(GetKits); var snapshotService = (PublishedSnapshotService)_snapshotService; var contentStore = snapshotService.GetContentStore(); @@ -1313,14 +1363,13 @@ namespace Umbraco.Tests.PublishedContent public void MultipleCacheIteration() { //see https://github.com/umbraco/Umbraco-CMS/issues/7798 - this.Init(this.GetInvariantKits()); + Init(GetInvariantKits); var snapshot = this._snapshotService.CreatePublishedSnapshot(previewToken: null); - this._snapshotAccessor.PublishedSnapshot = snapshot; + _snapshotAccessor.PublishedSnapshot = snapshot; var items = snapshot.Content.GetByXPath("/root/itype"); Assert.AreEqual(items.Count(), items.Count()); } - private void AssertLinkedNode(ContentNode node, int parent, int prevSibling, int nextSibling, int firstChild, int lastChild) { Assert.AreEqual(parent, node.ParentContentId); diff --git a/src/Umbraco.Tests/Services/RelationServiceTests.cs b/src/Umbraco.Tests/Services/RelationServiceTests.cs index 2ec10811b7..06de405cec 100644 --- a/src/Umbraco.Tests/Services/RelationServiceTests.cs +++ b/src/Umbraco.Tests/Services/RelationServiceTests.cs @@ -110,8 +110,8 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Test", rt.Name); Assert.AreEqual("repeatedEventOccurence", rt.Alias); Assert.AreEqual(false, rt.IsBidirectional); - Assert.AreEqual(Constants.ObjectTypes.Document, rt.ChildObjectType.Value); - Assert.AreEqual(Constants.ObjectTypes.Media, rt.ParentObjectType.Value); + Assert.AreEqual(Constants.ObjectTypes.Document, rt.ParentObjectType.Value); + Assert.AreEqual(Constants.ObjectTypes.Media, rt.ChildObjectType.Value); } [Test] diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index e7b4223424..57a71a2cf0 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -348,6 +348,9 @@ 8700 / http://localhost:8700 + 8610 + / + http://localhost:8610 False False @@ -429,4 +432,4 @@ - \ No newline at end of file + diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index 28cc70f776..862837381a 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -192,6 +192,10 @@ namespace Umbraco.Web.PropertyEditors public IEnumerable GetReferences(object value) { var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString(); + + if (rawJson.IsNullOrWhiteSpace()) + yield break; + DeserializeGridValue(rawJson, out var richTextEditorValues, out var mediaValues); foreach (var umbracoEntityReference in richTextEditorValues.SelectMany(x => diff --git a/src/Umbraco.Web/PropertyEditors/MediaPickerPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/MediaPickerPropertyEditor.cs index ece210b9d1..fa8060bd15 100644 --- a/src/Umbraco.Web/PropertyEditors/MediaPickerPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/MediaPickerPropertyEditor.cs @@ -45,8 +45,11 @@ namespace Umbraco.Web.PropertyEditors if (string.IsNullOrEmpty(asString)) yield break; - if (Udi.TryParse(asString, out var udi)) - yield return new UmbracoEntityReference(udi); + foreach (var udiStr in asString.Split(',')) + { + if (Udi.TryParse(udiStr, out var udi)) + yield return new UmbracoEntityReference(udi); + } } } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index f92d8adebb..34d21497a2 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -873,9 +873,11 @@ namespace Umbraco.Web.PublishedCache.NuCache var id = content.FirstChildContentId; while (id > 0) { + // get the required link node, this ensures that both `link` and `link.Value` are not null var link = GetRequiredLinkedNode(id, "child", null); - ClearBranchLocked(link.Value); - id = link.Value.NextSiblingContentId; + var linkValue = link.Value; // capture local since clearing in recurse can clear it + ClearBranchLocked(linkValue); // recurse + id = linkValue.NextSiblingContentId; } }