refactor(schemas): centralize null/default handling and extract ObjectSchema helpers #65
Conversation
…tSchema helpers - Add handleNullInput() and processClonedDefault() template methods to AckSchema base class - Composite schemas (Object, List, Discriminated) override processClonedDefault for recursive validation - ListSchema overrides handleNullInput entirely to handle List<Object?> → List<V> type conversion - Extract helper methods from ObjectSchema.parseAndValidate for improved readability: - _validateDefinedProperties() - _validateExistingProperty() - _handleMissingProperty() - _handleAdditionalProperties() This refactoring addresses two issues identified in code review: 1. Duplicated null/default handling across schema subclasses (consistency risk) 2. Monolithic ObjectSchema.parseAndValidate method (clarity issue) All 905 tests pass with no regressions.
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
- Update version references from beta.4 to beta.5 - Add note about object schema strict-by-default behavior - Clarify refinements run after constraints pass
…Schema - Remove unsafe `as DartType` cast from handleNullInput by routing cloned defaults through parseAndValidate for proper type conversion - Change processClonedDefault to accept Object? and call parseAndValidate, eliminating the need for schema-specific overrides - Remove redundant processClonedDefault overrides from ObjectSchema and DiscriminatedObjectSchema (base class now handles correctly) - Remove ListSchema.handleNullInput override (base class handles List<Object?> → List<V> conversion safely via parseAndValidate) - Fix TransformedSchema to clone defaults with cloneDefault() to prevent mutation of shared default values
There was a problem hiding this comment.
Pull request overview
This PR refactors the schema validation library to centralize null/default handling logic and improve code maintainability. The refactoring introduces template methods handleNullInput() and processClonedDefault() in the base AckSchema class to standardize how schemas handle null inputs and default values.
Changes:
- Introduced centralized null handling via
handleNullInput()andprocessClonedDefault()template methods in base AckSchema class - Extracted helper methods from
ObjectSchema.parseAndValidate(_validateDefinedProperties, _validateExistingProperty, _handleMissingProperty, _handleAdditionalProperties) for improved readability - Updated all scalar schemas (Any, Enum) and composite schemas (Object, List, DiscriminatedObject) to use centralized null handling, removing duplicated code
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ack/lib/src/schemas/schema.dart | Added handleNullInput() and processClonedDefault() template methods to centralize null/default handling logic across all schema types |
| packages/ack/lib/src/schemas/object_schema.dart | Refactored parseAndValidate by extracting helper methods and adopting centralized null handling |
| packages/ack/lib/src/schemas/list_schema.dart | Replaced custom null handling with centralized handleNullInput() call |
| packages/ack/lib/src/schemas/discriminated_object_schema.dart | Replaced custom null handling with centralized handleNullInput() call |
| packages/ack/lib/src/schemas/enum_schema.dart | Replaced custom null handling with centralized handleNullInput() call |
| packages/ack/lib/src/schemas/any_schema.dart | Replaced custom null handling with centralized handleNullInput() call |
| packages/ack/lib/src/schemas/transformed_schema.dart | Updated comments for clarity; maintains custom null handling due to OutputType/InputType distinction |
| llms.txt | Updated version references from beta.4 to beta.5 and improved documentation with examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Handle TransformedSchema's own defaultValue for null input. | ||
| // Clone the default to prevent mutation of shared state. | ||
| // This must happen BEFORE delegating to wrapped schema, because the wrapped | ||
| // schema might not accept null (e.g., non-nullable StringSchema) | ||
| if (inputValue == null) { | ||
| if (defaultValue case final dv?) { | ||
| // Use centralized constraints and refinements check | ||
| return applyConstraintsAndRefinements(dv, context); | ||
| } | ||
| // schema might not accept null (e.g., non-nullable StringSchema). | ||
| if (inputValue == null && defaultValue != null) { | ||
| final clonedDefault = cloneDefault(defaultValue!) as OutputType; | ||
| return applyConstraintsAndRefinements(clonedDefault, context); | ||
| } |
There was a problem hiding this comment.
TransformedSchema has custom null handling logic that differs from the centralized pattern introduced in this PR. While the documentation in schema.dart (line 224-225) suggests schemas where the default value type differs from input type should override processClonedDefault, TransformedSchema cannot use handleNullInput because it must handle its own defaultValue BEFORE delegating to the wrapped schema (as noted in the comment on lines 28-29). Consider adding a comment explaining why TransformedSchema doesn't use the centralized pattern, to help future maintainers understand this design decision.
| /// For composite schemas (Object, List, Discriminated), override [processClonedDefault] | ||
| /// to recursively validate the default structure instead of just applying constraints. |
There was a problem hiding this comment.
The documentation comment is slightly misleading. It says "For composite schemas (Object, List, Discriminated), override processClonedDefault to recursively validate" but composite schemas don't need to override this method - they can use the base implementation which already calls parseAndValidate recursively. Consider rephrasing to: "The base implementation calls parseAndValidate, ensuring composite schemas (Object, List, Discriminated) recursively validate their structure. Override only in schemas where the default value type differs from input type (e.g., TransformedSchema)."
| /// For composite schemas (Object, List, Discriminated), override [processClonedDefault] | |
| /// to recursively validate the default structure instead of just applying constraints. | |
| /// The base implementation of [processClonedDefault] calls [parseAndValidate], | |
| /// ensuring composite schemas (Object, List, Discriminated) recursively validate | |
| /// their structure. Override only in schemas where the default value type differs | |
| /// from the input type (e.g., TransformedSchema). |
- Update handleNullInput docstring to clarify that composite schemas no longer need to override processClonedDefault (base calls parseAndValidate) - Add design decision comment to TransformedSchema explaining why it cannot use the centralized handleNullInput/processClonedDefault pattern (defaultValue is OutputType, not InputType)
- AnyOfSchema: Add proper cloning for defaults, clarify null-handling semantics for union types (member schemas tried first when no default) - ObjectSchema: Pass null to child schemas for missing optional fields with defaults, letting child's handleNullInput clone properly - TransformedSchema: Add runtime type detection fallback for collection defaults to avoid TypeError while documenting mutation risk - Add tests documenting TransformedSchema collection default behavior
…prove logging - Fix multipleOf validation to handle IEEE 754 edge cases where remainder is close to the multiple itself (e.g., 0.6 % 0.1 = 0.0999...) - Extract magic epsilon to named constant with documentation - Replace print() with structured Logger in generator and field_info
Summary
AckSchemabase class viahandleNullInput()andprocessClonedDefault()template methodsObjectSchema.parseAndValidatefor improved readabilityListSchemadefaults by overridinghandleNullInputentirelyTest plan