Efcore community fixes - #38715
Open
ilkertskn wants to merge 3 commits into
Open
Conversation
- DateOnly/DateTime/DateTimeOffset -> string converters formatted using the current culture but parsed using the invariant culture, so round-tripping corrupted values under cultures whose default calendar is not Gregorian (e.g. th-TH adds 543 years to the year) - Pass CultureInfo.InvariantCulture when formatting, matching the parse side and the sibling TimeOnly/TimeSpan/number converters which already pin the invariant culture on both sides - Add round-trip tests under a non-Gregorian culture
- The FormattableString overload called _stringBuilder.Append(value) and therefore never emitted the trailing new line documented by its summary, unlike the string overload which uses AppendLine - Append the new line after the value, matching AppendLine(string) - Add a test for the FormattableString overload (existing tests only pass interpolated string literals, which bind to the string overload)
…string indexer - The this[string] setter set the complex collection value but then fell through to GetProperty(propertyName), which throws for a non-scalar property name, so setting a complex collection via the string indexer always threw (after mutating state) - Return once the complex property has been handled, mirroring the getter - Add a test in PropertyValuesTestBase, overridden for the in-memory provider which does not support complex collections
Contributor
Author
|
@dotnet-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR tightens culture-invariant round-tripping behavior for EF Core’s date/time string value converters and fixes/extends related infrastructure behavior with new regression tests.
Changes:
- Ensure
DateOnly,DateTime, andDateTimeOffsetstring conversions useCultureInfo.InvariantCultureduring formatting to avoid non-Gregorian culture pitfalls (e.g.,th-THBuddhist calendar). - Fix
PropertyValuesstring-indexer setting for complex collections by ensuring the complex-property branch exits early. - Correct
IndentedStringBuilder.AppendLine(FormattableString)to actually append a newline, and add a regression test.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.Tests/Storage/ValueConversion/DateStringConverterCultureTest.cs | Adds culture-specific regression coverage (Thai Buddhist calendar) for date/time string converter round-trips. |
| test/EFCore.Tests/Infrastructure/IndentedStringBuilderTest.cs | Adds regression test ensuring AppendLine(FormattableString) appends a newline. |
| test/EFCore.Specification.Tests/PropertyValuesTestBase.cs | Adds specification coverage for setting complex collections via the string indexer. |
| test/EFCore.InMemory.FunctionalTests/PropertyValuesInMemoryTest.cs | Marks the new complex-collection indexer test as unsupported for InMemory (expects exception). |
| src/EFCore/Storage/ValueConversion/Internal/StringDateTimeOffsetConverter.cs | Formats DateTimeOffset using invariant culture for stable serialization. |
| src/EFCore/Storage/ValueConversion/Internal/StringDateTimeConverter.cs | Formats DateTime using invariant culture for stable serialization. |
| src/EFCore/Storage/ValueConversion/Internal/StringDateOnlyConverter.cs | Formats DateOnly using invariant culture for stable serialization. |
| src/EFCore/Infrastructure/IndentedStringBuilder.cs | Fixes AppendLine(FormattableString) to append a newline. |
| src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs | Prevents fall-through after handling complex properties in the string indexer setter. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.