fix(http): restore web UI yield for layout templates (#101)

go/http/render.go

@@ -17,9 +17,11 @@
 package http
 
 import (
+	"bytes"
 	"encoding/json"
 	"html/template"
 	"net/http"
+	"os"
 	"path/filepath"
 	"sync"
 
@@ -35,50 +37,94 @@ func renderJSON(w http.ResponseWriter, status int, data interface{}) {
 	}
 }
 
-// templateCache caches parsed templates.
-var templateCache = struct {
+// contentTemplateCache caches parsed content templates (without layout).
+var contentTemplateCache = struct {
 	sync.RWMutex
 	m map[string]*template.Template
 }{m: make(map[string]*template.Template)}
 
+var (
+	layoutOnce    sync.Once
+	layoutSource  string
+	layoutLoadErr error
+)
+
 const templateDir = "resources"
 const layoutFile = "templates/layout"
 
-// getTemplate returns a cached template or parses and caches it.
-func getTemplate(name string) (*template.Template, error) {
-	templateCache.RLock()
-	if t, ok := templateCache.m[name]; ok {
-		templateCache.RUnlock()
+func loadLayoutSource() {
+	layoutPath := filepath.Join(templateDir, layoutFile+".tmpl")
+	b, err := os.ReadFile(layoutPath)
+	if err != nil {
+		layoutLoadErr = err
+		return
+	}
+	layoutSource = string(b)
+}
+
+// getContentTemplate returns a cached content template or parses and caches it.
+func getContentTemplate(name string) (*template.Template, error) {
+	contentTemplateCache.RLock()
+	if t, ok := contentTemplateCache.m[name]; ok {
+		contentTemplateCache.RUnlock()
 		return t, nil
 	}
-	templateCache.RUnlock()
+	contentTemplateCache.RUnlock()
 
-	layoutPath := filepath.Join(templateDir, layoutFile+".tmpl")
 	tmplPath := filepath.Join(templateDir, name+".tmpl")
-	t, err := template.ParseFiles(layoutPath, tmplPath)
+	t, err := template.ParseFiles(tmplPath)
 	if err != nil {
 		return nil, err
 	}
 
-	templateCache.Lock()
-	templateCache.m[name] = t
-	templateCache.Unlock()
+	contentTemplateCache.Lock()
+	contentTemplateCache.m[name] = t
+	contentTemplateCache.Unlock()
 	return t, nil
 }
 
 // renderHTML renders an HTML template with the given data.
 // The template name should be like "templates/clusters".
+// Content is injected into layout.tmpl via {{yield}}, matching the
+// martini-contrib/render convention used by the existing templates.
 func renderHTML(w http.ResponseWriter, status int, name string, data interface{}) {
-	t, err := getTemplate(name)
+	content, err := getContentTemplate(name)
 	if err != nil {
 		_ = log.Errorf("Error parsing template %s: %+v", name, err)
 		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
 		return
 	}
+
+	var contentBuf bytes.Buffer
+	if err := content.Execute(&contentBuf, data); err != nil {
+		_ = log.Errorf("Error executing template %s: %+v", name, err)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+
+	layoutOnce.Do(loadLayoutSource)
+	if layoutLoadErr != nil {
+		_ = log.Errorf("Error loading layout template: %+v", layoutLoadErr)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+
+	// Per-request layout parse so the yield closure is concurrent-safe.
+	layout, err := template.New("layout").Funcs(template.FuncMap{
+		"yield": func() template.HTML {
+			return template.HTML(contentBuf.String())
+		},
+	}).Parse(layoutSource)
+	if err != nil {
+		_ = log.Errorf("Error parsing layout template for %s: %+v", name, err)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+
 	w.Header().Set("Content-Type", "text/html; charset=UTF-8")
 	w.WriteHeader(status)
-	if err := t.Execute(w, data); err != nil {
-		_ = log.Errorf("Error executing template %s: %+v", name, err)
+	if err := layout.Execute(w, data); err != nil {
+		_ = log.Errorf("Error executing layout for template %s: %+v", name, err)
 	}
 }
 

go/http/render_test.go

@@ -0,0 +1,195 @@
+/*
+   Copyright 2014 Outbrain Inc.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package http
+
+import (
+	"html/template"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// chdirToRepoRoot finds the repository root (directory containing resources/templates)
+// so template paths resolve during tests.
+func chdirToRepoRoot(t *testing.T) {
+	t.Helper()
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	dir := wd
+	for {
+		if _, err := os.Stat(filepath.Join(dir, "resources", "templates", "layout.tmpl")); err == nil {
+			if err := os.Chdir(dir); err != nil {
+				t.Fatal(err)
+			}
+			t.Cleanup(func() { _ = os.Chdir(wd) })
+			return
+		}
+		parent := filepath.Dir(dir)
+		if parent == dir {
+			t.Fatal("could not find repo root containing resources/templates/layout.tmpl")
+		}
+		dir = parent
+	}
+}
+
+func clearContentTemplateCache() {
+	contentTemplateCache.Lock()
+	contentTemplateCache.m = make(map[string]*template.Template)
+	contentTemplateCache.Unlock()
+}
+
+// contentTemplateNames returns every content template under resources/templates
+// (everything except layout). Discovered from disk so new templates are covered.
+func contentTemplateNames(t *testing.T) []string {
+	t.Helper()
+	matches, err := filepath.Glob(filepath.Join("resources", "templates", "*.tmpl"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var names []string
+	for _, path := range matches {
+		base := filepath.Base(path)
+		if base == "layout.tmpl" {
+			continue
+		}
+		name := strings.TrimSuffix(base, ".tmpl")
+		names = append(names, "templates/"+name)
+	}
+	if len(names) == 0 {
+		t.Fatal("no content templates found under resources/templates")
+	}
+	return names
+}
+
+func sampleTemplateData() map[string]interface{} {
+	return map[string]interface{}{
+		"title":                         "test",
+		"prefix":                        "",
+		"agentsHttpActive":              false,
+		"autoshow_problems":             false,
+		"authorizedForAction":           false,
+		"userId":                        "",
+		"removeTextFromHostnameDisplay": "",
+		"webMessage":                    "",
+		"clusterName":                   "test-cluster",
+		"page":                          0,
+		"searchString":                  "",
+		"auditHostname":                 "",
+		"auditPort":                     0,
+		"agentHost":                     "",
+		"seedId":                        "",
+		"detectionId":                   0,
+		"clusterAlias":                  "",
+		"recoveryId":                    "",
+		"recoveryUid":                   "",
+		"pseudoGTIDModeEnabled":         false,
+		"contextMenuVisible":            false,
+	}
+}
+
+func TestRenderHTMLYield(t *testing.T) {
+	chdirToRepoRoot(t)
+	clearContentTemplateCache()
+
+	rec := httptest.NewRecorder()
+	renderHTML(rec, http.StatusOK, "templates/clusters", map[string]interface{}{
+		"title":  "clusters",
+		"prefix": "",
+	})
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String())
+	}
+	body := rec.Body.String()
+	if strings.Contains(body, "Internal Server Error") {
+		t.Fatalf("got error body: %s", body)
+	}
+	if !strings.Contains(body, `id="clusters"`) {
+		t.Fatalf("expected clusters content via yield, body snippet: %s", truncate(body, 500))
+	}
+	if !strings.Contains(body, "<!doctype html>") {
+		t.Fatalf("expected layout wrapper, body snippet: %s", truncate(body, 200))
+	}
+	if !strings.Contains(body, "Orchestrator - clusters") {
+		t.Fatalf("expected title from layout data, body snippet: %s", truncate(body, 300))
+	}
+}
+
+// TestLayoutRequiresYield guards the martini-contrib/render contract: layout.tmpl
+// uses {{yield}} to inject page content. Parsing layout without that FuncMap must fail.
+func TestLayoutRequiresYield(t *testing.T) {
+	chdirToRepoRoot(t)
+
+	layoutPath := filepath.Join("resources", "templates", "layout.tmpl")
+	src, err := os.ReadFile(layoutPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(src), "{{yield}}") {
+		t.Fatal("layout.tmpl no longer uses {{yield}}; update renderHTML if composition changed")
+	}
+
+	_, err = template.New("layout").Parse(string(src))
+	if err == nil {
+		t.Fatal("expected layout parse without yield FuncMap to fail")
+	}
+	if !strings.Contains(err.Error(), `function "yield" not defined`) {
+		t.Fatalf("unexpected parse error: %v", err)
+	}
+
+	_, err = template.New("layout").Funcs(template.FuncMap{
+		"yield": func() template.HTML { return "" },
+	}).Parse(string(src))
+	if err != nil {
+		t.Fatalf("layout should parse when yield is registered: %v", err)
+	}
+}
+
+func TestRenderHTMLAllWebTemplates(t *testing.T) {
+	chdirToRepoRoot(t)
+	clearContentTemplateCache()
+
+	data := sampleTemplateData()
+	for _, name := range contentTemplateNames(t) {
+		t.Run(name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			renderHTML(rec, http.StatusOK, name, data)
+			if rec.Code != http.StatusOK {
+				t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String())
+			}
+			body := rec.Body.String()
+			if strings.Contains(body, "Internal Server Error") {
+				t.Fatalf("error body: %s", body)
+			}
+			if !strings.Contains(body, "<!doctype html>") {
+				t.Fatalf("missing layout for %s", name)
+			}
+		})
+	}
+}
+
+func truncate(s string, n int) string {
+	if len(s) <= n {
+		return s
+	}
+	return s[:n] + "..."
+}