-
Notifications
You must be signed in to change notification settings - Fork 3
Closed
Description
🔍 Duplicate Code Pattern: HTTP Response Writer Wrappers
Part of duplicate code analysis: #375
Summary
Two nearly identical HTTP response writer wrapper implementations exist in the server package, differing only in field type (bytes.Buffer vs []byte). This duplication was introduced as part of the SDK logging changes in PR #372.
Duplication Details
Pattern: Response Writer Wrapper for Capturing Output
- Severity: Medium
- Occurrences: 2 instances
- Locations:
internal/server/sdk_logging.go(lines 40-54) -sdkLoggingResponseWriterinternal/server/transport.go(lines 52-66) -loggingResponseWriter
Code Comparison:
sdk_logging.go (uses bytes.Buffer):
type sdkLoggingResponseWriter struct {
http.ResponseWriter
body bytes.Buffer
statusCode int
}
func (w *sdkLoggingResponseWriter) WriteHeader(statusCode int) {
w.statusCode = statusCode
w.ResponseWriter.WriteHeader(statusCode)
}
func (w *sdkLoggingResponseWriter) Write(b []byte) (int, error) {
w.body.Write(b)
return w.ResponseWriter.Write(b)
}transport.go (uses []byte):
type loggingResponseWriter struct {
http.ResponseWriter
body []byte
statusCode int
}
func (w *loggingResponseWriter) WriteHeader(statusCode int) {
w.statusCode = statusCode
w.ResponseWriter.WriteHeader(statusCode)
}
func (w *loggingResponseWriter) Write(b []byte) (int, error) {
w.body = append(w.body, b...)
return w.ResponseWriter.Write(b)
}Key Difference: Only the body storage type differs (bytes.Buffer vs []byte with append)
Impact Analysis
- Maintainability: Changes to response capture logic need duplication across both types
- Bug Risk: Low - implementations are simple, but inconsistency could cause confusion
- Code Bloat: ~15 lines duplicated
- Testing: Requires separate tests for identical functionality
Refactoring Recommendations
1. Extract to Shared Middleware Utility (Recommended)
- Create
internal/server/middleware.gowith unified response writer:// responseCapturingWriter wraps http.ResponseWriter to capture response body type responseCapturingWriter struct { http.ResponseWriter body *bytes.Buffer statusCode int }
- Location:
internal/server/middleware.go(new file) - Estimated effort: 1 hour
- Benefits:
- Single implementation for all logging needs
- Consistent behavior across different logging contexts
- Easier to enhance (e.g., add size limits, compression)
2. Standardize on bytes.Buffer
- Use
bytes.Bufferin both locations (more efficient for large responses) - Update
transport.goto matchsdk_logging.gopattern - Estimated effort: 15 minutes
- Benefits:
- Quick fix
- Better memory efficiency with Buffer API
3. Wrapper Factory Function
- Create factory:
func newLoggingResponseWriter(w http.ResponseWriter) *loggingResponseWriter - Consolidate initialization logic
- Estimated effort: 30 minutes
Implementation Checklist
- Create
internal/server/middleware.gowith shared response writer - Add unit tests for response writer wrapper
- Update
sdk_logging.goto use shared implementation - Update
transport.goto use shared implementation - Remove duplicate type definitions
- Verify all logging functionality still works correctly
- Consider adding response size limits or streaming support
Parent Issue
See parent analysis report: #375
Related to #375
AI generated by Duplicate Code Detector
Reactions are currently unavailable