Feat/schema test coverage#79
Closed
lior1997k wants to merge 6 commits intoIvanMurzak:mainfrom
Closed
Conversation
GetSchemaTypeId() now percent-encodes [ ] < > characters in type IDs to produce valid JSON Schema $ref URI-reference keys. This fixes invalid $defs keys like System.String[] and IEnumerable<System.Int32> which violate RFC 3986. Also updates TestSchemaTypeId assertions to match the new sanitized output. Ultraworked with Sisyphus Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands JSON Schema-related test coverage for nested, deeply nested, and generic nested C# types, and adjusts the schema type-id generation used for $defs keys and $ref targets to use percent-encoded identifiers.
Changes:
- Updated
TypeUtils.GetSchemaTypeIdto return a percent-encoded form ofGetTypeId(encoding[],<>, and+). - Added/updated tests validating
GetSchemaTypeIdoutput for nested types, generics, and arrays. - Added new nested and generic nested model types in the outer test assembly and introduced schema generation tests covering
$defsand$refvalidity for nested scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ReflectorNet/src/Utils/TypeUtils.Name.cs | Changes schema type-id generation to return percent-encoded IDs used by schema $defs/$ref. |
| ReflectorNet.Tests/src/SchemaTests/TestSchemaTypeId.cs | Extends and updates assertions for encoded schema type IDs (nested, deeply nested, generic nested, arrays, generics). |
| ReflectorNet.Tests/src/SchemaTests/NestedClassSchemaTests.cs | Adds new schema generation tests ensuring nested-type schemas produce resolvable refs/defs and expected properties. |
| ReflectorNet.Tests.OuterAssembly/src/Model/NestedClass.cs | Adds deeply nested and generic nested model types used by the new schema tests. |
Comment on lines
+118
to
+132
| public static string GetSchemaTypeId<T>() => GetSchemaTypeId(typeof(T)); | ||
| public static string GetSchemaTypeId(Type type) | ||
| { | ||
| var typeId = GetTypeId(type); | ||
| return SanitizeForJsonSchemaRef(typeId); | ||
| } | ||
|
|
||
| private static string SanitizeForJsonSchemaRef(string typeId) | ||
| { | ||
| if (string.IsNullOrEmpty(typeId)) | ||
| return typeId; | ||
| return typeId.Replace("[", "%5B").Replace("]", "%5D") | ||
| .Replace("<", "%3C").Replace(">", "%3E") | ||
| .Replace("+", "%2B"); | ||
| } |
Comment on lines
+118
to
+123
| public static string GetSchemaTypeId<T>() => GetSchemaTypeId(typeof(T)); | ||
| public static string GetSchemaTypeId(Type type) | ||
| { | ||
| var typeId = GetTypeId(type); | ||
| return SanitizeForJsonSchemaRef(typeId); | ||
| } |
| private ParentClass.NestedClass NestedClassReturnMethod() => new ParentClass.NestedClass(); | ||
| private LevelOne.LevelTwo.LevelThree.LevelFour DeeplyNestedClassReturnMethod() => new LevelOne.LevelTwo.LevelThree.LevelFour(); | ||
| private GenericOuter<int>.GenericInner<string> GenericNestedClassReturnMethod() => new GenericOuter<int>.GenericInner<string>(); | ||
| private ParentClass.NestedClass[] NestedClassArrayReturnMethod() => new ParentClass.NestedClass[0]; |
…a tests instead of hardcoding
…n' into fix/nested-class-ref-sanitization
- Add deeply nested (LevelOne.LevelTwo.LevelThree.LevelFour) test types - Add generic nested (GenericOuter<T>.GenericInner<U>) test types - Add type ID sanitization tests for +, <, > characters - Add schema generation tests for nested class / validation - Add return schema, argument schema, and array schema tests for nested types - Add cross-assembly nested class schema validation All 1438 tests pass (including 27 new ones).
lior1997k
force-pushed
the
feat/schema-test-coverage
branch
from
3cb132e to
46d88b1
Compare
Owner
|
Use the first PR which you made to add this changes there. Don't need to add new PRs for the same thing. Just combine all together. |
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.
Here's a PR description for feat/schema-test-coverage:
Summary
This PR significantly expands JSON Schema test coverage for nested class scenarios, ensuring that $defs keys and $ref URI references remain valid for complex C# type hierarchies.
Motivation
Following the fix for + symbol sanitization in nested class names (#78), there was limited test coverage for:
Deeply nested classes (A.B.C.D)
Generic nested classes (Outer.Inner)
Schema generation validation for nested types
Cross-assembly nested type resolution
Changes
New Test Types (ReflectorNet.Tests.OuterAssembly)
LevelOne.LevelTwo.LevelThree.LevelFour — deeply nested class hierarchy
GenericOuter.GenericInner — generic nested classes with self-referencing
Type ID Sanitization Tests (TestSchemaTypeId.cs)
GetSchemaTypeId_NestedClass_ShouldEncodePlusSymbol — verifies + → %2B
GetSchemaTypeId_DeeplyNestedClass_ShouldEncodeAllPlusSymbols — verifies multiple + encoding
GetSchemaTypeId_GenericNestedClass_ShouldEncodePlusAndAngleBrackets — verifies +, <, > all encoded
Schema Generation Tests (NestedClassSchemaTests.cs)
Schema generation for nested, deeply nested, and generic nested classes
$defs key validation for deeply nested hierarchies
Return schema, argument schema, and array schema tests
Cross-assembly nested class schema validation
Self-referencing generic nested class schema validation
Verification
All 1438 tests pass (27 new, 1411 existing)
No breaking changes
No public API changes