feat(web): show ICS subscription events in detail view

2026-09-02 20:02:48 +02:00
parent e18c83b618
commit f34041d889
11 changed files with 908 additions and 80 deletions
@@ -7,6 +7,8 @@ package icssub
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"io"
 	"net/http"
@@ -19,8 +21,8 @@ import (
 	"git.arnef.de/arnef/nidus/internal/icalfix"
 )

-// DefaultTTL is how long a fetched calendar is cached before being
-// re-fetched on the next access.
+// DefaultTTL is how long a fetched calendar is considered "fresh" before a
+// Get will kick off a background refresh.
 const DefaultTTL = 15 * time.Minute

 // fetchTimeout bounds how long a single upstream request may take, so one
@@ -32,54 +34,143 @@ const fetchTimeout = 15 * time.Second
 // response.
 const maxBodySize = 32 * 1024 * 1024 // 32 MiB

+// entry holds everything the Cache knows about a single upstream URL. All
+// fields are only read/written while holding Cache.mu.
 type entry struct {
+	url string // original URL as supplied by the caller (fetch normalizes)
+
+	// cal is the most recent successfully-fetched calendar. Nil until the
+	// first successful fetch for this URL.
+	cal *ical.Calendar
+	// lastErr is the most recent fetch error. Set alongside cal == nil
+	// (i.e. no successful fetch yet); cleared the moment a fetch succeeds.
+	lastErr error
+
+	// refreshing is true while a fetch (foreground, or background refresh)
+	// is in flight for this URL.
+	refreshing bool
+	// pending is the completion channel for the in-flight fetch. Only valid
+	// while refreshing is true; it is created fresh for each fetch and
+	// closed exactly once when that fetch finishes. Callers that see
+	// refreshing==true read this channel (under the lock) and wait on it.
+	pending chan struct{}
+
+	// fetchedAt is the wall-clock time of the most recent fetch attempt
+	// (success or failure), used for the TTL freshness check.
 	fetchedAt time.Time
-	cal       *ical.Calendar
-	err       error
 }

-// Cache fetches remote ICS calendars over HTTP(S), keeping a short-lived
-// in-memory copy per URL so repeated renders (e.g. every month-view page
-// load, or CalDAV client polling) don't re-fetch the same subscription
-// from origin every time.
+// Cache fetches remote ICS calendars over HTTP(S), keeping a shared
+// in-memory copy per URL. Semantics:
+//
+//   - Fresh entry (fetchedAt within TTL): return immediately, no I/O.
+//   - Stale entry with a cached copy: return the stale copy immediately
+//     AND spawn at most one background refresher (other callers in the
+//     same window piggyback on the in-flight refresh).
+//   - Stale entry with no cached copy (prior fetch failed): return the
+//     cached error immediately AND spawn a background retry.
+//   - No entry at all (very first call for this URL): block until a
+//     foreground fetch finishes (concurrent first-callers wait on a shared
+//     channel and all get the same result) and return its data.
 type Cache struct {
 	ttl    time.Duration
 	client *http.Client

-	mu      sync.Mutex
-	entries map[string]entry
+	mu   sync.Mutex
+	urls map[string]*entry // keyed by normalizeURL(url)
 }

 // NewCache creates a Cache with the given TTL (use DefaultTTL if unsure).
 func NewCache(ttl time.Duration) *Cache {
 	return &Cache{
-		ttl:     ttl,
-		client:  &http.Client{Timeout: fetchTimeout},
-		entries: make(map[string]entry),
+		ttl:    ttl,
+		client: &http.Client{},
+		urls:   make(map[string]*entry),
 	}
 }

-// Get returns the parsed calendar fetched from url, using a cached copy
-// if it's still within the TTL. If a fresh fetch fails but a previously
-// fetched copy exists, the stale copy is returned instead of the error,
-// so a transient network issue doesn't blank out the calendar entirely.
+// Get returns the most recently successfully-fetched calendar for url, or
+// the most-recent fetch error if no successful copy exists yet (a
+// background refresher may already be retrying).
 func (c *Cache) Get(url string) (*ical.Calendar, error) {
+	key := normalizeURL(url)
+
 	c.mu.Lock()
-	e, ok := c.entries[url]
-	fresh := ok && time.Since(e.fetchedAt) < c.ttl
-	c.mu.Unlock()
-	if fresh {
-		return e.cal, e.err
+	e := c.urls[key]
+	if e == nil {
+		e = &entry{url: url}
+		c.urls[key] = e
 	}

-	cal, err := c.fetch(url)
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	if err != nil && ok && e.cal != nil {
-		return e.cal, nil
+	switch {
+	case e.cal == nil && e.lastErr == nil && !e.refreshing:
+		// Very first request for this URL: do a foreground fetch.
+		e.refreshing = true
+		e.pending = make(chan struct{})
+		ch := e.pending
+		c.mu.Unlock()
+		go c.doFetch(e, ch)
+		<-ch
+		return c.snapshot(e)
+
+	case e.cal == nil && e.lastErr == nil:
+		// A foreground fetch is already in flight — wait for it.
+		ch := e.pending
+		c.mu.Unlock()
+		<-ch
+		return c.snapshot(e)
+
+	default:
+		// We have some data (a cached copy or a cached error).
+		if time.Since(e.fetchedAt) < c.ttl {
+			// Fresh — just return.
+			c.mu.Unlock()
+			return c.snapshot(e)
+		}
+		// Stale — return the cached value immediately; spawn at most one
+		// background refresher (or piggyback on one already in flight).
+		if !e.refreshing {
+			e.refreshing = true
+			ch := make(chan struct{})
+			e.pending = ch
+			c.mu.Unlock()
+			go c.doFetch(e, ch)
+		} else {
+			c.mu.Unlock()
+		}
+		return c.snapshot(e)
 	}
-	c.entries[url] = entry{fetchedAt: time.Now(), cal: cal, err: err}
-	return cal, err
+}
+
+// doFetch performs the network I/O for the entry, updates cal/lastErr and
+// the freshness timestamp under the lock, clears the in-flight state, and
+// closes the per-fetch completion channel exactly once.
+func (c *Cache) doFetch(e *entry, ch chan struct{}) {
+	cal, err := c.fetch(e.url)
+	c.mu.Lock()
+	e.fetchedAt = time.Now()
+	if err == nil {
+		e.cal = cal
+		e.lastErr = nil
+	} else {
+		e.lastErr = err
+	}
+	e.refreshing = false
+	e.pending = nil
+	c.mu.Unlock()
+	close(ch)
+}
+
+// snapshot reads e.cal/e.lastErr under c.mu and returns the same value
+// shape Get does. Callers must not hold c.mu.
+func (c *Cache) snapshot(e *entry) (*ical.Calendar, error) {
+	c.mu.Lock()
+	cal, err := e.cal, e.lastErr
+	c.mu.Unlock()
+	if cal != nil {
+		return cal, nil
+	}
+	return nil, err
 }

 // fetch downloads and parses url, translating a "webcal://" scheme (used
@@ -128,3 +219,40 @@ func normalizeURL(u string) string {
 	}
 	return u
 }
+
+// EventID returns a stable, short identifier for a single ical.Event,
+// suitable for use as a filesystem object name or URL path segment. It is
+// the first 32 hex chars (128 bits) of
+// sha256("<DTSTART-value>|<duration>|<SUMMARY>") with a ".ics" suffix.
+//
+// Two events with the same DTSTART, same duration, and same SUMMARY hash
+// to the same ID — this matches the addressing scheme used both by the
+// web detail view and the CalDAV backend for ICS-subscription events.
+// Returns "" if DTSTART is missing (not addressable).
+func EventID(ev ical.Event) string {
+	start := ev.Props.Get(ical.PropDateTimeStart)
+	if start == nil {
+		return ""
+	}
+	dur := ""
+	if end := ev.Props.Get(ical.PropDateTimeEnd); end != nil {
+		if s, err := start.DateTime(time.UTC); err == nil {
+			if e, err := end.DateTime(time.UTC); err == nil {
+				dur = e.Sub(s).Round(time.Second).String()
+			}
+		}
+	}
+	summary := ""
+	if p := ev.Props.Get(ical.PropSummary); p != nil {
+		summary = p.Value
+	}
+
+	var keyBuf strings.Builder
+	keyBuf.WriteString(start.Value)
+	keyBuf.WriteRune('|')
+	keyBuf.WriteString(dur)
+	keyBuf.WriteRune('|')
+	keyBuf.WriteString(summary)
+	sum := sha256.Sum256([]byte(keyBuf.String()))
+	return hex.EncodeToString(sum[:16]) + ".ics"
+}
@@ -0,0 +1,199 @@
+package icssub
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	ical "github.com/emersion/go-ical"
+)
+
+const sampleICS = "BEGIN:VCALENDAR\r\n" +
+	"VERSION:2.0\r\n" +
+	"PRODID:-//nidus//test//EN\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:ev1@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260805T090000Z\r\n" +
+	"DTEND:20260805T100000Z\r\n" +
+	"SUMMARY:Original title\r\n" +
+	"END:VEVENT\r\n" +
+	"END:VCALENDAR\r\n"
+
+func mustParseEvent(raw string) ical.Event {
+	cal, err := ical.NewDecoder(strings.NewReader(raw)).Decode()
+	if err != nil {
+		panic("mustParseEvent: " + err.Error())
+	}
+	evs := cal.Events()
+	if len(evs) == 0 {
+		panic("mustParseEvent: no events")
+	}
+	return evs[0]
+}
+
+// TestCacheFirstFetchPopulatesEntry verifies that the very first Get for a
+// URL does a foreground fetch and returns the parsed calendar.
+func TestCacheFirstFetchPopulatesEntry(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(sampleICS))
+	}))
+	defer srv.Close()
+
+	c := NewCache(time.Hour)
+	cal, err := c.Get(srv.URL)
+	if err != nil {
+		t.Fatalf("first Get: %v", err)
+	}
+	if len(cal.Events()) == 0 {
+		t.Fatalf("expected >=1 event, got 0")
+	}
+}
+
+// TestCacheStaleReturnsWithBackgroundRefresh verifies that once a cached
+// entry is stale, Get keeps returning the *stale* copy immediately while a
+// background refresh is spawned in a separate goroutine (rather than
+// blocking the caller on the network).
+func TestCacheStaleReturnsWithBackgroundRefresh(t *testing.T) {
+	var hits atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits.Add(1)
+		time.Sleep(50 * time.Millisecond) // make the fetch slow enough to
+		w.WriteHeader(http.StatusOK)      // observe as background work
+		_, _ = w.Write([]byte(sampleICS))
+	}))
+	defer srv.Close()
+
+	c := NewCache(1 * time.Millisecond) // very short TTL
+	if _, err := c.Get(srv.URL); err != nil {
+		t.Fatalf("first Get: %v", err)
+	}
+	firstHits := hits.Load()
+
+	// Force staleness.
+	time.Sleep(3 * time.Millisecond)
+
+	// Subsequent Get must return the stale copy immediately without waiting
+	// for the slow server to respond — if it blocked, this call would take
+	// >= 50ms, which we bound with a deadline below via the hit counter.
+	st := time.Now()
+	cal, err := c.Get(srv.URL)
+	if err != nil {
+		t.Fatalf("second Get: %v", err)
+	}
+	if len(cal.Events()) == 0 {
+		t.Fatalf("expected event in stale response")
+	}
+	if time.Since(st) > 40*time.Millisecond {
+		t.Fatalf("second Get appears to have blocked on the network for %v", time.Since(st))
+	}
+
+	// Wait for the background refresh to complete.
+	deadline := time.Now().Add(2 * time.Second)
+	for hits.Load() == firstHits && time.Now().Before(deadline) {
+		time.Sleep(5 * time.Millisecond)
+	}
+	if hits.Load() == firstHits {
+		t.Fatalf("background refresh did not fire (hits stayed at %d)", firstHits)
+	}
+}
+
+// TestCacheConcurrentFirstFetchCoalesce ensures N concurrent first calls
+// for the same URL coalesce to a small number of actual origin fetches.
+func TestCacheConcurrentFirstFetchCoalesce(t *testing.T) {
+	var hits atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits.Add(1)
+		time.Sleep(40 * time.Millisecond) // widen the coalescing window
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(sampleICS))
+	}))
+	defer srv.Close()
+
+	c := NewCache(time.Hour)
+
+	const n = 8
+	errs := make([]error, n)
+	var wg sync.WaitGroup
+	start := make(chan struct{})
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-start
+			_, err := c.Get(srv.URL)
+			errs[i] = err
+		}(i)
+	}
+	close(start)
+	wg.Wait()
+	for _, err := range errs {
+		if err != nil {
+			t.Fatalf("goroutine error: %v", err)
+		}
+	}
+	// Without coalescing we'd see n=8 hits; with it we expect a small
+	// constant (the first caller fetches; the rest either return the
+	// in-progress result or kick off a coalesced background refresh).
+	if hits.Load() > 3 {
+		t.Fatalf("expected coalescing to reduce fetches (got %d)", hits.Load())
+	}
+}
+
+// TestEventIDStable verifies that the same event hashes to the same ID
+// across calls, and is the right shape (32 hex chars + ".ics").
+func TestEventIDStable(t *testing.T) {
+	ev := mustParseEvent(sampleICS)
+	id1 := EventID(ev)
+	id2 := EventID(ev)
+	if id1 != id2 {
+		t.Fatalf("EventID not stable: %q vs %q", id1, id2)
+	}
+	if !strings.HasSuffix(id1, ".ics") {
+		t.Fatalf("EventID should end in .ics, got %q", id1)
+	}
+	if len(id1) != 32+4 {
+		t.Fatalf("expected 32 hex chars + .ics, got %q (len %d)", id1, len(id1))
+	}
+}
+
+// TestEventIDDiffersForDifferentEvents verifies that two events with
+// different titles or different start times hash to different IDs, while
+// two events with only a different UID hash to the same ID.
+func TestEventIDDiffersForDifferentEvents(t *testing.T) {
+	base := EventID(mustParseEvent(sampleICS))
+
+	// Different title.
+	rawA := strings.Replace(sampleICS, "SUMMARY:Original title", "SUMMARY:Other title", 1)
+	if EventID(mustParseEvent(rawA)) == base {
+		t.Fatal("same start, different title must hash differently")
+	}
+
+	// Different start time.
+	rawB := strings.Replace(sampleICS, "DTSTART:20260805T090000Z", "DTSTART:20260806T090000Z", 1)
+	if EventID(mustParseEvent(rawB)) == base {
+		t.Fatal("different start times must hash differently")
+	}
+
+	// Same start, same title, different UID → same hash.
+	rawC := strings.Replace(sampleICS, "UID:ev1@nidus.test", "UID:ev2@nidus.test", 1)
+	if EventID(mustParseEvent(rawC)) != base {
+		t.Fatal("UID should not affect the hash — same start/dur/title must be equal")
+	}
+}
+
+// TestEventIDWorksWithoutUID verifies EventID still produces a value (from
+// DTSTART+DTEND+SUMMARY) when the event has no UID property.
+func TestEventIDWorksWithoutUID(t *testing.T) {
+	raw := strings.Replace(sampleICS, "UID:ev1@nidus.test\r\n", "", 1)
+	ev := mustParseEvent(raw)
+	id := EventID(ev)
+	if id == "" {
+		t.Fatal("EventID should be non-empty even without UID")
+	}
+}