From 0f0f04240ab255d752c57ea9f889bc9313a93f96 Mon Sep 17 00:00:00 2001 From: Esa Kataja Date: Sun, 6 Sep 2026 00:23:10 +0300 Subject: [PATCH] fix(kirjaa): open days older than the first entry ever logged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a date from before the oldest log entry rendered nothing at all. history() truncates its window at min(date) — correct, there is no history before the first entry to be missing — and loadDays widened the window by inflating the day count, which that truncation then undid. No row for the selected day meant no board to log it in, so the one thing you would want an old empty day for was the one thing you could not do. The same happened past maxHistoryDays, reachable straight from the date picker. history() now takes the selected day as an explicit floor instead of the caller guessing a day count, and date() clamps the past at maxHistoryDays the way it already clamped the future — the list runs unbroken from today down to the selection, so a picker set to 1994 would otherwise ask for eleven thousand rows. While in there: - entriesBetween replaces the day-at-a-time entryFor loop. Two queries for the whole window rather than two per day; the widest window a URL can ask for was 3,600 round trips through a pool of exactly one connection. - softDeleteMain and updateMain filter on special = 0. The catalog never lists Tähteet, but a stale tab or a hand-made POST could still have removed the row migration 0002 guarantees. - WriteTimeout and IdleTimeout on the server. With one database connection, a reader stalling on a long history response blocks everything behind it. Two tests, both of which fail on the old code: a day 100 back with only today logged, and sides landing on their own day now that they arrive in one query. --- cmd/foodster/handlers.go | 21 +++++--- cmd/foodster/main.go | 5 ++ cmd/foodster/store.go | 103 ++++++++++++++++++++++++++++++++++--- cmd/foodster/store_test.go | 69 +++++++++++++++++++++++-- 4 files changed, 179 insertions(+), 19 deletions(-) diff --git a/cmd/foodster/handlers.go b/cmd/foodster/handlers.go index 85062f7..98fab4d 100644 --- a/cmd/foodster/handlers.go +++ b/cmd/foodster/handlers.go @@ -60,12 +60,19 @@ func render(w http.ResponseWriter, r *http.Request, c templ.Component) { // there is nothing to write down for a dinner that has not happened, and a // stray entry dated next year would sit at the top of the history forever. // Every read and write goes through here, so the clamp covers them all. +// +// The past is clamped too, at maxHistoryDays. The day list runs unbroken from +// today down to the selected day, so a picker set to 1994 would ask for eleven +// thousand rows. Same ceiling ?paivat= already has. func (a *app) date(r *http.Request) time.Time { now := today(a.loc) if raw := r.FormValue("pvm"); raw != "" { if d, err := time.ParseInLocation(dateLayout, raw, a.loc); err == nil { - if d.After(now) { + switch floor := now.AddDate(0, 0, -maxHistoryDays+1); { + case d.After(now): return now + case d.Before(floor): + return floor } return d } @@ -205,15 +212,13 @@ func (a *app) buildLog(r *http.Request, o logOptions) logView { // list underneath the tap. func (a *app) loadDays(r *http.Request, v *logView) { v.HistoryDays = historyWindow(r) - - // The window has to reach the selected day, or it would have nowhere to - // expand. - if reach := int(v.Today.Sub(v.Date).Hours()/24) + 1; reach > v.HistoryDays { - v.HistoryDays = min(reach, maxHistoryDays) - } v.HistoryMore = v.HistoryDays + historyDays - page, err := history(a.db, a.loc, v.Today, v.HistoryDays) + // The window has to reach the selected day, or it would have nowhere to + // expand. history() takes it as a floor rather than the caller inflating + // the day count, because the window also truncates at the first entry ever + // logged — and a day older than that still has to be loggable. + page, err := history(a.db, a.loc, v.Today, v.HistoryDays, v.Date) if err != nil { log.Printf("history: %v", err) } diff --git a/cmd/foodster/main.go b/cmd/foodster/main.go index 96d1078..8cb97f7 100644 --- a/cmd/foodster/main.go +++ b/cmd/foodster/main.go @@ -100,10 +100,15 @@ func run() error { // local instance can pick another port. addr := cmp.Or(os.Getenv("ADDR"), listenAddr) + // WriteTimeout and IdleTimeout matter more than they look: the pool holds + // exactly one database connection, so a reader stalling on a long history + // response blocks every other request behind it. srv := &http.Server{ Addr: addr, Handler: routes(db, loc, password), ReadHeaderTimeout: 10 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) diff --git a/cmd/foodster/store.go b/cmd/foodster/store.go index 161fae7..6c38a71 100644 --- a/cmd/foodster/store.go +++ b/cmd/foodster/store.go @@ -305,7 +305,8 @@ func updateMain(db *sql.DB, id int64, name string, categories []string, hasSides defer tx.Rollback() if _, err := tx.Exec( - `UPDATE main_dishes SET name = ?, has_sides = ? WHERE id = ? AND deleted_at IS NULL`, + `UPDATE main_dishes SET name = ?, has_sides = ? + WHERE id = ? AND deleted_at IS NULL AND special = 0`, name, hasSides, id, ); err != nil { return taken(err) @@ -345,9 +346,14 @@ func updateSide(db *sql.DB, id int64, name string) error { // Soft delete: the row stays so historical log entries keep resolving their // names, but it disappears from the catalog and every picker (PRD §6). +// +// `special = 0` here and in updateMain: the catalog never lists Tähteet, so +// the UI cannot reach it, but a stale tab or a hand-made POST could — and +// removing it would take away the row migration 0002 guarantees. func softDeleteMain(db *sql.DB, id int64) error { _, err := db.Exec( - `UPDATE main_dishes SET deleted_at = datetime('now') WHERE id = ?`, id) + `UPDATE main_dishes SET deleted_at = datetime('now') + WHERE id = ? AND special = 0`, id) return err } @@ -432,7 +438,12 @@ type HistoryPage struct { // history walks back day by day from a given day, so a day nobody wrote down // shows up as an explicit gap rather than silently missing. It stops at the // first entry ever recorded — before that there is no history to be missing. -func history(db *sql.DB, loc *time.Location, from time.Time, days int) (HistoryPage, error) { +// +// reach names a day that must appear whatever the window says. The log board +// opens inside the selected day's row, so a date picked from before the first +// entry ever logged used to render nothing at all: no row, no board, no way to +// log it. Pass the zero time to ask for the plain window. +func history(db *sql.DB, loc *time.Location, from time.Time, days int, reach time.Time) (HistoryPage, error) { var first sql.NullString if err := db.QueryRow(`SELECT min(date) FROM meal_log`).Scan(&first); err != nil { if err == sql.ErrNoRows { @@ -457,14 +468,92 @@ func history(db *sql.DB, loc *time.Location, from time.Time, days int) (HistoryP oldest = firstDate page.More = false } + if !reach.IsZero() && reach.Before(oldest) { + oldest = reach + page.More = firstDate.Before(oldest) + } page.Next = oldest.AddDate(0, 0, -1) + entries, err := entriesBetween(db, oldest, from) + if err != nil { + return HistoryPage{}, err + } for d := from; !d.Before(oldest); d = d.AddDate(0, 0, -1) { - entry, err := entryFor(db, d) - if err != nil { - return HistoryPage{}, err + row := HistoryRow{Date: d} + if e := entries[d.Format(dateLayout)]; e != nil { + e.Date = d + row.Entry = e } - page.Rows = append(page.Rows, HistoryRow{Date: d, Entry: entry}) + page.Rows = append(page.Rows, row) } return page, nil } + +// entriesBetween loads every logged day in the inclusive range, keyed by +// stored date string, in two queries rather than two per day. The widest +// window a URL can ask for is five years, which day-at-a-time made 3,600 round +// trips through a pool of exactly one connection. +func entriesBetween(db *sql.DB, from, to time.Time) (map[string]*Entry, error) { + lo, hi := from.Format(dateLayout), to.Format(dateLayout) + + rows, err := db.Query(` + SELECT l.id, l.date, m.id, m.name, m.has_sides, + coalesce((SELECT group_concat(c.category) + FROM main_dish_categories c + WHERE c.main_dish_id = m.id), '') + FROM meal_log l + JOIN main_dishes m ON m.id = l.main_dish_id + WHERE l.date BETWEEN ? AND ?`, lo, hi) + if err != nil { + return nil, err + } + defer rows.Close() + + byDate := map[string]*Entry{} + byLog := map[int64]*Entry{} + for rows.Next() { + var logID int64 + var date, cats string + var e Entry + if err := rows.Scan( + &logID, &date, &e.Main.ID, &e.Main.Name, &e.Main.HasSides, &cats, + ); err != nil { + return nil, err + } + if cats != "" { + e.Main.Categories = strings.Split(cats, ",") + } + byDate[date] = &e + byLog[logID] = &e + } + if err := rows.Err(); err != nil { + return nil, err + } + if len(byLog) == 0 { + return byDate, nil + } + + sides, err := db.Query(` + SELECT ls.meal_log_id, s.id, s.name + FROM meal_log_sides ls + JOIN side_dishes s ON s.id = ls.side_dish_id + JOIN meal_log l ON l.id = ls.meal_log_id + WHERE l.date BETWEEN ? AND ? + ORDER BY s.name`, lo, hi) + if err != nil { + return nil, err + } + defer sides.Close() + + for sides.Next() { + var logID int64 + var s Side + if err := sides.Scan(&logID, &s.ID, &s.Name); err != nil { + return nil, err + } + if e := byLog[logID]; e != nil { + e.Sides = append(e.Sides, s) + } + } + return byDate, sides.Err() +} diff --git a/cmd/foodster/store_test.go b/cmd/foodster/store_test.go index 1a2fdb7..a9a043d 100644 --- a/cmd/foodster/store_test.go +++ b/cmd/foodster/store_test.go @@ -235,7 +235,7 @@ func TestHistoryMarksUnloggedDaysAsGaps(t *testing.T) { t.Fatalf("save -3: %v", err) } - page, err := history(h.db, loc, now, 60) + page, err := history(h.db, loc, now, 60, time.Time{}) if err != nil { t.Fatalf("history: %v", err) } @@ -271,7 +271,7 @@ func TestHistoryPagesInWindows(t *testing.T) { t.Fatalf("save -9: %v", err) } - first, err := history(h.db, loc, now, 5) + first, err := history(h.db, loc, now, 5, time.Time{}) if err != nil { t.Fatalf("first window: %v", err) } @@ -286,7 +286,7 @@ func TestHistoryPagesInWindows(t *testing.T) { } // The windows must meet exactly: no day repeated, none skipped. - second, err := history(h.db, loc, first.Next, 5) + second, err := history(h.db, loc, first.Next, 5, time.Time{}) if err != nil { t.Fatalf("second window: %v", err) } @@ -305,7 +305,7 @@ func TestHistoryPagesInWindows(t *testing.T) { func TestHistoryEmptyWithoutEntries(t *testing.T) { h := seeded(t) - page, err := history(h.db, time.UTC, today(time.UTC), 60) + page, err := history(h.db, time.UTC, today(time.UTC), 60, time.Time{}) if err != nil { t.Fatalf("history: %v", err) } @@ -316,3 +316,64 @@ func TestHistoryEmptyWithoutEntries(t *testing.T) { t.Error("More is set although there is no history at all") } } + +// A date picked from before the first entry ever logged used to fall outside +// the window entirely: no row, so the log board had nothing to open in and the +// day could not be filled in at all. +func TestHistoryReachesDaysOlderThanTheFirstEntry(t *testing.T) { + h := seeded(t) + loc := time.UTC + now := today(loc) + + if err := saveEntry(h.db, now, h.mainNamed(t, "Lohikeitto"), nil); err != nil { + t.Fatalf("save today: %v", err) + } + + want := now.AddDate(0, 0, -100) + page, err := history(h.db, loc, now, 30, want) + if err != nil { + t.Fatalf("history: %v", err) + } + if len(page.Rows) != 101 { + t.Fatalf("%d rows, want 101 (today back to the selected day)", len(page.Rows)) + } + last := page.Rows[len(page.Rows)-1] + if !last.Date.Equal(want) { + t.Errorf("last row is %s, want the selected %s", + last.Date.Format(dateLayout), want.Format(dateLayout)) + } + if page.More { + t.Error("More is set although the window reached past the oldest entry") + } +} + +// The sides of every day come back in one query now; each still has to land on +// its own day. +func TestHistoryKeepsSidesWithTheirOwnDay(t *testing.T) { + h := seeded(t) + loc := time.UTC + now := today(loc) + + muusi := h.sideNamed(t, "Perunamuusi") + riisi := h.sideNamed(t, "Riisi") + if err := saveEntry(h.db, now, h.mainNamed(t, "Lohikeitto"), []int64{muusi}); err != nil { + t.Fatalf("save today: %v", err) + } + if err := saveEntry(h.db, now.AddDate(0, 0, -1), h.mainNamed(t, "Lihapullat"), []int64{riisi}); err != nil { + t.Fatalf("save -1: %v", err) + } + + page, err := history(h.db, loc, now, 30, time.Time{}) + if err != nil { + t.Fatalf("history: %v", err) + } + if len(page.Rows) != 2 { + t.Fatalf("%d rows, want 2", len(page.Rows)) + } + for i, want := range []string{"Perunamuusi", "Riisi"} { + got := page.Rows[i].Entry + if got == nil || len(got.Sides) != 1 || got.Sides[0].Name != want { + t.Errorf("row %d sides = %+v, want just %s", i, got.Sides, want) + } + } +}