Skip to content

Commit 774e987

Browse files
Merge pull request #2 from TimothyStiles/req-resp-refactor
Req resp refactor
2 parents 80ea18e + 27e50bd commit 774e987

File tree

4 files changed

+42
-116
lines changed

4 files changed

+42
-116
lines changed

.ditto/2706164f318da155

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

.ditto/e5daf97c0fa2c6f7

Lines changed: 0 additions & 1 deletion
This file was deleted.

ditto.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,11 @@ type CachedResponse struct {
3636
func (c *CachingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
3737

3838
// So I originally tried implementing this to be a lot cleaner with a majority of the logic in retrieve and cache functions. However,
39-
// I'm not sure what was going on but there was some sort of data race where the cache file was being written to correctly but if the
39+
// I'm not sure what was going on but there was some sort of data race where the cache file was being written to correctly but if
4040
// RoundTrip called cache then the GitHub example test would panic and not receive the data from the same response the cache file was
4141
// being written from in the same call to this function (RoundTrip).
4242

43-
endpoint := req.URL.String()
44-
45-
data, err := retrieve(endpoint)
43+
data, err := retrieve(req)
4644
if err == nil {
4745

4846
var cachedResp CachedResponse
@@ -83,7 +81,7 @@ func (c *CachingTransport) RoundTrip(req *http.Request) (*http.Response, error)
8381
return nil, err
8482
}
8583

86-
err = cache(endpoint, marshalledResponse)
84+
err = cache(req, marshalledResponse)
8785
if err != nil {
8886
return nil, err
8987
}
@@ -92,23 +90,26 @@ func (c *CachingTransport) RoundTrip(req *http.Request) (*http.Response, error)
9290
return resp, nil
9391
}
9492

95-
func getCacheFilePath(endpoint string) string {
93+
func getCacheFilePath(req *http.Request) string {
9694
hash := fnv.New64a()
97-
hash.Write([]byte(endpoint))
95+
url := req.URL.String()
96+
method := req.Method
97+
endpointPlusMethod := fmt.Sprintf("%s:%s", method, url)
98+
hash.Write([]byte(endpointPlusMethod))
9899
hashedEndpoint := fmt.Sprintf("%x", hash.Sum(nil))
99100
return filepath.Join(".ditto", hashedEndpoint)
100101
}
101102

102-
func retrieve(endpoint string) ([]byte, error) {
103-
cacheFilePath := getCacheFilePath(endpoint)
103+
func retrieve(req *http.Request) ([]byte, error) {
104+
cacheFilePath := getCacheFilePath(req)
104105
if _, err := os.Stat(cacheFilePath); os.IsNotExist(err) {
105106
return nil, err
106107
}
107108
return os.ReadFile(cacheFilePath)
108109
}
109110

110-
func cache(endpoint string, data []byte) error {
111-
cacheFilePath := getCacheFilePath(endpoint)
111+
func cache(req *http.Request, data []byte) error {
112+
cacheFilePath := getCacheFilePath(req)
112113
os.MkdirAll(filepath.Dir(cacheFilePath), os.ModePerm)
113114
return os.WriteFile(cacheFilePath, data, 0644)
114115
}

ditto_test.go

Lines changed: 29 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,128 +1,53 @@
11
package ditto
22

33
import (
4-
"bytes"
5-
"context"
6-
"fmt"
7-
"io"
4+
"net"
85
"net/http"
6+
"net/http/httptest"
97
"os"
10-
"path/filepath"
118
"testing"
12-
13-
"github.com/google/go-github/v57/github"
149
)
1510

16-
func TestMain(m *testing.M) {
17-
os.Exit(m.Run())
18-
}
19-
20-
func TestGetCacheFilePath(t *testing.T) {
21-
endpoint := "https://example.com/api"
22-
expected := filepath.Join(".ditto", "362656336a5f086c")
23-
result := getCacheFilePath(endpoint)
24-
if result != expected {
25-
t.Errorf("Expected` %s, but got %s", expected, result)
26-
}
27-
}
28-
func TestCache(t *testing.T) {
29-
endpoint := "https://example.com/api"
30-
data := []byte("test data")
31-
expectedFilePath := getCacheFilePath(endpoint)
32-
33-
err := cache(endpoint, data)
34-
if err != nil {
35-
t.Errorf("Unexpected error: %v", err)
36-
}
37-
38-
// Check if the cache file exists
39-
_, err = os.Stat(expectedFilePath)
40-
if err != nil {
41-
t.Errorf("Cache file does not exist: %v", err)
42-
}
43-
44-
// Clean up the cache file
45-
err = os.Remove(expectedFilePath)
46-
if err != nil {
47-
t.Errorf("Failed to remove cache file: %v", err)
48-
}
49-
}
50-
func TestRetrieve(t *testing.T) {
51-
endpoint := "https://example.com/api"
52-
expectedData := []byte("test data")
53-
expectedFilePath := getCacheFilePath(endpoint)
54-
55-
// Create a cache file with test data
56-
err := os.WriteFile(expectedFilePath, expectedData, 0644)
57-
if err != nil {
58-
t.Fatalf("Failed to create cache file: %v", err)
59-
}
60-
defer os.Remove(expectedFilePath)
61-
62-
// Test loading cache
63-
result, err := retrieve(endpoint)
64-
if err != nil {
65-
t.Errorf("Unexpected error: %v", err)
66-
}
11+
func TestCachingTransport_RoundTrip(t *testing.T) {
12+
// Create a test server
13+
server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
14+
w.WriteHeader(http.StatusOK)
15+
w.Write([]byte("Hello, World!"))
16+
}))
6717

68-
// Compare loaded data with expected data
69-
if !bytes.Equal(result, expectedData) {
70-
t.Errorf("Expected data: %s, but got: %s", expectedData, result)
71-
}
72-
}
73-
func TestCachingTransport_RoundTrip_CachedResponse(t *testing.T) {
74-
// Define the test URL and expected response
75-
url := "https://example.com/api"
18+
// Set the server to listen on a specific address
19+
server.Listener, _ = net.Listen("tcp", "localhost:56560")
7620

77-
// Remove any existing cache for the test URL
78-
cacheFilePath := getCacheFilePath(url)
79-
_ = os.Remove(cacheFilePath)
21+
server.Start()
22+
defer server.Close()
8023

81-
// Create a new caching HTTP client
82-
client := &CachingTransport{
24+
// Create a new CachingTransport
25+
cachingTransport := &CachingTransport{
8326
Transport: http.DefaultTransport,
8427
}
8528

86-
// Create a new HTTP request
87-
req, err := http.NewRequest("GET", url, nil)
29+
// Create a new request
30+
req, err := http.NewRequest(http.MethodGet, server.URL, nil)
8831
if err != nil {
89-
t.Fatalf("Failed to create HTTP request: %v", err)
32+
t.Fatalf("Failed to create request: %v", err)
9033
}
9134

92-
// Make the HTTP request using the caching HTTP client
93-
resp, err := client.RoundTrip(req)
94-
if err != nil {
95-
t.Fatalf("Failed to make HTTP request: %v", err)
96-
}
97-
defer resp.Body.Close()
35+
// remove the cache file if it exists and defer removing it again until the end of the test
36+
cacheFilePath := getCacheFilePath(req)
37+
os.Remove(cacheFilePath)
38+
defer os.Remove(cacheFilePath)
9839

99-
// Read the response body
100-
_, err = io.ReadAll(resp.Body)
40+
// Call the RoundTrip method
41+
resp, err := cachingTransport.RoundTrip(req)
10142
if err != nil {
102-
t.Fatalf("Failed to read response body: %v", err)
43+
t.Fatalf("RoundTrip failed: %v", err)
10344
}
45+
defer resp.Body.Close()
10446

105-
// clean up the cache file again just for good measure
106-
_ = os.Remove(cacheFilePath)
107-
108-
}
109-
110-
func TestGitHubExampleRegression(t *testing.T) {
111-
os.Remove("ditto/028587485a8f03d6")
112-
defer os.Remove(".ditto/028587485a8f03d6")
113-
token := os.Getenv("GITHUB_TOKEN")
114-
115-
// instead of http.DefaultClient we use ditto.Client()
116-
client := github.NewClient(Client()).WithAuthToken(token) // auth token is optional
117-
118-
// Use client...
119-
repos, _, err := client.Repositories.List(context.Background(), "TimothyStiles", nil)
120-
if err != nil {
121-
fmt.Println("Error:", err)
122-
return
47+
// Check the response
48+
if resp.StatusCode != http.StatusOK {
49+
t.Errorf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode)
12350
}
12451

125-
if repos[0].GetName() != "action-discord" {
126-
t.Errorf("Expected %s, but got %s", "action-discord", repos[0].GetName())
127-
}
52+
// TODO: Add more assertions as needed
12853
}

0 commit comments

Comments
 (0)