From f04396809df5fe8f4564f0d2c1ff18e4dc14c200 Mon Sep 17 00:00:00 2001 From: Johannes Date: Sat, 24 Sep 2022 15:01:33 +0200 Subject: [PATCH] rework template rendering, cache parsed templates adapt error wrapping principle --- controllers/addOfficeHourHandler.go | 7 +- controllers/confirmRequestHandler.go | 7 +- controllers/deleteOfficeHourHandler.go | 5 +- controllers/getHandlers.go | 3 +- controllers/templates.go | 60 -------- controllers/timetable.go | 17 ++- main.go | 15 +- repositories/request.go | 5 +- templating/templates.go | 138 ++++++++++++++++++ .../templates}/addFailure.html | 0 .../templates}/addMask.html | 0 .../templates}/addSuccess.html | 0 {templates => templating/templates}/base.html | 0 .../templates}/confirmationMail | 0 .../templates}/deleteSuccess.html | 0 .../templates}/executeFailure.html | 0 .../templates}/executeSuccess.html | 0 .../templates}/index.html | 0 .../templates}/officeHourTable.html | 0 .../templates}/requestNotFound.html | 0 {templates => templating/templates}/td.html | 0 21 files changed, 175 insertions(+), 82 deletions(-) delete mode 100644 controllers/templates.go create mode 100644 templating/templates.go rename {templates => templating/templates}/addFailure.html (100%) rename {templates => templating/templates}/addMask.html (100%) rename {templates => templating/templates}/addSuccess.html (100%) rename {templates => templating/templates}/base.html (100%) rename {templates => templating/templates}/confirmationMail (100%) rename {templates => templating/templates}/deleteSuccess.html (100%) rename {templates => templating/templates}/executeFailure.html (100%) rename {templates => templating/templates}/executeSuccess.html (100%) rename {templates => templating/templates}/index.html (100%) rename {templates => templating/templates}/officeHourTable.html (100%) rename {templates => templating/templates}/requestNotFound.html (100%) rename {templates => templating/templates}/td.html (100%) diff --git a/controllers/addOfficeHourHandler.go b/controllers/addOfficeHourHandler.go index 97d30ef..a99a40a 100644 --- a/controllers/addOfficeHourHandler.go +++ b/controllers/addOfficeHourHandler.go @@ -7,6 +7,7 @@ import ( "net/mail" "officeHours/config" "officeHours/models" + "officeHours/templating" "strconv" "strings" ) @@ -149,7 +150,7 @@ func (b *BaseHandler) AddOfficeHourHandler(w http.ResponseWriter, req *http.Requ id, err := b.officeHourRepo.Add(officeHour) if err != nil { w.WriteHeader(http.StatusInternalServerError) - b.serveTemplate(w, "addFailure", err) + templating.ServeTemplate(w, "addFailure", err) return } officeHour, err = b.officeHourRepo.FindById(id) @@ -160,7 +161,7 @@ func (b *BaseHandler) AddOfficeHourHandler(w http.ResponseWriter, req *http.Requ if err != nil { log.Printf("Error adding request: %s", err.Error()) } - b.serveTemplate(w, "addSuccess", nil) + templating.ServeTemplate(w, "addSuccess", nil) } } @@ -172,5 +173,5 @@ func (b *BaseHandler) writeAddOfficeHourMask(w http.ResponseWriter, req *http.Re if len(data.Errors) != 0 { w.WriteHeader(http.StatusBadRequest) } - b.serveTemplate(w, "addMask", data) + templating.ServeTemplate(w, "addMask", data) } diff --git a/controllers/confirmRequestHandler.go b/controllers/confirmRequestHandler.go index 33d3287..98a485d 100644 --- a/controllers/confirmRequestHandler.go +++ b/controllers/confirmRequestHandler.go @@ -2,6 +2,7 @@ package controllers import ( "net/http" + "officeHours/templating" ) func (b *BaseHandler) ConfirmRequestHandler(w http.ResponseWriter, req *http.Request) { @@ -10,16 +11,16 @@ func (b *BaseHandler) ConfirmRequestHandler(w http.ResponseWriter, req *http.Req if err != nil { // TODO: header 404 - b.serveTemplate(w, "requestNotFound", nil) + templating.ServeTemplate(w, "requestNotFound", nil) return } err = b.requestRepo.Execute(request) if err != nil { // TODO: write header 500 - b.serveTemplate(w, "executeFailure", err.Error()) + templating.ServeTemplate(w, "executeFailure", err.Error()) return } - b.serveTemplate(w, "executeSuccess", nil) + templating.ServeTemplate(w, "executeSuccess", nil) } diff --git a/controllers/deleteOfficeHourHandler.go b/controllers/deleteOfficeHourHandler.go index 233dc34..f694872 100644 --- a/controllers/deleteOfficeHourHandler.go +++ b/controllers/deleteOfficeHourHandler.go @@ -4,10 +4,13 @@ package controllers import ( "net/http" "officeHours/models" + "officeHours/templating" "strconv" ) func (b *BaseHandler) DeleteOfficeHourHandler(w http.ResponseWriter, req *http.Request) { + // TODO: error handling here is by no means sufficient, furthermore + // 400 BadRequest is for technically wrong stuff (most promimently GET instead of POST) if req.FormValue("id") != "" { id, err := strconv.Atoi(req.FormValue("id")) if err != nil { @@ -18,7 +21,7 @@ func (b *BaseHandler) DeleteOfficeHourHandler(w http.ResponseWriter, req *http.R w.WriteHeader(http.StatusBadRequest) } _, err = b.requestRepo.Add(officeHour, models.RequestDelete) - b.serveTemplate(w, "deleteSuccess", nil) + templating.ServeTemplate(w, "deleteSuccess", nil) } else { officeHours, _ := b.officeHourRepo.GetAll(true) timetable, slots := b.GetTimetable(officeHours) diff --git a/controllers/getHandlers.go b/controllers/getHandlers.go index 0be1781..5bdc516 100644 --- a/controllers/getHandlers.go +++ b/controllers/getHandlers.go @@ -5,6 +5,7 @@ import ( "log" "net/http" "officeHours/models" + "officeHours/templating" "strconv" ) @@ -58,5 +59,5 @@ func (b *BaseHandler) writeTimetablePage(w http.ResponseWriter, req *http.Reques SelectedRoom int SelectedCourse int }{courses, rooms, timetable, selectedRoom, selectedCourse} - b.serveTemplate(w, "index", data) + templating.ServeTemplate(w, "index", data) } diff --git a/controllers/templates.go b/controllers/templates.go deleted file mode 100644 index 1f4eb3b..0000000 --- a/controllers/templates.go +++ /dev/null @@ -1,60 +0,0 @@ -package controllers - -import ( - "fmt" - "html/template" - "log" - "net/http" - "officeHours/models" - "os" -) - -var funcs = template.FuncMap{ - "DayName": models.DayName, - "divide": func(i int, j int) int { return i / j }, -} -var baseTemplate = template.Must(template.ParseFiles("templates/base.html")).New("base.html").Funcs(funcs) - -// Execute a rendered full HTML page. -// -// If you just want to fill a template snippet, use the RawTemplates object. -// -// Parameters: -// - name is the name of the template file inside templates/, without .html suffix. -// - data is passed through to the template -func (b *BaseHandler) serveTemplate(w http.ResponseWriter, name string, data any) { - full_name := "templates/" + name + ".html" - // check that template exists - info, err := os.Stat(full_name) - if (err != nil && os.IsNotExist(err)) || info.IsDir() { - errMsg := fmt.Sprintf("Template %s nicht gefunden", full_name) - log.Println(errMsg) - w.WriteHeader(http.StatusNotFound) - w.Write([]byte(errMsg)) - return - } - - tmpl, err := baseTemplate.ParseFiles(full_name) - if err != nil { - errMsg := fmt.Sprintf("Template %s konnte nicht geparst werden : %s", full_name, err.Error()) - log.Println(errMsg) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(errMsg)) - return - } - // TODO: cache templates in parsed state, but not executed - err = template.Must(tmpl.Clone()).Execute(w, data) - if err != nil { - errMsg := fmt.Sprintf("Template %s konnte nicht ausgeführt werden : %s", full_name, err.Error()) - log.Println(errMsg) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(errMsg)) - return - } -} - -// standalone templates that should not be wrapped into the base.html -var RawTemplates, TemplateError = template.New("").Funcs(funcs).ParseFiles( - "templates/confirmationMail", - "templates/td.html", - "templates/officeHourTable.html") diff --git a/controllers/timetable.go b/controllers/timetable.go index 5479329..b1d5eb1 100644 --- a/controllers/timetable.go +++ b/controllers/timetable.go @@ -7,6 +7,7 @@ import ( "html/template" "log" "officeHours/models" + "officeHours/templating" ) func (b *BaseHandler) GetTimetable(officeHours []models.OfficeHour) (timetable map[models.Date]map[int]models.OfficeHour, slots []int) { @@ -85,9 +86,11 @@ func (b *BaseHandler) printTimetable(timetable map[models.Date]map[int]models.Of MinuteGranularity int DeleteIcons bool }{OfficeHour: current, MinuteGranularity: b.config.Date.MinuteGranularity, DeleteIcons: deleteIcons} - templateError := RawTemplates.ExecuteTemplate(&celldata, "td.html", data) - if templateError != nil { - log.Printf("Error executing template td.html: %s", templateError.Error()) + err := templating.WriteTemplate(&celldata, "td", data) + if err != nil { + err = fmt.Errorf("writing table cell failed:\n%w", err) + log.Println(err.Error()) + // TODO: better error wrapping up to top-level request handler } tableBody += celldata.String() } @@ -120,9 +123,11 @@ func (b *BaseHandler) printTimetable(timetable map[models.Date]map[int]models.Of slots[4], template.HTML(tableBody), } - templateError := RawTemplates.ExecuteTemplate(&table, "officeHourTable.html", tableData) - if templateError != nil { - log.Printf("Error executing template officeHourTable.html: %s", templateError.Error()) + err := templating.WriteTemplate(&table, "officeHourTable", tableData) + if err != nil { + err = fmt.Errorf("writing table failed:\n%w", err) + log.Println(err.Error()) + // TODO: better error wrapping up to top-level request handler } return template.HTML(table.String()) } diff --git a/main.go b/main.go index eea1f9f..00423f5 100644 --- a/main.go +++ b/main.go @@ -11,6 +11,7 @@ import ( "officeHours/controllers" "officeHours/repositories" "officeHours/sqldb" + "officeHours/templating" "os" ) @@ -20,10 +21,6 @@ func main() { log.SetOutput(logwriter) } - if controllers.TemplateError != nil { - log.Fatalf("Error parsing templates: %s", controllers.TemplateError.Error()) - } - configFile := flag.String( "config", "config/config.json", @@ -42,10 +39,16 @@ func main() { // serve static files staticHandler := http.FileServer(http.Dir("./static")) - + // parse templates + if err = templating.InitTemplates(); err != nil { + log.Print(err.Error()) + log.Fatal(">>>\nFatal error occurred, aborting program\n<<<\n") + } + // database connection db, err := sqldb.Connect(conf) if err != nil { - log.Fatalf(err.Error()) + log.Print(err.Error()) + log.Fatal(">>>\nFatal error occurred, aborting program\n<<<\n") } // Create repos roomRepo := repositories.NewRoomRepo(db) diff --git a/repositories/request.go b/repositories/request.go index 5411a34..e6273e9 100644 --- a/repositories/request.go +++ b/repositories/request.go @@ -11,8 +11,8 @@ import ( "math/big" "net/smtp" "officeHours/config" - "officeHours/controllers" "officeHours/models" + "officeHours/templating" ) type RequestRepo struct { @@ -131,12 +131,13 @@ func (r *RequestRepo) sendConfirmationMail(request models.Request) error { Config config.Config Request models.Request }{r.config, request} - err := controllers.RawTemplates.ExecuteTemplate(&message, "confirmationMail", data) + err := templating.WriteTemplate(&message, "confirmationMail", data) if err != nil { err = fmt.Errorf("Error parsing confirmation Mail: %w", err) log.Println(err.Error()) return err } + switch r.config.Mailer.Type { case "Stdout": fmt.Println(message.String()) diff --git a/templating/templates.go b/templating/templates.go new file mode 100644 index 0000000..982b2c2 --- /dev/null +++ b/templating/templates.go @@ -0,0 +1,138 @@ +package templating + +import ( + "fmt" + "html/template" + "io" + "log" + "net/http" + "officeHours/models" + "os" + "time" +) + +// parsed templates available for execution +var templates map[string]*template.Template = map[string]*template.Template{} + +// Initialise and parse templates. +// +// Should only be called once. +// +// Since this is something which may error and feels like +// it should not be done automatically on import, +// put it into a function. +func InitTemplates() error { + const templateDir = "templating/templates/" + var funcs = template.FuncMap{ + "DayName": models.DayName, + "divide": func(i int, j int) int { return i / j }, + } + var emptyTemplate = template.New("").Funcs(funcs) + var baseTemplate, err = template.ParseFiles(templateDir + "base.html") + if err != nil { + return fmt.Errorf("parsing base template failed:\n%w", err) + } + baseTemplate.Funcs(funcs) + + type toCache struct { + filename string + standalone bool + } + var toParse = map[string]toCache{ + // full html templates + "addFailure": {"addFailure.html", false}, + "addMask": {"addMask.html", false}, + "addSuccess": {"addSuccess.html", false}, + "deleteSuccess": {"deleteSuccess.html", false}, + "executeFailure": {"executeFailure.html", false}, + "executeSuccess": {"executeSuccess.html", false}, + "index": {"index.html", false}, + "requestNotFound": {"requestNotFound.html", false}, + // standalone templates + "confirmationMail": {"confirmationMail", true}, + "officeHourTable": {"officeHourTable.html", true}, + "td": {"td.html", true}, + } + + // parse templates and add to global mapping + for key, tmpl := range toParse { + if _, exists := templates[key]; exists { + return fmt.Errorf("template '%s' already parsed", key) + } + fullName := templateDir + tmpl.filename + // check that template file + info, err := os.Stat(fullName) + if err != nil { + return fmt.Errorf("adding template %s failed:\n%w", tmpl.filename, err) + } + if info.IsDir() { + return fmt.Errorf("adding template %s failed: is a directory", tmpl.filename) + } + // parse + var parsed *template.Template + if tmpl.standalone { + parsed, err = emptyTemplate.Clone() + parsed = parsed.New(tmpl.filename) + } else { + parsed, err = baseTemplate.Clone() + } + if err != nil { + return fmt.Errorf("cloning base template failed:\n%w", err) + } + parsed, err = parsed.ParseFiles(fullName) + if err != nil { + return fmt.Errorf("parsing template %s failed:\n%w", tmpl.filename, err) + } + templates[key] = parsed + } + return nil +} + +// Execute a template and write it to the given writer. +// +// Parameters: +// - name: name of the template to execute +// - data: passed through to the template +func WriteTemplate(w io.Writer, name string, data any) error { + tmpl, exists := templates[name] + if !exists { + return fmt.Errorf("template %s not available", name) + } + tmpl, err := tmpl.Clone() + if err != nil { + return fmt.Errorf("cloning template failed:\n%w", err) + } + err = tmpl.Execute(w, data) + if err != nil { + return fmt.Errorf("template execution failed:\n%w", err) + } + return nil +} + +// Execute a template and write it to a http writer. +// +// Similar to WriteTemplate, but in error case this adds a corresponding +// status code and writes an error message to the writer. +// +// Typically, this is the final action in handling an http request. +// +// Parameters: +// - name: name of the template to execute +// - data: passed through to the template +func ServeTemplate(w http.ResponseWriter, name string, data any) { + // TODO: make this return an error, handle on top of every request handler + err := WriteTemplate(w, name, data) + if err != nil { + err = fmt.Errorf("writing template failed:\n%w", err) + log.Println(err.Error()) + w.WriteHeader(http.StatusInternalServerError) + io.WriteString(w, `Internal Server Error. + +Du kannst uns helfen, indem du folgende Fehlermeldung per Mail +an sprechstunden@mathebau.de sendest: + +Fehler um\n +`+time.Now().String()+"\n"+err.Error()) + return + } +} diff --git a/templates/addFailure.html b/templating/templates/addFailure.html similarity index 100% rename from templates/addFailure.html rename to templating/templates/addFailure.html diff --git a/templates/addMask.html b/templating/templates/addMask.html similarity index 100% rename from templates/addMask.html rename to templating/templates/addMask.html diff --git a/templates/addSuccess.html b/templating/templates/addSuccess.html similarity index 100% rename from templates/addSuccess.html rename to templating/templates/addSuccess.html diff --git a/templates/base.html b/templating/templates/base.html similarity index 100% rename from templates/base.html rename to templating/templates/base.html diff --git a/templates/confirmationMail b/templating/templates/confirmationMail similarity index 100% rename from templates/confirmationMail rename to templating/templates/confirmationMail diff --git a/templates/deleteSuccess.html b/templating/templates/deleteSuccess.html similarity index 100% rename from templates/deleteSuccess.html rename to templating/templates/deleteSuccess.html diff --git a/templates/executeFailure.html b/templating/templates/executeFailure.html similarity index 100% rename from templates/executeFailure.html rename to templating/templates/executeFailure.html diff --git a/templates/executeSuccess.html b/templating/templates/executeSuccess.html similarity index 100% rename from templates/executeSuccess.html rename to templating/templates/executeSuccess.html diff --git a/templates/index.html b/templating/templates/index.html similarity index 100% rename from templates/index.html rename to templating/templates/index.html diff --git a/templates/officeHourTable.html b/templating/templates/officeHourTable.html similarity index 100% rename from templates/officeHourTable.html rename to templating/templates/officeHourTable.html diff --git a/templates/requestNotFound.html b/templating/templates/requestNotFound.html similarity index 100% rename from templates/requestNotFound.html rename to templating/templates/requestNotFound.html diff --git a/templates/td.html b/templating/templates/td.html similarity index 100% rename from templates/td.html rename to templating/templates/td.html