Fix: Event with `RECURRENCE-ID` overrides occurrence from orginal event #864

axunonb · 2025-09-17T13:44:16Z

Resolves #863
Resolves #865

Resolves #863

codecov · 2025-09-17T13:44:36Z

Codecov Report

❌ Patch coverage is 65.90909% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Ical.Net/Calendar.cs	65.9%	0 Missing and 15 partials ⚠️

❌ Your patch status has failed because the patch coverage (65.9%) is below the target coverage (80.0%). You can increase the patch coverage or adjust the target coverage.

@@           Coverage Diff           @@
##            main    #864     +/-   ##
=======================================
- Coverage   68.0%   67.9%   -0.1%     
=======================================
  Files        112     112             
  Lines       4267    4297     +30     
  Branches     956     973     +17     
=======================================
+ Hits        2901    2917     +16     
+ Misses      1038    1037      -1     
- Partials     328     343     +15

Files with missing lines	Coverage Δ
Ical.Net/Calendar.cs	`72.5% <65.9%> (-6.0%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ramonsmits · 2025-09-17T15:16:34Z

@axunonb I applied this fix to my earlier branch and minimized the ics data but that still seems to not generate all occurrences:

main...ramonsmits:ical.net:occurence-issue

I also pushed the same test but for v4 that does generate the expected occurrences:

main...ramonsmits:occurrence-issue-v4

axunonb · 2025-09-17T16:14:21Z

Yes, I didn't use the same DTSTART in Master and Override event. That seems to be the reason, and I got excited too soon ;-)
The Override event is not part of occurrences, while it is expected to be. That's why the Master gets filtered out and so the expected date is missing. We're getting closer though.

axunonb · 2025-09-18T06:51:17Z

@ramonsmits Should work now also when the Override Event has the same DTSTART

Filter UID/RECURRENCE-ID combinations that replace other occurrences before using `.OrderedDistinct()`

Ical.Net/Calendar.cs

ramonsmits · 2025-09-18T13:20:44Z

Ical.Net/Calendar.cs

            .OrderedMergeMany()

            // Remove duplicates and take advantage of being ordered to avoid full enumeration.
            .OrderedDistinct()


What is the expected order? Looking at the implementation of OrderedDistinct it it keeps the order of the enumerator and the first of its "sequence" is kept. However, in the code above I do not see any sorting happening. Is there some order implied from the RecurringItems items?

It ordered by Period.StartTime

Actually the question is legit. The sequence is ordered by StartTime, true, but OrderedDistinct() uses Ocurrences.Equals() to compare for equality, which compares more than just the StartTime, i.e. also the Source. But the sequence is not ordered by Source, so this could become a problem.

Actually the real core of the problem we are dealing with here is a shortcoming (or misuse) of Occurrences.Equals, which compares the Source property via UniqueComponent.Equals under the hood, which in turn only looks at the objects' Uid. In case of VEVENTs with RECURRENCE-ID there are multiple objects having the same UID. In the case of this issue they also have the same DTSTART, so the resulting Occurrence instances are considered equal although they aren't. OrderedDistinct() will therefore, more or less randomly, return only one of both (except if there are even more entries with the same StartDate and the other issue kicks in that the sequence is not properly ordered due to the Source property). As a consequence the filter for the RECURRENCE-ID that has formerly been placed after the OrderedDistinct() couldn't see it.

While this change is a significant improvement, it is not a final fix for that problem. Actually it should be clearly defined how Occurrence instances are compared for equality and I'd say this should not happen based on just the UID. Actually I'd say that two Occurrences should be considered equal only if the Sources are equal by reference.

To keep the issues apart I created #865, #866.

ramonsmits · 2025-09-18T13:22:35Z

btw, tested last against my own ical and it seems to now correctly generate the occurrences 💪

* Move tests from GetOccurrencesTests to RecurrenceTests * Delete GetOccurrencesTests * Minor refactoring of some unit tests

minichma

Wow, this is a subtle one. Please see which of my comments make sense. I think the issue is just a symptom for problems that go deeper. Created follow-up tickets #865, #866 so we can keep things apart.

minichma · 2025-09-18T18:25:21Z

Ical.Net.Tests/RecurrenceTests.cs

+    }
+
+    [Test]
+    public void GetOccurrencesWithRecurrenceId_DateOnly_ShouldEnumerate()


The new tests actually don't reproduce the original issue. Maybe add the test from the ticket?

#863 (comment)

Will flag the tests with Category `RECURRENCE-ID" and add a comment referencing to the test covering the #863

minichma · 2025-09-18T18:26:32Z

Ical.Net/Calendar.cs

-            .Where(r => r.Uid != null)
+            // Create a value tuple instead of an anonymous type, because
+            // anonymous types don't work well as dictionary keys due to equality semantics.
+            .Select(r => ((r as IUniqueComponent)?.Uid, r.RecurrenceId!.Value))


I fully agree that it is better to use tuples here rather than anonymous types but I don't think this is the root cause of the problem (as suspected in the issue's comments).

Fully agree

minichma · 2025-09-18T18:40:28Z

Ical.Net/Calendar.cs

+                // Exclude occurrences that are overridden by other components with the same UID and RECURRENCE-ID.
+                // This must happen before .OrderedDistinct() because that method would remove duplicates
+                // based on the occurrence time, and we need to remove them based on UID + RECURRENCE-ID.
+                .Where(r =>


Probably just a matter of taste: For simplicity I'd rather place the filter after the ToList(). The idea is that before the ToList() we only deal with the individual sequences of occurrences, but not with the occurrences themselves, which is done only after the ToList(). If the filter is placed here, the effect is the same, but the code is less linear to read. I suggest to place it after the OrderedMergeMany() and before the OrderedDistinct().

Btw, due to #455 it might be a good idea to keep this filter here, or at least before the OrderedMerge.

Will leave the position unchanged

minichma · 2025-09-18T19:05:44Z

Ical.Net/Calendar.cs

            .OrderedMergeMany()

            // Remove duplicates and take advantage of being ordered to avoid full enumeration.
            .OrderedDistinct()


Actually the question is legit. The sequence is ordered by StartTime, true, but OrderedDistinct() uses Ocurrences.Equals() to compare for equality, which compares more than just the StartTime, i.e. also the Source. But the sequence is not ordered by Source, so this could become a problem.

Actually the real core of the problem we are dealing with here is a shortcoming (or misuse) of Occurrences.Equals, which compares the Source property via UniqueComponent.Equals under the hood, which in turn only looks at the objects' Uid. In case of VEVENTs with RECURRENCE-ID there are multiple objects having the same UID. In the case of this issue they also have the same DTSTART, so the resulting Occurrence instances are considered equal although they aren't. OrderedDistinct() will therefore, more or less randomly, return only one of both (except if there are even more entries with the same StartDate and the other issue kicks in that the sequence is not properly ordered due to the Source property). As a consequence the filter for the RECURRENCE-ID that has formerly been placed after the OrderedDistinct() couldn't see it.

While this change is a significant improvement, it is not a final fix for that problem. Actually it should be clearly defined how Occurrence instances are compared for equality and I'd say this should not happen based on just the UID. Actually I'd say that two Occurrences should be considered equal only if the Sources are equal by reference.

To keep the issues apart I created #865, #866.

* Allow more than one `VEVENT` with same `UID` and `RECURRENCE-ID` (threw before) * Use the latest `UID` / `RECURRENCE-ID`: * either last override in the calendar * or override with the highest `SEQUENCE`

sonarqubecloud · 2025-09-19T21:44:03Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

minichma

Very nice!

axunonb · 2025-09-21T19:36:29Z

@minichma Thanks for your support and review

minichma · 2025-09-21T19:42:11Z

@axunonb urw

Fix: Event with RECURRENCE-ID overrides orginal event

748fc40

Resolves #863

ramonsmits added a commit to ramonsmits/ical.net that referenced this pull request Sep 17, 2025

Fix from ical-org#864

6c2ada5

Change filtering occurrences where RECURRENCE-ID exists

7c17f4b

Filter UID/RECURRENCE-ID combinations that replace other occurrences before using `.OrderedDistinct()`

axunonb force-pushed the wip/axunonb/pr/event-recurrence-id branch 2 times, most recently from 830295e to 0c92c39 Compare September 18, 2025 12:58

ramonsmits reviewed Sep 18, 2025

View reviewed changes

Refactor RecurrenceTests using RECURRENCE-ID

c79005e

* Move tests from GetOccurrencesTests to RecurrenceTests * Delete GetOccurrencesTests * Minor refactoring of some unit tests

axunonb force-pushed the wip/axunonb/pr/event-recurrence-id branch from 0c92c39 to c79005e Compare September 18, 2025 13:28

Add more comment to Calendar.GetOccurrences

53fc64f

axunonb requested a review from minichma September 18, 2025 13:49

axunonb marked this pull request as ready for review September 18, 2025 13:49

minichma mentioned this pull request Sep 18, 2025

Chained RECURRENCE-ID not working #865

Closed

minichma previously approved these changes Sep 18, 2025

View reviewed changes

axunonb dismissed minichma’s stale review via 8adedbb September 19, 2025 10:04

axunonb requested a review from minichma September 19, 2025 10:07

axunonb force-pushed the wip/axunonb/pr/event-recurrence-id branch from 8adedbb to bae25e0 Compare September 19, 2025 16:39

Support chained RECURRENCE-ID

3cb8fdd

* Allow more than one `VEVENT` with same `UID` and `RECURRENCE-ID` (threw before) * Use the latest `UID` / `RECURRENCE-ID`: * either last override in the calendar * or override with the highest `SEQUENCE`

axunonb force-pushed the wip/axunonb/pr/event-recurrence-id branch from bae25e0 to 3cb8fdd Compare September 19, 2025 21:43

minichma approved these changes Sep 21, 2025

View reviewed changes

axunonb merged commit 1fd6dde into main Sep 21, 2025
10 of 11 checks passed

axunonb deleted the wip/axunonb/pr/event-recurrence-id branch September 21, 2025 19:27

axunonb mentioned this pull request Oct 1, 2025

feat: RecurringComponent supports RECURRENCE-ID with optional RANGE parameter #870

Merged

axunonb mentioned this pull request Oct 5, 2025

Event Series with a missing event override from GetOccurrences() #871

Closed

This was referenced Oct 6, 2025

Bump Ical.Net from 5.1.0 to 5.1.1 microting/eform-backendconfiguration-plugin#521

Merged

Bump the all-updates group with 4 updates mikkelkaas/HaandboldGrabber#16

Closed

Bump Ical.Net from 4.3.1 to 5.1.1 Atrejoe/Inky-Calendar-Server#257

Open

dependabot bot mentioned this pull request Oct 13, 2025

Bump the all-updates group with 4 updates mikkelkaas/IntraGrabber#18

Closed

Fix: Event with RECURRENCE-ID overrides occurrence from orginal event #864

Fix: Event with RECURRENCE-ID overrides occurrence from orginal event #864

Uh oh!

Conversation

axunonb commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ramonsmits commented Sep 17, 2025

Uh oh!

axunonb commented Sep 17, 2025

Uh oh!

axunonb commented Sep 18, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ramonsmits commented Sep 18, 2025

Uh oh!

minichma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 19, 2025

Quality Gate passed

Uh oh!

minichma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

axunonb commented Sep 21, 2025

Uh oh!

minichma commented Sep 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix: Event with `RECURRENCE-ID` overrides occurrence from orginal event #864

Fix: Event with `RECURRENCE-ID` overrides occurrence from orginal event #864

axunonb commented Sep 17, 2025 •

edited

Loading

codecov bot commented Sep 17, 2025 •

edited

Loading