Skip to content

UTF-8 Serializer - #983

Draft
maknapp wants to merge 4 commits into
mainfrom
maknapp/serializer
Draft

maknapp wants to merge 4 commits into
mainfrom
maknapp/serializer

Conversation

@maknapp

@maknapp maknapp commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

This is a mostly functional serializer with some parts completely untested. I did change a few tests because they seemed like mistakes to me.

I think deprecating in v6 is the way to go here. The public surface area of the current serializer methods is too large and won't allow for minor breaking changes to be made with the new serializer (like changing exception types). For example, I tried changing the existing SerializeToString methods to use the new TryParse methods, but many SerializeToString methods differ how they throw exceptions vs returning null.

Some of the SerializeToString methods are copied directly into the TryParse or converter methods just to be 100% sure not to break anything with the old serializer. Some TryParse methods are different to test reducing allocations.

Some tests are changed to test only the new TryParse methods instead of continuing to test the corresponding SerializeToString methods - this was just me being lazy for now. The latest commit is testing out how to test both serializers at the same time.

Type mapping is simplified. The new serializer matches only by property name. A CalendarPropertyConverter is expected to handle all VALUE types for its property. Activator is not used at all, so this should avoid any issues mentioned in #977 too.

Undefined properties use the default converter, which assumes all values are strings even if VALUE is defined. This is the same behavior as the old serializer.

Backslash encoding is applied only to TEXT value types. Most/all(?) other value types specifically say no other encoding (backslash) applies.

There is no encoding option. It currently writes without BOM by default, same as old serializer. Do we need an option to include BOM?

Todo

  • Custom exception type with properties like LineNumber to point directly to the error
  • Ensure all name/property/value types contain strictly valid characters as specified in the RFC
  • Option or callback to control how exceptions are handled
  • Remove logging
  • Items listed in Serialize to UTF-8 byte stream #973
  • Improve copied parsing methods
  • Improve test coverage
  • Make sure all TryParse methods have good parameter names and will never throw exceptions
  • Check if OnDeserializing and OnDeserialized events are required
  • Mark all old serializer methods as obsolete
  • SerializeAsync
  • PipeReader and PipeWriter (might make SerializeAsync easier too)
Benchmarks
Method Runtime Mean Gen0 Gen1 Allocated
Deserialize .NET 10.0 44.281 us 11.0474 1.6479 90.52 KB
Deserialize .NET 8.0 51.969 us 11.3525 1.2817 92.94 KB
Deserialize .NET Framework 4.8 125.079 us 16.8457 2.4414 104.2 KB
Deserialize2 .NET 10.0 21.894 us 6.9580 0.9460 56.96 KB
Deserialize2 .NET 8.0 27.703 us 7.0190 0.9460 57.35 KB
Deserialize2 .NET Framework 4.8 70.102 us 10.6201 1.4648 65.82 KB
BenchmarkSerializeCalendar .NET 10.0 10.407 us 3.1433 0.0610 25.76 KB
BenchmarkSerializeCalendar .NET 8.0 12.658 us 3.1891 0.0610 26.14 KB
BenchmarkSerializeCalendar .NET Framework 4.8 26.722 us 5.2185 0.0916 32.2 KB
BenchmarkSerializeCalendar2 .NET 10.0 3.187 us 0.6638 - 5.42 KB
BenchmarkSerializeCalendar2 .NET 8.0 4.422 us 0.6866 - 5.66 KB
BenchmarkSerializeCalendar2 .NET Framework 4.8 11.868 us 1.0986 - 6.77 KB
SerializeMultibyte .NET 10.0 12.816 us 4.4708 0.1068 36.57 KB
SerializeMultibyte .NET 8.0 14.896 us 4.5013 0.1068 36.82 KB
SerializeMultibyte .NET Framework 4.8 27.128 us 7.2327 0.1831 44.62 KB
SerializeMultibyte2 .NET 10.0 3.524 us 0.6104 - 5 KB
SerializeMultibyte2 .NET 8.0 4.544 us 0.6256 - 5.14 KB
SerializeMultibyte2 .NET Framework 4.8 10.519 us 0.9308 - 5.79 KB
DeserializeCalendar .NET 10.0 14.748 us 3.9215 0.1984 32.11 KB
DeserializeCalendar .NET 8.0 19.620 us 3.9978 0.1831 32.82 KB
DeserializeCalendar .NET Framework 4.8 47.291 us 6.0425 0.3052 37.24 KB
DeserializeCalendar2 .NET 10.0 5.820 us 1.9379 0.0610 15.88 KB
DeserializeCalendar2 .NET 8.0 8.423 us 1.9531 0.0610 16.01 KB
DeserializeCalendar2 .NET Framework 4.8 18.050 us 3.0518 - 18.85 KB

Fixes #973, #978, #702, #937
Partial #908
Partial #977

@maknapp
maknapp requested a review from axunonb September 12, 2026 16:15
@maknapp

maknapp commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator Author

Failed DeserializesSingleEventCalendar [8 ms]
Error Message:
Assert.That(output, Is.EqualTo(ics))
Expected string length 867 but was 903. Strings differ at index 15.
Expected: "BEGIN:VCALENDAR\nVERSION:2.0\nPRODID:-//Mozilla.org/NONSGML Moz..."
But was: "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Mozilla.org/NONSGML M..."

Not sure why the test failed... expected should be using \r\n line endings.

@axunonb

axunonb commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

Not sure why the test failed... expected should be using \r\n line endings.

IcsFiles.Attachment4 reads the raw embedded .ics file with StreamReader.ReadToEnd(), so the string retains whatever line endings are actually stored in the embedded resource file — in this case, I think the Attachment4.ics file on disk has LF-only line endings.

The fix is either
var ics = IcsFiles.Attachment4.ReplaceLineEndings("\r\n");
or to save the file with CR LF line ending.

We experienced this issue in the past already. Is there something wrong with the line ending definition for the repo?
[edit]
Yes, see #984 that should fix the issue

@axunonb

axunonb commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

There is no encoding option. It currently writes without BOM by default, same as old serializer. Do we need an option to include BOM?

If I remember correctly the BOM option had compatibility reasons. The BOM is clearly excluded in RFC, so I vote for not allowing uncompliant output. A related question might arise with line endings, which are also well defined as CR LF, not LF-only. Different output is up to users.

Changes test classes that have a lot of serializer tests
to test both old and new serializers with the same data.
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.99059% with 373 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Ical.Net/Serialization/CalendarReader.cs 80.3% 37 Missing and 21 partials ⚠️
...et/Serialization/CalendarSerializer.Deserialize.cs 58.4% 38 Missing and 4 partials ⚠️
Ical.Net/Serialization/CalendarWriter.cs 79.8% 33 Missing and 4 partials ⚠️
Ical.Net/DataTypes/WeekDay.cs 68.2% 9 Missing and 12 partials ⚠️
....Net/Serialization/Converters/FreeBusyConverter.cs 13.6% 13 Missing and 6 partials ⚠️
...cal.Net/Serialization/CalendarPropertyConverter.cs 69.6% 13 Missing and 4 partials ⚠️
.../Serialization/Converters/RecurrenceIdConverter.cs 20.0% 14 Missing and 2 partials ⚠️
...Serialization/Converters/RequestStatusConverter.cs 36.4% 11 Missing and 3 partials ⚠️
Ical.Net/DataTypes/Duration.cs 81.2% 4 Missing and 9 partials ⚠️
Ical.Net/DataTypes/GeographicLocation.cs 40.9% 12 Missing and 1 partial ⚠️
... and 21 more

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

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #983     +/-   ##
=======================================
- Coverage   72.9%   72.2%   -0.6%     
=======================================
  Files        116     143     +27     
  Lines       4822    6161   +1339     
  Branches    1109    1433    +324     
=======================================
+ Hits        3513    4450    +937     
- Misses       953    1255    +302     
- Partials     356     456    +100     
Files with missing lines Coverage Δ
Ical.Net/Calendar.cs 76.2% <ø> (ø)
Ical.Net/CalendarCollection.cs 54.8% <ø> (ø)
Ical.Net/CalendarComponents/CalendarComponent.cs 84.0% <100.0%> (+0.7%) ⬆️
Ical.Net/CalendarComponents/CalendarEvent.cs 56.9% <100.0%> (+1.4%) ⬆️
Ical.Net/DataTypes/Attachment.cs 58.3% <100.0%> (-23.5%) ⬇️
Ical.Net/DataTypes/RecurrenceRule.cs 93.8% <100.0%> (+4.1%) ⬆️
Ical.Net/DataTypes/RequestStatus.cs 24.1% <ø> (+1.6%) ⬆️
Ical.Net/DataTypes/Trigger.cs 69.7% <ø> (ø)
Ical.Net/Serialization/CalendarSerializer.cs 31.6% <ø> (ø)
Ical.Net/Serialization/ConverterMap.cs 100.0% <100.0%> (ø)
... and 35 more

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Serialize to UTF-8 byte stream

2 participants