-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add partial key formatting methods #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds two new methods for writing key strings;
- `ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true)`
in `IPrimaryKey<T>`
- `ToSortKeyString(int throughPartIndex, bool
includeTrailingDelimiter = true)` in `ICompositePrimaryKey<T>`
These methods enable formatting keys up to a specific index, with optional
trailing delimiters. This is useful for hierarchical queries and prefix
matching in databases like DynamoDB.
The `throughPartIndex` parameter is zero-based and inclusive, counting only
properties and constants (not delimiters). For example, with a key template
`"{Country}#{County}#{Locality}"` and values "UK", "Derbyshire", "Matlock":
- `ToPartitionKeyString(0, false)` returns `"UK"`
- `ToPartitionKeyString(0, true)` returns `"UK#"`
- `ToPartitionKeyString(1, false)` returns `"UK#Derbyshire"`
- `ToPartitionKeyString(1, true)` returns `"UK#Derbyshire#"`
WalkthroughThis pull request introduces new method overloads to the primary and composite primary key interfaces, enabling dynamic formatting of partition and sort keys up to a specified part index with optional trailing delimiters. The source generator is updated to emit these new overload implementations, and comprehensive test coverage is added across multiple test files. The public API surface is updated accordingly. Changes
Sequence DiagramsequenceDiagram
participant Caller
participant GeneratedClass
participant FormatLogic
Caller->>GeneratedClass: ToPartitionKeyString(throughPartIndex, includeTrailingDelimiter)
activate GeneratedClass
Note over GeneratedClass: Validate throughPartIndex
alt Valid throughPartIndex
GeneratedClass->>FormatLogic: BuildFormatStringForKeyParts(throughPartIndex)
activate FormatLogic
FormatLogic->>FormatLogic: Switch on index & assemble<br/>format string from parts
FormatLogic-->>GeneratedClass: format string
deactivate FormatLogic
GeneratedClass->>FormatLogic: Format key parts using<br/>constructed format string
activate FormatLogic
Note over FormatLogic: Handle trailing delimiter
FormatLogic-->>GeneratedClass: formatted string
deactivate FormatLogic
GeneratedClass-->>Caller: return formatted key
else Invalid throughPartIndex
GeneratedClass-->>Caller: throw InvalidOperationException
end
deactivate GeneratedClass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #33 +/- ##
==========================================
+ Coverage 91.55% 91.67% +0.12%
==========================================
Files 34 34
Lines 1503 1525 +22
Branches 246 249 +3
==========================================
+ Hits 1376 1398 +22
- Misses 59 60 +1
+ Partials 68 67 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/CompositeKey/IPrimaryKey`1.cs (1)
23-37: Document exception behaviour for invalid index/delimiter combinationsThe overload and example read clearly, but the contract doesn’t mention that certain
throughPartIndex/includeTrailingDelimitercombinations are invalid and will throw (per the tests and generated implementation). It would be helpful to explicitly document that anInvalidOperationExceptionis thrown when the index is out of range for the available key parts, or whenincludeTrailingDelimiteristruebut there is no following delimiter for that part. This will make the behaviour of the new overload predictable for consumers.src/CompositeKey/ICompositePrimaryKey`1.cs (1)
17-31: Align sort-key overload docs with actual exception behaviourThis overload mirrors the partition-key overload, including the same implicit constraints on
throughPartIndexandincludeTrailingDelimiter. As with the partition key, it would be good to call out that invalid combinations (e.g. index outside the available sort-key parts, orincludeTrailingDelimiter == truewhen there is no following delimiter) result in anInvalidOperationException, so callers know what to expect.src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (2)
278-319: Tests correctly pin partial formatting semantics (minor naming nit)The parametrised test nicely exercises all valid combinations of
throughPartIndex/includeTrailingDelimiterforPrimaryKeyWithFastPathFormattingand matches the intended behaviour of counting only value/constant parts and optionally including the following delimiter. One small readability nit: the parameter namecompositeKeyis misleading for a primary key; renaming it toprimaryKeywould better reflect the type and avoid confusion with the composite-key tests.
321-329: Invalid-combination test is good; consider a negative-index caseThe invalid-case test asserting
InvalidOperationExceptionfor(3, true)and(4, false)is a good guard for “too-far” and “no trailing delimiter” scenarios. If you want to be exhaustive, you could add a negativethroughPartIndexcase (e.g.-1) to document that such inputs are also rejected in the same way, but this is optional given the current coverage.src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (1)
54-55: Dynamic partial-format generation matches the intended semantics (with a small edge-case to consider)The new
WriteDynamicFormatMethodBodyForKeyPartsplus its use in both primary and composite key paths looks solid:
throughPartIndexis derived by incrementingkeyPartIndexonly for non-delimiter parts, so it counts exactly properties/constants as documented.- For each value/constant part N, you emit
(N, false)(up to that part) and, if a delimiter immediately follows,(N, true)including that delimiter; this lines up with the tests’ expectations and the interface comments.- The default arm throwing
InvalidOperationExceptioncleanly handles out-of-range indices and unsupported delimiter combinations, and the message is explicit enough.Centralising format-string construction in
BuildFormatStringForKeyParts(and reusing it fromWriteFormatMethodBodyForKeyParts) also removes duplication and guarantees that full-key and partial-key paths stay consistent.One edge case to be aware of: if a key template ever produced consecutive
DelimiterKeyParts with no value/constant between them, this loop would emit multiple switch cases with the same(throughPartIndex, true)pattern but different format strings, causing a compile-time “duplicate pattern” error in the generated code. If such templates are impossible or already rejected by your analyzers, you’re fine; otherwise, you may want to either disallow them at validation time or skip additional delimiter-only cases for the samekeyPartIndex.Also applies to: 140-145, 576-580, 588-617, 619-635
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (2)
67-75: Consider adding test coverage for negative indices.The boundary tests appropriately validate that trailing delimiters at the last index and out-of-bounds indices throw
InvalidOperationException. However, consider adding test cases for negativethroughPartIndexvalues (e.g.,-1) to ensure they're also handled correctly.Example test case to add:
[Theory, AutoData] public static void CompositePrimaryKey_ToPartitionKeyString_WithNegativePartIndex_ShouldThrowInvalidOperationException( CompositePrimaryKey compositeKey) { var act = () => compositeKey.ToPartitionKeyString(-1, false); act.ShouldThrow<InvalidOperationException>(); }
123-131: Consider adding test coverage for negative indices.Similar to the partition key tests, consider adding test cases for negative
throughPartIndexvalues to ensure complete boundary validation.Example test case to add:
[Theory, AutoData] public static void CompositePrimaryKey_ToSortKeyString_WithNegativePartIndex_ShouldThrowInvalidOperationException( CompositePrimaryKey compositeKey) { var act = () => compositeKey.ToSortKeyString(-1, false); act.ShouldThrow<InvalidOperationException>(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs(2 hunks)src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs(1 hunks)src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs(6 hunks)src/CompositeKey/ICompositePrimaryKey1.cs` (1 hunks)src/CompositeKey/IPrimaryKey1.cs` (1 hunks)src/CompositeKey/PublicAPI.Unshipped.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings configuration (likely through ImplicitUsings or similar mechanisms) for common imports like xUnit and Shouldly, eliminating the need for explicit using directives in individual test files.
Applied to files:
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cssrc/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings (typically in GlobalUsings.cs files) for common imports like xUnit and Shouldly, so individual test files don't need explicit using directives for these frameworks.
Applied to files:
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cssrc/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.503Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.503Z
Learning: In CompositeKey projects, test files use global usings (typically in GlobalUsings.cs files or via ImplicitUsings in project files) for common imports like xUnit and Shouldly, so individual test files don't need explicit using directives for these frameworks.
Applied to files:
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cssrc/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
📚 Learning: 2025-08-28T19:57:32.504Z
Learnt from: DrBarnabus
Repo: DrBarnabus/CompositeKey PR: 30
File: src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs:11-42
Timestamp: 2025-08-28T19:57:32.504Z
Learning: CompositeKey test projects use MSBuild's <Using Include="..." /> elements in .csproj files combined with <ImplicitUsings>enable</ImplicitUsings> in Directory.Build.props to automatically include Xunit and Shouldly namespaces, eliminating the need for explicit using statements in test files.
Applied to files:
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs
🧬 Code graph analysis (3)
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (2)
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (16)
Theory(9-16)Theory(18-27)Theory(29-38)Theory(40-65)Theory(67-75)Theory(77-86)Theory(88-121)Theory(123-131)Theory(133-145)Theory(147-152)Theory(154-166)Theory(168-173)Theory(175-187)Theory(189-195)Theory(197-209)Theory(211-217)src/CompositeKey/IPrimaryKey`1.cs (2)
ToPartitionKeyString(21-21)ToPartitionKeyString(37-37)
src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (1)
src/CompositeKey.SourceGeneration/Core/SourceWriter.cs (5)
SourceWriter(7-120)StartBlock(31-38)WriteLine(46-46)WriteLine(48-52)EndBlock(40-44)
src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (3)
src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeyTests.cs (16)
Theory(9-16)Theory(18-25)Theory(27-34)Theory(36-44)Theory(46-51)Theory(53-61)Theory(63-69)Theory(85-92)Theory(94-101)Theory(103-110)Theory(112-123)Theory(125-130)Theory(132-143)Theory(145-151)Theory(171-178)Theory(180-187)src/CompositeKey/IPrimaryKey`1.cs (2)
ToPartitionKeyString(21-21)ToPartitionKeyString(37-37)src/CompositeKey/ICompositePrimaryKey`1.cs (2)
ToSortKeyString(15-15)ToSortKeyString(31-31)
🔇 Additional comments (5)
src/CompositeKey/PublicAPI.Unshipped.txt (1)
2-3: API surface entries match interface contractsThe new PublicAPI entries for
ToSortKeyString(int, bool)andToPartitionKeyString(int, bool)(withincludeTrailingDelimiter = true) match the interface signatures and intended behaviour, so the public surface looks consistent.src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (2)
407-415: Nice clean-up of unused pattern variableChanging the
ParseType.Stringcase pattern to drop the unusedpartvariable simplifies the match without altering behaviour and removes potential compiler warnings.
665-670: Comment and attribute emission remain clear and accurateThe updated comment around
GeneratedCodeAttributeand the emitted attribute itself look correct and keep the intent of the generated class annotation clear.src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (2)
40-65: LGTM! Comprehensive test coverage for partition key formatting.The test correctly validates partial partition key formatting with different parameter combinations. The switch statement properly handles all tested scenarios, and the expected string formats align with the template
{GuidValue}#{DecimalValue}.
88-121: LGTM! Thorough test coverage for sort key formatting.The test comprehensively validates partial sort key formatting, including testing middle parts with both delimiter options. All expected string formats correctly match the template
Constant~{EnumValue}@{StringValue}, and the switch statement properly handles all tested scenarios.
Adds two new methods for writing key strings;
ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true)inIPrimaryKey<T>ToSortKeyString(int throughPartIndex, bool includeTrailingDelimiter = true)inICompositePrimaryKey<T>These methods enable formatting keys up to a specific index, with optional trailing delimiters. This is useful for hierarchical queries and prefix matching in databases like DynamoDB.
The
throughPartIndexparameter is zero-based and inclusive, counting only properties and constants (not delimiters). For example, with a key template"{Country}#{County}#{Locality}"and values "UK", "Derbyshire", "Matlock":ToPartitionKeyString(0, false)returns"UK"ToPartitionKeyString(0, true)returns"UK#"ToPartitionKeyString(1, false)returns"UK#Derbyshire"ToPartitionKeyString(1, true)returns"UK#Derbyshire#"Summary by CodeRabbit
New Features
Tests