From 113f7855338bc024c26a78bc72a548d18a81ca4d Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 3 Mar 2026 23:10:50 +0530 Subject: [PATCH 1/2] feat: generate VTIMEZONEs and select components to merge --- pyproject.toml | 2 +- src/mergecal/calendar_merger.py | 105 +++++++++++++++++++-- src/mergecal/cli.py | 23 ++++- tests/calendars/no_vtimezone_google.ics | 34 +++++++ tests/calendars/test_empty_calendar.ics | 5 + tests/calendars/test_single_event.ics | 12 +++ tests/test_color_merging.py | 12 ++- tests/test_component_selection.py | 107 ++++++++++++++++++++++ tests/test_issue_7_generate_vtimezones.py | 91 ++++++++++++++++++ uv.lock | 8 +- 10 files changed, 379 insertions(+), 20 deletions(-) create mode 100644 tests/calendars/no_vtimezone_google.ics create mode 100644 tests/calendars/test_empty_calendar.ics create mode 100644 tests/calendars/test_single_event.ics create mode 100644 tests/test_component_selection.py create mode 100644 tests/test_issue_7_generate_vtimezones.py diff --git a/pyproject.toml b/pyproject.toml index c1ff1f6..e131b0d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ classifiers = [ ] dependencies = [ - "icalendar>=7.0.2", + "icalendar>=7.0.3", "rich>=10", "typer>=0.15,<1", "x-wr-timezone>=2.0.1" diff --git a/src/mergecal/calendar_merger.py b/src/mergecal/calendar_merger.py index 5a4daa9..2817239 100644 --- a/src/mergecal/calendar_merger.py +++ b/src/mergecal/calendar_merger.py @@ -1,10 +1,18 @@ +from __future__ import annotations + from dataclasses import dataclass, field -from icalendar import Calendar, Component +from icalendar import Calendar, Component, Timezone from x_wr_timezone import to_standard ComponentId = tuple[str, int, str | None] +# Avoids expensive re-generation of VTIMEZONE components across CalendarMerger +# instances. +_timezone_cache: dict[str, Timezone] = {} + +DEFAULT_COMPONENTS = ["VEVENT", "VTIMEZONE"] + def calendars_from_ical(data: bytes) -> list[Calendar]: """Parse ICS data, returning one Calendar per VCALENDAR component found.""" @@ -52,6 +60,8 @@ def __init__( version: str = "2.0", calscale: str = "GREGORIAN", method: str | None = None, + generate_vtimezone: bool = True, + components: list[str] | None = None, ): self.merged_calendar = Calendar() @@ -64,13 +74,72 @@ def __init__( self.calendars: list[Calendar] = [] self._merged = False + self.generate_vtimezone = generate_vtimezone + self.components = ( + [c.strip().upper() for c in components if c.strip()] + if components is not None + else list(DEFAULT_COMPONENTS) + ) for calendar in calendars: self.add_calendar(calendar) + def _get_components(self, cal: Calendar) -> list[Component]: + result: list[Component] = [] + if "VEVENT" in self.components: + result.extend(cal.events) + if "VTODO" in self.components: + result.extend(cal.todos) + if "VJOURNAL" in self.components: + result.extend(cal.walk("VJOURNAL")) + return result + + def _should_generate_timezones(self) -> bool: + return "VTIMEZONE" in self.components and self.generate_vtimezone + def add_calendar(self, calendar: Calendar) -> None: """Add a calendar to be merged.""" - self.calendars.append(to_standard(calendar, add_timezone_component=True)) + cal = to_standard( + calendar, add_timezone_component=self._should_generate_timezones() + ) + + if self._should_generate_timezones(): + for tz in cal.timezones: + if tz.tz_name not in _timezone_cache: + _timezone_cache[tz.tz_name] = tz + + # to_standard() may add a VTIMEZONE even when one already exists for + # the same TZID, causing get_missing_tzids() to raise KeyError + # (collective/icalendar#1124). Deduplicate before calling + # add_missing_timezones(). Also drop VTIMEZONEs not referenced by + # any component — get_missing_tzids() assumes every VTIMEZONE is + # used, so an unreferenced one causes a KeyError too. + used_tzids = cal.get_used_tzids() + seen_tzids: set[str] = set() + deduped: list[Component] = [] + for c in cal.subcomponents: + if isinstance(c, Timezone): + if c.tz_name in seen_tzids or c.tz_name not in used_tzids: + continue + seen_tzids.add(c.tz_name) + deduped.append(c) + cal.subcomponents[:] = deduped + + if cal.get_used_tzids(): + missing_tzids = cal.get_missing_tzids() + + for tzid in missing_tzids: + if tzid in _timezone_cache: + cal.add_component(_timezone_cache[tzid]) + + remaining = cal.get_missing_tzids() + if remaining: + cal.add_missing_timezones() + for tz in cal.timezones: + if tz.tz_name in remaining: + _timezone_cache[tz.tz_name] = tz + + self.calendars.append(cal) def merge(self) -> Calendar: """Merge the calendars.""" @@ -90,20 +159,36 @@ def merge(self) -> Calendar: if calendar_color and not self.merged_calendar.color: self.merged_calendar.color = calendar_color - for timezone in cal.timezones: - if timezone.tz_name not in tzids: - self.merged_calendar.add_component(timezone) - tzids.add(timezone.tz_name) - - for component in cal.events + cal.todos + cal.journals: + if "VTIMEZONE" in self.components: + for timezone in cal.timezones: + if timezone.tz_name == "UTC": + # UTC needs no VTIMEZONE per RFC 5545 §3.6.5; skip spurious + # entries generated by add_missing_timezones() + # (collective/icalendar#1124) + continue + if timezone.tz_name not in tzids: + self.merged_calendar.add_component(timezone) + tzids.add(timezone.tz_name) + + for component in self._get_components(cal): tracker.add(component, calendar_color) return self.merged_calendar -def merge_calendars(calendars: list[Calendar], **kwargs: object) -> Calendar: +def merge_calendars( + calendars: list[Calendar], + generate_vtimezone: bool = True, + components: list[str] | None = None, + **kwargs: object, +) -> Calendar: """Convenience function to merge calendars.""" - merger = CalendarMerger(calendars, **kwargs) # type: ignore + merger = CalendarMerger( + calendars, + generate_vtimezone=generate_vtimezone, + components=components, + **kwargs, # type: ignore[arg-type] + ) return merger.merge() diff --git a/src/mergecal/cli.py b/src/mergecal/cli.py index 4f0be7c..73b4092 100644 --- a/src/mergecal/cli.py +++ b/src/mergecal/cli.py @@ -7,13 +7,22 @@ app = typer.Typer() -# Define arguments and options outside of the function calendars_arg = typer.Argument(..., help="Paths to the calendar files to merge") output_opt = typer.Option( "merged_calendar.ics", "--output", "-o", help="Output file path" ) prodid_opt = typer.Option(None, "--prodid", help="Product ID for the merged calendar") method_opt = typer.Option(None, "--method", help="Calendar method") +no_generate_vtimezone_opt = typer.Option( + False, + "--no-generate-vtimezone", + help="Disable VTIMEZONE generation for performance", +) +components_opt = typer.Option( + None, + "--components", + help="Comma-separated component types to merge (VEVENT,VTODO,VJOURNAL,VTIMEZONE)", +) @app.command() @@ -22,6 +31,8 @@ def main( output: Path = output_opt, prodid: str | None = prodid_opt, method: str | None = method_opt, + no_generate_vtimezone: bool = no_generate_vtimezone_opt, + components: str | None = components_opt, ) -> None: """Merge multiple iCalendar files into one.""" try: @@ -31,7 +42,15 @@ def main( calendar_objects.extend(calendars_from_ical(cal_file.read())) merger = CalendarMerger( - calendars=calendar_objects, prodid=prodid, method=method + calendars=calendar_objects, + prodid=prodid, + method=method, + generate_vtimezone=not no_generate_vtimezone, + components=( + [p.strip() for p in components.split(",") if p.strip()] + if components + else None + ), ) merged_calendar = merger.merge() diff --git a/tests/calendars/no_vtimezone_google.ics b/tests/calendars/no_vtimezone_google.ics new file mode 100644 index 0000000..23585a9 --- /dev/null +++ b/tests/calendars/no_vtimezone_google.ics @@ -0,0 +1,34 @@ +BEGIN:VCALENDAR +PRODID:-//Google Inc//Google Calendar 70.9054//EN +VERSION:2.0 +CALSCALE:GREGORIAN +METHOD:PUBLISH +X-WR-CALNAME:Test Calendar +X-WR-TIMEZONE:America/New_York +BEGIN:VEVENT +DTSTART;TZID=America/New_York:20240315T090000 +DTEND;TZID=America/New_York:20240315T100000 +DTSTAMP:20240301T120000Z +UID:test-event-1@google.com +CREATED:20240301T120000Z +DESCRIPTION:Test event with timezone reference but no VTIMEZONE component +LAST-MODIFIED:20240301T120000Z +SEQUENCE:0 +STATUS:CONFIRMED +SUMMARY:Meeting in New York timezone +TRANSP:OPAQUE +END:VEVENT +BEGIN:VEVENT +DTSTART;TZID=Europe/London:20240320T140000 +DTEND;TZID=Europe/London:20240320T150000 +DTSTAMP:20240301T120000Z +UID:test-event-2@google.com +CREATED:20240301T120000Z +DESCRIPTION:Another event with different timezone but no VTIMEZONE +LAST-MODIFIED:20240301T120000Z +SEQUENCE:0 +STATUS:CONFIRMED +SUMMARY:London meeting +TRANSP:OPAQUE +END:VEVENT +END:VCALENDAR diff --git a/tests/calendars/test_empty_calendar.ics b/tests/calendars/test_empty_calendar.ics new file mode 100644 index 0000000..45529f1 --- /dev/null +++ b/tests/calendars/test_empty_calendar.ics @@ -0,0 +1,5 @@ +BEGIN:VCALENDAR +PRODID:-//Test//Test//EN +VERSION:2.0 +CALSCALE:GREGORIAN +END:VCALENDAR diff --git a/tests/calendars/test_single_event.ics b/tests/calendars/test_single_event.ics new file mode 100644 index 0000000..35d0533 --- /dev/null +++ b/tests/calendars/test_single_event.ics @@ -0,0 +1,12 @@ +BEGIN:VCALENDAR +PRODID:-//Test//Test//EN +VERSION:2.0 +CALSCALE:GREGORIAN +BEGIN:VEVENT +UID:test-single-event@test.com +SUMMARY:Test Single Event +DTSTART;TZID=America/Chicago:20240101T120000 +DTEND;TZID=America/Chicago:20240101T130000 +DTSTAMP:20240101T000000Z +END:VEVENT +END:VCALENDAR diff --git a/tests/test_color_merging.py b/tests/test_color_merging.py index f84105e..b142a07 100644 --- a/tests/test_color_merging.py +++ b/tests/test_color_merging.py @@ -4,7 +4,10 @@ def test_component_inherits_calendar_color(calendars, component_type): - result = merge_calendars(calendars.color_rfc7986.stream) + result = merge_calendars( + calendars.color_rfc7986.stream, + components=["VEVENT", "VTODO", "VJOURNAL"], + ) assert result.walk(component_type)[0].color == "turquoise" @@ -14,7 +17,10 @@ def test_event_inherits_apple_calendar_color(calendars): def test_component_own_color_not_overwritten(calendars, component_type): - result = merge_calendars(calendars.color_event_own.stream) + result = merge_calendars( + calendars.color_event_own.stream, + components=["VEVENT", "VTODO", "VJOURNAL"], + ) assert result.walk(component_type)[0].color == "navy" @@ -38,7 +44,7 @@ def test_merged_calendar_color_when_only_one_has_color(calendars): def test_component_own_color_preserved_across_calendars(calendars, component_type): cals = calendars.color_event_own.stream + calendars.color_rfc7986.stream - result = merge_calendars(cals) + result = merge_calendars(cals, components=["VEVENT", "VTODO", "VJOURNAL"]) assert result.walk(component_type)[0].color == "navy" diff --git a/tests/test_component_selection.py b/tests/test_component_selection.py new file mode 100644 index 0000000..cdf5ab9 --- /dev/null +++ b/tests/test_component_selection.py @@ -0,0 +1,107 @@ +"""Select which components to merge (Issue #182).""" + +from mergecal import CalendarMerger, merge_calendars + + +def test_default_components_vevent_and_vtimezone_only(calendars): + """Only VEVENT and VTIMEZONE are merged by default.""" + result = merge_calendars(calendars.color_rfc7986.stream) + + assert len(list(result.events)) == 1 + assert len(list(result.todos)) == 0 + assert len(list(result.walk("VJOURNAL"))) == 0 + + +def test_select_events_only(calendars): + """Only VEVENTs are merged; no timezones when VTIMEZONE excluded.""" + result = merge_calendars(calendars.color_rfc7986.stream, components=["VEVENT"]) + + assert len(list(result.events)) == 1 + assert len(list(result.todos)) == 0 + assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(list(result.timezones)) == 0 + + +def test_select_todos_only(calendars): + """Only VTODOs are merged.""" + result = merge_calendars(calendars.color_rfc7986.stream, components=["VTODO"]) + + assert len(list(result.events)) == 0 + assert len(list(result.todos)) == 1 + assert len(list(result.timezones)) == 0 + + +def test_select_journals_only(calendars): + """Only VJOURNALs are merged.""" + result = merge_calendars(calendars.color_rfc7986.stream, components=["VJOURNAL"]) + + assert len(list(result.events)) == 0 + assert len(list(result.todos)) == 0 + assert len(list(result.walk("VJOURNAL"))) == 1 + assert len(list(result.timezones)) == 0 + + +def test_select_events_and_timezones(calendars): + """Timezones are generated when VTIMEZONE is included in components.""" + result = merge_calendars( + calendars.no_vtimezone_google.stream, components=["VEVENT", "VTIMEZONE"] + ) + + assert len(list(result.events)) == 2 + assert len(list(result.timezones)) > 0 + + +def test_calendar_merger_components_parameter(calendars): + """CalendarMerger respects the components parameter.""" + result = CalendarMerger( + calendars.color_rfc7986.stream, components=["VEVENT", "VTODO"] + ).merge() + + assert len(list(result.events)) == 1 + assert len(list(result.todos)) == 1 + assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(list(result.timezones)) == 0 + + +def test_empty_components_list(calendars): + """An empty components list produces an empty calendar.""" + result = merge_calendars(calendars.color_rfc7986.stream, components=[]) + + assert len(list(result.events)) == 0 + assert len(list(result.todos)) == 0 + assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(list(result.timezones)) == 0 + + +def test_timezone_generation_only_when_vtimezone_in_components(calendars): + """generate_vtimezone has no effect when VTIMEZONE is not in the components list.""" + cals = calendars.no_vtimezone_google.stream + + result_with = merge_calendars( + cals, components=["VEVENT", "VTIMEZONE"], generate_vtimezone=True + ) + result_without = merge_calendars( + cals, components=["VEVENT"], generate_vtimezone=True + ) + + assert len(list(result_with.timezones)) > 0 + assert len(list(result_without.timezones)) == 0 + + +def test_multiple_calendars_component_filtering(calendars): + """Component filtering applies across all merged calendars.""" + cals = calendars.one_event.stream + calendars.color_rfc7986.stream + result = merge_calendars(cals, components=["VEVENT"]) + + assert len(list(result.events)) == 2 + assert len(list(result.todos)) == 0 + assert len(list(result.walk("VJOURNAL"))) == 0 + + +def test_unknown_components_silently_ignored(calendars): + """Unknown component names do not crash the merger.""" + cals = calendars.test_empty_calendar.stream + + result = CalendarMerger(cals, components=["VEVENT", "UNKNOWN"]).merge() + + assert len(list(result.events)) == 0 diff --git a/tests/test_issue_7_generate_vtimezones.py b/tests/test_issue_7_generate_vtimezones.py new file mode 100644 index 0000000..e0abce7 --- /dev/null +++ b/tests/test_issue_7_generate_vtimezones.py @@ -0,0 +1,91 @@ +"""Generate VTIMEZONEs when not available (Issue #7).""" + +from mergecal import CalendarMerger, merge_calendars +from mergecal.calendar_merger import _timezone_cache + + +def test_generate_vtimezone_enabled_by_default(calendars): + """Missing VTIMEZONEs are generated and included in the merged calendar.""" + _timezone_cache.clear() + + result = merge_calendars(calendars.no_vtimezone_google.stream) + + timezone_names = {tz.tz_name for tz in result.timezones} + assert "America/New_York" in timezone_names + assert "Europe/London" in timezone_names + + +def test_utc_timezone_filtered_out(calendars): + """UTC needs no VTIMEZONE per RFC 5545 §3.6.5.""" + result = merge_calendars(calendars.no_vtimezone_google.stream) + + assert "UTC" not in {tz.tz_name for tz in result.timezones} + + +def test_generate_vtimezone_can_be_disabled(calendars): + """Generation can be disabled; no VTIMEZONE components are added.""" + result = merge_calendars( + calendars.no_vtimezone_google.stream, generate_vtimezone=False + ) + + assert len(list(result.timezones)) == 0 + + +def test_calendar_merger_generate_vtimezone_parameter(calendars): + """CalendarMerger respects generate_vtimezone=False.""" + cals = calendars.no_vtimezone_google.stream + + result_on = CalendarMerger(cals, generate_vtimezone=True).merge() + result_off = CalendarMerger(cals, generate_vtimezone=False).merge() + + assert len(list(result_on.timezones)) > 0 + assert len(list(result_off.timezones)) == 0 + + +def test_existing_vtimezone_components_preserved(calendars): + """Existing VTIMEZONE components survive the merge.""" + result = merge_calendars(calendars.x_wr_timezone.stream) + + assert len(list(result.timezones)) > 0 + + +def test_x_wr_timezone_conversion_still_works(calendars): + """X-WR-TIMEZONE events are still present when generation is disabled.""" + result = merge_calendars(calendars.x_wr_timezone.stream, generate_vtimezone=False) + + assert len(list(result.events)) == 2 + assert len(list(result.timezones)) == 0 + + +def test_timezone_deduplication(calendars): + """Each timezone appears exactly once even when merged from duplicate calendars.""" + cals = calendars.no_vtimezone_google.stream * 2 + result = merge_calendars(cals, generate_vtimezone=True) + + timezone_names = [tz.tz_name for tz in result.timezones] + assert timezone_names.count("America/New_York") == 1 + assert timezone_names.count("Europe/London") == 1 + + +def test_generated_timezones_cached_across_instances(calendars): + """Generated timezones are cached so subsequent merges avoid re-generation.""" + _timezone_cache.clear() + + merge_calendars(calendars.no_vtimezone_google.stream) + + assert "America/New_York" in _timezone_cache + assert "Europe/London" in _timezone_cache + + +def test_cache_not_populated_when_vtimezone_excluded(calendars): + """Cache is not populated when VTIMEZONE is excluded from the components list.""" + _timezone_cache.clear() + + merge_calendars( + calendars.no_vtimezone_google.stream, + components=["VEVENT"], + generate_vtimezone=True, + ) + + assert "America/New_York" not in _timezone_cache + assert "Europe/London" not in _timezone_cache diff --git a/uv.lock b/uv.lock index fc2e21c..2307877 100644 --- a/uv.lock +++ b/uv.lock @@ -303,16 +303,16 @@ wheels = [ [[package]] name = "icalendar" -version = "7.0.2" +version = "7.0.3" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "python-dateutil" }, { name = "typing-extensions", marker = "python_full_version < '3.13'" }, { name = "tzdata" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/8c/51/43458f01e229763b05dd937b5e9d41ef506b6eb8b4bf939f8ea34350b853/icalendar-7.0.2.tar.gz", hash = "sha256:de844ff5cde32f539bea7644e36d8494032a926b933bedb92621f2f239760806", size = 440039, upload-time = "2026-02-24T16:13:42.887Z" } +sdist = { url = "https://files.pythonhosted.org/packages/b8/60/6b0356a2ed1c9689ae14bd8e44f22eac67c420a0ecca4df8306b70906600/icalendar-7.0.3.tar.gz", hash = "sha256:95027ece087ab87184d765f03761f25875821f74cdd18d3b57e9c868216d8fde", size = 443788, upload-time = "2026-03-03T12:00:10.952Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/62/ab/e0d44b1de0beb703bbc507ca064300b34046f9f9628f052d1a97ffa61b95/icalendar-7.0.2-py3-none-any.whl", hash = "sha256:ad31a5825b39522a30b073c6ced3ffcdf6c02cbb7dab69ba2e4de32ddbf77cc9", size = 437913, upload-time = "2026-02-24T16:13:40.631Z" }, + { url = "https://files.pythonhosted.org/packages/fa/c6/431fbf9063a6a4306d4cedae7823d69baf0979ba6ca57ab24a9d898cd0aa/icalendar-7.0.3-py3-none-any.whl", hash = "sha256:8c9fea6d3a89671bba8b6938d8565b4d0ec465c6a2796ef0f92790dcb9e627cd", size = 442406, upload-time = "2026-03-03T12:00:09.228Z" }, ] [[package]] @@ -470,7 +470,7 @@ docs = [ [package.metadata] requires-dist = [ - { name = "icalendar", specifier = ">=7.0.2" }, + { name = "icalendar", specifier = ">=7.0.3" }, { name = "rich", specifier = ">=10" }, { name = "typer", specifier = ">=0.15,<1" }, { name = "x-wr-timezone", specifier = ">=2.0.1" }, From 339eff29d1040b9ffd93b8668943243b758236a9 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 5 Mar 2026 10:53:00 +0530 Subject: [PATCH 2/2] refactor: address code review feedback --- src/mergecal/calendar_merger.py | 71 ++++++++-------- src/mergecal/cli.py | 5 +- tests/test_component_selection.py | 99 ++++++++++++----------- tests/test_issue_7_generate_vtimezones.py | 44 +--------- 4 files changed, 96 insertions(+), 123 deletions(-) diff --git a/src/mergecal/calendar_merger.py b/src/mergecal/calendar_merger.py index 2817239..1d99bfb 100644 --- a/src/mergecal/calendar_merger.py +++ b/src/mergecal/calendar_merger.py @@ -7,11 +7,7 @@ ComponentId = tuple[str, int, str | None] -# Avoids expensive re-generation of VTIMEZONE components across CalendarMerger -# instances. -_timezone_cache: dict[str, Timezone] = {} - -DEFAULT_COMPONENTS = ["VEVENT", "VTIMEZONE"] +DEFAULT_COMPONENTS = ["VEVENT", "VTODO", "VJOURNAL", "VTIMEZONE"] def calendars_from_ical(data: bytes) -> list[Calendar]: @@ -63,6 +59,22 @@ def __init__( generate_vtimezone: bool = True, components: list[str] | None = None, ): + """ + Initialize the merger. + + Args: + calendars: Calendars to merge. + prodid: PRODID for the merged calendar. Defaults to the mergecal PRODID. + version: iCalendar version. Defaults to "2.0". + calscale: Calendar scale. Defaults to "GREGORIAN". + method: Calendar method (e.g. "PUBLISH"). + generate_vtimezone: Generate missing VTIMEZONE components for any + referenced timezone IDs. Disable for performance when timezone + accuracy is not needed. + components: Component types to include in the merge. Defaults to all + types: VEVENT, VTODO, VJOURNAL, VTIMEZONE. Pass a subset to filter. + + """ self.merged_calendar = Calendar() self.merged_calendar.add("prodid", prodid or generate_default_prodid()) @@ -75,6 +87,7 @@ def __init__( self.calendars: list[Calendar] = [] self._merged = False self.generate_vtimezone = generate_vtimezone + self._timezone_cache: dict[str, Timezone] = {} self.components = ( [c.strip().upper() for c in components if c.strip()] if components is not None @@ -91,11 +104,11 @@ def _get_components(self, cal: Calendar) -> list[Component]: if "VTODO" in self.components: result.extend(cal.todos) if "VJOURNAL" in self.components: - result.extend(cal.walk("VJOURNAL")) + result.extend(cal.journals) return result def _should_generate_timezones(self) -> bool: - return "VTIMEZONE" in self.components and self.generate_vtimezone + return self.generate_vtimezone def add_calendar(self, calendar: Calendar) -> None: """Add a calendar to be merged.""" @@ -105,15 +118,13 @@ def add_calendar(self, calendar: Calendar) -> None: if self._should_generate_timezones(): for tz in cal.timezones: - if tz.tz_name not in _timezone_cache: - _timezone_cache[tz.tz_name] = tz - - # to_standard() may add a VTIMEZONE even when one already exists for - # the same TZID, causing get_missing_tzids() to raise KeyError - # (collective/icalendar#1124). Deduplicate before calling - # add_missing_timezones(). Also drop VTIMEZONEs not referenced by - # any component — get_missing_tzids() assumes every VTIMEZONE is - # used, so an unreferenced one causes a KeyError too. + if tz.tz_name not in self._timezone_cache: + self._timezone_cache[tz.tz_name] = tz + + # to_standard() may add a duplicate VTIMEZONE for the same TZID when + # the calendar already has one; deduplicate before calling + # add_missing_timezones(). Also drop VTIMEZONEs not referenced by any + # component — get_missing_tzids() assumes every VTIMEZONE is used. used_tzids = cal.get_used_tzids() seen_tzids: set[str] = set() deduped: list[Component] = [] @@ -125,19 +136,18 @@ def add_calendar(self, calendar: Calendar) -> None: deduped.append(c) cal.subcomponents[:] = deduped - if cal.get_used_tzids(): - missing_tzids = cal.get_missing_tzids() - + missing_tzids = cal.get_missing_tzids() + if missing_tzids: for tzid in missing_tzids: - if tzid in _timezone_cache: - cal.add_component(_timezone_cache[tzid]) + if tzid in self._timezone_cache: + cal.add_component(self._timezone_cache[tzid]) remaining = cal.get_missing_tzids() if remaining: cal.add_missing_timezones() for tz in cal.timezones: if tz.tz_name in remaining: - _timezone_cache[tz.tz_name] = tz + self._timezone_cache[tz.tz_name] = tz self.calendars.append(cal) @@ -159,16 +169,13 @@ def merge(self) -> Calendar: if calendar_color and not self.merged_calendar.color: self.merged_calendar.color = calendar_color - if "VTIMEZONE" in self.components: - for timezone in cal.timezones: - if timezone.tz_name == "UTC": - # UTC needs no VTIMEZONE per RFC 5545 §3.6.5; skip spurious - # entries generated by add_missing_timezones() - # (collective/icalendar#1124) - continue - if timezone.tz_name not in tzids: - self.merged_calendar.add_component(timezone) - tzids.add(timezone.tz_name) + for timezone in cal.timezones: + if timezone.tz_name == "UTC": + # UTC needs no VTIMEZONE per RFC 5545 §3.6.5 + continue + if timezone.tz_name not in tzids: + self.merged_calendar.add_component(timezone) + tzids.add(timezone.tz_name) for component in self._get_components(cal): tracker.add(component, calendar_color) diff --git a/src/mergecal/cli.py b/src/mergecal/cli.py index 73b4092..821f656 100644 --- a/src/mergecal/cli.py +++ b/src/mergecal/cli.py @@ -16,7 +16,10 @@ no_generate_vtimezone_opt = typer.Option( False, "--no-generate-vtimezone", - help="Disable VTIMEZONE generation for performance", + help=( + "Do not generate missing VTIMEZONE components although they might be" + " required. This increases performance." + ), ) components_opt = typer.Option( None, diff --git a/tests/test_component_selection.py b/tests/test_component_selection.py index cdf5ab9..90a7970 100644 --- a/tests/test_component_selection.py +++ b/tests/test_component_selection.py @@ -1,54 +1,61 @@ """Select which components to merge (Issue #182).""" +import pytest + from mergecal import CalendarMerger, merge_calendars -def test_default_components_vevent_and_vtimezone_only(calendars): - """Only VEVENT and VTIMEZONE are merged by default.""" +def test_default_components_include_all_types(calendars): + """All component types are merged by default.""" result = merge_calendars(calendars.color_rfc7986.stream) assert len(list(result.events)) == 1 - assert len(list(result.todos)) == 0 - assert len(list(result.walk("VJOURNAL"))) == 0 - - -def test_select_events_only(calendars): - """Only VEVENTs are merged; no timezones when VTIMEZONE excluded.""" - result = merge_calendars(calendars.color_rfc7986.stream, components=["VEVENT"]) - - assert len(list(result.events)) == 1 - assert len(list(result.todos)) == 0 - assert len(list(result.walk("VJOURNAL"))) == 0 - assert len(list(result.timezones)) == 0 - - -def test_select_todos_only(calendars): - """Only VTODOs are merged.""" - result = merge_calendars(calendars.color_rfc7986.stream, components=["VTODO"]) - - assert len(list(result.events)) == 0 assert len(list(result.todos)) == 1 + assert len(result.journals) == 1 assert len(list(result.timezones)) == 0 -def test_select_journals_only(calendars): - """Only VJOURNALs are merged.""" - result = merge_calendars(calendars.color_rfc7986.stream, components=["VJOURNAL"]) - - assert len(list(result.events)) == 0 - assert len(list(result.todos)) == 0 - assert len(list(result.walk("VJOURNAL"))) == 1 +@pytest.mark.parametrize( + "components,num_events,num_todos,num_journals", + [ + (["VEVENT"], 1, 0, 0), + (["VTODO"], 0, 1, 0), + (["VJOURNAL"], 0, 0, 1), + ], +) +def test_select_single_component_type( + calendars, components, num_events, num_todos, num_journals +): + """Only the selected component type is merged.""" + result = merge_calendars(calendars.color_rfc7986.stream, components=components) + + assert len(list(result.events)) == num_events + assert len(list(result.todos)) == num_todos + assert len(result.journals) == num_journals assert len(list(result.timezones)) == 0 -def test_select_events_and_timezones(calendars): - """Timezones are generated when VTIMEZONE is included in components.""" +def test_events_include_required_timezones(calendars): + """ + VTIMEZONEs required by events are generated even without VTIMEZONE in components. + + Timezones follow events automatically; the components list only filters event types. + """ result = merge_calendars( - calendars.no_vtimezone_google.stream, components=["VEVENT", "VTIMEZONE"] + calendars.no_vtimezone_google.stream, components=["VEVENT"] ) assert len(list(result.events)) == 2 - assert len(list(result.timezones)) > 0 + assert "America/New_York" in {tz.tz_name for tz in result.timezones} + assert "Europe/London" in {tz.tz_name for tz in result.timezones} + + +def test_select_vtimezone_only(calendars): + """components=['VTIMEZONE'] copies existing VTIMEZONE components, no events.""" + result = merge_calendars(calendars.one_event.stream, components=["VTIMEZONE"]) + + assert len(list(result.events)) == 0 + assert len(list(result.timezones)) == 1 def test_calendar_merger_components_parameter(calendars): @@ -59,7 +66,7 @@ def test_calendar_merger_components_parameter(calendars): assert len(list(result.events)) == 1 assert len(list(result.todos)) == 1 - assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(result.journals) == 0 assert len(list(result.timezones)) == 0 @@ -69,23 +76,17 @@ def test_empty_components_list(calendars): assert len(list(result.events)) == 0 assert len(list(result.todos)) == 0 - assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(result.journals) == 0 assert len(list(result.timezones)) == 0 -def test_timezone_generation_only_when_vtimezone_in_components(calendars): - """generate_vtimezone has no effect when VTIMEZONE is not in the components list.""" - cals = calendars.no_vtimezone_google.stream - - result_with = merge_calendars( - cals, components=["VEVENT", "VTIMEZONE"], generate_vtimezone=True - ) - result_without = merge_calendars( - cals, components=["VEVENT"], generate_vtimezone=True +def test_generate_vtimezone_false_disables_generation(calendars): + """generate_vtimezone=False suppresses all VTIMEZONE output.""" + result = merge_calendars( + calendars.no_vtimezone_google.stream, generate_vtimezone=False ) - assert len(list(result_with.timezones)) > 0 - assert len(list(result_without.timezones)) == 0 + assert len(list(result.timezones)) == 0 def test_multiple_calendars_component_filtering(calendars): @@ -95,13 +96,13 @@ def test_multiple_calendars_component_filtering(calendars): assert len(list(result.events)) == 2 assert len(list(result.todos)) == 0 - assert len(list(result.walk("VJOURNAL"))) == 0 + assert len(result.journals) == 0 def test_unknown_components_silently_ignored(calendars): """Unknown component names do not crash the merger.""" - cals = calendars.test_empty_calendar.stream - - result = CalendarMerger(cals, components=["VEVENT", "UNKNOWN"]).merge() + result = CalendarMerger( + calendars.test_empty_calendar.stream, components=["VEVENT", "UNKNOWN"] + ).merge() assert len(list(result.events)) == 0 diff --git a/tests/test_issue_7_generate_vtimezones.py b/tests/test_issue_7_generate_vtimezones.py index e0abce7..8efdeaf 100644 --- a/tests/test_issue_7_generate_vtimezones.py +++ b/tests/test_issue_7_generate_vtimezones.py @@ -1,13 +1,10 @@ """Generate VTIMEZONEs when not available (Issue #7).""" -from mergecal import CalendarMerger, merge_calendars -from mergecal.calendar_merger import _timezone_cache +from mergecal import merge_calendars def test_generate_vtimezone_enabled_by_default(calendars): """Missing VTIMEZONEs are generated and included in the merged calendar.""" - _timezone_cache.clear() - result = merge_calendars(calendars.no_vtimezone_google.stream) timezone_names = {tz.tz_name for tz in result.timezones} @@ -31,22 +28,11 @@ def test_generate_vtimezone_can_be_disabled(calendars): assert len(list(result.timezones)) == 0 -def test_calendar_merger_generate_vtimezone_parameter(calendars): - """CalendarMerger respects generate_vtimezone=False.""" - cals = calendars.no_vtimezone_google.stream - - result_on = CalendarMerger(cals, generate_vtimezone=True).merge() - result_off = CalendarMerger(cals, generate_vtimezone=False).merge() - - assert len(list(result_on.timezones)) > 0 - assert len(list(result_off.timezones)) == 0 - - def test_existing_vtimezone_components_preserved(calendars): """Existing VTIMEZONE components survive the merge.""" result = merge_calendars(calendars.x_wr_timezone.stream) - assert len(list(result.timezones)) > 0 + assert "America/New_York" in {tz.tz_name for tz in result.timezones} def test_x_wr_timezone_conversion_still_works(calendars): @@ -60,32 +46,8 @@ def test_x_wr_timezone_conversion_still_works(calendars): def test_timezone_deduplication(calendars): """Each timezone appears exactly once even when merged from duplicate calendars.""" cals = calendars.no_vtimezone_google.stream * 2 - result = merge_calendars(cals, generate_vtimezone=True) + result = merge_calendars(cals) timezone_names = [tz.tz_name for tz in result.timezones] assert timezone_names.count("America/New_York") == 1 assert timezone_names.count("Europe/London") == 1 - - -def test_generated_timezones_cached_across_instances(calendars): - """Generated timezones are cached so subsequent merges avoid re-generation.""" - _timezone_cache.clear() - - merge_calendars(calendars.no_vtimezone_google.stream) - - assert "America/New_York" in _timezone_cache - assert "Europe/London" in _timezone_cache - - -def test_cache_not_populated_when_vtimezone_excluded(calendars): - """Cache is not populated when VTIMEZONE is excluded from the components list.""" - _timezone_cache.clear() - - merge_calendars( - calendars.no_vtimezone_google.stream, - components=["VEVENT"], - generate_vtimezone=True, - ) - - assert "America/New_York" not in _timezone_cache - assert "Europe/London" not in _timezone_cache