From 98bda6759eeff194536974cc39509630d88ffe1d Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 8 Sep 2020 11:25:37 +1000 Subject: [PATCH] Adds more tests, cleans up tests --- .../BlockEditorComponentTests.cs | 318 ++++++++++++------ .../Compose/BlockEditorComponent.cs | 156 +++++---- .../Compose/NestedContentPropertyComponent.cs | 2 +- 3 files changed, 301 insertions(+), 175 deletions(-) diff --git a/src/Umbraco.Tests/PropertyEditors/BlockEditorComponentTests.cs b/src/Umbraco.Tests/PropertyEditors/BlockEditorComponentTests.cs index aafc05a640..bfd8b8c77b 100644 --- a/src/Umbraco.Tests/PropertyEditors/BlockEditorComponentTests.cs +++ b/src/Umbraco.Tests/PropertyEditors/BlockEditorComponentTests.cs @@ -1,6 +1,9 @@ using Newtonsoft.Json; using NUnit.Framework; using System; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core; using Umbraco.Web.Compose; namespace Umbraco.Tests.PropertyEditors @@ -15,52 +18,37 @@ namespace Umbraco.Tests.PropertyEditors }; + private const string _contentGuid1 = "036ce82586a64dfba2d523a99ed80f58"; + private const string _contentGuid2 = "48288c21a38a40ef82deb3eda90a58f6"; + private const string _settingsGuid1 = "ffd35c4e2eea4900abfa5611b67b2492"; + private const string _subContentGuid1 = "4c44ce6b3a5c4f5f8f15e3dc24819a9e"; + private const string _subContentGuid2 = "a062c06d6b0b44ac892b35d90309c7f8"; + private const string _subSettingsGuid1 = "4d998d980ffa4eee8afdc23c4abd6d29"; + + [Test] + public void Cannot_Have_Null_Udi() + { + var component = new BlockEditorComponent(); + var json = GetBlockListJson(null, string.Empty); + Assert.Throws(() => component.ReplaceBlockListUdis(json)); + } + [Test] public void No_Nesting() { - var guids = new[] { Guid.NewGuid(), Guid.NewGuid() }; + var guids = Enumerable.Range(0, 3).Select(x => Guid.NewGuid()).ToList(); var guidCounter = 0; Func guidFactory = () => guids[guidCounter++]; - var json = @"{ - ""layout"": - { - ""Umbraco.BlockList"": [ - { - ""contentUdi"": ""umb://element/036ce82586a64dfba2d523a99ed80f58"" - }, - { - ""contentUdi"": ""umb://element/48288c21a38a40ef82deb3eda90a58f6"" - } - ] - }, - ""contentData"": [ - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/036ce82586a64dfba2d523a99ed80f58"", - ""featureName"": ""Hello"", - ""featureDetails"": ""World"" - }, - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/48288c21a38a40ef82deb3eda90a58f6"", - ""featureName"": ""Another"", - ""featureDetails"": ""Feature"" - } - ], - ""settingsData"": [] -}"; + var json = GetBlockListJson(null); - - var expected = json - .Replace("036ce82586a64dfba2d523a99ed80f58", guids[0].ToString("N")) - .Replace("48288c21a38a40ef82deb3eda90a58f6", guids[1].ToString("N")); + var expected = ReplaceGuids(json, guids, _contentGuid1, _contentGuid2, _settingsGuid1); var component = new BlockEditorComponent(); - var result = component.ReplaceBlockListUdis(json, false, guidFactory); + var result = component.ReplaceBlockListUdis(json, guidFactory); - var expectedJson = JsonConvert.DeserializeObject(expected, _serializerSettings).ToString(); - var resultJson = JsonConvert.DeserializeObject(result, _serializerSettings).ToString(); + var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings); + var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings); Console.WriteLine(expectedJson); Console.WriteLine(resultJson); Assert.AreEqual(expectedJson, resultJson); @@ -69,91 +57,205 @@ namespace Umbraco.Tests.PropertyEditors [Test] public void One_Level_Nesting_Escaped() { - var guids = new[] { Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid() }; + var guids = Enumerable.Range(0, 6).Select(x => Guid.NewGuid()).ToList(); + var guidCounter = 0; Func guidFactory = () => guids[guidCounter++]; + var innerJson = GetBlockListJson(null, _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + // we need to ensure the escaped json is consistent with how it will be re-escaped after parsing // and this is how to do that, the result will also include quotes around it. - var innerJson = JsonConvert.DeserializeObject(@"{ - ""layout"": - { - ""Umbraco.BlockList"": [ - { - ""contentUdi"": ""umb://element/4C44CE6B3A5C4F5F8F15E3DC24819A9E"" - }, + var innerJsonEscaped = JsonConvert.ToString(innerJson); - { - ""contentUdi"": ""umb://element/A062C06D6B0B44AC892B35D90309C7F8"" - } - ] - }, - ""contentData"": [ - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/4C44CE6B3A5C4F5F8F15E3DC24819A9E"", - ""featureName"": ""Hello"", - ""featureDetails"": ""World"" - }, - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/A062C06D6B0B44AC892B35D90309C7F8"", - ""featureName"": ""Another"", - ""featureDetails"": ""Feature"" - } - ], - ""settingsData"": [] -}", _serializerSettings); - - var serializedInnerJson = JsonConvert.SerializeObject(innerJson, _serializerSettings); - - var subJsonEscaped = JsonConvert.ToString(serializedInnerJson); - - var json = @"{ - ""layout"": - { - ""Umbraco.BlockList"": [ - { - ""contentUdi"": ""umb://element/036ce82586a64dfba2d523a99ed80f58"" - }, - { - ""contentUdi"": ""umb://element/48288c21a38a40ef82deb3eda90a58f6"" - } - ] - }, - ""contentData"": [ - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/036ce82586a64dfba2d523a99ed80f58"", - ""featureName"": ""Hello"", - ""featureDetails"": ""World"", - ""subFeatures"": " + subJsonEscaped + @" - }, - { - ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", - ""udi"": ""umb://element/48288c21a38a40ef82deb3eda90a58f6"", - ""featureName"": ""Another"", - ""featureDetails"": ""Feature"" - } - ], - ""settingsData"": [] -}"; - - var expected = json - .Replace("036ce82586a64dfba2d523a99ed80f58", guids[0].ToString("N")) - .Replace("48288c21a38a40ef82deb3eda90a58f6", guids[1].ToString("N")) - .Replace("4C44CE6B3A5C4F5F8F15E3DC24819A9E", guids[2].ToString("N")) - .Replace("A062C06D6B0B44AC892B35D90309C7F8", guids[3].ToString("N")); + // get the json with the subFeatures as escaped + var json = GetBlockListJson(innerJsonEscaped); var component = new BlockEditorComponent(); - var result = component.ReplaceBlockListUdis(json, false, guidFactory); + var result = component.ReplaceBlockListUdis(json, guidFactory); - var expectedJson = JsonConvert.DeserializeObject(expected, _serializerSettings).ToString(); - var resultJson = JsonConvert.DeserializeObject(result, _serializerSettings).ToString(); + // the expected result is that the subFeatures data is no longer escaped + var expected = ReplaceGuids(GetBlockListJson(innerJson), guids, + _contentGuid1, _contentGuid2, _settingsGuid1, + _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + + var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings); + var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings); Console.WriteLine(expectedJson); Console.WriteLine(resultJson); Assert.AreEqual(expectedJson, resultJson); } + [Test] + public void One_Level_Nesting_Unescaped() + { + var guids = Enumerable.Range(0, 6).Select(x => Guid.NewGuid()).ToList(); + var guidCounter = 0; + Func guidFactory = () => guids[guidCounter++]; + + // nested blocks without property value escaping used in the conversion + var innerJson = GetBlockListJson(null, _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + + // get the json with the subFeatures as unescaped + var json = GetBlockListJson(innerJson); + + var expected = ReplaceGuids(GetBlockListJson(innerJson), guids, + _contentGuid1, _contentGuid2, _settingsGuid1, + _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + + var component = new BlockEditorComponent(); + var result = component.ReplaceBlockListUdis(json, guidFactory); + + var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings); + var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings); + Console.WriteLine(expectedJson); + Console.WriteLine(resultJson); + Assert.AreEqual(expectedJson, resultJson); + } + + [Test] + public void Nested_In_Complex_Editor_Escaped() + { + var guids = Enumerable.Range(0, 6).Select(x => Guid.NewGuid()).ToList(); + var guidCounter = 0; + Func guidFactory = () => guids[guidCounter++]; + + var innerJson = GetBlockListJson(null, _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + + // we need to ensure the escaped json is consistent with how it will be re-escaped after parsing + // and this is how to do that, the result will also include quotes around it. + var innerJsonEscaped = JsonConvert.ToString(innerJson); + + // Complex editor such as the grid + var complexEditorJsonEscaped = GetGridJson(innerJsonEscaped); + + var json = GetBlockListJson(complexEditorJsonEscaped); + + var component = new BlockEditorComponent(); + var result = component.ReplaceBlockListUdis(json, guidFactory); + + // the expected result is that the subFeatures data is no longer escaped + var expected = ReplaceGuids(GetBlockListJson(GetGridJson(innerJson)), guids, + _contentGuid1, _contentGuid2, _settingsGuid1, + _subContentGuid1, _subContentGuid2, _subSettingsGuid1); + + var expectedJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(expected, _serializerSettings), _serializerSettings); + var resultJson = JsonConvert.SerializeObject(JsonConvert.DeserializeObject(result, _serializerSettings), _serializerSettings); + Console.WriteLine(expectedJson); + Console.WriteLine(resultJson); + Assert.AreEqual(expectedJson, resultJson); + } + + private string GetBlockListJson(string subFeatures, + string contentGuid1 = _contentGuid1, + string contentGuid2 = _contentGuid2, + string settingsGuid1 = _settingsGuid1) + { + return @"{ + ""layout"": + { + ""Umbraco.BlockList"": [ + { + ""contentUdi"": """ + (contentGuid1.IsNullOrWhiteSpace() ? string.Empty : GuidUdi.Create(Constants.UdiEntityType.Element, Guid.Parse(contentGuid1)).ToString()) + @""" + }, + { + ""contentUdi"": ""umb://element/" + contentGuid2 + @""", + ""settingsUdi"": ""umb://element/" + settingsGuid1 + @""" + } + ] + }, + ""contentData"": [ + { + ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", + ""udi"": """ + (contentGuid1.IsNullOrWhiteSpace() ? string.Empty : GuidUdi.Create(Constants.UdiEntityType.Element, Guid.Parse(contentGuid1)).ToString()) + @""", + ""featureName"": ""Hello"", + ""featureDetails"": ""World"" + }, + { + ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", + ""udi"": ""umb://element/" + contentGuid2 + @""", + ""featureName"": ""Another"", + ""featureDetails"": ""Feature""" + (subFeatures == null ? string.Empty : (@", ""subFeatures"": " + subFeatures)) + @" + } + ], + ""settingsData"": [ + { + ""contentTypeKey"": ""d6ce4a86-91a2-45b3-a99c-8691fc1fb020"", + ""udi"": ""umb://element/" + settingsGuid1 + @""", + ""featureName"": ""Setting 1"", + ""featureDetails"": ""Setting 2"" + }, + ] +}"; + } + + private string GetGridJson(string subBlockList) + { + return @"{ + ""name"": ""1 column layout"", + ""sections"": [ + { + ""grid"": ""12"", + ""rows"": [ + { + ""name"": ""Article"", + ""id"": ""b4f6f651-0de3-ef46-e66a-464f4aaa9c57"", + ""areas"": [ + { + ""grid"": ""4"", + ""controls"": [ + { + ""value"": ""I am quote"", + ""editor"": { + ""alias"": ""quote"", + ""view"": ""textstring"" + }, + ""styles"": null, + ""config"": null + }], + ""styles"": null, + ""config"": null + }, + { + ""grid"": ""8"", + ""controls"": [ + { + ""value"": ""Header"", + ""editor"": { + ""alias"": ""headline"", + ""view"": ""textstring"" + }, + ""styles"": null, + ""config"": null + }, + { + ""value"": " + subBlockList + @", + ""editor"": { + ""alias"": ""madeUpNestedContent"", + ""view"": ""madeUpNestedContentInGrid"" + }, + ""styles"": null, + ""config"": null + }], + ""styles"": null, + ""config"": null + }], + ""styles"": null, + ""config"": null + }] + }] +}"; + } + + private string ReplaceGuids(string json, List newGuids, params string[] oldGuids) + { + for (var i = 0; i < oldGuids.Length; i++) + { + var old = oldGuids[i]; + json = json.Replace(old, newGuids[i].ToString("N")); + } + return json; + } + } } diff --git a/src/Umbraco.Web/Compose/BlockEditorComponent.cs b/src/Umbraco.Web/Compose/BlockEditorComponent.cs index ecd94ff2fb..0666ba4461 100644 --- a/src/Umbraco.Web/Compose/BlockEditorComponent.cs +++ b/src/Umbraco.Web/Compose/BlockEditorComponent.cs @@ -10,8 +10,6 @@ using Umbraco.Core.PropertyEditors; namespace Umbraco.Web.Compose { - - /// /// A component for Block editors used to bind to events /// @@ -29,10 +27,10 @@ namespace Umbraco.Web.Compose public void Terminate() => _handler?.Dispose(); - private string ReplaceBlockListUdis(string rawJson, bool onlyMissingUdis) => ReplaceBlockListUdis(rawJson, onlyMissingUdis, null); + private string ReplaceBlockListUdis(string rawJson, bool onlyMissingUdis) => ReplaceBlockListUdis(rawJson, null); // internal for tests - internal string ReplaceBlockListUdis(string rawJson, bool onlyMissingUdis, Func createGuid = null, JsonSerializerSettings serializerSettings = null) + internal string ReplaceBlockListUdis(string rawJson, Func createGuid = null) { // used so we can test nicely if (createGuid == null) @@ -42,18 +40,19 @@ namespace Umbraco.Web.Compose return rawJson; // Parse JSON + // This will throw a FormatException if there are null UDIs (expected) var blockListValue = _converter.Deserialize(rawJson); - UpdateBlockListRecursively(blockListValue, onlyMissingUdis, createGuid, serializerSettings); + UpdateBlockListRecursively(blockListValue, createGuid); - return JsonConvert.SerializeObject(blockListValue.BlockValue, serializerSettings); + return JsonConvert.SerializeObject(blockListValue.BlockValue); } - private void UpdateBlockListRecursively(BlockEditorData blockListData, bool onlyMissingKeys, Func createGuid, JsonSerializerSettings serializerSettings) + private void UpdateBlockListRecursively(BlockEditorData blockListData, Func createGuid) { var oldToNew = new Dictionary(); - MapOldToNewUdis(oldToNew, blockListData.BlockValue.ContentData, onlyMissingKeys, createGuid); - MapOldToNewUdis(oldToNew, blockListData.BlockValue.SettingsData, onlyMissingKeys, createGuid); + MapOldToNewUdis(oldToNew, blockListData.BlockValue.ContentData, createGuid); + MapOldToNewUdis(oldToNew, blockListData.BlockValue.SettingsData, createGuid); for (var i = 0; i < blockListData.References.Count; i++) { @@ -83,68 +82,39 @@ namespace Umbraco.Web.Compose } - RecursePropertyValues(blockListData.BlockValue.ContentData, onlyMissingKeys, createGuid, serializerSettings); - RecursePropertyValues(blockListData.BlockValue.SettingsData, onlyMissingKeys, createGuid, serializerSettings); + RecursePropertyValues(blockListData.BlockValue.ContentData, createGuid); + RecursePropertyValues(blockListData.BlockValue.SettingsData, createGuid); } - private void RecursePropertyValues(IEnumerable blockData, bool onlyMissingKeys, Func createGuid, JsonSerializerSettings serializerSettings) + private void RecursePropertyValues(IEnumerable blockData, Func createGuid) { foreach (var data in blockData) { // check if we need to recurse (make a copy of the dictionary since it will be modified) foreach (var propertyAliasToBlockItemData in new Dictionary(data.RawPropertyValues)) { - var asString = propertyAliasToBlockItemData.Value?.ToString(); - - if (asString != null && asString.DetectIsJson()) + if (propertyAliasToBlockItemData.Value is JToken jtoken) { - // this gets a little ugly because there could be some other complex editor that contains another block editor - // and since we would have no idea how to parse that, all we can do is try JSON Path to find another block editor - // of our type - var json = JToken.Parse(asString); - - // select all tokens (flatten) - var allProperties = json.SelectTokens("$..*").Select(x => x.Parent as JProperty).WhereNotNull().ToList(); - foreach (var prop in allProperties) + if (ProcessJToken(jtoken, createGuid, out var result)) { - if (prop.Name == Constants.PropertyEditors.Aliases.BlockList) - { - // get it's parent 'layout' and it's parent's container - var layout = prop.Parent?.Parent as JProperty; - if (layout != null && layout.Parent is JObject layoutJson) - { - // recurse - var blockListValue = _converter.ConvertFrom(layoutJson); - UpdateBlockListRecursively(blockListValue, onlyMissingKeys, createGuid, serializerSettings); + // need to re-save this back to the RawPropertyValues + data.RawPropertyValues[propertyAliasToBlockItemData.Key] = result; + } + } + else + { + var asString = propertyAliasToBlockItemData.Value?.ToString(); - // set new value - if (layoutJson.Parent != null) - { - // we can replace the sub string - layoutJson.Replace(JsonConvert.SerializeObject(blockListValue.BlockValue, serializerSettings)); - } - else - { - // this was the root string - data.RawPropertyValues[propertyAliasToBlockItemData.Key] = JsonConvert.SerializeObject(blockListValue.BlockValue, serializerSettings); - } - } - } - else if (prop.Name != "layout" && prop.Name != "contentData" && prop.Name != "settingsData" && prop.Name != "contentTypeKey") + if (asString != null && asString.DetectIsJson()) + { + // this gets a little ugly because there could be some other complex editor that contains another block editor + // and since we would have no idea how to parse that, all we can do is try JSON Path to find another block editor + // of our type + var json = JToken.Parse(asString); + if (ProcessJToken(json, createGuid, out var result)) { - // this is an arbitrary property that could contain a nested complex editor - var propVal = prop.Value?.ToString(); - // check if this might contain a nested Block Editor - if (!propVal.IsNullOrWhiteSpace() && propVal.DetectIsJson() && propVal.InvariantContains(Constants.PropertyEditors.Aliases.BlockList)) - { - if (_converter.TryDeserialize(propVal, out var nestedBlockData)) - { - // recurse - UpdateBlockListRecursively(nestedBlockData, onlyMissingKeys, createGuid, serializerSettings); - // set the value to the updated one - prop.Value = JsonConvert.SerializeObject(nestedBlockData.BlockValue, serializerSettings); - } - } + // need to re-save this back to the RawPropertyValues + data.RawPropertyValues[propertyAliasToBlockItemData.Key] = result; } } } @@ -152,20 +122,74 @@ namespace Umbraco.Web.Compose } } - private void MapOldToNewUdis(Dictionary oldToNew, IEnumerable blockData, bool onlyMissingKeys, Func createGuid) + private bool ProcessJToken(JToken json, Func createGuid, out JToken result) + { + var updated = false; + result = json; + + // select all tokens (flatten) + var allProperties = json.SelectTokens("$..*").Select(x => x.Parent as JProperty).WhereNotNull().ToList(); + foreach (var prop in allProperties) + { + if (prop.Name == Constants.PropertyEditors.Aliases.BlockList) + { + // get it's parent 'layout' and it's parent's container + var layout = prop.Parent?.Parent as JProperty; + if (layout != null && layout.Parent is JObject layoutJson) + { + // recurse + var blockListValue = _converter.ConvertFrom(layoutJson); + UpdateBlockListRecursively(blockListValue, createGuid); + + // set new value + if (layoutJson.Parent != null) + { + // we can replace the object + layoutJson.Replace(JObject.FromObject(blockListValue.BlockValue)); + updated = true; + } + else + { + // if there is no parent it means that this json property was the root, in which case we just return + result = JObject.FromObject(blockListValue.BlockValue); + return true; + } + } + } + else if (prop.Name != "layout" && prop.Name != "contentData" && prop.Name != "settingsData" && prop.Name != "contentTypeKey") + { + // this is an arbitrary property that could contain a nested complex editor + var propVal = prop.Value?.ToString(); + // check if this might contain a nested Block Editor + if (!propVal.IsNullOrWhiteSpace() && propVal.DetectIsJson() && propVal.InvariantContains(Constants.PropertyEditors.Aliases.BlockList)) + { + if (_converter.TryDeserialize(propVal, out var nestedBlockData)) + { + // recurse + UpdateBlockListRecursively(nestedBlockData, createGuid); + // set the value to the updated one + prop.Value = JObject.FromObject(nestedBlockData.BlockValue); + updated = true; + } + } + } + } + + return updated; + } + + private void MapOldToNewUdis(Dictionary oldToNew, IEnumerable blockData, Func createGuid) { foreach (var data in blockData) { + // This should never happen since a FormatException will be thrown if one is empty but we'll keep this here if (data.Udi == null) throw new InvalidOperationException("Block data cannot contain a null UDI"); // replace the UDIs - if (!onlyMissingKeys) - { - var newUdi = GuidUdi.Create(Constants.UdiEntityType.Element, createGuid()); - oldToNew[data.Udi] = newUdi; - data.Udi = newUdi; - } + var newUdi = GuidUdi.Create(Constants.UdiEntityType.Element, createGuid()); + oldToNew[data.Udi] = newUdi; + data.Udi = newUdi; } } } diff --git a/src/Umbraco.Web/Compose/NestedContentPropertyComponent.cs b/src/Umbraco.Web/Compose/NestedContentPropertyComponent.cs index 9414a1a836..633e814bd9 100644 --- a/src/Umbraco.Web/Compose/NestedContentPropertyComponent.cs +++ b/src/Umbraco.Web/Compose/NestedContentPropertyComponent.cs @@ -42,7 +42,7 @@ namespace Umbraco.Web.Compose if (string.IsNullOrWhiteSpace(rawJson) || !rawJson.DetectIsJson()) return rawJson; - // Parse JSON + // Parse JSON var complexEditorValue = JToken.Parse(rawJson); UpdateNestedContentKeysRecursively(complexEditorValue, onlyMissingKeys, createGuid);