chore: refactor net/rest to use Ferret HTTP client abstraction - #31
Merged
Conversation
… tests for network context handling, and improve request encoding
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the net/rest module’s internal REST client to use Ferret’s network HTTP client abstraction (context-provided ferretnet.HTTPClient and ferrethttp.{Request,Response}) instead of net/http.Client, aiming to improve policy enforcement and testability.
Changes:
- Reworked request execution to fetch an HTTP client from the execution context and issue requests via
ferrethttp.Request. - Updated request/response encoding/decoding to operate on Ferret HTTP types and
[]bytebodies. - Expanded tests with Ferret network context usage, including a new policy enforcement test and a recording HTTP client test double.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/net/rest/core/client.go | Removes direct *http.Client storage from the REST client handle. |
| modules/net/rest/core/execute.go | Executes requests via ferretnet.HTTPClientFrom(ctx) and ferrethttp.Request. |
| modules/net/rest/core/codec.go | Changes request body encoding to return []byte (and updates call sites). |
| modules/net/rest/core/response.go | Decodes ferrethttp.Response and builds “full” response objects using the request URL. |
| modules/net/rest/core/client_test.go | Updates tests to provide a Ferret network context and adds new coverage for context-provided HTTP clients. |
| modules/net/rest/core/recording_http_client_test.go | Adds a test double to record outgoing Ferret HTTP requests. |
| modules/net/rest/http_test.go | Adds a module-level test verifying Ferret HTTP policy enforcement blocks outbound requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
34
to
36
| if binary, ok := value.(runtime.Binary); ok { | ||
| return bytes.NewReader(binary), "application/octet-stream", nil | ||
| return []byte(binary), "application/octet-stream", nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the REST client in the
net/restmodule to remove its direct dependency on the standard libraryhttp.Clientand instead use the Ferret network interface (ferretnet.HTTPClient). This enables better testability, greater flexibility for custom network policies, and improved context management. The change is accompanied by updates to the test suite and several new tests to ensure correct behavior and coverage.The most important changes are:
Core REST client refactor:
Clientstruct inclient.gono longer holds anhttp.Client; instead, it retrieves aferretnet.HTTPClientfrom the context for each request, making the client more flexible and testable. (client.go,execute.go) [1] [2] [3] [4] [5]ferrethttp.Requestandferrethttp.Response) throughout, including in the codec and response handling logic. (codec.go,response.go) [1] [2] [3] [4][]byteinstead of anio.Reader, reflecting the new Ferret HTTP interface. (codec.go) [1] [2]Test improvements and coverage:
networkContext()helper to provide the necessary Ferret network in the context. (client_test.go) [1] [2] [3] [4] [5] [6] [7] [8]TestClientUsesFerretHTTPClientFromContext).TestClientRequiresNetworkContext).TestModuleHonorsFerretHTTPPolicy). [1] [2]Test utilities:
recordingHTTPClienttest double is introduced to capture and inspect outgoing Ferret HTTP requests for assertions in tests. (recording_http_client_test.go)These changes make the REST client more robust, modular, and secure, while improving testability and aligning it with Ferret's network abstraction.