Skip to content

Commit af710a6

Browse files
authored
refactor: avoid creating temp files (#6)
* refactor: avoid creating temp files * add test for upload via reader * introduce content type method * ci: add go test: * upload artifact action update
1 parent 8978877 commit af710a6

File tree

7 files changed

+328
-57
lines changed

7 files changed

+328
-57
lines changed

.github/workflows/go-test.yml

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
name: Go Tests
2+
3+
on:
4+
push:
5+
branches: [main]
6+
paths:
7+
- "upload-file/**"
8+
- ".github/workflows/go-test.yml"
9+
pull_request:
10+
branches: [main]
11+
paths:
12+
- "upload-file/**"
13+
- ".github/workflows/go-test.yml"
14+
15+
jobs:
16+
upload-file:
17+
runs-on: ubuntu-latest
18+
19+
steps:
20+
- uses: actions/checkout@v4
21+
22+
- name: Set up Go
23+
uses: actions/setup-go@v4
24+
with:
25+
go-version: "1.24.5"
26+
27+
- name: Cache Go modules
28+
uses: actions/cache@v3
29+
with:
30+
path: |
31+
~/.cache/go-build
32+
~/go/pkg/mod
33+
key: ${{ runner.os }}-go-${{ hashFiles('upload-file/go.sum') }}
34+
restore-keys: |
35+
${{ runner.os }}-go-
36+
37+
- name: Download dependencies
38+
working-directory: ./upload-file
39+
run: go mod download
40+
41+
- name: Verify dependencies
42+
working-directory: ./upload-file
43+
run: go mod verify
44+
45+
- name: Run tests
46+
working-directory: ./upload-file
47+
run: go test -v -race -coverprofile=coverage.out ./...
48+
49+
- name: Generate coverage report
50+
working-directory: ./upload-file
51+
run: go tool cover -html=coverage.out -o coverage.html
52+
53+
- name: Upload coverage reports
54+
uses: actions/upload-artifact@v4
55+
with:
56+
name: coverage-report
57+
path: upload-file/coverage.html
58+
59+
- name: Check test coverage
60+
working-directory: ./upload-file
61+
run: |
62+
COVERAGE=$(go tool cover -func=coverage.out | grep total | awk '{print substr($3, 1, length($3)-1)}')
63+
echo "Total test coverage: ${COVERAGE}%"
64+
if (( $(echo "$COVERAGE < 50" | bc -l) )); then
65+
echo "Warning: Test coverage is below 50%"
66+
fi

upload-file/upload/buildkite.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package upload
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
67
"errors"
@@ -79,15 +80,6 @@ func uploadBuildkiteMetadata(ctx context.Context, c *Client, in *input.Input) er
7980
if err != nil {
8081
return fmt.Errorf("failed to marshal metadata: %v", err)
8182
}
82-
metadataFile, err := os.CreateTemp("", "buildkite-metadata.json")
83-
if err != nil {
84-
return fmt.Errorf("failed to create temp metadata file: %v", err)
85-
}
86-
defer os.Remove(metadataFile.Name())
87-
_, err = metadataFile.Write(metadataJSON)
88-
if err != nil {
89-
return fmt.Errorf("failed to write metadata to temp file: %v", err)
90-
}
9183

9284
buildkiteBranch := os.Getenv("BUILDKITE_BRANCH")
9385
buildkiteTag := os.Getenv("BUILDKITE_TAG")
@@ -109,7 +101,7 @@ func uploadBuildkiteMetadata(ctx context.Context, c *Client, in *input.Input) er
109101
uploadPath = servicePath(serviceName, fmt.Sprintf("buildkite/%s/%s", buildkitePipelineSlug, uploadPath))
110102

111103
log.Println("Uploading Buildkite metadata")
112-
err = c.UploadFile(ctx, metadataFile.Name(), uploadPath)
104+
err = c.UploadViaReader(ctx, bytes.NewReader(metadataJSON), ContentTypeJSON, uploadPath)
113105
if err != nil {
114106
return fmt.Errorf("failed to upload metadata: %v", err)
115107
}

upload-file/upload/client.go

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,13 @@ func (c *Client) UploadFile(ctx context.Context, sourcePath, destination string)
109109
)
110110
}
111111

112-
func (c *Client) rawUpload(ctx context.Context, uploadURL string, contentType string, contentSize int64, r io.Reader) error {
112+
func (c *Client) rawUpload(ctx context.Context, uploadURL string, contentType ContentType, contentSize int64, r io.Reader) error {
113113
req, err := http.NewRequestWithContext(ctx, "PUT", uploadURL, r)
114114
if err != nil {
115115
return fmt.Errorf("failed to create HTTP request: %v", err)
116116
}
117117
req.ContentLength = contentSize
118-
req.Header.Set("Content-Type", contentType)
118+
req.Header.Set("Content-Type", string(contentType))
119119

120120
resp, err := c.httpClient.Do(req)
121121
if err != nil {
@@ -131,26 +131,22 @@ func (c *Client) rawUpload(ctx context.Context, uploadURL string, contentType st
131131
return nil
132132
}
133133

134-
func getContentType(filePath string) string {
135-
ext := filepath.Ext(filePath)
136-
switch ext {
137-
case ".json":
138-
return "application/json"
139-
case ".txt":
140-
return "text/plain"
141-
case ".csv":
142-
return "text/csv"
143-
case ".xml":
144-
return "application/xml"
145-
case ".pdf":
146-
return "application/pdf"
147-
case ".zip":
148-
return "application/zip"
149-
case ".tar":
150-
return "application/x-tar"
151-
case ".gz":
152-
return "application/gzip"
153-
default:
154-
return "application/octet-stream"
134+
func (c *Client) UploadViaReader(ctx context.Context, r io.Reader, contentType ContentType, destination string) error {
135+
// Buffer the content to determine size
136+
var buf bytes.Buffer
137+
size, err := io.Copy(&buf, r)
138+
if err != nil {
139+
return fmt.Errorf("failed to read content: %v", err)
140+
}
141+
142+
presignedURL, err := c.presignedUploadURL(ctx, destination)
143+
if err != nil {
144+
return fmt.Errorf("failed to get presigned upload URL: %w", err)
155145
}
146+
147+
return c.rawUpload(ctx,
148+
presignedURL,
149+
contentType, size,
150+
&buf,
151+
)
156152
}

upload-file/upload/client_test.go

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
package upload
2+
3+
import (
4+
"context"
5+
"errors"
6+
"io"
7+
"net/http"
8+
"net/http/httptest"
9+
"strings"
10+
"testing"
11+
"time"
12+
)
13+
14+
func TestClient_UploadViaReader_Success(t *testing.T) {
15+
// Create a test server for the upload endpoint
16+
uploadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
17+
if r.Method != "PUT" {
18+
t.Errorf("Expected PUT method, got %s", r.Method)
19+
}
20+
21+
// Verify content type
22+
expectedContentType := "application/json"
23+
if r.Header.Get("Content-Type") != expectedContentType {
24+
t.Errorf("Expected Content-Type %s, got %s", expectedContentType, r.Header.Get("Content-Type"))
25+
}
26+
27+
// Read and verify content
28+
body, err := io.ReadAll(r.Body)
29+
if err != nil {
30+
t.Errorf("Failed to read request body: %v", err)
31+
}
32+
33+
expectedContent := `{"test": "data"}`
34+
if string(body) != expectedContent {
35+
t.Errorf("Expected content %s, got %s", expectedContent, string(body))
36+
}
37+
38+
w.WriteHeader(http.StatusOK)
39+
w.Write([]byte("Upload successful"))
40+
}))
41+
defer uploadServer.Close()
42+
43+
// Create a test server for the GraphQL endpoint
44+
graphqlServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
45+
if r.Method != "POST" {
46+
t.Errorf("Expected POST method for GraphQL, got %s", r.Method)
47+
}
48+
49+
// Return a mock presigned URL response
50+
response := `{
51+
"data": {
52+
"storage_presigned_upload_url": {
53+
"url": "` + uploadServer.URL + `",
54+
"expired_at": "` + time.Now().Add(time.Hour).Format(time.RFC3339) + `"
55+
}
56+
}
57+
}`
58+
59+
w.Header().Set("Content-Type", "application/json")
60+
w.WriteHeader(http.StatusOK)
61+
w.Write([]byte(response))
62+
}))
63+
defer graphqlServer.Close()
64+
65+
// Create client with test servers
66+
client := NewClient(graphqlServer.URL, "test-api-key")
67+
68+
// Test content
69+
testContent := `{"test": "data"}`
70+
reader := strings.NewReader(testContent)
71+
72+
// Call the function under test
73+
err := client.UploadViaReader(context.Background(), reader, "application/json", "test/file.json")
74+
75+
if err != nil {
76+
t.Errorf("Expected no error, but got: %v", err)
77+
}
78+
}
79+
80+
func TestClient_UploadViaReader_ReaderError(t *testing.T) {
81+
// Test case where the reader itself fails
82+
client := NewClient("https://test.example.com/graphql", "test-key")
83+
84+
// Create a reader that will fail
85+
failingReader := &failingReader{err: errors.New("reader error")}
86+
87+
err := client.UploadViaReader(context.Background(), failingReader, "text/plain", "test/file.txt")
88+
89+
expectedError := "failed to read content: reader error"
90+
if err == nil {
91+
t.Errorf("Expected error containing %q, but got no error", expectedError)
92+
} else if !strings.Contains(err.Error(), expectedError) {
93+
t.Errorf("Expected error containing %q, but got: %v", expectedError, err)
94+
}
95+
}
96+
97+
// failingReader is a mock reader that always returns an error
98+
type failingReader struct {
99+
err error
100+
}
101+
102+
func (f *failingReader) Read(p []byte) (n int, err error) {
103+
return 0, f.err
104+
}
105+
106+
func TestClient_UploadViaReader_GraphQLError(t *testing.T) {
107+
// Create a test server that returns GraphQL error
108+
graphqlServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
109+
w.Header().Set("Content-Type", "application/json")
110+
w.WriteHeader(http.StatusBadRequest)
111+
w.Write([]byte(`{"errors": [{"message": "Invalid query"}]}`))
112+
}))
113+
defer graphqlServer.Close()
114+
115+
client := NewClient(graphqlServer.URL, "test-key")
116+
reader := strings.NewReader("test content")
117+
118+
err := client.UploadViaReader(context.Background(), reader, "text/plain", "test/file.txt")
119+
120+
if err == nil {
121+
t.Error("Expected error due to GraphQL failure, but got no error")
122+
} else if !strings.Contains(err.Error(), "failed to get presigned upload URL") {
123+
t.Errorf("Expected GraphQL error, but got: %v", err)
124+
}
125+
}
126+
127+
func TestClient_UploadViaReader_UploadError(t *testing.T) {
128+
// Create upload server that returns error
129+
uploadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
130+
w.WriteHeader(http.StatusForbidden)
131+
w.Write([]byte("Access denied"))
132+
}))
133+
defer uploadServer.Close()
134+
135+
// Create GraphQL server that returns the upload server URL
136+
graphqlServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
137+
response := `{
138+
"data": {
139+
"storage_presigned_upload_url": {
140+
"url": "` + uploadServer.URL + `",
141+
"expired_at": "` + time.Now().Add(time.Hour).Format(time.RFC3339) + `"
142+
}
143+
}
144+
}`
145+
w.Header().Set("Content-Type", "application/json")
146+
w.WriteHeader(http.StatusOK)
147+
w.Write([]byte(response))
148+
}))
149+
defer graphqlServer.Close()
150+
151+
client := NewClient(graphqlServer.URL, "test-key")
152+
reader := strings.NewReader("test content")
153+
154+
err := client.UploadViaReader(context.Background(), reader, "text/plain", "test/file.txt")
155+
if err == nil {
156+
t.Error("Expected upload error, but got no error")
157+
} else if !strings.Contains(err.Error(), "upload failed with status 403") {
158+
t.Errorf("Expected upload status error, but got: %v", err)
159+
}
160+
}
161+
162+
func TestClient_UploadViaReader_EmptyContent(t *testing.T) {
163+
// Test with empty content
164+
uploadServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
165+
body, _ := io.ReadAll(r.Body)
166+
if len(body) != 0 {
167+
t.Errorf("Expected empty body, got %d bytes", len(body))
168+
}
169+
w.WriteHeader(http.StatusOK)
170+
}))
171+
defer uploadServer.Close()
172+
173+
graphqlServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
174+
response := `{
175+
"data": {
176+
"storage_presigned_upload_url": {
177+
"url": "` + uploadServer.URL + `",
178+
"expired_at": "` + time.Now().Add(time.Hour).Format(time.RFC3339) + `"
179+
}
180+
}
181+
}`
182+
w.Header().Set("Content-Type", "application/json")
183+
w.WriteHeader(http.StatusOK)
184+
w.Write([]byte(response))
185+
}))
186+
defer graphqlServer.Close()
187+
188+
client := NewClient(graphqlServer.URL, "test-key")
189+
reader := strings.NewReader("")
190+
191+
err := client.UploadViaReader(context.Background(), reader, "text/plain", "test/empty.txt")
192+
193+
if err != nil {
194+
t.Errorf("Expected no error for empty content, but got: %v", err)
195+
}
196+
}

upload-file/upload/content_type.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package upload
2+
3+
import "path/filepath"
4+
5+
type ContentType string
6+
7+
const (
8+
ContentTypeJSON ContentType = "application/json"
9+
ContentTypeText ContentType = "text/plain"
10+
ContentTypeCSV ContentType = "text/csv"
11+
ContentTypeXML ContentType = "application/xml"
12+
ContentTypePDF ContentType = "application/pdf"
13+
ContentTypeZIP ContentType = "application/zip"
14+
ContentTypeTAR ContentType = "application/x-tar"
15+
ContentTypeGZ ContentType = "application/gzip"
16+
ContentTypeOctetStream ContentType = "application/octet-stream"
17+
)
18+
19+
func getContentType(filePath string) ContentType {
20+
ext := filepath.Ext(filePath)
21+
switch ext {
22+
case ".json":
23+
return ContentTypeJSON
24+
case ".txt":
25+
return ContentTypeText
26+
case ".csv":
27+
return ContentTypeCSV
28+
case ".xml":
29+
return ContentTypeXML
30+
case ".pdf":
31+
return ContentTypePDF
32+
case ".zip":
33+
return ContentTypeZIP
34+
case ".tar":
35+
return ContentTypeTAR
36+
case ".gz":
37+
return ContentTypeGZ
38+
default:
39+
return ContentTypeOctetStream
40+
}
41+
}

0 commit comments

Comments
 (0)