feat: adjust validation openapi#59
Conversation
- Update core structs (Server, Discriminator, XML) with new OpenAPI 3.2.0 fields. - Implement high-level options (ServerName, ContentSummary) for improved configuration. - Enhance validation suite to strictly enforce version-specific rules: - Enforce numeric exclusive boundaries in 3.1+ (JSON Schema 2020-12). - Validate server name uniqueness and reserved header names. - Restrict Encoding Object fields based on media type context. - Refactor URI validation for technical accuracy (IsNonRelativeURI) and harden discovery endpoints. - Update reflector to automatically migrate deprecated XML fields to NodeType in 3.2.0. - Update documentation and golden files to reflect specification alignment. - Fix linting issues by extracting validation helpers and improving formatting. - Comprehensive test coverage added in audit_test.go.
…ecks - Introduce Severity types (SeverityError, SeverityWarning, SeverityInfo) and a custom ValidationError wrapper in internal/validate. - Refactor all existing strict validation logic to use validate.Errorf. - Implement Warningf and Infof helper functions for non-fatal feedback. - Update spec.ValidationErrors to correctly aggregate and filter errors based on severity using the new HasSeverity method. - Update builder package to explicitly generate SeverityError errors during path construction. - Add new OpenAPI best practice checks for missing tags, contact, license, servers, operationId, summary, and description. - Add deprecation warnings for operations, parameters, headers, schemas, and security schemes. - Refactor internal test suite helpers to tolerate advisory warnings while asserting strict errors. - Expose severity constants and ValidationError via aliases in the public spec package for external inspection.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions.
This PR modernizes the spec library to align with OpenAPI 3.2.0 and introduces a validation severity system. The changes are well-structured and covered by tests. However, a behavioral change in joinErrors may silently drop warnings/info from validation results, and a minor inconsistency in version-gated component checks could surprise users.
🌟 Strengths
- Full test coverage for new severity levels and OpenAPI 3.2.0 fields.
- Clean separation of severity types and clear refactoring of hardcoded
fmt.Errorfto structured errors.
💡 Suggestions (P2)
- errors.go: The
joinErrorsfunction now returnsnilwhen noSeverityErroris present, meaning best-practice recommendations (missingoperationId,summary, etc.) will no longer propagate to callers ofValidate(). This is a deliberate design choice, but consumers relying onValidate()to surface all issues may see fewer errors. Consider documenting this breaking change in notes or offering a strict mode. - internal/validate/components.go: The validation condition for
components.PathItemsandcomponents.MediaTypeschanged fromlen > 0to!= nil. While empty maps are rarely used, this stricter check may surprise upgraders who inadvertently pass an empty non-nil map.
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| return false | ||
| } | ||
|
|
||
| func joinErrors(errs []error) error { |
There was a problem hiding this comment.
P2 | Confidence: High
The joinErrors function now returns nil when the collection contains no error with SeverityError. Previously any non-nil validation error (including those now reclassified as Warning or Info) caused a non-nil return. This change alters the contract of all Validate() calls: adapters (e.g., adapter/muxopenapi/router.go) and the generator's MarshalYAML (via router.go) will now silently accept spec generation even when best-practice issues like missing operationId, summary, or description are present. Consumers that relied on Validate() to catch any validation issue will no longer be notified of these recommendations, potentially producing suboptimal specifications. This is a deliberate design choice to align with severity levels, but it is a breaking behavioral change for existing callers.
…ecific schema and server checks.
…he internal package and clean up document validation structure
…alidateReport method
…ng parameters in reflector
…n messaging, add mediaType tag support to reflector, and move tests to package validate_test
…e yaml marshal to use single quote
…or handling consistency
1 participant
Description
Type of Change
Checklist
make checkpasses locally (sync + tidy + lint + test)make test-update)Notes for Reviewers
Breaking Change Notes