refactor: render mail templates with html/template directly (#8272)

internal/email/email.go

@@ -1,15 +1,19 @@
 package email
 
 import (
+	"bytes"
 	"fmt"
 	"html/template"
+	"io/fs"
 	"net/mail"
+	"os"
 	"path/filepath"
+	"slices"
+	"strings"
 	"sync"
 	"time"
 
 	"github.com/cockroachdb/errors"
-	"gopkg.in/macaron.v1"
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/markup"
@@ -37,46 +41,130 @@ const (
 )
 
 var (
-	tplRender     *macaron.TplRender
+	tplSet     *template.Template
-	tplRenderOnce sync.Once
+	tplSetOnce sync.Once
+	tplSetErr  error
 )
 
-// render renders a mail template with given data.
+func funcMap() template.FuncMap {
-func render(tpl string, data map[string]any) (string, error) {
+	return template.FuncMap{
-	tplRenderOnce.Do(func() {
+		"AppName": func() string {
-		customDir := filepath.Join(conf.CustomDir(), "templates")
+			return conf.App.BrandName
-		opt := &macaron.RenderOptions{
+		},
-			Directory:         filepath.Join(conf.WorkDir(), "templates", "mail"),
+		"AppURL": func() string {
-			AppendDirectories: []string{filepath.Join(customDir, "mail")},
+			return conf.Server.ExternalURL
-			Extensions:        []string{".tmpl", ".html"},
+		},
-			Funcs: []template.FuncMap{map[string]any{
+		"Year": func() int {
-				"AppName": func() string {
+			return time.Now().Year()
-					return conf.App.BrandName
+		},
-				},
+		"Str2HTML": func(raw string) template.HTML {
-				"AppURL": func() string {
+			return template.HTML(markup.Sanitize(raw))
-					return conf.Server.ExternalURL
+		},
-				},
+	}
-				"Year": func() int {
+}
-					return time.Now().Year()
-				},
-				"Str2HTML": func(raw string) template.HTML {
-					return template.HTML(markup.Sanitize(raw))
-				},
-			}},
-		}
-		if !conf.Server.LoadAssetsFromDisk {
-			opt.TemplateFileSystem = templates.NewTemplateFileSystem("mail", customDir)
-		}
 
-		ts := macaron.NewTemplateSet()
+// Recognized mail-template file extensions. A template's name is its path
-		ts.Set(macaron.DEFAULT_TPL_SET_NAME, opt)
+// relative to the "mail" directory, without extension (e.g. "auth/activate").
-		tplRender = &macaron.TplRender{
+var mailTemplateExts = []string{".tmpl", ".html"}
-			TemplateSet: ts,
+
-			Opt:         opt,
+// loadMailTemplates parses every mail template under the embedded "mail" tree
+// (or "<work>/templates/mail" when LoadAssetsFromDisk is set), then overlays
+// files from "<custom>/templates/mail" so an admin can override any builtin.
+func loadMailTemplates() (*template.Template, error) {
+	root := template.New("").Funcs(funcMap())
+	parse := func(name string, data []byte) error {
+		_, err := root.New(name).Parse(string(data))
+		return errors.Wrapf(err, "parse %q", name)
+	}
+
+	if conf.Server.LoadAssetsFromDisk {
+		baseRoot := filepath.Join(conf.WorkDir(), "templates", "mail")
+		if _, err := os.Stat(baseRoot); err != nil {
+			return nil, errors.Wrapf(err, "stat base mail templates %q", baseRoot)
 		}
+		if err := overlayDiskMailTemplates(baseRoot, parse); err != nil {
+			return nil, err
+		}
+	} else {
+		for _, name := range templates.MailFileNames() {
+			ext := strings.ToLower(filepath.Ext(name))
+			if !slices.Contains(mailTemplateExts, ext) {
+				continue
+			}
+			data, err := templates.ReadMailFile(name)
+			if err != nil {
+				return nil, errors.Wrapf(err, "read embedded %q", name)
+			}
+			if err := parse(strings.TrimSuffix(filepath.ToSlash(name), ext), data); err != nil {
+				return nil, err
+			}
+		}
+	}
+	if err := overlayDiskMailTemplates(filepath.Join(conf.CustomDir(), "templates", "mail"), parse); err != nil {
+		return nil, err
+	}
+	return root, nil
+}
+
+// overlayDiskMailTemplates walks root and parses every recognized template
+// file via parse. A missing root is not an error: custom overrides are optional.
+func overlayDiskMailTemplates(root string, parse func(name string, data []byte) error) error {
+	return filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error {
+		if err != nil {
+			if errors.Is(err, fs.ErrNotExist) {
+				return fs.SkipAll
+			}
+			return err
+		}
+		if d.IsDir() {
+			return nil
+		}
+		ext := strings.ToLower(filepath.Ext(p))
+		if !slices.Contains(mailTemplateExts, ext) {
+			return nil
+		}
+		data, err := os.ReadFile(p)
+		if err != nil {
+			return errors.Wrapf(err, "read %q", p)
+		}
+		rel, err := filepath.Rel(root, p)
+		if err != nil {
+			return err
+		}
+		return parse(strings.TrimSuffix(filepath.ToSlash(rel), ext), data)
 	})
+}
 
-	return tplRender.HTMLString(tpl, data)
+func render(tpl string, data map[string]any) (string, error) {
+	set, err := mailTemplateSet()
+	if err != nil {
+		return "", errors.Wrap(err, "load mail templates")
+	}
+	t := set.Lookup(tpl)
+	if t == nil {
+		return "", errors.Newf("mail template %q not found", tpl)
+	}
+	var buf bytes.Buffer
+	if err := t.Execute(&buf, data); err != nil {
+		return "", errors.Wrapf(err, "execute %q", tpl)
+	}
+	return buf.String(), nil
+}
+
+// mailTemplateSet returns the parsed template set. When assets are loaded from
+// disk, templates are reloaded on every call so admin edits under
+// <work>/templates/mail or <custom>/templates/mail take effect without a
+// restart — matching the hot-reload behavior of the previous macaron renderer
+// for non-production environments. When assets are embedded, the set is loaded
+// once and cached for the process lifetime.
+func mailTemplateSet() (*template.Template, error) {
+	if conf.Server.LoadAssetsFromDisk {
+		return loadMailTemplates()
+	}
+	tplSetOnce.Do(func() {
+		tplSet, tplSetErr = loadMailTemplates()
+	})
+	return tplSet, tplSetErr
 }
 
 func SendTestMail(email string) error {

internal/email/email_test.go

@@ -0,0 +1,124 @@
+package email
+
+import (
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"gogs.io/gogs/internal/conf"
+)
+
+// TestRenderEmbeddedTemplates ensures every builtin mail template parses and
+// executes against the data shape its production caller supplies, so a syntax
+// regression or missing field is caught at build time, not on the first email.
+func TestRenderEmbeddedTemplates(t *testing.T) {
+	conf.SetMockApp(t, conf.AppOpts{BrandName: "Gogs"})
+	conf.SetMockServer(t, conf.ServerOpts{
+		ExternalURL:        "https://example.test/",
+		LoadAssetsFromDisk: false,
+	})
+	resetTemplateCache(t)
+
+	tests := []struct {
+		name string
+		data map[string]any
+	}{
+		{
+			name: tmplAuthActivate,
+			data: map[string]any{
+				"Username":          "alice",
+				"ActiveCodeLives":   1440,
+				"ResetPwdCodeLives": 1440,
+				"Code":              "abc",
+			},
+		},
+		{
+			name: tmplAuthActivateEmail,
+			data: map[string]any{
+				"Username":        "alice",
+				"ActiveCodeLives": 1440,
+				"Code":            "abc",
+				"Email":           "alice@example.test",
+			},
+		},
+		{
+			name: tmplAuthResetPassword,
+			data: map[string]any{
+				"Username":          "alice",
+				"ActiveCodeLives":   1440,
+				"ResetPwdCodeLives": 1440,
+				"Code":              "abc",
+			},
+		},
+		{
+			name: tmplAuthRegisterNotify,
+			data: map[string]any{"Username": "alice"},
+		},
+		{
+			name: tmplNotifyCollaborator,
+			data: map[string]any{
+				"Subject":  "alice added you to bob/repo",
+				"RepoName": "bob/repo",
+				"Link":     "https://example.test/bob/repo",
+			},
+		},
+		{
+			name: tmplIssueComment,
+			data: map[string]any{
+				"Subject": "[bob/repo] Re: Issue title",
+				"Body":    "<p>comment body</p>",
+				"Link":    "https://example.test/bob/repo/issues/1",
+				"Doer":    testDoer{},
+			},
+		},
+		{
+			name: tmplIssueMention,
+			data: map[string]any{
+				"Subject": "[bob/repo] @alice mentioned you",
+				"Body":    "<p>mention body</p>",
+				"Link":    "https://example.test/bob/repo/issues/1",
+				"Doer":    testDoer{},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			body, err := render(tc.name, tc.data)
+			require.NoError(t, err)
+			assert.NotEmpty(t, body)
+			assert.False(t, strings.Contains(body, "<no value>"), "template referenced a missing data key")
+		})
+	}
+}
+
+// TestRenderUnknownTemplate asserts callers get a useful error rather than an
+// empty body when asking for a name that doesn't exist.
+func TestRenderUnknownTemplate(t *testing.T) {
+	conf.SetMockServer(t, conf.ServerOpts{LoadAssetsFromDisk: false})
+	resetTemplateCache(t)
+
+	_, err := render("does/not/exist", nil)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "not found")
+}
+
+// resetTemplateCache forces the next render call to reload templates, so each
+// test starts from a clean state regardless of execution order.
+func resetTemplateCache(t *testing.T) {
+	t.Helper()
+	tplSet = nil
+	tplSetErr = nil
+	tplSetOnce = sync.Once{}
+}
+
+// testDoer satisfies the User interface for fields the issue templates touch.
+type testDoer struct{}
+
+func (testDoer) ID() int64                               { return 1 }
+func (testDoer) DisplayName() string                     { return "alice" }
+func (testDoer) Email() string                           { return "alice@example.test" }
+func (testDoer) GenerateEmailActivateCode(string) string { return "abc" }

templates/embed.go

@@ -39,6 +39,9 @@ func (fs *fileSystem) Get(name string) (io.Reader, error) {
 func mustNames(fsys fs.FS) []string {
 	var names []string
 	walkDirFunc := func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
 		if !d.IsDir() {
 			names = append(names, path)
 		}
@@ -50,6 +53,24 @@ func mustNames(fsys fs.FS) []string {
 	return names
 }
 
+// MailFileNames returns the embedded template file paths under "mail/",
+// each relative to the "mail" directory (e.g. "auth/activate.tmpl").
+func MailFileNames() []string {
+	var names []string
+	for _, name := range mustNames(files) {
+		if rel, ok := strings.CutPrefix(name, "mail/"); ok {
+			names = append(names, rel)
+		}
+	}
+	return names
+}
+
+// ReadMailFile returns the embedded mail template bytes at the given path
+// relative to the "mail" directory.
+func ReadMailFile(name string) ([]byte, error) {
+	return files.ReadFile(path.Join("mail", name))
+}
+
 // NewTemplateFileSystem returns a macaron.TemplateFileSystem instance for embedded assets.
 // The argument "dir" can be used to serve subset of embedded assets. Template file
 // found under the "customDir" on disk has higher precedence over embedded assets.