fix: caching middleware's partRequest getUpstreamReader range request…#38
fix: caching middleware's partRequest getUpstreamReader range request…#38
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the getContents function in the caching middleware to eliminate code duplication by delegating HTTP request cloning and range header setting to the existing getUpstreamReader method.
Changes:
- Removed manual request cloning and range header manipulation in favor of using the
getUpstreamReadermethod which already handles these operations - Added clarifying comments indicating that request cloning is handled automatically by
getUpstreamReader
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| newRange := fmt.Sprintf("bytes=%d-%d", fromByte, toByte) | ||
| req.Header.Set("Range", newRange) | ||
|
|
||
| // auto-copy request with c.req |
There was a problem hiding this comment.
The comment "auto-copy request with c.req" is grammatically awkward. Consider rephrasing to "Request is automatically cloned by getUpstreamReader" or "getUpstreamReader handles request cloning and range header setting" for better clarity.
| // auto-copy request with c.req | |
| // getUpstreamReader handles request cloning and range header setting |
| req := c.req.Clone(context.Background()) | ||
| req.Header.Set("Range", newRange) | ||
|
|
||
| // auto-copy request with c.req |
There was a problem hiding this comment.
The comment "auto-copy request with c.req" is grammatically awkward. Consider rephrasing to "Request is automatically cloned by getUpstreamReader" or "getUpstreamReader handles request cloning and range header setting" for better clarity.
e086297 to
b037391
Compare
This pull request simplifies the logic in the
getContentsfunction by removing manual HTTP request cloning and range header setting, instead delegating these responsibilities to thegetUpstreamReadermethod.Refactoring and simplification:
c.reqand manual setting of theRangeheader in favor of usinggetUpstreamReader, which now handles copying the request and setting the appropriate headers. (server/middleware/caching/internal.go) [1] [2]