Automatic Value Object Validation take3 #263
Merged
Conversation
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
Introduce IScalarValueObject<TSelf, TPrimitive> interface and refactor all scalar value object base types (ScalarValueObject, RequiredGuid, RequiredString) to use the CRTP pattern for improved type safety and static interface support. Add ASP.NET Core integration: model binders and JSON converters for value objects, with automatic validation via AddScalarValueObjectValidation(). Update source generator, tests, and documentation to use the new pattern and enable seamless, reflection-free validation in web APIs.
Deleted ITryCreatable.cs, including the ITryCreatable<T> interface, all XML documentation, usage examples, and implementation notes. This interface previously defined a contract for validated creation of types from strings using a static TryCreate method.
- Add README section on ASP.NET Core integration and usage of IScalarValueObject for automatic model validation - Update code samples and feature table to highlight seamless ASP.NET Core support - Generated value objects now explicitly implement IScalarValueObject<TSelf, T> - Add required TryCreate overloads for model binding compatibility - Expand XML docs and examples in RequiredGuid/RequiredString to show automatic validation and error handling in controllers - Clarify difference between manual and automatic validation approaches - Improves developer experience by reducing boilerplate for value object validation in ASP.NET Core apps
Refactored ScalarValueObject<TSelf, T> and all derived types to explicitly implement the IScalarValueObject<TSelf, TPrimitive> interface. Updated model binder and JSON converter providers to use [DynamicallyAccessedMembers] and [UnconditionalSuppressMessage] attributes for improved trimming and AOT compatibility. Enhanced documentation and remarks regarding reflection and AOT support. Updated tests and sample classes to ensure consistent interface usage and added overloads for TryCreate methods where appropriate. This improves robustness, clarity, and future compatibility of the value object infrastructure.
Add .AddScalarValueObjectValidation() to service registration for automatic model binding validation of value objects. Introduce RegisterUserDto using value objects for registration, and add RegisterWithAutoValidation endpoint to demonstrate the feature. Update .http files with sample requests and add AUTOMATIC_VALIDATION.md documentation. Clean up namespaces in ServiceCollectionExtensions.cs. This simplifies validation, removes manual TryCreate/Combine calls, and leverages ASP.NET Core's built-in validation pipeline.
Introduce comprehensive value object validation for ASP.NET Core: - Add ValidationErrorsContext for per-request error collection - Replace old JSON converter with ValidatingJsonConverter & factory - Add PropertyNameAwareConverter for property-level error reporting - Add ValueObjectValidationMiddleware and ValueObjectValidationFilter - Update IScalarValueObject.TryCreate to accept fieldName - Update all value object implementations and codegen for new signature - Revise AddScalarValueObjectValidation to wire up new infrastructure - Improve documentation and usage examples throughout This enables property-aware, multi-error collection during JSON deserialization and model binding, providing robust and user-friendly validation error reporting.
Added a sample HTTP request to SampleMinimalApi.http to demonstrate a successful registration using the /users/RegisterWithAutoValidation endpoint with valid user data. This helps in testing and documenting the expected behavior for successful user registration.
Introduce integration and unit tests to verify that when the same value object type (Name) is used for multiple DTO properties (FirstName, LastName), validation errors are correctly attributed to the property names, not the type name. Add RegisterWithNameDto and Name value object, update UsersController with RegisterWithSharedNameType endpoint, and expand Register.http with relevant scenarios. Includes comprehensive tests for validation context, JSON converters, and error attribution.
Introduce extension methods and endpoint filter for property-aware value object validation in Minimal APIs. - Add AddScalarValueObjectValidationForMinimalApi and WithValueObjectValidation extensions. - Implement ValueObjectValidationEndpointFilter for automatic validation error responses. - Update ValidatingJsonConverter to write primitive values directly for source-gen compatibility. - Register new endpoints and DTOs to demonstrate and test property-level validation, including scenarios with shared value object types. - Expand HTTP test cases to verify correct error attribution to property names.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #263 +/- ##
==========================================
+ Coverage 89.65% 90.07% +0.42%
==========================================
Files 72 83 +11
Lines 1875 2187 +312
Branches 373 448 +75
==========================================
+ Hits 1681 1970 +289
- Misses 122 133 +11
- Partials 72 84 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Introduce ScalarValueObjectTypeHelper to centralize reflection logic for detecting IScalarValueObject<TSelf, TPrimitive> implementations. Refactor all usages to use this helper, improving maintainability and consistency. Add ValidationError.ToDictionary() for easier integration with ASP.NET Core validation responses, and update filters to use it. Add comprehensive unit tests for the new helper, error dictionary conversion, and validation middleware/filter behavior. Remove redundant code and streamline error handling.
Introduce AspSourceGenerator: a Roslyn source generator that auto-generates AOT-friendly System.Text.Json converters and serializer context entries for all IScalarValueObject<TSelf, TPrimitive> types. Integrate the generator as an analyzer in sample projects. Refactor reflection-based code to use a new CreateGenericInstance helper for DRYness. Add documentation and improve XML comments. This enables Native AOT support and eliminates runtime reflection for value object serialization.
- Rewrote and expanded README.md with clearer intro, feature list, quick starts, and best practices for value object validation and result conversion. - Added REFLECTION-FALLBACK.md explaining reflection vs. source generator validation, migration, and troubleshooting. - Introduced AddValueObjectValidation extension for unified MVC/Minimal API setup, with full XML docs and usage guidance. - Updated .gitignore to exclude .claude files. - Improves onboarding, clarifies AOT/reflection, and simplifies setup for all ASP.NET Core app types.
Introduce unit tests for model binding, JSON converters, service registration, and MVC validation filter related to value object infrastructure. Tests cover various value object types, error handling, registration in DI, and integration scenarios, ensuring robust and predictable behavior.
Introduces SampleMinimalApiNoAot, a new example project demonstrating FunctionalDDD.Asp usage without source generation or JsonSerializerContext. Includes comprehensive README, ready-to-use HTTP test file, and API routes for user and todo endpoints. Adds EXAMPLES-GUIDE.md comparing all examples and guiding users on when to use reflection fallback vs. AOT. Updates solution and configuration files. Emphasizes that reflection fallback is production-ready with negligible overhead, ideal for learning and most .NET apps.
Comprehensive unit and integration tests were added for the FunctionalDdd.Asp value object and validation infrastructure, covering normal, edge, and concurrency scenarios. Null argument checks were introduced in ScalarValueObjectTypeHelper for improved robustness, and generic type instantiation now safely returns null on constraint violations. These changes enhance reliability, error reporting, and maintainability.
ConvertToPrimitive now returns Result<T>, enabling precise parse error reporting and better ModelState messages. Updated unit tests to expect parse errors for invalid input. Added comprehensive tests for all supported primitive types and edge cases.
Added new test value objects (TestEmail, TestAge) and a variety of DTOs to ServiceCollectionExtensionsTests. Introduced comprehensive tests for JSON deserialization and validation error collection, covering multiple, mixed, nested, and nullable value object scenarios. Made TestName sealed for consistency. These changes ensure robust validation across diverse DTO structures.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new IScalarValueObject<TSelf, TPrimitive>-based model for scalar value objects and wires it into ASP.NET Core so value objects can be used directly in DTOs with automatic validation during model binding and JSON deserialization. It replaces the old ITryCreatable<T> pattern, adds a reflection-based validation pipeline plus an optional AOT-friendly source generator, and updates examples, tests, and documentation to demonstrate both MVC and Minimal API flows.
Changes:
- Replace
ITryCreatable<T>with the CRTP-basedIScalarValueObject<TSelf, TPrimitive>and updateScalarValueObject,RequiredString,RequiredGuid, andEmailAddressaccordingly, along with tests and docs. - Add a full ASP.NET Core integration layer (
FunctionalDdd.Asp) for automatic value object validation, including JSON converters, model binders, middleware, filters, endpoint filters, and a Roslyn source generator for AOT scenarios. - Add and update sample apps (MVC, Minimal API with and without AOT) plus documentation and .http files to illustrate automatic validation, reflection fallback behavior, and usage patterns.
Reviewed changes
Copilot reviewed 90 out of 91 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| RailwayOrientedProgramming/src/ITryCreatable.cs | Removes the obsolete ITryCreatable<T> interface to migrate to the new scalar value object pattern. |
| RailwayOrientedProgramming/src/Errors/Types/ValidationError.cs | Adds ToDictionary() to project validation errors into the IDictionary<string,string[]> shape needed by ASP.NET Core validation responses. |
| PrimitiveValueObjects/tests/RequiredStringTests.cs | Updates test value objects to inherit from RequiredString<TSelf> and adjusts TryCreate invocations to the new (string?, string?) signature. |
| PrimitiveValueObjects/tests/RequiredGuidTests.cs | Updates the test GUID value object to use RequiredGuid<TSelf>. |
| PrimitiveValueObjects/tests/ITryCreatableImplementationTests.cs | Reworks tests to validate TryCreate implementations and field-name behavior instead of ITryCreatable<T> interface conformance; renames the test class accordingly. |
| PrimitiveValueObjects/src/RequiredString.cs | Converts RequiredString to RequiredString<TSelf> : ScalarValueObject<TSelf,string> with IScalarValueObject<TSelf,string> constraint, and expands XML docs and examples to cover the new validation and ASP.NET integration story. |
| PrimitiveValueObjects/src/RequiredGuid.cs | Converts RequiredGuid to RequiredGuid<TSelf> : ScalarValueObject<TSelf,Guid> with IScalarValueObject<TSelf,Guid> constraint, and updates documentation to show the richer generated API surface and ASP.NET scenarios. |
| PrimitiveValueObjects/src/PrimitiveValueObjectTraceProviderBuilderExtensions.cs | Updates XML docs to reference the new EmailAddress.TryCreate(string?, string?) overload. |
| PrimitiveValueObjects/src/ParsableJsonConverter.cs | Updates documentation to refer to the new two-parameter ScalarValueObject<TSelf,T> base class. |
| PrimitiveValueObjects/src/EmailAddress.cs | Migrates EmailAddress to ScalarValueObject<EmailAddress,string> + IScalarValueObject, wires TryCreate(string?, string?) into the new interface, and updates Parse/TryParse to delegate through the new overload. |
| PrimitiveValueObjects/README.md | Updates examples and the core concepts table to use RequiredString<TSelf>, RequiredGuid<TSelf> and to document IScalarValueObject-based ASP.NET Core automatic validation. |
| FunctionalDDD.sln | Adds the Asp source generator and new SampleMinimalApiNoAot project to the solution and introduces a new src solution folder. |
| FluentValidation/tests/ValueObject/ZipCode.cs | Updates ZipCode to use ScalarValueObject<ZipCode,string> and implement IScalarValueObject<ZipCode,string>, and adjusts TryCreate signature to match the new convention. |
| FluentValidation/tests/ValueObject/UserId.cs | Updates the test UserId to inherit from RequiredGuid<UserId>. |
| FluentValidation/tests/ValueObject/LastName.cs | Updates the test LastName to inherit from RequiredString<LastName>. |
| FluentValidation/tests/ValueObject/FirstName.cs | Updates the test FirstName to inherit from RequiredString<FirstName>. |
| Examples/Xunit/ValueObject/LastName.cs | Updates the example LastName value object to inherit from RequiredString<LastName>. |
| Examples/Xunit/ValueObject/FirstName.cs | Updates the example FirstName value object to inherit from RequiredString<FirstName>. |
| Examples/Xunit/ValidationExample.cs | Adjusts optional value object conversion examples to use the new generic RequiredString types and the updated TryCreate signatures. |
| Examples/Xunit/DomainDrivenDesignSamplesTests.cs | Updates example value objects to use ScalarValueObject<TSelf,T> + IScalarValueObject, adds TryCreate overloads with fieldName, and adjusts Temperature to return validation errors with the correct field name. |
| Examples/SampleWebApplication/src/Program.cs | Enables automatic scalar value object validation by adding AddScalarValueObjectValidation() and the UseValueObjectValidation() middleware. |
| Examples/SampleWebApplication/src/Controllers/UsersController.cs | Adds new endpoints demonstrating automatic validation via DTOs with value objects and shared-name-type scenarios, and removes a now-unnecessary pragma. |
| Examples/SampleWebApplication/Requests/Register.http | Extends the HTTP request examples to cover automatic validation flows and shared-name-type edge cases. |
| Examples/SampleWebApplication/AUTOMATIC_VALIDATION.md | New documentation explaining before/after controller code, how automatic validation works under the hood, and the shape of successful vs validation-error responses. |
| Examples/SampleUserLibrary/ValueObject/UserId.cs | Updates UserId to inherit from RequiredGuid<UserId>. |
| Examples/SampleUserLibrary/ValueObject/Name.cs | Introduces a generic Name : RequiredString<Name> value object used to test property-name attribution when the same type backs multiple properties. |
| Examples/SampleUserLibrary/ValueObject/LastName.cs | Updates LastName to inherit from RequiredString<LastName>. |
| Examples/SampleUserLibrary/ValueObject/FirstName.cs | Updates FirstName to inherit from RequiredString<FirstName>. |
| Examples/SampleUserLibrary/SampleUserLibrary.csproj | Adds the Asp source generator as an analyzer reference alongside the PrimitiveValueObjects generator and Asp/PrimitiveValueObjects runtime projects. |
| Examples/SampleUserLibrary/Model/RegisterWithNameDto.cs | Adds a DTO using the shared Name value object for both first and last name to test property-level error attribution, plus XML docs. |
| Examples/SampleUserLibrary/Model/RegisterUserDto.cs | Adds a DTO using concrete value objects (FirstName, LastName, EmailAddress) to demonstrate automatic validation in controllers and Minimal APIs. |
| Examples/SampleMinimalApiNoAot/appsettings.json | Standard ASP.NET Core appsettings for the new non-AOT sample Minimal API. |
| Examples/SampleMinimalApiNoAot/SampleMinimalApiNoAot.http | HTTP examples for the non-AOT Minimal API, covering auto-validation, shared-name-type behavior, manual validation, and error responses. |
| Examples/SampleMinimalApiNoAot/SampleMinimalApiNoAot.csproj | New Minimal API project (no AOT, no generator reference) consuming Asp and SampleUserLibrary to demonstrate reflection fallback. |
| Examples/SampleMinimalApiNoAot/README.md | Detailed documentation of the reflection-fallback Minimal API sample, performance characteristics, and when to choose reflection vs generator. |
| Examples/SampleMinimalApiNoAot/Properties/launchSettings.json | Launch profile for the non-AOT Minimal API sample. |
| Examples/SampleMinimalApiNoAot/Program.cs | Configures Minimal APIs with AddScalarValueObjectValidationForMinimalApi, value object validation middleware, sample routes, and OpenTelemetry tracing. |
| Examples/SampleMinimalApiNoAot/API/UserRoutes.cs | Defines Minimal API user routes demonstrating manual Result chaining and the new .WithValueObjectValidation()-based automatic validation endpoints. |
| Examples/SampleMinimalApiNoAot/API/ToDoRoutes.cs | Simple TODO endpoints used in the non-AOT Minimal API sample. |
| Examples/SampleMinimalApi/SampleMinimalApi.http | Extends the AOT Minimal API .http file with automatic validation and shared-name-type test cases and introduces a common @host variable. |
| Examples/SampleMinimalApi/SampleMinimalApi.csproj | Adds AspSourceGenerator as an analyzer for the AOT Minimal API sample to generate AOT-safe value object converters. |
| Examples/SampleMinimalApi/Program.cs | Integrates the generated AppJsonSerializerContext with HttpJsonOptions, adds Minimal API validation config/middleware, and includes the DTOs in the json context. |
| Examples/SampleMinimalApi/API/UserRoutes.cs | Adds Minimal API endpoints that consume RegisterUserDto and RegisterWithNameDto directly with .WithValueObjectValidation(). |
| Examples/EcommerceExample/ValueObjects/ProductId.cs | Updates ProductId to inherit from RequiredGuid<ProductId>. |
| Examples/EcommerceExample/ValueObjects/OrderId.cs | Updates OrderId to inherit from RequiredGuid<OrderId>. |
| Examples/EcommerceExample/ValueObjects/Money.cs | Updates Money to ScalarValueObject<Money,decimal> + IScalarValueObject, and splits TryCreate into overloads that accept an optional field name for better error attribution. |
| Examples/EcommerceExample/ValueObjects/CustomerId.cs | Updates CustomerId to inherit from RequiredGuid<CustomerId>. |
| Examples/EXAMPLES-GUIDE.md | New high-level guide describing all sample projects, their trade-offs (AOT vs reflection, Minimal API vs MVC), and a recommended learning path; includes one incorrect GitHub issues link noted in review. |
| Examples/BankingExample/ValueObjects/TransactionId.cs | Updates TransactionId to inherit from RequiredGuid<TransactionId>. |
| Examples/BankingExample/ValueObjects/Money.cs | Updates Money to ScalarValueObject<Money,decimal> + IScalarValueObject and uses the field name in validation errors. |
| Examples/BankingExample/ValueObjects/CustomerId.cs | Updates CustomerId to inherit from RequiredGuid<CustomerId>. |
| Examples/BankingExample/ValueObjects/AccountId.cs | Updates AccountId to inherit from RequiredGuid<AccountId>. |
| DomainDrivenDesign/tests/ValueObjects/ScalarValueObjectTests.cs | Updates test scalar value objects to use the new generic ScalarValueObject<TSelf,T> + IScalarValueObject pattern and adds static TryCreate implementations for use with Asp helpers. |
| DomainDrivenDesign/tests/ValueObjects/Money.cs | Updates the test Money value object to the new ScalarValueObject<Money,decimal> + IScalarValueObject shape with a static TryCreate method. |
| DomainDrivenDesign/src/ScalarValueObject.cs | Refactors the base scalar value object to the CRTP form ScalarValueObject<TSelf,T> with IScalarValueObject<TSelf,T> constraint and updates docs and implicit conversion accordingly. |
| DomainDrivenDesign/src/IScalarValueObject.cs | Introduces IScalarValueObject<TSelf,TPrimitive> with a static TryCreate and Value property, forming the core contract used by validation, model binding, and converters. |
| DomainDrivenDesign/src/DomainDrivenDesign.csproj | Adds a reference to the RailwayOrientedProgramming project to use Result and Error types in the new scalar value object abstractions. |
| Benchmark/BenchmarkROP.cs | Updates the benchmark FirstName value object to inherit from RequiredString<FirstName>. |
| Asp/tests/ValueObjectValidationMiddlewareTests.cs | New tests verifying that the middleware creates/disposes validation scopes correctly, handles exceptions/cancellation, and maintains isolation across requests. |
| Asp/tests/ValueObjectValidationFilterTests.cs | New tests covering filter ordering, interaction with ModelState, clearing of pre-existing errors, case/nesting behavior, and the shape of ValidationProblemDetails. |
| Asp/tests/ValueObjectValidationEndpointFilterTests.cs | New tests ensuring the Minimal API endpoint filter short-circuits on errors, passes through successful results, and respects concurrency/isolation. |
| Asp/tests/ValidationErrorsContextConcurrencyTests.cs | Concurrency tests validating that ValidationErrorsContext correctly aggregates errors across many threads/scopes without duplication or leaks. |
| Asp/tests/ScalarValueObjectTypeHelperTests.cs | Tests for the reflection helper that detects value object types, extracts primitive types, instantiates generic helpers, and handles edge cases. |
| Asp/tests/PropertyNameAwareConverterTests.cs | Tests ensuring property names are applied to the context, restored correctly (including nested cases and exceptions), and preserved in errors. |
| Asp/src/Validation/ValueObjectValidationMiddleware.cs | Introduces middleware that opens/closes a ValidationErrorsContext scope per request so converters can collect validation errors. |
| Asp/src/Validation/ValueObjectValidationFilter.cs | Adds an MVC action filter that reads collected validation errors, updates ModelState, and returns a 400 ValidationProblemDetails response instead of executing the action. |
| Asp/src/Validation/ValueObjectValidationEndpointFilter.cs | Adds a Minimal API endpoint filter that checks ValidationErrorsContext and returns Results.ValidationProblem() when errors exist. |
| Asp/src/Validation/ValidationErrorsContext.cs | Implements the AsyncLocal-backed error collection context with scoped ErrorCollector, property-name tracking, and thread-safe aggregation into a ValidationError. |
| Asp/src/Validation/ValidatingJsonConverterFactory.cs | Adds a JsonConverterFactory that detects IScalarValueObject implementations and creates ValidatingJsonConverter<TValueObject,TPrimitive> instances via reflection. |
| Asp/src/Validation/ValidatingJsonConverter.cs | Implements a generic JSON converter that deserializes the primitive, calls TryCreate, and collects validation errors instead of throwing, while writing primitive values efficiently. |
| Asp/src/Validation/ScalarValueObjectTypeHelper.cs | Provides shared reflection helpers to detect IScalarValueObject<TSelf,TPrimitive> implementations and construct generic helpers (converters/model binders). |
| Asp/src/Validation/PropertyNameAwareConverter.cs | Wraps converters to set ValidationErrorsContext.CurrentPropertyName for per-property error attribution during deserialization. |
| Asp/src/ModelBinding/ScalarValueObjectModelBinderProvider.cs | Introduces a model binder provider that detects scalar value objects and supplies ScalarValueObjectModelBinder<TValueObject,TPrimitive>. |
| Asp/src/ModelBinding/ScalarValueObjectModelBinder.cs | Implements a model binder that parses primitives from standard sources and calls TryCreate, pushing validation errors into ModelState. |
| Asp/src/Extensions/ServiceCollectionExtensions.cs | Adds extension methods to configure validation for MVC and Minimal APIs, wire up JSON options, register middleware, and expose .WithValueObjectValidation() on route handlers. |
| Asp/src/Asp.csproj | Adds a reference to the DomainDrivenDesign project to leverage the new scalar value object abstractions. |
| Asp/generator/ScalarValueObjectInfo.cs | Defines a simple metadata type describing discovered value objects (namespace, type name, primitive type) used by the generator. |
| Asp/generator/README.md | Documents the Asp source generator’s purpose, usage, and benefits over reflection. |
| Asp/generator/AspSourceGenerator.csproj | New project file for the Asp source generator, targeting netstandard2.0 and packaging as an analyzer. |
| Asp/docs/REFLECTION-FALLBACK.md | Deep-dive documentation describing reflection fallback vs source generator behavior, performance trade-offs, and migration paths. |
| Asp/README.md | Rewrites the Asp README to cover value object validation, result conversion, MVC/Minimal integrations, AOT vs reflection, and pointers to examples/docs. |
| .gitignore | Ignores a new /.claude directory used by tooling. |
Comment on lines
+289
to
+291
| Found an issue or want to add an example? | ||
| - Open an issue at [github.com/anthropics/claude-code/issues](https://github.com/anthropics/claude-code/issues) | ||
| - Examples should be simple, focused, and well-documented |
There was a problem hiding this comment.
The "Contributing" section still links to github.com/anthropics/claude-code/issues, which appears unrelated to this repository and will confuse users trying to file issues. Please update this URL to point to the actual FunctionalDDD GitHub repository issues page or a more appropriate contribution channel.
Comment on lines
+135
to
+154
| foreach (var property in typeInfo.Properties) | ||
| { | ||
| // Check if it's a value object (IScalarValueObject<TSelf, T>) | ||
| if (!IsScalarValueObjectProperty(property)) | ||
| continue; | ||
|
|
||
| var propertyType = property.PropertyType; | ||
|
|
||
| // Create a validating converter for this value object | ||
| var innerConverter = CreateValidatingConverter(propertyType); | ||
| if (innerConverter is null) | ||
| continue; | ||
|
|
||
| // Wrap it with property name awareness | ||
| var wrappedConverter = CreatePropertyNameAwareConverter(innerConverter, property.Name, propertyType); | ||
| if (wrappedConverter is not null) | ||
| { | ||
| property.CustomConverter = wrappedConverter; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
Comment on lines
+56
to
+62
| foreach (var key in context.ModelState.Keys.ToList()) | ||
| { | ||
| if (key.EndsWith("." + fieldError.FieldName, StringComparison.OrdinalIgnoreCase) || | ||
| key.Equals(fieldError.FieldName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| context.ModelState.Remove(key); | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
Suggested change
| foreach (var key in context.ModelState.Keys.ToList()) | |
| { | |
| if (key.EndsWith("." + fieldError.FieldName, StringComparison.OrdinalIgnoreCase) || | |
| key.Equals(fieldError.FieldName, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| context.ModelState.Remove(key); | |
| } | |
| foreach (var key in context.ModelState.Keys | |
| .Where(key => key.EndsWith("." + fieldError.FieldName, StringComparison.OrdinalIgnoreCase) || | |
| key.Equals(fieldError.FieldName, StringComparison.OrdinalIgnoreCase)) | |
| .ToList()) | |
| { | |
| context.ModelState.Remove(key); |
Comment on lines
+169
to
+175
| foreach (var detail in fieldError.Details) | ||
| { | ||
| if (!errors.Contains(detail)) | ||
| { | ||
| errors.Add(detail); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
Comment on lines
+130
to
+140
| foreach (var baseType in c.BaseList.Types) | ||
| { | ||
| var typeName = baseType.Type.ToString(); | ||
| if (typeName.Contains("ScalarValueObject") || | ||
| typeName.Contains("RequiredString") || | ||
| typeName.Contains("RequiredGuid") || | ||
| typeName.Contains(ScalarValueObjectInterfaceName)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
Suggested change
| foreach (var baseType in c.BaseList.Types) | |
| { | |
| var typeName = baseType.Type.ToString(); | |
| if (typeName.Contains("ScalarValueObject") || | |
| typeName.Contains("RequiredString") || | |
| typeName.Contains("RequiredGuid") || | |
| typeName.Contains(ScalarValueObjectInterfaceName)) | |
| { | |
| return true; | |
| } | |
| } | |
| return c.BaseList.Types | |
| .Select(baseType => baseType.Type.ToString()) | |
| .Any(typeName => | |
| typeName.Contains("ScalarValueObject") || | |
| typeName.Contains("RequiredString") || | |
| typeName.Contains("RequiredGuid") || | |
| typeName.Contains(ScalarValueObjectInterfaceName)); |
| { | ||
| for (int i = 0; i < iterations; i++) | ||
| { | ||
| var error = ValidationErrorsContext.GetValidationError(); |
| for (int i = 0; i < iterations; i++) | ||
| { | ||
| var error = ValidationErrorsContext.GetValidationError(); | ||
| var hasErrors = ValidationErrorsContext.HasErrors; |
Comment on lines
+218
to
+247
| if (baseName == "RequiredString" && baseType.IsGenericType) | ||
| { | ||
| // The type argument should be the class itself (CRTP), primitive is string | ||
| if (baseType.TypeArguments.Length == 1 && | ||
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | ||
| return ("RequiredString", null); | ||
| } | ||
| } | ||
|
|
||
| if (baseName == "RequiredGuid" && baseType.IsGenericType) | ||
| { | ||
| // The type argument should be the class itself (CRTP), primitive is Guid | ||
| if (baseType.TypeArguments.Length == 1 && | ||
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | ||
| return ("RequiredGuid", null); | ||
| } | ||
| } | ||
|
|
||
| // Check for ScalarValueObject<TSelf, TPrimitive> | ||
| if (baseName == "ScalarValueObject" && baseType.IsGenericType && baseType.TypeArguments.Length == 2) | ||
| { | ||
| // Verify CRTP pattern: first type arg should be the class itself | ||
| if (SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| return ("ScalarValueObject", baseType.TypeArguments[1]); | ||
| } |
There was a problem hiding this comment.
These 'if' statements can be combined.
Suggested change
| if (baseName == "RequiredString" && baseType.IsGenericType) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is string | |
| if (baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | |
| return ("RequiredString", null); | |
| } | |
| } | |
| if (baseName == "RequiredGuid" && baseType.IsGenericType) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is Guid | |
| if (baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | |
| return ("RequiredGuid", null); | |
| } | |
| } | |
| // Check for ScalarValueObject<TSelf, TPrimitive> | |
| if (baseName == "ScalarValueObject" && baseType.IsGenericType && baseType.TypeArguments.Length == 2) | |
| { | |
| // Verify CRTP pattern: first type arg should be the class itself | |
| if (SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| return ("ScalarValueObject", baseType.TypeArguments[1]); | |
| } | |
| if (baseName == "RequiredString" && | |
| baseType.IsGenericType && | |
| baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | |
| return ("RequiredString", null); | |
| } | |
| if (baseName == "RequiredGuid" && | |
| baseType.IsGenericType && | |
| baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | |
| return ("RequiredGuid", null); | |
| } | |
| // Check for ScalarValueObject<TSelf, TPrimitive> | |
| if (baseName == "ScalarValueObject" && | |
| baseType.IsGenericType && | |
| baseType.TypeArguments.Length == 2 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| return ("ScalarValueObject", baseType.TypeArguments[1]); |
Comment on lines
+218
to
+237
| if (baseName == "RequiredString" && baseType.IsGenericType) | ||
| { | ||
| // The type argument should be the class itself (CRTP), primitive is string | ||
| if (baseType.TypeArguments.Length == 1 && | ||
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | ||
| return ("RequiredString", null); | ||
| } | ||
| } | ||
|
|
||
| if (baseName == "RequiredGuid" && baseType.IsGenericType) | ||
| { | ||
| // The type argument should be the class itself (CRTP), primitive is Guid | ||
| if (baseType.TypeArguments.Length == 1 && | ||
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | ||
| return ("RequiredGuid", null); | ||
| } |
There was a problem hiding this comment.
These 'if' statements can be combined.
Suggested change
| if (baseName == "RequiredString" && baseType.IsGenericType) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is string | |
| if (baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | |
| return ("RequiredString", null); | |
| } | |
| } | |
| if (baseName == "RequiredGuid" && baseType.IsGenericType) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is Guid | |
| if (baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | |
| return ("RequiredGuid", null); | |
| } | |
| if (baseName == "RequiredString" && | |
| baseType.IsGenericType && | |
| baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is string | |
| // Return a marker that we'll convert to "string" in GetPrimitiveTypeName | |
| return ("RequiredString", null); | |
| } | |
| if (baseName == "RequiredGuid" && | |
| baseType.IsGenericType && | |
| baseType.TypeArguments.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | |
| { | |
| // The type argument should be the class itself (CRTP), primitive is Guid | |
| // Return a marker that we'll convert to "Guid" in GetPrimitiveTypeName | |
| return ("RequiredGuid", null); |
Comment on lines
+241
to
+248
| if (baseName == "ScalarValueObject" && baseType.IsGenericType && baseType.TypeArguments.Length == 2) | ||
| { | ||
| // Verify CRTP pattern: first type arg should be the class itself | ||
| if (SymbolEqualityComparer.Default.Equals(baseType.TypeArguments[0], classSymbol)) | ||
| { | ||
| return ("ScalarValueObject", baseType.TypeArguments[1]); | ||
| } | ||
| } |
There was a problem hiding this comment.
These 'if' statements can be combined.
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.
Eliminate manual Combine chains by using value objects directly in your DTOs. Validation errors are automatically collected during JSON deserialization.