-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(compressor): added gzip/deflate compression support on request #12
Conversation
Reviewer's Guide by SourceryThis pull request introduces gzip and deflate compression support for requests. The Sequence diagram for HTTP request compression flowsequenceDiagram
participant Client
participant App
participant Compressor
participant ResponseWriter
Client->>App: HTTP Request with Accept-Encoding
App->>App: createWriter()
App->>Compressor: Check AcceptEncoding()
alt Compression supported
Compressor->>ResponseWriter: Create compressed writer
ResponseWriter-->>App: Return compressed writer
else No compression
App->>ResponseWriter: Create standard writer
ResponseWriter-->>App: Return standard writer
end
App->>Client: Compressed/Standard Response
Class diagram for compression-related typesclassDiagram
class Compressor {
<<interface>>
+AcceptEncoding() string
+New(rw ResponseWriter) ResponseWriter
}
class ResponseWriter {
<<interface>>
+Write(p []byte) (int, error)
+Header() Header
+WriteHeader(statusCode int)
+Close()
}
class GzipCompressor {
+AcceptEncoding() string
+New(rw ResponseWriter) ResponseWriter
}
class DeflateCompressor {
+AcceptEncoding() string
+New(rw ResponseWriter) ResponseWriter
}
class gzipResponseWriter {
-w *gzip.Writer
+Write(p []byte) (int, error)
+Close()
}
class deflateResponseWriter {
-w *flate.Writer
+Write(p []byte) (int, error)
+Close()
}
Compressor <|.. GzipCompressor
Compressor <|.. DeflateCompressor
ResponseWriter <|.. gzipResponseWriter
ResponseWriter <|.. deflateResponseWriter
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 found some issues that need to be addressed.
Blocking issues:
- The error from flate.NewWriter() should be handled rather than ignored (link)
Overall Comments:
- Consider adding proper error handling for compression writer creation instead of discarding errors silently, especially in DeflateCompressor.New()
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 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 #12 +/- ##
==========================================
+ Coverage 82.28% 83.12% +0.83%
==========================================
Files 21 26 +5
Lines 988 1037 +49
==========================================
+ Hits 813 862 +49
Misses 138 138
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. |
Added
gzip
anddeflate
compressors to support Accept-Encoding request. fixed [feat]support Accept-Encoding #10DeepSource
feature