-
Notifications
You must be signed in to change notification settings - Fork 4
fix(context): improved accept match rules #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's Guide by SourceryThis pull request improves the matching logic for MIME types and refactors the view rendering process to use the new matching logic. It also introduces a MimeType struct to represent MIME types. Sequence diagram for improved view matching processsequenceDiagram
participant C as Context
participant V as Viewer
participant MT as MimeType
C->>C: View(data, options...)
C->>C: getViewer(name)
alt Named viewer exists
C->>V: MimeType()
V-->>C: viewer mime type
C->>MT: Match(accept)
MT-->>C: match result
else No named viewer
loop For each Accept header
C->>V: MimeType()
V-->>C: viewer mime type
C->>MT: Match(accept)
MT-->>C: match result
end
end
C->>V: Render(response, request, data)
Class diagram for the new MimeType structure and Viewer interface changesclassDiagram
class MimeType {
+String Type
+String SubType
+Match(accept MimeType) bool
+String() string
}
class Viewer {
<<interface>>
+MimeType() *MimeType
+Render(w http.ResponseWriter, r *http.Request, data any) error
}
class JsonViewer {
+MimeType() *MimeType
+Render(w http.ResponseWriter, r *http.Request, data any) error
}
class HtmlViewer {
+MimeType() *MimeType
+Render(w http.ResponseWriter, r *http.Request, data any) error
}
class FileViewer {
+MimeType() *MimeType
+Render(w http.ResponseWriter, r *http.Request, data any) error
}
Viewer <|.. JsonViewer
Viewer <|.. HtmlViewer
Viewer <|.. FileViewer
note for MimeType "New struct for MIME type handling"
note for Viewer "Updated to return *MimeType"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Here's the code health analysis summary for commits Analysis Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cnlangzi - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
+ Coverage 89.79% 89.98% +0.18%
==========================================
Files 32 32
Lines 1176 1198 +22
==========================================
+ Hits 1056 1078 +22
Misses 83 83
Partials 37 37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Changed
Fixed
DefaultView
/View
should be type-safe #5Added
Tests
Tasks to complete before merging PR:
make unit-test
to check for any regressions 📋make lint
to check for any issuesSummary by Sourcery
Improve MIME type matching rules and update the View method signature. Use the first matching viewer based on the Accept header, or the first available viewer if no match is found.
Bug Fixes:
Enhancements: