From 4eb54e4ed941b26376546233402721d3415c3f31 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 10 Aug 2020 16:08:34 +0100 Subject: [PATCH 1/9] Add Parser to find Udi's in Macro Parameters in Macros in RTE + Grid --- .../Templates/HtmlMacroParameterParser.cs | 108 ++++++++++++++++++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + 2 files changed, 109 insertions(+) create mode 100644 src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs diff --git a/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs b/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs new file mode 100644 index 000000000000..c944ab45024d --- /dev/null +++ b/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs @@ -0,0 +1,108 @@ +using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Web.Macros; + +namespace Umbraco.Web.Templates +{ + + public sealed class HtmlMacroParameterParser + { + public HtmlMacroParameterParser() + { + + } + + /// + /// Parses out media UDIs from an html string based on embedded macro parameter values + /// + /// + /// + public IEnumerable FindUdisFromMacroParameters(string text) + { + // we already have a method for finding Macros within a Rich Text Editor using regexes: Umbraco.Web.Macros.MacroTagParser + // it searches for 'text blocks' which I think are the old way of embedding macros in a template, to allow inline code - we're not interested in those in V8 + // but we are interested in the foundMacros, and in particular their parameters and values + // (aside TODO: at somepoint we might want to track Macro usage?) + // ideally we'd determine each type of parameter editor and call it's GetReferences implementation + // however this information isn't stored in the embedded macro markup and would involve 'looking up' types for each parameter + // (or we could change the macros markup to include details of the editor) + // but pragmatically we don't really care what type of property editor it is if it stores media udis + // we want to track media udis wherever they are used regardless of how they are picked? + // so here we parse the parameters and check to see if they are in the format umb://media + // see also https://github.com/umbraco/Umbraco-CMS/pull/8388 + + List macroParameterValues = new List(); + MacroTagParser.ParseMacros(text, textblock => { }, (macroAlias, macroAttributes) => macroParameterValues.AddRange(macroAttributes.Values.ToList().Where(f=>f.StartsWith("umb://" + Constants.UdiEntityType.Media + "/")))); + + if (macroParameterValues.Any()) + { + // we have media udis! - but we could also have a csv of multiple media udis items + foreach (var macroParameterValue in macroParameterValues.Distinct()) + { + string[] potentialUdis = macroParameterValue.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries); + foreach (var potentialUdi in potentialUdis.Distinct()) + { + if(Udi.TryParse(potentialUdi, out var udi)) + { + yield return udi; + } + } + } + } + else + { + yield break; + } + } + /// + /// Parses out media UDIs from Macro Grid Control Parameters + /// + /// + public IEnumerable FindUdisFromGridControlMacroParameters(IEnumerable macroValues) + { + List macroParameterValues = new List(); + foreach (var macroValue in macroValues) + { + //deserialise JSON of Macro Grid Control to a class + var gridMacro = macroValue.Value.ToObject(); + //collect any macro parameters that contain the media udi format + if (gridMacro != null && gridMacro.MacroParameters!=null && gridMacro.MacroParameters.Any()) + { + macroParameterValues.AddRange(gridMacro.MacroParameters.Values.ToList().Where(f => f.Contains("umb://" + Constants.UdiEntityType.Media + "/"))); + } + } + if (macroParameterValues.Any()) + { + // we have media udis! - but we could also have a csv of multiple items + foreach (var macroParameterValue in macroParameterValues.Distinct()) + { + string[] potentialUdis = macroParameterValue.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries); + foreach (var potentialUdi in potentialUdis.Distinct()) + { + if (Udi.TryParse(potentialUdi, out var udi)) + { + yield return udi; + } + } + } + } + else + { + yield break; + } + } + // poco class to deserialise the Json for a Macro Control + private class GridMacro + { + [JsonProperty("macroAlias")] + public string MacroAlias { get; set; } + [JsonProperty("macroParamsDictionary")] + public Dictionary MacroParameters { get; set; } + } + } +} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 2ca7d6d7ae65..b599e606cabf 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -272,6 +272,7 @@ + From bf5b356b39464f4e4a038accb0217f3498b7f68e Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 10 Aug 2020 16:09:06 +0100 Subject: [PATCH 2/9] Register HtmlMacroParameterParser with DI --- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index 60521e6d907b..b0172e16a443 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -109,6 +109,7 @@ public override void Compose(Composition composition) composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); + composition.RegisterUnique(); composition.RegisterUnique(); // register the umbraco helper - this is Transient! very important! From 5a0bc3dcbce0cb2e83cba5b500eba111508f6d90 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 10 Aug 2020 16:10:06 +0100 Subject: [PATCH 3/9] Some Tests to cover the parsing of text/json for Macros and their parameters --- .../HtmlMacroParameterParserTests.cs | 154 ++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 2 files changed, 155 insertions(+) create mode 100644 src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs diff --git a/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs b/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs new file mode 100644 index 000000000000..39a48bbe4c9e --- /dev/null +++ b/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs @@ -0,0 +1,154 @@ +using Umbraco.Core.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Tests.Testing.Objects.Accessors; +using Umbraco.Web.Templates; +using Umbraco.Web; +using Umbraco.Core.Models.PublishedContent; +using Umbraco.Web.Routing; +using Umbraco.Tests.Testing.Objects; +using System.Web; +using System; +using System.Linq; +using Umbraco.Core.Models; +using Umbraco.Core; +using System.Diagnostics; +using Newtonsoft.Json.Linq; +using System.Collections.Generic; + +namespace Umbraco.Tests.Templates +{ + [TestFixture] + public class HtmlMacroParameterParserTests + { + [Test] + public void Returns_Udis_From_Single_MediaPicker_Macro_Parameters_In_Macros_In_Html() + { + //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker + var input = @"

This is some normal text before the macro

+ +

This is a paragraph after the macro and before the next

+ +

some more text

"; + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); + Assert.AreEqual(3, result.Count); + Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); + Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); + Assert.AreEqual(Udi.Parse("umb://media/90ba0d3dba6e4c9fa1953db78352ba73"), result[2]); + } + [Test] + public void Returns_Empty_With_From_Single_MediaPicker_With_No_Macro_Parameters_In_Macros_In_Html() + { + //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker + var input = @"

This is some normal text before the macro

+ +

This is a paragraph after the macro and before the next

+ Some other value

"" /> +

some more text

"; + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); + Assert.AreEqual(0, result.Count); + } + //NB: When multiple media pickers store udis instead of ints! - see https://github.com/umbraco/Umbraco-CMS/pull/8388 + [Test] + public void Returns_Empty_When_No_Macros_In_Html() + { + //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker + var input = @"

This is some normal text before the macro

+

This is a paragraph after the macro and before the next

+

some more text

"; + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); + Assert.AreEqual(0, result.Count); + } + [Test] + public void Returns_Udis_From_Multiple_MediaPicker_Macro_Parameters_In_Macros_In_Html() + { + //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker + var input = @"

This is some normal text before the macro

+ +

This is a paragraph after the macro

"; + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); + Assert.AreEqual(3, result.Count); + Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); + Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); + Assert.AreEqual(Udi.Parse("umb://media/bb763e0d0ceb408c8720365d57e06444"), result[2]); + } + [Test] + public void Returns_Udis_From_Single_MediaPicker_Macro_Parameters_In_Grid_Macros() + { + // create a list of GridValue.GridControls with Editor GridEditor alias macro + List gridControls = new List(); + + // single media picker macro parameter + var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": { ""singleMediaPicker"": ""umb://media/90ba0d3dba6e4c9fa1953db78352ba73"" }}"); + gridControls.Add(macroGridControl); + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); + Assert.AreEqual(1, result.Count); + Assert.AreEqual(Udi.Parse("umb://media/90ba0d3dba6e4c9fa1953db78352ba73"), result[0]); + + } + [Test] + public void Returns_Empty_From_Single_MediaPicker_With_No_Macro_Parameters_In_Grid_Macros() + { + // create a list of GridValue.GridControls with Editor GridEditor alias macro + List gridControls = new List(); + + // single media picker macro parameter + var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": {}}"); + gridControls.Add(macroGridControl); + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); + Assert.AreEqual(0, result.Count); + + } + //NB: When multiple media pickers store udis instead of ints! - see https://github.com/umbraco/Umbraco-CMS/pull/8388 + [Test] + public void Returns_Udis_From_Multiple_MediaPicker_Macro_Parameters_In_Grid_Macros() + { + // create a list of GridValue.GridControls with Editor GridEditor alias macro + List gridControls = new List(); + + // multiple media picker macro parameter + var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": { ""multipleMediaPicker"": ""umb://media/eee91c05b2e84031a056dcd7f28eff89,umb://media/fa763e0d0ceb408c8720365d57e06e32,umb://media/bb763e0d0ceb408c8720365d57e06444"" }}"); + gridControls.Add(macroGridControl); + + var macroParameterParser = new HtmlMacroParameterParser(); + + var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); + Assert.AreEqual(3, result.Count); + Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); + Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); + Assert.AreEqual(Udi.Parse("umb://media/bb763e0d0ceb408c8720365d57e06444"), result[2]); + + } + + //setup a Macro Grid Control based on Json of the Macro + private GridValue.GridControl GetMacroGridControl(string macroJson) + { + var macroGridEditor = new GridValue.GridEditor(); + macroGridEditor.Alias = "macro"; + macroGridEditor.View = "macro"; + var macroGridControl = new GridValue.GridControl(); + macroGridControl.Editor = macroGridEditor; + macroGridControl.Value = JToken.Parse(macroJson); + return macroGridControl; + } + + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 3c359cdde82b..5604c6915eeb 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -167,6 +167,7 @@ + From 3b38bc64ee39caa2421afa810c9c9b2d32e547cb Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 10 Aug 2020 16:10:47 +0100 Subject: [PATCH 4/9] Inject the HtmlMacroParameterParser into GridPropertyEditor + RichTextPropertyEditor to hunt for media Udis in Macro Parameters --- .../PropertyEditors/GridPropertyEditor.cs | 57 +++++++++++++++---- .../PropertyEditors/RichTextPropertyEditor.cs | 23 +++++--- 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index 862837381a24..eafbe303672a 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -29,6 +29,7 @@ public class GridPropertyEditor : DataEditor { private IUmbracoContextAccessor _umbracoContextAccessor; private readonly HtmlImageSourceParser _imageSourceParser; + private readonly HtmlMacroParameterParser _macroParameterParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly HtmlLocalLinkParser _localLinkParser; private readonly IImageUrlGenerator _imageUrlGenerator; @@ -39,15 +40,25 @@ public GridPropertyEditor(ILogger logger, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser) - : this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, Current.ImageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, Current.Factory.GetInstance(),Current.ImageUrlGenerator) + { + } + [Obsolete("Use the constructor which takes an HtmlMacroParameterParser")] + public GridPropertyEditor(ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, Current.Factory.GetInstance(), imageUrlGenerator) { } - public GridPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, + HtmlMacroParameterParser macroParameterParser, IImageUrlGenerator imageUrlGenerator) : base(logger) { @@ -55,6 +66,7 @@ public GridPropertyEditor(ILogger logger, _imageSourceParser = imageSourceParser; _pastedImages = pastedImages; _localLinkParser = localLinkParser; + _macroParameterParser = macroParameterParser; _imageUrlGenerator = imageUrlGenerator; } @@ -72,6 +84,7 @@ internal class GridPropertyValueEditor : DataValueEditor, IDataValueReference { private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly HtmlImageSourceParser _imageSourceParser; + private readonly HtmlMacroParameterParser _macroParameterParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly RichTextPropertyEditor.RichTextPropertyValueEditor _richTextPropertyValueEditor; private readonly MediaPickerPropertyEditor.MediaPickerPropertyValueEditor _mediaPickerPropertyValueEditor; @@ -83,23 +96,34 @@ public GridPropertyValueEditor(DataEditorAttribute attribute, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser) - : this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, Current.ImageUrlGenerator) + : this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser,Current.Factory.GetInstance(), Current.ImageUrlGenerator) + { + } + [Obsolete("Use the constructor which takes an HtmlMacroParameterParser")] + public GridPropertyValueEditor(DataEditorAttribute attribute, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator) + : this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, Current.Factory.GetInstance(), imageUrlGenerator) { } - public GridPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, + HtmlMacroParameterParser macroParameterParser, IImageUrlGenerator imageUrlGenerator) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; _imageSourceParser = imageSourceParser; _pastedImages = pastedImages; - _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator); + _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, macroParameterParser, pastedImages, _imageUrlGenerator); _mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute); + _macroParameterParser = macroParameterParser; _imageUrlGenerator = imageUrlGenerator; } @@ -125,7 +149,7 @@ public override object FromEditor(ContentPropertyData editorValue, object curren var mediaParent = config?.MediaParentId; var mediaParentId = mediaParent == null ? Guid.Empty : mediaParent.Guid; - var grid = DeserializeGridValue(rawJson, out var rtes, out _); + var grid = DeserializeGridValue(rawJson, out var rtes, out _, out _); var userId = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser?.Id ?? Constants.Security.SuperUserId; @@ -158,7 +182,7 @@ public override object ToEditor(Property property, IDataTypeService dataTypeServ var val = property.GetValue(culture, segment)?.ToString(); if (val.IsNullOrWhiteSpace()) return string.Empty; - var grid = DeserializeGridValue(val, out var rtes, out _); + var grid = DeserializeGridValue(val, out var rtes, out _, out _); //process the rte values foreach (var rte in rtes.ToList()) @@ -172,7 +196,7 @@ public override object ToEditor(Property property, IDataTypeService dataTypeServ return grid; } - private GridValue DeserializeGridValue(string rawJson, out IEnumerable richTextValues, out IEnumerable mediaValues) + private GridValue DeserializeGridValue(string rawJson, out IEnumerable richTextValues, out IEnumerable mediaValues, out IEnumerable macroValues) { var grid = JsonConvert.DeserializeObject(rawJson); @@ -180,7 +204,8 @@ private GridValue DeserializeGridValue(string rawJson, out IEnumerable x.Rows.SelectMany(r => r.Areas).SelectMany(a => a.Controls)).ToArray(); richTextValues = controls.Where(x => x.Editor.Alias.ToLowerInvariant() == "rte"); mediaValues = controls.Where(x => x.Editor.Alias.ToLowerInvariant() == "media"); - + // Find all the macros + macroValues = controls.Where(x => x.Editor.Alias.ToLowerInvariant() == "macro"); return grid; } @@ -196,7 +221,7 @@ public IEnumerable GetReferences(object value) if (rawJson.IsNullOrWhiteSpace()) yield break; - DeserializeGridValue(rawJson, out var richTextEditorValues, out var mediaValues); + DeserializeGridValue(rawJson, out var richTextEditorValues, out var mediaValues, out var macroValues); foreach (var umbracoEntityReference in richTextEditorValues.SelectMany(x => _richTextPropertyValueEditor.GetReferences(x.Value))) @@ -205,6 +230,18 @@ public IEnumerable GetReferences(object value) foreach (var umbracoEntityReference in mediaValues.SelectMany(x => _mediaPickerPropertyValueEditor.GetReferences(x.Value["udi"]))) yield return umbracoEntityReference; + + //Macros don't really have a Property Editor for which we can call GetRefererences + //where does the responsibility lie for MacroParametersEditors to report their references? + //when we don't easily know the property type for a parameter - without 'looking up' which would be expensive? + //pragmatically we only care if the parameter has a value that is a media udi eg umb://media + //so we 'could' just loop through all parameter values here and add references for any values that are media udis... eg they are in use! however they are picked + //Is the HtmlMacroParameterParser the right place to put this method? + var udis = _macroParameterParser.FindUdisFromGridControlMacroParameters(macroValues); + foreach (var udi in udis) + { + yield return new UmbracoEntityReference(udi); + } } } } diff --git a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs index 42777f11add0..2d8dd6312c9a 100644 --- a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs @@ -30,6 +30,7 @@ public class RichTextPropertyEditor : DataEditor private IUmbracoContextAccessor _umbracoContextAccessor; private readonly HtmlImageSourceParser _imageSourceParser; private readonly HtmlLocalLinkParser _localLinkParser; + private readonly HtmlMacroParameterParser _macroParameterParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; @@ -39,19 +40,24 @@ public class RichTextPropertyEditor : DataEditor /// [Obsolete("Use the constructor which takes an IImageUrlGenerator")] public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages) - : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, Current.ImageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser,Current.Factory.GetInstance(), pastedImages, Current.ImageUrlGenerator) + { + } + [Obsolete("Use the constructor which takes a HtmlMacroParameterParser instead")] + public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, Current.Factory.GetInstance(), pastedImages, imageUrlGenerator) { } - /// /// The constructor will setup the property editor based on the attribute if one is found /// - public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, HtmlMacroParameterParser macroParameterParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) : base(logger) { _umbracoContextAccessor = umbracoContextAccessor; _imageSourceParser = imageSourceParser; _localLinkParser = localLinkParser; + _macroParameterParser = macroParameterParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; } @@ -60,7 +66,7 @@ public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoCon /// Create a custom value editor /// /// - protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator); + protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _macroParameterParser,_pastedImages, _imageUrlGenerator); protected override IConfigurationEditor CreateConfigurationEditor() => new RichTextConfigurationEditor(); @@ -74,15 +80,17 @@ internal class RichTextPropertyValueEditor : DataValueEditor, IDataValueReferenc private IUmbracoContextAccessor _umbracoContextAccessor; private readonly HtmlImageSourceParser _imageSourceParser; private readonly HtmlLocalLinkParser _localLinkParser; + private readonly HtmlMacroParameterParser _macroParameterParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; - public RichTextPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser,HtmlMacroParameterParser macroParameterParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; _imageSourceParser = imageSourceParser; _localLinkParser = localLinkParser; + _macroParameterParser = macroParameterParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; } @@ -156,10 +164,11 @@ public IEnumerable GetReferences(object value) foreach (var udi in _imageSourceParser.FindUdisFromDataAttributes(asString)) yield return new UmbracoEntityReference(udi); - + foreach (var udi in _macroParameterParser.FindUdisFromMacroParameters(asString)) + yield return new UmbracoEntityReference(udi); foreach (var udi in _localLinkParser.FindUdisFromLocalLinks(asString)) yield return new UmbracoEntityReference(udi); - + // is the TODO below a reference to needing to scan for media used in Macro parameters (see _macroParameterParser above) or a more general future TODO for some kind of 'tracking macro use' function? (is there an issue for this?) //TODO: Detect Macros too ... but we can save that for a later date, right now need to do media refs } } From b7c937b2b536b6dd20c2237dd24f907cec0ed012 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 17 Aug 2020 12:55:56 +0100 Subject: [PATCH 5/9] Add GetAll implementation to MacroService for multiple Macro Aliases Calling new MacroRepository methods cached with a CustomCachePolicy which gets cleared in the MacroCacheRefresher --- .../Repositories/IMacroRepository.cs | 3 +++ .../Repositories/Implement/MacroRepository.cs | 24 ++++++++++++++++++- src/Umbraco.Core/Services/IMacroService.cs | 6 ++--- .../Services/Implement/MacroService.cs | 10 ++++++-- .../Services/MacroServiceTests.cs | 14 +++++++++++ src/Umbraco.Web/Cache/MacroCacheRefresher.cs | 5 +++- 6 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/IMacroRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IMacroRepository.cs index 1ed08352ed64..dc957a3bcc7f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IMacroRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IMacroRepository.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Umbraco.Core.Models; namespace Umbraco.Core.Persistence.Repositories @@ -7,6 +8,8 @@ public interface IMacroRepository : IReadWriteQueryRepository, IRea { //IEnumerable GetAll(params string[] aliases); + IMacro GetByAlias(string alias); + IEnumerable GetAllByAlias(string[] aliases); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs index 1f0b45614b1d..b140e51213c6 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MacroRepository.cs @@ -15,9 +15,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { internal class MacroRepository : NPocoRepositoryBase, IMacroRepository { + private readonly IRepositoryCachePolicy _macroByAliasCachePolicy; public MacroRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger) : base(scopeAccessor, cache, logger) - { } + { + _macroByAliasCachePolicy = new DefaultRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, DefaultOptions); + } protected override IMacro PerformGet(int id) { @@ -213,5 +216,24 @@ protected override void PersistUpdatedItem(IMacro entity) entity.ResetDirtyProperties(); } + + public IMacro GetByAlias(string alias) + { + return _macroByAliasCachePolicy.Get(alias, PerformGetByAlias, PerformGetAllByAlias); + } + public IEnumerable GetAllByAlias(string[] aliases) + { + return _macroByAliasCachePolicy.GetAll(aliases, PerformGetAllByAlias); + } + private IMacro PerformGetByAlias(string alias) + { + var query = Query().Where(x => x.Alias.Equals(alias)); + return PerformGetByQuery(query).FirstOrDefault(); + } + private IEnumerable PerformGetAllByAlias(params string[] aliases) + { + var query = Query().WhereIn(x => x.Alias, aliases); + return PerformGetByQuery(query); + } } } diff --git a/src/Umbraco.Core/Services/IMacroService.cs b/src/Umbraco.Core/Services/IMacroService.cs index 597c986f3769..cb120d951b07 100644 --- a/src/Umbraco.Core/Services/IMacroService.cs +++ b/src/Umbraco.Core/Services/IMacroService.cs @@ -18,12 +18,12 @@ public interface IMacroService : IService IMacro GetByAlias(string alias); ///// - ///// Gets a list all available objects + ///// Gets a list of available objects by Alias ///// ///// Optional array of aliases to limit the results ///// An enumerable list of objects - //IEnumerable GetAll(params string[] aliases); - + IEnumerable GetAll(params string[] aliases); + IEnumerable GetAll(); IEnumerable GetAll(params int[] ids); diff --git a/src/Umbraco.Core/Services/Implement/MacroService.cs b/src/Umbraco.Core/Services/Implement/MacroService.cs index a6631aae4c2e..db71bbf114dd 100644 --- a/src/Umbraco.Core/Services/Implement/MacroService.cs +++ b/src/Umbraco.Core/Services/Implement/MacroService.cs @@ -34,8 +34,14 @@ public IMacro GetByAlias(string alias) { using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { - var q = Query().Where(x => x.Alias == alias); - return _macroRepository.Get(q).FirstOrDefault(); + return _macroRepository.GetByAlias(alias); + } + } + public IEnumerable GetAll(params string[] aliases) + { + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) + { + return _macroRepository.GetAllByAlias(aliases); } } diff --git a/src/Umbraco.Tests/Services/MacroServiceTests.cs b/src/Umbraco.Tests/Services/MacroServiceTests.cs index 69e816585eb4..97c3bea2e0b4 100644 --- a/src/Umbraco.Tests/Services/MacroServiceTests.cs +++ b/src/Umbraco.Tests/Services/MacroServiceTests.cs @@ -54,7 +54,21 @@ public void Can_Get_By_Alias() Assert.IsNotNull(macro); Assert.AreEqual("Test1", macro.Name); } + [Test] + public void Can_Get_By_Aliases() + { + // Arrange + var macroService = ServiceContext.MacroService; + // Act + var macros = macroService.GetAll("test1","test2"); + + //assert + Assert.IsNotNull(macros); + Assert.AreEqual(2, macros.Count()); + Assert.AreEqual("Test1", macros.ToArray()[0].Name); + Assert.AreEqual("Test2", macros.ToArray()[1].Name); + } [Test] public void Can_Get_All() { diff --git a/src/Umbraco.Web/Cache/MacroCacheRefresher.cs b/src/Umbraco.Web/Cache/MacroCacheRefresher.cs index 0cecba7b7bec..5c19fb4c6301 100644 --- a/src/Umbraco.Web/Cache/MacroCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/MacroCacheRefresher.cs @@ -52,6 +52,7 @@ public override void Refresh(string json) if (macroRepoCache) { macroRepoCache.Result.Clear(RepositoryCacheKeys.GetKey(payload.Id)); + macroRepoCache.Result.Clear(RepositoryCacheKeys.GetKey(payload.Alias));//repository caching of macro definition by alias } } @@ -99,12 +100,14 @@ internal static string[] GetAllMacroCacheKeys() return new[] { CacheKeys.MacroContentCacheKey, // macro render cache - CacheKeys.MacroFromAliasCacheKey, // lookup macro by alias + CacheKeys.MacroFromAliasCacheKey // lookup macro by alias + }; } internal static string[] GetCacheKeysForAlias(string alias) { + return GetAllMacroCacheKeys().Select(x => x + alias).ToArray(); } From ed0180f3a2380537ce6ee47cf6e20eeaaa4117fe Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 17 Aug 2020 12:56:26 +0100 Subject: [PATCH 6/9] Remove previous tests based on scraping the embedded parameter udis --- .../HtmlMacroParameterParserTests.cs | 154 ------------------ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 - 2 files changed, 155 deletions(-) delete mode 100644 src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs diff --git a/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs b/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs deleted file mode 100644 index 39a48bbe4c9e..000000000000 --- a/src/Umbraco.Tests/Templates/HtmlMacroParameterParserTests.cs +++ /dev/null @@ -1,154 +0,0 @@ -using Umbraco.Core.Logging; -using Moq; -using NUnit.Framework; -using Umbraco.Tests.Testing.Objects.Accessors; -using Umbraco.Web.Templates; -using Umbraco.Web; -using Umbraco.Core.Models.PublishedContent; -using Umbraco.Web.Routing; -using Umbraco.Tests.Testing.Objects; -using System.Web; -using System; -using System.Linq; -using Umbraco.Core.Models; -using Umbraco.Core; -using System.Diagnostics; -using Newtonsoft.Json.Linq; -using System.Collections.Generic; - -namespace Umbraco.Tests.Templates -{ - [TestFixture] - public class HtmlMacroParameterParserTests - { - [Test] - public void Returns_Udis_From_Single_MediaPicker_Macro_Parameters_In_Macros_In_Html() - { - //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker - var input = @"

This is some normal text before the macro

- -

This is a paragraph after the macro and before the next

- -

some more text

"; - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); - Assert.AreEqual(3, result.Count); - Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); - Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); - Assert.AreEqual(Udi.Parse("umb://media/90ba0d3dba6e4c9fa1953db78352ba73"), result[2]); - } - [Test] - public void Returns_Empty_With_From_Single_MediaPicker_With_No_Macro_Parameters_In_Macros_In_Html() - { - //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker - var input = @"

This is some normal text before the macro

- -

This is a paragraph after the macro and before the next

- Some other value

"" /> -

some more text

"; - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); - Assert.AreEqual(0, result.Count); - } - //NB: When multiple media pickers store udis instead of ints! - see https://github.com/umbraco/Umbraco-CMS/pull/8388 - [Test] - public void Returns_Empty_When_No_Macros_In_Html() - { - //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker - var input = @"

This is some normal text before the macro

-

This is a paragraph after the macro and before the next

-

some more text

"; - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); - Assert.AreEqual(0, result.Count); - } - [Test] - public void Returns_Udis_From_Multiple_MediaPicker_Macro_Parameters_In_Macros_In_Html() - { - //Two macros, one single parameter, single Media Picker, one multiple paramters of single media picker - var input = @"

This is some normal text before the macro

- -

This is a paragraph after the macro

"; - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromMacroParameters(input).ToList(); - Assert.AreEqual(3, result.Count); - Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); - Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); - Assert.AreEqual(Udi.Parse("umb://media/bb763e0d0ceb408c8720365d57e06444"), result[2]); - } - [Test] - public void Returns_Udis_From_Single_MediaPicker_Macro_Parameters_In_Grid_Macros() - { - // create a list of GridValue.GridControls with Editor GridEditor alias macro - List gridControls = new List(); - - // single media picker macro parameter - var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": { ""singleMediaPicker"": ""umb://media/90ba0d3dba6e4c9fa1953db78352ba73"" }}"); - gridControls.Add(macroGridControl); - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); - Assert.AreEqual(1, result.Count); - Assert.AreEqual(Udi.Parse("umb://media/90ba0d3dba6e4c9fa1953db78352ba73"), result[0]); - - } - [Test] - public void Returns_Empty_From_Single_MediaPicker_With_No_Macro_Parameters_In_Grid_Macros() - { - // create a list of GridValue.GridControls with Editor GridEditor alias macro - List gridControls = new List(); - - // single media picker macro parameter - var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": {}}"); - gridControls.Add(macroGridControl); - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); - Assert.AreEqual(0, result.Count); - - } - //NB: When multiple media pickers store udis instead of ints! - see https://github.com/umbraco/Umbraco-CMS/pull/8388 - [Test] - public void Returns_Udis_From_Multiple_MediaPicker_Macro_Parameters_In_Grid_Macros() - { - // create a list of GridValue.GridControls with Editor GridEditor alias macro - List gridControls = new List(); - - // multiple media picker macro parameter - var macroGridControl = GetMacroGridControl(@"{ ""macroAlias"": ""SingleMediaPicker"", ""macroParamsDictionary"": { ""multipleMediaPicker"": ""umb://media/eee91c05b2e84031a056dcd7f28eff89,umb://media/fa763e0d0ceb408c8720365d57e06e32,umb://media/bb763e0d0ceb408c8720365d57e06444"" }}"); - gridControls.Add(macroGridControl); - - var macroParameterParser = new HtmlMacroParameterParser(); - - var result = macroParameterParser.FindUdisFromGridControlMacroParameters(gridControls).ToList(); - Assert.AreEqual(3, result.Count); - Assert.AreEqual(Udi.Parse("umb://media/eee91c05b2e84031a056dcd7f28eff89"), result[0]); - Assert.AreEqual(Udi.Parse("umb://media/fa763e0d0ceb408c8720365d57e06e32"), result[1]); - Assert.AreEqual(Udi.Parse("umb://media/bb763e0d0ceb408c8720365d57e06444"), result[2]); - - } - - //setup a Macro Grid Control based on Json of the Macro - private GridValue.GridControl GetMacroGridControl(string macroJson) - { - var macroGridEditor = new GridValue.GridEditor(); - macroGridEditor.Alias = "macro"; - macroGridEditor.View = "macro"; - var macroGridControl = new GridValue.GridControl(); - macroGridControl.Editor = macroGridEditor; - macroGridControl.Value = JToken.Parse(macroJson); - return macroGridControl; - } - - } -} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 5604c6915eeb..3c359cdde82b 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -167,7 +167,6 @@ - From e3989263204d0b9b339084a282f149c9b66587c2 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 17 Aug 2020 12:57:43 +0100 Subject: [PATCH 7/9] Update Macro Parameter Parser, to use the Macro Configuration, to find the types of Macro Parameter Property Editors To GetUdiReferences Utilises the new GetAll endpoint in the MacroService --- .../Templates/HtmlMacroParameterParser.cs | 149 +++++++++++------- 1 file changed, 93 insertions(+), 56 deletions(-) diff --git a/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs b/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs index c944ab45024d..d851f23a8170 100644 --- a/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs +++ b/src/Umbraco.Web/Templates/HtmlMacroParameterParser.cs @@ -4,97 +4,134 @@ using System.Linq; using System.Text.RegularExpressions; using Umbraco.Core; +using Umbraco.Core.Composing; using Umbraco.Core.Models; +using Umbraco.Core.Models.Editors; +using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Services; using Umbraco.Web.Macros; +using Umbraco.Core.Logging; namespace Umbraco.Web.Templates { public sealed class HtmlMacroParameterParser { - public HtmlMacroParameterParser() + private readonly IMacroService _macroService; + private readonly ILogger _logger; + //private readonly ParameterEditorCollection _parameterEditors; + public HtmlMacroParameterParser(IMacroService macroService, ILogger logger)//, ParameterEditorCollection parameterEditorCollection) { - + //I get an error at boot injecting this here, what would be the best way of referencing it? + //_parameterEditors = parameterEditorCollection; + _macroService = macroService; + _logger = logger; } + private IEnumerable GetUmbracoEntityReferencesFromMacros(List>> macros) + { + var uniqueMacroAliases = macros.Select(f => f.Item1).Distinct(); + //TODO: tracking Macro references...? am finding the used Macros Udis here - but not sure what to do with them - eg seems like there should be a Related Macro relation type - but Relations don't accept 'Macro' as an option + var foundMacroUmbracoEntityReferences = new List(); + //Get all the macro configs in one hit for these unique macro aliases - this is now cached with a custom cache policy + var macroConfigs = _macroService.GetAll(uniqueMacroAliases.ToArray()); + //get the registered parametereditors with the implmementation to avoid looking up for each parameter + // can we inject this instead of using Current?? - I get a boot error when I attempt this directly into the parser constructor - could/should it be passed in via the method? + var parameterEditors = Current.ParameterEditors; + foreach (var macro in macros) + { + var macroConfig = macroConfigs.FirstOrDefault(f => f.Alias == macro.Item1); + foundMacroUmbracoEntityReferences.Add(new UmbracoEntityReference(Udi.Create(Constants.UdiEntityType.Macro, macroConfig.Key))); + // only do this if the macros actually have parameters + if (macroConfig.Properties != null && macroConfig.Properties.Keys.Any(f=>f!= "macroAlias")) + { + foreach (var umbracoEntityReference in GetUmbracoEntityReferencesFromMacroParameters(macro.Item1, macro.Item2, macroConfig, parameterEditors)) + { + yield return umbracoEntityReference; + } + } + } + } /// - /// Parses out media UDIs from an html string based on embedded macro parameter values + /// Finds media UDIs in Macro Parameter Values by calling the GetReference method for all the Macro Parameter Editors for a particular Macro /// - /// + /// The alias of the macro + /// The parameters for the macro a dictionary of key/value strings + /// The macro configuration for this particular macro - contains the types of editors used for each parameter + /// A list of all the registered parameter editors used in the Umbraco implmentation - to look up the corresponding property editor for a macro parameter /// - public IEnumerable FindUdisFromMacroParameters(string text) + private IEnumerable GetUmbracoEntityReferencesFromMacroParameters(string macroAlias, Dictionary macroParameters, IMacro macroConfig, ParameterEditorCollection parameterEditors) { - // we already have a method for finding Macros within a Rich Text Editor using regexes: Umbraco.Web.Macros.MacroTagParser - // it searches for 'text blocks' which I think are the old way of embedding macros in a template, to allow inline code - we're not interested in those in V8 - // but we are interested in the foundMacros, and in particular their parameters and values - // (aside TODO: at somepoint we might want to track Macro usage?) - // ideally we'd determine each type of parameter editor and call it's GetReferences implementation - // however this information isn't stored in the embedded macro markup and would involve 'looking up' types for each parameter - // (or we could change the macros markup to include details of the editor) - // but pragmatically we don't really care what type of property editor it is if it stores media udis - // we want to track media udis wherever they are used regardless of how they are picked? - // so here we parse the parameters and check to see if they are in the format umb://media - // see also https://github.com/umbraco/Umbraco-CMS/pull/8388 - - List macroParameterValues = new List(); - MacroTagParser.ParseMacros(text, textblock => { }, (macroAlias, macroAttributes) => macroParameterValues.AddRange(macroAttributes.Values.ToList().Where(f=>f.StartsWith("umb://" + Constants.UdiEntityType.Media + "/")))); - - if (macroParameterValues.Any()) - { - // we have media udis! - but we could also have a csv of multiple media udis items - foreach (var macroParameterValue in macroParameterValues.Distinct()) + var foundUmbracoEntityReferences = new List(); + foreach (var parameter in macroConfig.Properties) + { + if (macroParameters.TryGetValue(parameter.Alias, out string parameterValue)) { - string[] potentialUdis = macroParameterValue.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries); - foreach (var potentialUdi in potentialUdis.Distinct()) + var parameterEditorAlias = parameter.EditorAlias; + //lookup propertyEditor from core current ParameterEditorCollection + var parameterEditor = parameterEditors.FirstOrDefault(f => String.Equals(f.Alias,parameterEditorAlias,StringComparison.OrdinalIgnoreCase)); + if (parameterEditor != null) { - if(Udi.TryParse(potentialUdi, out var udi)) + //get the ParameterValueEditor for this PropertyEditor (where the GetReferences method is implemented) - cast As IDataValueReference to determine if 'it is' implemented for the editor + IDataValueReference parameterValueEditor = parameterEditor.GetValueEditor() as IDataValueReference; + if (parameterValueEditor != null) + { + foreach (var entityReference in parameterValueEditor.GetReferences(parameterValue)) + { + foundUmbracoEntityReferences.Add(entityReference); + } + } + else { - yield return udi; + _logger.Info("{0} doesn't have a ValueEditor that implements IDataValueReference", parameterEditor.Alias); } } } } - else + return foundUmbracoEntityReferences; + } + /// + /// Parses out media UDIs from an html string based on embedded macro parameter values + /// + /// + /// + public IEnumerable FindUmbracoEntityReferencesFromEmbeddedMacros(string text) + { + // we already have a method for finding Macros within a Rich Text Editor using regexes: Umbraco.Web.Macros.MacroTagParser + // it searches for 'text blocks' which I think are the old way of embedding macros in a template, to allow inline code - we're not interested in those in V8 + // but we are interested in the foundMacros, + // there may be more than one macro with the same alias on the page so using a tuple + List>> foundMacros = new List>>(); + + //though this legacy ParseMacros does find the macros it seems to lowercase the macro parameter alias - so making the dictionary case insensitive + MacroTagParser.ParseMacros(text, textblock => { }, (macroAlias, macroAttributes) => foundMacros.Add(new Tuple>(macroAlias, new Dictionary(macroAttributes,StringComparer.OrdinalIgnoreCase)))); + foreach (var umbracoEntityReference in GetUmbracoEntityReferencesFromMacros(foundMacros)) { - yield break; - } + yield return umbracoEntityReference; + }; } /// /// Parses out media UDIs from Macro Grid Control Parameters /// /// - public IEnumerable FindUdisFromGridControlMacroParameters(IEnumerable macroValues) + public IEnumerable FindUmbracoEntityReferencesFromGridControlMacros(IEnumerable macroGridControls) { - List macroParameterValues = new List(); - foreach (var macroValue in macroValues) + List>> foundMacros = new List>>(); + + foreach (var macroGridControl in macroGridControls) { //deserialise JSON of Macro Grid Control to a class - var gridMacro = macroValue.Value.ToObject(); + var gridMacro = macroGridControl.Value.ToObject(); //collect any macro parameters that contain the media udi format - if (gridMacro != null && gridMacro.MacroParameters!=null && gridMacro.MacroParameters.Any()) + if (gridMacro != null && gridMacro.MacroParameters != null && gridMacro.MacroParameters.Any()) { - macroParameterValues.AddRange(gridMacro.MacroParameters.Values.ToList().Where(f => f.Contains("umb://" + Constants.UdiEntityType.Media + "/"))); - } - } - if (macroParameterValues.Any()) - { - // we have media udis! - but we could also have a csv of multiple items - foreach (var macroParameterValue in macroParameterValues.Distinct()) - { - string[] potentialUdis = macroParameterValue.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries); - foreach (var potentialUdi in potentialUdis.Distinct()) - { - if (Udi.TryParse(potentialUdi, out var udi)) - { - yield return udi; - } - } + foundMacros.Add(new Tuple>(gridMacro.MacroAlias, gridMacro.MacroParameters)); } } - else + foreach (var umbracoEntityReference in GetUmbracoEntityReferencesFromMacros(foundMacros)) { - yield break; - } + yield return umbracoEntityReference; + }; } // poco class to deserialise the Json for a Macro Control private class GridMacro @@ -102,7 +139,7 @@ private class GridMacro [JsonProperty("macroAlias")] public string MacroAlias { get; set; } [JsonProperty("macroParamsDictionary")] - public Dictionary MacroParameters { get; set; } + public Dictionary MacroParameters { get; set; } } } } From 7ab1028273de5b43515cdc0c097823a811325e45 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 17 Aug 2020 12:59:13 +0100 Subject: [PATCH 8/9] Update Grid + RichTextEditor Property Editors to use the MacroParameterParser --- .../PropertyEditors/GridPropertyEditor.cs | 15 ++++----------- .../PropertyEditors/RichTextPropertyEditor.cs | 9 ++++++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index eafbe303672a..cbdcc505de22 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -230,18 +230,11 @@ public IEnumerable GetReferences(object value) foreach (var umbracoEntityReference in mediaValues.SelectMany(x => _mediaPickerPropertyValueEditor.GetReferences(x.Value["udi"]))) yield return umbracoEntityReference; + + foreach (var umbracoEntityReference in _macroParameterParser.FindUmbracoEntityReferencesFromGridControlMacros(macroValues)) + yield return umbracoEntityReference; - //Macros don't really have a Property Editor for which we can call GetRefererences - //where does the responsibility lie for MacroParametersEditors to report their references? - //when we don't easily know the property type for a parameter - without 'looking up' which would be expensive? - //pragmatically we only care if the parameter has a value that is a media udi eg umb://media - //so we 'could' just loop through all parameter values here and add references for any values that are media udis... eg they are in use! however they are picked - //Is the HtmlMacroParameterParser the right place to put this method? - var udis = _macroParameterParser.FindUdisFromGridControlMacroParameters(macroValues); - foreach (var udi in udis) - { - yield return new UmbracoEntityReference(udi); - } + } } } diff --git a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs index 2d8dd6312c9a..93baf045b4cf 100644 --- a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs @@ -164,12 +164,15 @@ public IEnumerable GetReferences(object value) foreach (var udi in _imageSourceParser.FindUdisFromDataAttributes(asString)) yield return new UmbracoEntityReference(udi); - foreach (var udi in _macroParameterParser.FindUdisFromMacroParameters(asString)) - yield return new UmbracoEntityReference(udi); + foreach (var udi in _localLinkParser.FindUdisFromLocalLinks(asString)) yield return new UmbracoEntityReference(udi); - // is the TODO below a reference to needing to scan for media used in Macro parameters (see _macroParameterParser above) or a more general future TODO for some kind of 'tracking macro use' function? (is there an issue for this?) + + foreach (var umbracoEntityReference in _macroParameterParser.FindUmbracoEntityReferencesFromEmbeddedMacros(asString)) + yield return umbracoEntityReference; //TODO: Detect Macros too ... but we can save that for a later date, right now need to do media refs + //UPDATE: We are getting the Macros in 'FindUmbracoEntityReferencesFromEmbeddedMacros' do we just return the Macro Udis here too? do they need their own relationAlias? + } } From 27007cee0a2d4a31d0c915611e0a06c8fd7a5465 Mon Sep 17 00:00:00 2001 From: Marc Moriyamas Date: Mon, 17 Aug 2020 13:00:34 +0100 Subject: [PATCH 9/9] Update MultiMediaPicker Parameter editor to have it's own ValueEditor to handle legacy Ids (there is a PR to store these as Udis in the future, this will handle both scenarios) --- .../MultipleMediaPickerParameterEditor.cs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PropertyEditors/ParameterEditors/MultipleMediaPickerParameterEditor.cs b/src/Umbraco.Web/PropertyEditors/ParameterEditors/MultipleMediaPickerParameterEditor.cs index 1208a5eecc33..63107f048385 100644 --- a/src/Umbraco.Web/PropertyEditors/ParameterEditors/MultipleMediaPickerParameterEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/ParameterEditors/MultipleMediaPickerParameterEditor.cs @@ -1,6 +1,12 @@ -using Umbraco.Core; +using System; +using System.Collections.Generic; +using Umbraco.Core; +using Umbraco.Core.Composing; using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; +using static Umbraco.Web.PropertyEditors.MediaPickerPropertyEditor; namespace Umbraco.Web.PropertyEditors.ParameterEditors { @@ -23,5 +29,42 @@ public MultipleMediaPickerParameterEditor(ILogger logger) { DefaultConfiguration.Add("multiPicker", "1"); } + protected override IDataValueEditor CreateValueEditor() => new MultipleMediaPickerPropertyValueEditor(Attribute); + + internal class MultipleMediaPickerPropertyValueEditor : DataValueEditor, IDataValueReference + { + public MultipleMediaPickerPropertyValueEditor(DataEditorAttribute attribute) : base(attribute) + { + } + + public IEnumerable GetReferences(object value) + { + var asString = value is string str ? str : value?.ToString(); + + if (string.IsNullOrEmpty(asString)) yield break; + + foreach (var udiStr in asString.Split(',')) + { + if (Udi.TryParse(udiStr, out var udi)) + { + yield return new UmbracoEntityReference(udi); + } + // for legacy reasons the multiple media picker parameter editor is configured to store as ints not udis - there is a PR to perhaps change this: https://github.com/umbraco/Umbraco-CMS/pull/8388 + // but adding below should support both scenarios, or should this be added as a fallback on the MediaPickerPropertyValueEditor + if (int.TryParse(udiStr, out var id)) + { + //TODO: inject the service? + var guidAttempt = Current.Services.EntityService.GetKey(id, UmbracoObjectTypes.Media); + var guid = guidAttempt.Success ? guidAttempt.Result : Guid.Empty; + if (guid != Guid.Empty) + { + yield return new UmbracoEntityReference(new GuidUdi(Constants.UdiEntityType.Media, guid)); + } + + } + } + } + } + } }