fix(kirjaa): open days older than the first entry ever logged
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.
This commit is contained in:
@@ -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
|
// 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.
|
// 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.
|
// 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 {
|
func (a *app) date(r *http.Request) time.Time {
|
||||||
now := today(a.loc)
|
now := today(a.loc)
|
||||||
if raw := r.FormValue("pvm"); raw != "" {
|
if raw := r.FormValue("pvm"); raw != "" {
|
||||||
if d, err := time.ParseInLocation(dateLayout, raw, a.loc); err == nil {
|
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
|
return now
|
||||||
|
case d.Before(floor):
|
||||||
|
return floor
|
||||||
}
|
}
|
||||||
return d
|
return d
|
||||||
}
|
}
|
||||||
@@ -205,15 +212,13 @@ func (a *app) buildLog(r *http.Request, o logOptions) logView {
|
|||||||
// list underneath the tap.
|
// list underneath the tap.
|
||||||
func (a *app) loadDays(r *http.Request, v *logView) {
|
func (a *app) loadDays(r *http.Request, v *logView) {
|
||||||
v.HistoryDays = historyWindow(r)
|
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
|
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 {
|
if err != nil {
|
||||||
log.Printf("history: %v", err)
|
log.Printf("history: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -100,10 +100,15 @@ func run() error {
|
|||||||
// local instance can pick another port.
|
// local instance can pick another port.
|
||||||
addr := cmp.Or(os.Getenv("ADDR"), listenAddr)
|
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{
|
srv := &http.Server{
|
||||||
Addr: addr,
|
Addr: addr,
|
||||||
Handler: routes(db, loc, password),
|
Handler: routes(db, loc, password),
|
||||||
ReadHeaderTimeout: 10 * time.Second,
|
ReadHeaderTimeout: 10 * time.Second,
|
||||||
|
WriteTimeout: 30 * time.Second,
|
||||||
|
IdleTimeout: 120 * time.Second,
|
||||||
}
|
}
|
||||||
|
|
||||||
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
|
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
|
||||||
|
|||||||
+96
-7
@@ -305,7 +305,8 @@ func updateMain(db *sql.DB, id int64, name string, categories []string, hasSides
|
|||||||
defer tx.Rollback()
|
defer tx.Rollback()
|
||||||
|
|
||||||
if _, err := tx.Exec(
|
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,
|
name, hasSides, id,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
return taken(err)
|
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
|
// Soft delete: the row stays so historical log entries keep resolving their
|
||||||
// names, but it disappears from the catalog and every picker (PRD §6).
|
// 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 {
|
func softDeleteMain(db *sql.DB, id int64) error {
|
||||||
_, err := db.Exec(
|
_, 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
|
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
|
// 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
|
// 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.
|
// 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
|
var first sql.NullString
|
||||||
if err := db.QueryRow(`SELECT min(date) FROM meal_log`).Scan(&first); err != nil {
|
if err := db.QueryRow(`SELECT min(date) FROM meal_log`).Scan(&first); err != nil {
|
||||||
if err == sql.ErrNoRows {
|
if err == sql.ErrNoRows {
|
||||||
@@ -457,14 +468,92 @@ func history(db *sql.DB, loc *time.Location, from time.Time, days int) (HistoryP
|
|||||||
oldest = firstDate
|
oldest = firstDate
|
||||||
page.More = false
|
page.More = false
|
||||||
}
|
}
|
||||||
|
if !reach.IsZero() && reach.Before(oldest) {
|
||||||
|
oldest = reach
|
||||||
|
page.More = firstDate.Before(oldest)
|
||||||
|
}
|
||||||
page.Next = oldest.AddDate(0, 0, -1)
|
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) {
|
for d := from; !d.Before(oldest); d = d.AddDate(0, 0, -1) {
|
||||||
entry, err := entryFor(db, d)
|
row := HistoryRow{Date: d}
|
||||||
if err != nil {
|
if e := entries[d.Format(dateLayout)]; e != nil {
|
||||||
return HistoryPage{}, err
|
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
|
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()
|
||||||
|
}
|
||||||
|
|||||||
@@ -235,7 +235,7 @@ func TestHistoryMarksUnloggedDaysAsGaps(t *testing.T) {
|
|||||||
t.Fatalf("save -3: %v", err)
|
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 {
|
if err != nil {
|
||||||
t.Fatalf("history: %v", err)
|
t.Fatalf("history: %v", err)
|
||||||
}
|
}
|
||||||
@@ -271,7 +271,7 @@ func TestHistoryPagesInWindows(t *testing.T) {
|
|||||||
t.Fatalf("save -9: %v", err)
|
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 {
|
if err != nil {
|
||||||
t.Fatalf("first window: %v", err)
|
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.
|
// 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 {
|
if err != nil {
|
||||||
t.Fatalf("second window: %v", err)
|
t.Fatalf("second window: %v", err)
|
||||||
}
|
}
|
||||||
@@ -305,7 +305,7 @@ func TestHistoryPagesInWindows(t *testing.T) {
|
|||||||
func TestHistoryEmptyWithoutEntries(t *testing.T) {
|
func TestHistoryEmptyWithoutEntries(t *testing.T) {
|
||||||
h := seeded(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 {
|
if err != nil {
|
||||||
t.Fatalf("history: %v", err)
|
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")
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user