feat: add irisopenapi adapter#60
Conversation
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 introduces a new Iris adapter for OpenAPI spec generation, following the established adapter patterns with solid test coverage. Two minor maintainability suggestions are noted below that do not block approval but improve future-proofing.
📄 Documentation Diagram
This diagram documents the Iris adapter's route registration and spec generation flow.
sequenceDiagram
participant User
participant Adapter as irisopenapi
participant IrisParty as Iris Party
participant SpecGen as spec.Generator
User->>Adapter: NewRouter(party, opts)
note over Adapter: PR #35;60: New Iris adapter
Adapter->>IrisParty: register docs endpoints
Adapter->>SpecGen: create spec router
User->>Adapter: Get("/ping", handler)
Adapter->>IrisParty: party.Handle(...)
Adapter->>SpecGen: specRouter.Add("GET","/ping")
User->>Adapter: GenerateSchema()
Adapter->>SpecGen: gen.GenerateSchema()
SpecGen-->>User: schema bytes
🌟 Strengths
- Comprehensive test coverage including golden file testing for petstore example.
- Clean separation of concerns following the existing adapter architecture.
💡 Suggestions
- adapter/irisopenapi/route.go: Coupling to internal Iris type
irisrouter.Routecreates a latent risk of breakage on future Iris upgrades. - adapter/irisopenapi/route.go: Ignoring the return value of
specRoute.Withmay cause applied OpenAPI options to be silently lost if the underlying implementation changes.
💡 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.
| type route struct { | ||
| irisRoute *irisrouter.Route | ||
| specRoute spec.Route | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: High
The route struct stores a pointer to irisrouter.Route, which is an internal type of the Iris framework (github.com/kataras/iris/v12/core/router). This creates a tight coupling to an internal API that is not covered by Go module compatibility guarantees. While this pattern is shared across all existing adapters, it introduces a latent risk of breakage on minor Iris version upgrades where internal types may change. If the internal struct is altered in a future Iris release, the adapter will fail to compile and require a patch. A more robust approach would be to use only the public iris.Route interface if it exposes the needed fields (Method, Path, Name) or to define a thin wrapper that extracts these values at registration time.
| return r.irisRoute.Name | ||
| } | ||
|
|
||
| func (r *route) With(opts ...option.OperationOption) Route { |
There was a problem hiding this comment.
P2 | Confidence: Medium
The With method calls r.specRoute.With(opts...) but ignores its return value. If specRoute.With returns a new route (immutable pattern) rather than mutating in place, then r.specRoute would remain unchanged, causing the applied OpenAPI options to be silently lost. Based on the implementation of other adapters in this repository, it is likely that specRoute.With mutates the receiver. However, the ignored return value is a code smell and a potential future bug if the underlying implementation changes. For idiomatic safety and consistency with Go chaining practices, the result should be reassigned to r.specRoute. This is a defensive improvement that eliminates the risk entirely without performance cost.
| func (r *route) With(opts ...option.OperationOption) Route { | |
| func (r *route) With(opts ...option.OperationOption) Route { | |
| if r.specRoute == nil { | |
| return r | |
| } | |
| r.specRoute = r.specRoute.With(opts...) | |
| return r | |
| } |
1 participant
Description
add irisopenapi adapter
Type of Change
Checklist
make checkpasses locally (sync + tidy + lint + test)make test-update)Notes for Reviewers
Breaking Change Notes