fix(caldav): expand RRULE for recurring events

2026-09-06 15:49:09 +02:00
parent f35121b3dc
commit 8e946964ec
11 changed files with 1223 additions and 126 deletions
@@ -197,3 +197,273 @@ func TestEventIDWorksWithoutUID(t *testing.T) {
 		t.Fatal("EventID should be non-empty even without UID")
 	}
 }
+
+// outlookStyleSeries mimics the canonical Outlook/Exchange shape: one UID
+// carrying a base VEVENT (with RRULE) plus bare per-occurrence instance
+// VEVENTs (the "fully expanded" ICS model). GroupSeries must fold all of
+// them into ONE Series (base + 2 instances), not two separate entities.
+const outlookStyleSeries = "BEGIN:VCALENDAR\r\n" +
+	"VERSION:2.0\r\n" +
+	"PRODID:-//nidus//test//EN\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:series@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260306T103000Z\r\n" +
+	"DTEND:20260306T113000Z\r\n" +
+	"RRULE:FREQ=WEEKLY;UNTIL=20260903T083000Z;INTERVAL=3;BYDAY=FR;WKST=SU\r\n" +
+	"SUMMARY:#smarttouch Planning\r\n" +
+	"END:VEVENT\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:series@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260320T103000Z\r\n" +
+	"DTEND:20260320T113000Z\r\n" +
+	"SUMMARY:#smarttouch Planning\r\n" +
+	"END:VEVENT\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:series@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260403T103000Z\r\n" +
+	"DTEND:20260403T123000Z\r\n" +
+	"SUMMARY:#smarttouch Planning (extended)\r\n" +
+	"END:VEVENT\r\n" +
+	"END:VCALENDAR\r\n"
+
+func TestGroupSeriesFoldsOutlookSeriesShape(t *testing.T) {
+	cal, err := ical.NewDecoder(strings.NewReader(outlookStyleSeries)).Decode()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := len(cal.Events()); got != 3 {
+		t.Fatalf("fixture must have 3 VEVENTs, got %d", got)
+	}
+	series := GroupSeries(cal)
+	if len(series) != 1 {
+		t.Fatalf("GroupSeries: expected 1 series, got %d", len(series))
+	}
+	s := series[0]
+	if s.Base == nil {
+		t.Fatal("the base (RRULE) VEVENT must be the series Base")
+	}
+	if s.Base.Props.Get(ical.PropUID).Value != "series@nidus.test" {
+		t.Fatalf("wrong base UID: %q", s.Base.Props.Get(ical.PropUID).Value)
+	}
+	if len(s.Instances) != 2 {
+		t.Fatalf("expected 2 explicit instances, got %d", len(s.Instances))
+	}
+	if s.ID() == "" {
+		t.Fatal("series ID must be non-empty")
+	}
+	// Anchor must be the base.
+	if got := s.Anchor().Props.Get(ical.PropUID).Value; got != "series@nidus.test" {
+		t.Fatalf("Anchor must be the base, got UID %q", got)
+	}
+	// Calendar() must contain all three VEVENTs.
+	if got := s.Calendar().Events(); len(got) != 3 {
+		t.Fatalf("Calendar() must contain 3 events, got %d", len(got))
+	}
+}
+
+// tzAwareSeries mimics a real Outlook/Exchange feed (already run through
+// icalfix.NormalizeTimeZones, hence "Europe/Berlin" rather than the raw
+// Windows zone name): a VCALENDAR with one VTIMEZONE the events actually
+// reference, plus a second, unrelated VTIMEZONE no event references.
+const tzAwareSeries = "BEGIN:VCALENDAR\r\n" +
+	"VERSION:2.0\r\n" +
+	"PRODID:-//nidus//test//EN\r\n" +
+	"BEGIN:VTIMEZONE\r\n" +
+	"TZID:Europe/Berlin\r\n" +
+	"END:VTIMEZONE\r\n" +
+	"BEGIN:VTIMEZONE\r\n" +
+	"TZID:America/New_York\r\n" +
+	"END:VTIMEZONE\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:tz-series@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART;TZID=Europe/Berlin:20260910T100000\r\n" +
+	"DTEND;TZID=Europe/Berlin:20260910T110000\r\n" +
+	"RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=WE\r\n" +
+	"SUMMARY:Frontend Weekly\r\n" +
+	"END:VEVENT\r\n" +
+	"END:VCALENDAR\r\n"
+
+// TestSeriesCalendarEmbedsReferencedTimezone is the regression test for
+// the bug report "Frontend Weekly / Abstimmung Next-Release Package /
+// Prd-Deployment don't show up in the CalDAV client": Series.Calendar()
+// used to build a VTIMEZONE-less VCALENDAR for every ICS-subscription
+// series, even though its VEVENTs' DTSTART/DTEND still carried a
+// TZID=Europe/Berlin parameter — an RFC 5545 §3.6.5 violation that made
+// strict clients (notably ical4j, which DAVx5 is built on) fail to
+// resolve the timezone for recurrence-rule expansion, silently dropping
+// the whole (recurring) VEVENT from sync. The fix must embed exactly the
+// VTIMEZONE(s) the series' own VEVENTs reference — no more, no less.
+func TestSeriesCalendarEmbedsReferencedTimezone(t *testing.T) {
+	cal, err := ical.NewDecoder(strings.NewReader(tzAwareSeries)).Decode()
+	if err != nil {
+		t.Fatal(err)
+	}
+	series := GroupSeries(cal)
+	if len(series) != 1 {
+		t.Fatalf("expected 1 series, got %d", len(series))
+	}
+	out := series[0].Calendar()
+
+	var tzids []string
+	for _, child := range out.Children {
+		if child.Name == ical.CompTimezone {
+			tzids = append(tzids, child.Props.Get(ical.PropTimezoneID).Value)
+		}
+	}
+	if len(tzids) != 1 || tzids[0] != "Europe/Berlin" {
+		t.Fatalf("expected exactly the referenced VTIMEZONE (Europe/Berlin) to be embedded, got %v", tzids)
+	}
+
+	// Re-encoding and re-decoding must round-trip: the VEVENT's DTSTART
+	// TZID must resolve to a VTIMEZONE actually present in the same
+	// object (this is what a strict client like ical4j checks).
+	if got := len(out.Events()); got != 1 {
+		t.Fatalf("expected 1 VEVENT in the series calendar, got %d", got)
+	}
+}
+
+// twoDistinctSeries are two completely unrelated events (different UIDs).
+// GroupSeries must NOT fold them together.
+const twoDistinctSeries = "BEGIN:VCALENDAR\r\n" +
+	"VERSION:2.0\r\n" +
+	"PRODID:-//nidus//test//EN\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:a@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260809T090000Z\r\n" +
+	"DTEND:20260809T100000Z\r\n" +
+	"SUMMARY:meeting A\r\n" +
+	"END:VEVENT\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:b@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260809T110000Z\r\n" +
+	"DTEND:20260809T120000Z\r\n" +
+	"SUMMARY:meeting B\r\n" +
+	"END:VEVENT\r\n" +
+	"END:VCALENDAR\r\n"
+
+func TestGroupSeriesKeepsDistinctSeries(t *testing.T) {
+	cal, err := ical.NewDecoder(strings.NewReader(twoDistinctSeries)).Decode()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := len(GroupSeries(cal)); got != 2 {
+		t.Fatalf("two distinct UIDs must produce two series, got %d", got)
+	}
+}
+
+// seriesOnlyInstances is the shape of a set of one-off events on one UID
+// with no RRULE. The earliest instance must anchor the series.
+const seriesOnlyInstances = "BEGIN:VCALENDAR\r\n" +
+	"VERSION:2.0\r\n" +
+	"PRODID:-//nidus//test//EN\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:g@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260810T090000Z\r\n" +
+	"DTEND:20260810T100000Z\r\n" +
+	"SUMMARY:later\r\n" +
+	"END:VEVENT\r\n" +
+	"BEGIN:VEVENT\r\n" +
+	"UID:g@nidus.test\r\n" +
+	"DTSTAMP:20260101T000000Z\r\n" +
+	"DTSTART:20260809T090000Z\r\n" +
+	"DTEND:20260809T100000Z\r\n" +
+	"SUMMARY:earlier\r\n" +
+	"END:VEVENT\r\n" +
+	"END:VCALENDAR\r\n"
+
+func TestGroupSeriesAnchorWithoutBase(t *testing.T) {
+	cal, err := ical.NewDecoder(strings.NewReader(seriesOnlyInstances)).Decode()
+	if err != nil {
+		t.Fatal(err)
+	}
+	series := GroupSeries(cal)
+	if len(series) != 1 {
+		t.Fatalf("expected 1 series, got %d", len(series))
+	}
+	s := series[0]
+	if s.Base != nil {
+		t.Fatalf("no event carries RRULE, so Base must be nil, got %v", s.Base)
+	}
+	if len(s.Instances) != 2 {
+		t.Fatalf("expected 2 instances, got %d", len(s.Instances))
+	}
+	anchor := s.Anchor()
+	if anchor == nil {
+		t.Fatal("Anchor must be non-nil for a non-empty series")
+	}
+	sp := anchor.Props.Get(ical.PropSummary)
+	if sp == nil || sp.Value != "earlier" {
+		t.Fatalf("Anchor should be the earliest instance, got %v", sp)
+	}
+}
+
+// TestSeriesOccurrencesInReturnsOneDayPerDay verifies the crux of the
+// web-grid fix: on a day the RRULE base covers AND an explicit instance
+// covers, OccurrencesIn returns exactly one event for that day (the
+// instance, which "wins" per the Exchange override model), and does not
+// double-paint.
+func TestSeriesOccurrencesInReturnsOneDayPerDay(t *testing.T) {
+	cal, err := ical.NewDecoder(strings.NewReader(outlookStyleSeries)).Decode()
+	if err != nil {
+		t.Fatal(err)
+	}
+	series := GroupSeries(cal)
+	if len(series) != 1 {
+		t.Fatalf("expected 1 series, got %d", len(series))
+	}
+	s := series[0]
+
+	loc := time.UTC
+	// Window that contains 2026-03-20 (which is both an RRULE date and a
+	// separate explicit instance in the fixture).
+	gs := time.Date(2026, 3, 1, 0, 0, 0, 0, loc)
+	ge := time.Date(2026, 4, 10, 0, 0, 0, 0, loc)
+
+	occ := s.OccurrencesIn(gs, ge, loc)
+	if len(occ) < 3 {
+		t.Fatalf("expected at least 3 days of occurrences, got %d: %v", len(occ), keysOf(occ))
+	}
+	// The explicit instance on 2026-03-20 (title "#smarttouch Planning")
+	// must be the representative event for that day (the base also covers
+	// that day via RRULE — but the instance wins).
+	evOnMar20, ok := occ["2026-03-20"]
+	if !ok {
+		t.Fatalf("2026-03-20 is in the window AND is an explicit instance — must be present, got %v", keysOf(occ))
+	}
+	if p := evOnMar20.Props.Get(ical.PropSummary); p == nil || p.Value != "#smarttouch Planning" {
+		t.Fatalf("instance on 2026-03-20 should win over the base; got %v", p)
+	}
+	// 2026-04-03 has an explicit instance with a different title — should
+	// be that title (not the base title) for that day.
+	evOnApr3, ok := occ["2026-04-03"]
+	if !ok {
+		t.Fatalf("2026-04-03 must be present, got %v", keysOf(occ))
+	}
+	if p := evOnApr3.Props.Get(ical.PropSummary); p == nil || p.Value != "#smarttouch Planning (extended)" {
+		t.Fatalf("instance on 2026-04-03 should have the overridden title; got %v", p)
+	}
+	// 2026-03-06 is the RRULE base's DTSTART and NOT explicitly overridden —
+	// the base should be the representative for that day.
+	evOnBase, ok := occ["2026-03-06"]
+	if !ok {
+		t.Fatalf("2026-03-06 (base DTSTART) must be present, got %v", keysOf(occ))
+	}
+	if p := evOnBase.Props.Get(ical.PropSummary); p == nil || p.Value != "#smarttouch Planning" {
+		t.Fatalf("base should represent 2026-03-06; got %v", p)
+	}
+}
+
+func keysOf(m map[string]ical.Event) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	return out
+}