From 6571a2b2798804e18062fd5813a7cc4cfd238cc0 Mon Sep 17 00:00:00 2001 From: David Peck Date: Fri, 29 Sep 2023 19:15:03 +0100 Subject: [PATCH 1/3] Moved Attr setting to separate method (SVGs) --- Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs index d1dd8e1..b715b4a 100644 --- a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs +++ b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs @@ -201,6 +201,17 @@ public override void Process(TagHelperContext context, TagHelperOutput output) @"syntax:error:", RegexOptions.IgnoreCase | RegexOptions.Singleline); + cleanedFileContents = ParseAndSetAttrs(cleanedFileContents); + + return cleanedFileContents; + } + + /// + /// Set CSS Class, Viewbox, and/or width/height + /// + /// SVG file contents + /// SVG file contents with attributes + private string ParseAndSetAttrs (string cleanedFileContents) { if ((EnsureViewBox || (_globalSettings.OurSVG.EnsureViewBox && !IgnoreAppSettings)) || !string.IsNullOrEmpty(CssClass)) { HtmlDocument doc = new HtmlDocument(); From dbb84031fcec5b1debf2f19c34e4f43243622d18 Mon Sep 17 00:00:00 2001 From: David Peck Date: Fri, 29 Sep 2023 19:20:13 +0100 Subject: [PATCH 2/3] Moved the Attr setting to after cached content (SVGs) --- Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs index b715b4a..ca20a3f 100644 --- a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs +++ b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs @@ -130,6 +130,9 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return; } + // Set CSS Class, Viewbox, and/or width/height + cleanedFileContents = ParseAndSetAttrs(cleanedFileContents); + // Remove the src attribute or media-item from the output.Attributes.RemoveAll("src"); output.Attributes.RemoveAll("media-item"); @@ -201,8 +204,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) @"syntax:error:", RegexOptions.IgnoreCase | RegexOptions.Singleline); - cleanedFileContents = ParseAndSetAttrs(cleanedFileContents); - return cleanedFileContents; } From c35b0b8914a7c2c0fb10bffcce33433eeae975ad Mon Sep 17 00:00:00 2001 From: David Peck Date: Fri, 29 Sep 2023 19:59:53 +0100 Subject: [PATCH 3/3] Refactor to cache SVG with attributes separately to SVG content --- Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs | 98 ++++++++++++++------ 1 file changed, 69 insertions(+), 29 deletions(-) diff --git a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs index ca20a3f..8bede85 100644 --- a/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs +++ b/Our.Umbraco.TagHelpers/InlineSvgTagHelper.cs @@ -85,6 +85,31 @@ public InlineSvgTagHelper(MediaFileManager mediaFileManager, IWebHostEnvironment [HtmlAttributeName("ignore-appsettings")] public bool IgnoreAppSettings { get; set; } + + + private string CalculateSvgContentCacheKey () { + if (MediaItem is not null) + { + return string.Concat("MediaItem-SvgContents (", MediaItem.Key.ToString(), ")"); + } + + if (string.IsNullOrWhiteSpace(FileSource) == false) + { + return string.Concat("File-SvgContents (", FileSource, ")"); + } + + return string.Empty; + } + + private string CalculateSetAttrsCacheKey (string cleanedFileContents) + => $"SvgWithAttributes ({CssClass?.GetHashCode()}_{cleanedFileContents.GetHashCode()})"; + + private int FetchCacheMinutes () + => CacheMinutes > 0 ? CacheMinutes : _globalSettings.OurSVG.CacheMinutes; + + private bool FetchEnsureViewBoxStatus () => + EnsureViewBox || (_globalSettings.OurSVG.EnsureViewBox && !IgnoreAppSettings); + public override void Process(TagHelperContext context, TagHelperOutput output) { // Can only use media-item OR src @@ -99,23 +124,14 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } string? cleanedFileContents = null; - - if(Cache || (_globalSettings.OurSVG.Cache && !IgnoreAppSettings)) + var doCache = Cache || (_globalSettings.OurSVG.Cache && !IgnoreAppSettings); + if (doCache) { - var cacheName = string.Empty; - var cacheMins = CacheMinutes > 0 ? CacheMinutes : _globalSettings.OurSVG.CacheMinutes; + var cacheKey = CalculateSvgContentCacheKey(); + var cacheMins = FetchCacheMinutes(); - if (MediaItem is not null) - { - cacheName = string.Concat("MediaItem-SvgContents (", MediaItem.Key.ToString(), ")"); - } - else if (string.IsNullOrWhiteSpace(FileSource) == false) - { - cacheName = string.Concat("File-SvgContents (", FileSource, ")"); - } - - cleanedFileContents = _appCaches.RuntimeCache.GetCacheItem(cacheName, () => - { + cleanedFileContents = _appCaches.RuntimeCache.GetCacheItem(cacheKey, () => + { return GetFileContents(); }, TimeSpan.FromMinutes(cacheMins)); } @@ -130,15 +146,28 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return; } - // Set CSS Class, Viewbox, and/or width/height - cleanedFileContents = ParseAndSetAttrs(cleanedFileContents); + string svgWithAttributes; + if (doCache) + { + var cacheKey = CalculateSetAttrsCacheKey(cleanedFileContents); + var cacheMins = FetchCacheMinutes(); + + svgWithAttributes = _appCaches.RuntimeCache.GetCacheItem(cacheKey, () => + { + return ParseAndSetAttrs(cleanedFileContents); + }, TimeSpan.FromMinutes(cacheMins)); + } + else + { + svgWithAttributes = ParseAndSetAttrs(cleanedFileContents); + } // Remove the src attribute or media-item from the output.Attributes.RemoveAll("src"); output.Attributes.RemoveAll("media-item"); output.TagName = null; // Remove - output.Content.SetHtmlContent(cleanedFileContents); + output.Content.SetHtmlContent(svgWithAttributes); } private string? GetFileContents() @@ -213,18 +242,29 @@ public override void Process(TagHelperContext context, TagHelperOutput output) /// SVG file contents /// SVG file contents with attributes private string ParseAndSetAttrs (string cleanedFileContents) { - if ((EnsureViewBox || (_globalSettings.OurSVG.EnsureViewBox && !IgnoreAppSettings)) || !string.IsNullOrEmpty(CssClass)) + var doEnsureViewBox = FetchEnsureViewBoxStatus(); + var hasCssClass = !string.IsNullOrEmpty(CssClass); + + // No work to do (save the unnecessary LoadHtml) + if (!doEnsureViewBox && !hasCssClass) { - HtmlDocument doc = new HtmlDocument(); - doc.LoadHtml(cleanedFileContents); - var svgs = doc.DocumentNode.SelectNodes("//svg"); - foreach (var svgNode in svgs) + return cleanedFileContents; + } + + HtmlDocument doc = new HtmlDocument(); + doc.LoadHtml(cleanedFileContents); + var svgs = doc.DocumentNode.SelectNodes("//svg"); + foreach (var svgNode in svgs) + { + if (hasCssClass) { - if (!string.IsNullOrEmpty(CssClass)) - { - svgNode.AddClass(CssClass); - } - if ((EnsureViewBox || (_globalSettings.OurSVG.EnsureViewBox && !IgnoreAppSettings)) && svgNode.Attributes.Contains("width") && svgNode.Attributes.Contains("height") && !svgNode.Attributes.Contains("viewbox")) + svgNode.AddClass(CssClass); + } + + if (doEnsureViewBox) + { + var hasDimensionsAndNoViewbox = svgNode.Attributes.Contains("width") && svgNode.Attributes.Contains("height") && !svgNode.Attributes.Contains("viewbox"); + if (hasDimensionsAndNoViewbox) { var width = StringUtils.GetDecimal(svgNode.GetAttributeValue("width", "0")); var height = StringUtils.GetDecimal(svgNode.GetAttributeValue("height", "0")); @@ -234,8 +274,8 @@ private string ParseAndSetAttrs (string cleanedFileContents) { svgNode.Attributes.Remove("height"); } } - cleanedFileContents = doc.DocumentNode.OuterHtml; } + cleanedFileContents = doc.DocumentNode.OuterHtml; return cleanedFileContents; }