mavcaldav: reconcile the render collection on startup

Withdrawal read published, which is in-memory, so the second loop only ever withdrew reminders this process had published. Fire a reminder, restart mavcaldav, and its event stayed in the collection forever with nothing left to revisit it. "Losing it costs nothing, the next tick rebuilds it" holds for events that should be there and not for the ones that should not. The first tick now PROPFINDs the collection and reconciles what it finds against what is pending. Only hrefs carrying ReminderUIDPrefix are read back, so the pass can never propose deleting a file maven did not create. A failed read is retried on the next tick rather than skipped for the life of the process. Two smaller things from the same review. checkRenderTarget takes the whole read set, so a second calendar to read cannot quietly fall outside the guarantee the package comment makes. writeIfChanged loses its confidence parameter, which every caller passed 1.0 and nothing read. Found in review of #56.
2026-08-01 14:11:07 +04:00
parent bddf52d1ee
commit 1c94df76b7
5 changed files with 264 additions and 34 deletions
@@ -2,9 +2,11 @@ package main

 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -27,12 +29,16 @@ func (c *reminderCore) ListReminders(context.Context, int) ([]ipc.Reminder, erro
 	return c.reminders, nil
 }

-// calSrv records what a CalDAV collection received.
+// calSrv records what a CalDAV collection received. existing seeds resources
+// that were already in the collection before this process started, which is
+// what a restart looks like from the renderer's side.
 type calSrv struct {
-	mu     sync.Mutex
-	puts   map[string]string
-	dels   []string
-	status int
+	mu       sync.Mutex
+	puts     map[string]string
+	dels     []string
+	existing []string
+	propfind int
+	status   int
 	*httptest.Server
 }

@@ -47,12 +53,43 @@ func newCalSrv() *calSrv {
 			s.puts[strings.TrimPrefix(r.URL.Path, "/cal/")] = string(body)
 		case http.MethodDelete:
 			s.dels = append(s.dels, strings.TrimPrefix(r.URL.Path, "/cal/"))
+		case "PROPFIND":
+			s.propfind++
+			w.Header().Set("Content-Type", "application/xml; charset=utf-8")
+			w.WriteHeader(http.StatusMultiStatus)
+			io.WriteString(w, s.multistatusLocked(r.URL.Path))
+			return
 		}
 		w.WriteHeader(s.status)
 	}))
 	return s
 }

+// multistatusLocked renders the collection listing. Caller holds the lock.
+func (s *calSrv) multistatusLocked(base string) string {
+	var b strings.Builder
+	b.WriteString(`<?xml version="1.0"?><D:multistatus xmlns:D="DAV:">`)
+	b.WriteString("<D:response><D:href>" + base + "</D:href></D:response>")
+	names := append([]string{}, s.existing...)
+	for name := range s.puts {
+		names = append(names, name)
+	}
+	for _, name := range names {
+		if slices.Contains(s.dels, name) {
+			continue
+		}
+		b.WriteString("<D:response><D:href>/cal/" + name + "</D:href></D:response>")
+	}
+	b.WriteString("</D:multistatus>")
+	return b.String()
+}
+
+func (s *calSrv) deleted() []string {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return append([]string{}, s.dels...)
+}
+
 func (s *calSrv) putCount() int {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -168,19 +205,97 @@ func TestRenderOnceUsesNextFireForRecurring(t *testing.T) {

 func TestCheckRenderTargetRefusesTheCalendarItReads(t *testing.T) {
 	read := "http://localhost:5232/kami/personal"
-	if err := checkRenderTarget(read, ""); err != nil {
+	if err := checkRenderTarget([]string{read}, ""); err != nil {
 		t.Fatalf("rendering off must be fine: %v", err)
 	}
-	if err := checkRenderTarget(read, "http://localhost:5232/kami/maven"); err != nil {
+	if err := checkRenderTarget([]string{read}, "http://localhost:5232/kami/maven"); err != nil {
 		t.Fatalf("a distinct collection must be accepted: %v", err)
 	}
-	if err := checkRenderTarget(read, read); err == nil {
+	if err := checkRenderTarget([]string{read}, read); err == nil {
 		t.Error("rendering into the read calendar must be refused")
 	}
-	if err := checkRenderTarget(read, read+"/"); err == nil {
+	if err := checkRenderTarget([]string{read}, read+"/"); err == nil {
 		t.Error("a trailing slash must not defeat the check")
 	}
-	if err := checkRenderTarget(read, strings.ToUpper(read)); err == nil {
+	if err := checkRenderTarget([]string{read}, strings.ToUpper(read)); err == nil {
 		t.Error("case must not defeat the check")
 	}
+	// Every read target is checked, not the first one. A second calendar to
+	// read must not fall outside the guarantee just by being added later.
+	work := "http://localhost:5232/kami/work"
+	if err := checkRenderTarget([]string{read, work}, work); err == nil {
+		t.Error("rendering into the second read calendar must be refused")
+	}
+	if err := checkRenderTarget([]string{read, work}, "http://localhost:5232/kami/maven"); err != nil {
+		t.Fatalf("a collection maven owns must still be accepted: %v", err)
+	}
+}
+
+// Withdrawal has to survive a restart. published is in-memory, so a fresh
+// process knows nothing about the events an earlier one wrote: fire a reminder,
+// restart mavcaldav, and its event used to sit in the collection forever
+// because nothing ever revisited it. The first tick reads the collection and
+// reconciles what it finds against what is pending.
+func TestRenderOnceWithdrawsAfterRestart(t *testing.T) {
+	srv := newCalSrv()
+	defer srv.Close()
+	// Left behind by a previous process: 4 is still pending, 5 has fired.
+	// The third file is not maven's and must not be touched.
+	srv.existing = []string{"maven-reminder-4.ics", "maven-reminder-5.ics", "dentist.ics"}
+
+	core := &reminderCore{reminders: []ipc.Reminder{
+		{ID: 4, FireTs: time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC), Payload: "выпить воды", Status: "pending"},
+		{ID: 5, FireTs: time.Date(2026, 8, 1, 8, 0, 0, 0, time.UTC), Payload: "уже прозвенело", Status: "fired"},
+	}}
+	r := newRenderer(core, srv.Client(), srv.URL+"/cal", "u", "p", 0)
+	r.renderOnce(context.Background())
+
+	dels := srv.deleted()
+	if len(dels) != 1 || dels[0] != "maven-reminder-5.ics" {
+		t.Fatalf("deleted %v, want only the fired reminder's event", dels)
+	}
+
+	// The collection is read once, not on every tick.
+	r.renderOnce(context.Background())
+	srv.mu.Lock()
+	n := srv.propfind
+	srv.mu.Unlock()
+	if n != 1 {
+		t.Errorf("PROPFIND ran %d times, want once per process", n)
+	}
+}
+
+// A collection maven cannot read is not a reason to stop publishing to it, and
+// the reconciliation must be retried rather than skipped for the process.
+func TestRenderOnceRetriesReconcile(t *testing.T) {
+	srv := newCalSrv()
+	defer srv.Close()
+	srv.existing = []string{"maven-reminder-6.ics"}
+	failing := &http.Client{Transport: &propfindFailure{base: srv.Client().Transport}}
+
+	core := &reminderCore{}
+	r := newRenderer(core, failing, srv.URL+"/cal", "u", "p", 0)
+	r.renderOnce(context.Background())
+	if got := srv.deleted(); len(got) != 0 {
+		t.Fatalf("nothing can be withdrawn on a failed read: %v", got)
+	}
+	if r.reconciled {
+		t.Fatal("a failed read must not count as reconciled")
+	}
+
+	r.http = srv.Client()
+	r.renderOnce(context.Background())
+	if got := srv.deleted(); len(got) != 1 || got[0] != "maven-reminder-6.ics" {
+		t.Fatalf("deleted %v, want the orphaned event on the retry", got)
+	}
+}
+
+// propfindFailure fails PROPFIND and passes everything else through.
+type propfindFailure struct{ base http.RoundTripper }
+
+func (f *propfindFailure) RoundTrip(req *http.Request) (*http.Response, error) {
+	if req.Method == "PROPFIND" {
+		return nil, errors.New("collection unreachable")
+	}
+	return f.base.RoundTrip(req)
 }