From 5db4c8db93de9e8792ff4fb07adb8a23b65b7850 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 5 Aug 2023 12:34:59 +0800 Subject: [PATCH] Refactor backend SVG package and add tests (#26335) Introduce a well-tested `svg.Normalize` function. Make `RenderHTML` faster and more stable. --- modules/html/html.go | 32 +++++++------------ modules/svg/processor.go | 59 +++++++++++++++++++++++++++++++++++ modules/svg/processor_test.go | 29 +++++++++++++++++ modules/svg/svg.go | 33 ++++++++------------ 4 files changed, 113 insertions(+), 40 deletions(-) create mode 100644 modules/svg/processor.go create mode 100644 modules/svg/processor_test.go diff --git a/modules/html/html.go b/modules/html/html.go index 6cb6b847ef..b1ebd584c6 100644 --- a/modules/html/html.go +++ b/modules/html/html.go @@ -6,28 +6,20 @@ package html // ParseSizeAndClass get size and class from string with default values // If present, "others" expects the new size first and then the classes to use func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int, string) { - if len(others) == 0 { - return defaultSize, defaultClass - } - size := defaultSize - _size, ok := others[0].(int) - if ok && _size != 0 { - size = _size - } - - if len(others) == 1 { - return size, defaultClass - } - - class := defaultClass - if _class, ok := others[1].(string); ok && _class != "" { - if defaultClass == "" { - class = _class - } else { - class = defaultClass + " " + _class + if len(others) >= 1 { + if v, ok := others[0].(int); ok && v != 0 { + size = v + } + } + class := defaultClass + if len(others) >= 2 { + if v, ok := others[1].(string); ok && v != "" { + if class != "" { + class += " " + } + class += v } } - return size, class } diff --git a/modules/svg/processor.go b/modules/svg/processor.go new file mode 100644 index 0000000000..82248fb0c1 --- /dev/null +++ b/modules/svg/processor.go @@ -0,0 +1,59 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package svg + +import ( + "bytes" + "fmt" + "regexp" + "sync" +) + +type normalizeVarsStruct struct { + reXMLDoc, + reComment, + reAttrXMLNs, + reAttrSize, + reAttrClassPrefix *regexp.Regexp +} + +var ( + normalizeVars *normalizeVarsStruct + normalizeVarsOnce sync.Once +) + +// Normalize normalizes the SVG content: set default width/height, remove unnecessary tags/attributes +// It's designed to work with valid SVG content. For invalid SVG content, the returned content is not guaranteed. +func Normalize(data []byte, size int) []byte { + normalizeVarsOnce.Do(func() { + normalizeVars = &normalizeVarsStruct{ + reXMLDoc: regexp.MustCompile(`(?s)<\?xml.*?>`), + reComment: regexp.MustCompile(`(?s)`), + + reAttrXMLNs: regexp.MustCompile(`(?s)\s+xmlns\s*=\s*"[^"]*"`), + reAttrSize: regexp.MustCompile(`(?s)\s+(width|height)\s*=\s*"[^"]+"`), + reAttrClassPrefix: regexp.MustCompile(`(?s)\s+class\s*=\s*"`), + } + }) + data = normalizeVars.reXMLDoc.ReplaceAll(data, nil) + data = normalizeVars.reComment.ReplaceAll(data, nil) + + data = bytes.TrimSpace(data) + svgTag, svgRemaining, ok := bytes.Cut(data, []byte(">")) + if !ok || !bytes.HasPrefix(svgTag, []byte(`') + normalized = append(normalized, svgRemaining...) + return normalized +} diff --git a/modules/svg/processor_test.go b/modules/svg/processor_test.go new file mode 100644 index 0000000000..a0286666ed --- /dev/null +++ b/modules/svg/processor_test.go @@ -0,0 +1,29 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package svg + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNormalize(t *testing.T) { + res := Normalize([]byte("foo"), 1) + assert.Equal(t, "foo", string(res)) + + res = Normalize([]byte(` + +content`), 1) + assert.Equal(t, `content`, string(res)) + + res = Normalize([]byte(`content`), 16) + + assert.Equal(t, `content`, string(res)) +} diff --git a/modules/svg/svg.go b/modules/svg/svg.go index fc96ea8e6a..016e1dc08b 100644 --- a/modules/svg/svg.go +++ b/modules/svg/svg.go @@ -7,42 +7,35 @@ import ( "fmt" "html/template" "path" - "regexp" "strings" - "code.gitea.io/gitea/modules/html" + gitea_html "code.gitea.io/gitea/modules/html" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/public" ) -var ( - // SVGs contains discovered SVGs - SVGs = map[string]string{} - - widthRe = regexp.MustCompile(`width="[0-9]+?"`) - heightRe = regexp.MustCompile(`height="[0-9]+?"`) -) +var svgIcons map[string]string const defaultSize = 16 -// Init discovers SVGs and populates the `SVGs` variable +// Init discovers SVG icons and populates the `svgIcons` variable func Init() error { - files, err := public.AssetFS().ListFiles("assets/img/svg") + const svgAssetsPath = "assets/img/svg" + files, err := public.AssetFS().ListFiles(svgAssetsPath) if err != nil { return err } - // Remove `xmlns` because inline SVG does not need it - reXmlns := regexp.MustCompile(`(]*?)\s+xmlns="[^"]*"`) + svgIcons = make(map[string]string, len(files)) for _, file := range files { if path.Ext(file) != ".svg" { continue } - bs, err := public.AssetFS().ReadFile("assets/img/svg", file) + bs, err := public.AssetFS().ReadFile(svgAssetsPath, file) if err != nil { log.Error("Failed to read SVG file %s: %v", file, err) } else { - SVGs[file[:len(file)-4]] = reXmlns.ReplaceAllString(string(bs), "$1") + svgIcons[file[:len(file)-4]] = string(Normalize(bs, defaultSize)) } } return nil @@ -50,12 +43,12 @@ func Init() error { // RenderHTML renders icons - arguments icon name (string), size (int), class (string) func RenderHTML(icon string, others ...any) template.HTML { - size, class := html.ParseSizeAndClass(defaultSize, "", others...) - - if svgStr, ok := SVGs[icon]; ok { + size, class := gitea_html.ParseSizeAndClass(defaultSize, "", others...) + if svgStr, ok := svgIcons[icon]; ok { + // the code is somewhat hacky, but it just works, because the SVG contents are all normalized if size != defaultSize { - svgStr = widthRe.ReplaceAllString(svgStr, fmt.Sprintf(`width="%d"`, size)) - svgStr = heightRe.ReplaceAllString(svgStr, fmt.Sprintf(`height="%d"`, size)) + svgStr = strings.Replace(svgStr, fmt.Sprintf(`width="%d"`, defaultSize), fmt.Sprintf(`width="%d"`, size), 1) + svgStr = strings.Replace(svgStr, fmt.Sprintf(`height="%d"`, defaultSize), fmt.Sprintf(`height="%d"`, size), 1) } if class != "" { svgStr = strings.Replace(svgStr, `class="`, fmt.Sprintf(`class="%s `, class), 1)