Skip to content

Commit 7dedb84

Browse files
committed
refactor: consolidate duplicated code patterns across packages
Extract shared helpers to eliminate ~200 lines of redundant code: - doRequest/marshalAndPost in GitHubClient (6 methods consolidated) - writeToFileWithFS for create/write/close/verbose file operations - encodeJSONIndented for JSON encoding across 4 call sites - resolveColorThresholds for gauge color threshold logic (3 files) - Export DetectFormat for cross-package format detection reuse - Reuse NormalizeBaseURL instead of inline strings.TrimRight
1 parent a991395 commit 7dedb84

12 files changed

Lines changed: 122 additions & 328 deletions

File tree

internal/checks/github.go

Lines changed: 42 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"time"
1515

1616
"github.com/boinger/confvis/internal/confidence"
17+
"github.com/boinger/confvis/internal/sources/httpclient"
1718
)
1819

1920
const (
@@ -66,19 +67,13 @@ type GitHubClientConfig struct {
6667

6768
// NewGitHubClient creates a new GitHub Checks API client.
6869
func NewGitHubClient(cfg GitHubClientConfig) *GitHubClient {
69-
baseURL := cfg.BaseURL
70-
if baseURL == "" {
71-
baseURL = defaultGitHubAPIURL
72-
}
73-
baseURL = strings.TrimRight(baseURL, "/")
74-
7570
timeout := cfg.Timeout
7671
if timeout == 0 {
7772
timeout = defaultTimeout
7873
}
7974

8075
return &GitHubClient{
81-
baseURL: baseURL,
76+
baseURL: httpclient.NormalizeBaseURL(cfg.BaseURL, defaultGitHubAPIURL),
8277
token: cfg.Token,
8378
httpClient: &http.Client{Timeout: timeout},
8479
}
@@ -87,14 +82,8 @@ func NewGitHubClient(cfg GitHubClientConfig) *GitHubClient {
8782
// NewGitHubClientWithHTTP creates a new client with a custom HTTP client.
8883
// This is primarily intended for testing.
8984
func NewGitHubClientWithHTTP(cfg GitHubClientConfig, httpClient *http.Client) *GitHubClient {
90-
baseURL := cfg.BaseURL
91-
if baseURL == "" {
92-
baseURL = defaultGitHubAPIURL
93-
}
94-
baseURL = strings.TrimRight(baseURL, "/")
95-
9685
return &GitHubClient{
97-
baseURL: baseURL,
86+
baseURL: httpclient.NormalizeBaseURL(cfg.BaseURL, defaultGitHubAPIURL),
9887
token: cfg.Token,
9988
httpClient: httpClient,
10089
}
@@ -165,20 +154,20 @@ func (c *GitHubClient) CreateCheck(ctx context.Context, report *confidence.Repor
165154
return c.postCheckRun(ctx, endpoint, req)
166155
}
167156

168-
func (c *GitHubClient) postCheckRun(ctx context.Context, endpoint string, req CheckRunRequest) (*CheckRunResponse, error) {
169-
body, err := json.Marshal(req)
170-
if err != nil {
171-
return nil, fmt.Errorf(errMarshalingRequest, err)
172-
}
173-
174-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
157+
// doRequest executes an HTTP request with standard GitHub API headers.
158+
// It returns the response body bytes on success, or an error if the request
159+
// fails or the status code doesn't match expectedStatus.
160+
func (c *GitHubClient) doRequest(ctx context.Context, method, endpoint string, body io.Reader, expectedStatus int) ([]byte, error) {
161+
httpReq, err := http.NewRequestWithContext(ctx, method, endpoint, body)
175162
if err != nil {
176163
return nil, fmt.Errorf(errCreatingRequest, err)
177164
}
178165

179166
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
180167
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
181-
httpReq.Header.Set(headerContentType, contentTypeJSON)
168+
if body != nil {
169+
httpReq.Header.Set(headerContentType, contentTypeJSON)
170+
}
182171
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
183172

184173
resp, err := c.httpClient.Do(httpReq)
@@ -192,10 +181,28 @@ func (c *GitHubClient) postCheckRun(ctx context.Context, endpoint string, req Ch
192181
return nil, fmt.Errorf(errReadingResponse, err)
193182
}
194183

195-
if resp.StatusCode != http.StatusCreated {
184+
if resp.StatusCode != expectedStatus {
196185
return nil, fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
197186
}
198187

188+
return respBody, nil
189+
}
190+
191+
// marshalAndPost marshals a request body and sends it via doRequest.
192+
func (c *GitHubClient) marshalAndPost(ctx context.Context, method, endpoint string, reqBody any, expectedStatus int) ([]byte, error) {
193+
jsonBody, err := json.Marshal(reqBody)
194+
if err != nil {
195+
return nil, fmt.Errorf(errMarshalingRequest, err)
196+
}
197+
return c.doRequest(ctx, method, endpoint, bytes.NewReader(jsonBody), expectedStatus)
198+
}
199+
200+
func (c *GitHubClient) postCheckRun(ctx context.Context, endpoint string, req CheckRunRequest) (*CheckRunResponse, error) {
201+
respBody, err := c.marshalAndPost(ctx, http.MethodPost, endpoint, req, http.StatusCreated)
202+
if err != nil {
203+
return nil, err
204+
}
205+
199206
var result CheckRunResponse
200207
if err := json.Unmarshal(respBody, &result); err != nil {
201208
return nil, fmt.Errorf(errDecodingResponse, err)
@@ -305,28 +312,9 @@ func (c *GitHubClient) FindComment(ctx context.Context, opts CommentOptions) (*C
305312

306313
endpoint := fmt.Sprintf(issueCommentsEndpoint, c.baseURL, opts.Owner, opts.Repo, opts.PR)
307314

308-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
309-
if err != nil {
310-
return nil, fmt.Errorf(errCreatingRequest, err)
311-
}
312-
313-
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
314-
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
315-
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
316-
317-
resp, err := c.httpClient.Do(httpReq)
318-
if err != nil {
319-
return nil, fmt.Errorf(errMakingRequest, err)
320-
}
321-
defer func() { _ = resp.Body.Close() }()
322-
323-
respBody, err := io.ReadAll(resp.Body)
315+
respBody, err := c.doRequest(ctx, http.MethodGet, endpoint, nil, http.StatusOK)
324316
if err != nil {
325-
return nil, fmt.Errorf(errReadingResponse, err)
326-
}
327-
328-
if resp.StatusCode != http.StatusOK {
329-
return nil, fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
317+
return nil, err
330318
}
331319

332320
var comments issueCommentsResponse
@@ -358,38 +346,11 @@ func (c *GitHubClient) PostComment(ctx context.Context, opts CommentOptions, bod
358346

359347
endpoint := fmt.Sprintf(issueCommentsEndpoint, c.baseURL, opts.Owner, opts.Repo, opts.PR)
360348

361-
reqBody := struct {
349+
respBody, err := c.marshalAndPost(ctx, http.MethodPost, endpoint, struct {
362350
Body string `json:"body"`
363-
}{Body: body}
364-
365-
jsonBody, err := json.Marshal(reqBody)
366-
if err != nil {
367-
return nil, fmt.Errorf(errMarshalingRequest, err)
368-
}
369-
370-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(jsonBody))
371-
if err != nil {
372-
return nil, fmt.Errorf(errCreatingRequest, err)
373-
}
374-
375-
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
376-
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
377-
httpReq.Header.Set(headerContentType, contentTypeJSON)
378-
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
379-
380-
resp, err := c.httpClient.Do(httpReq)
381-
if err != nil {
382-
return nil, fmt.Errorf(errMakingRequest, err)
383-
}
384-
defer func() { _ = resp.Body.Close() }()
385-
386-
respBody, err := io.ReadAll(resp.Body)
351+
}{Body: body}, http.StatusCreated)
387352
if err != nil {
388-
return nil, fmt.Errorf(errReadingResponse, err)
389-
}
390-
391-
if resp.StatusCode != http.StatusCreated {
392-
return nil, fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
353+
return nil, err
393354
}
394355

395356
var result CommentResponse
@@ -408,38 +369,11 @@ func (c *GitHubClient) UpdateComment(ctx context.Context, opts CommentOptions, c
408369

409370
endpoint := fmt.Sprintf("%s/repos/%s/%s/issues/comments/%d", c.baseURL, opts.Owner, opts.Repo, commentID)
410371

411-
reqBody := struct {
372+
respBody, err := c.marshalAndPost(ctx, http.MethodPatch, endpoint, struct {
412373
Body string `json:"body"`
413-
}{Body: body}
414-
415-
jsonBody, err := json.Marshal(reqBody)
416-
if err != nil {
417-
return nil, fmt.Errorf(errMarshalingRequest, err)
418-
}
419-
420-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPatch, endpoint, bytes.NewReader(jsonBody))
374+
}{Body: body}, http.StatusOK)
421375
if err != nil {
422-
return nil, fmt.Errorf(errCreatingRequest, err)
423-
}
424-
425-
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
426-
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
427-
httpReq.Header.Set(headerContentType, contentTypeJSON)
428-
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
429-
430-
resp, err := c.httpClient.Do(httpReq)
431-
if err != nil {
432-
return nil, fmt.Errorf(errMakingRequest, err)
433-
}
434-
defer func() { _ = resp.Body.Close() }()
435-
436-
respBody, err := io.ReadAll(resp.Body)
437-
if err != nil {
438-
return nil, fmt.Errorf(errReadingResponse, err)
439-
}
440-
441-
if resp.StatusCode != http.StatusOK {
442-
return nil, fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
376+
return nil, err
443377
}
444378

445379
var result CommentResponse
@@ -458,27 +392,8 @@ func (c *GitHubClient) DeleteComment(ctx context.Context, opts CommentOptions, c
458392

459393
endpoint := fmt.Sprintf("%s/repos/%s/%s/issues/comments/%d", c.baseURL, opts.Owner, opts.Repo, commentID)
460394

461-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil)
462-
if err != nil {
463-
return fmt.Errorf(errCreatingRequest, err)
464-
}
465-
466-
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
467-
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
468-
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
469-
470-
resp, err := c.httpClient.Do(httpReq)
471-
if err != nil {
472-
return fmt.Errorf(errMakingRequest, err)
473-
}
474-
defer func() { _ = resp.Body.Close() }()
475-
476-
if resp.StatusCode != http.StatusNoContent {
477-
respBody, _ := io.ReadAll(resp.Body)
478-
return fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
479-
}
480-
481-
return nil
395+
_, err := c.doRequest(ctx, http.MethodDelete, endpoint, nil, http.StatusNoContent)
396+
return err
482397
}
483398

484399
// FindAllConfvisComments finds all confvis comments on a PR.
@@ -493,28 +408,9 @@ func (c *GitHubClient) FindAllConfvisComments(ctx context.Context, opts CommentO
493408

494409
endpoint := fmt.Sprintf(issueCommentsEndpoint, c.baseURL, opts.Owner, opts.Repo, opts.PR)
495410

496-
httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
411+
respBody, err := c.doRequest(ctx, http.MethodGet, endpoint, nil, http.StatusOK)
497412
if err != nil {
498-
return nil, fmt.Errorf(errCreatingRequest, err)
499-
}
500-
501-
httpReq.Header.Set(headerAuthorization, bearerPrefix+c.token)
502-
httpReq.Header.Set(headerAccept, acceptGitHubJSON)
503-
httpReq.Header.Set(headerGitHubAPIVersion, gitHubAPIVersion)
504-
505-
resp, err := c.httpClient.Do(httpReq)
506-
if err != nil {
507-
return nil, fmt.Errorf(errMakingRequest, err)
508-
}
509-
defer func() { _ = resp.Body.Close() }()
510-
511-
respBody, err := io.ReadAll(resp.Body)
512-
if err != nil {
513-
return nil, fmt.Errorf(errReadingResponse, err)
514-
}
515-
516-
if resp.StatusCode != http.StatusOK {
517-
return nil, fmt.Errorf(errAPIStatus, resp.StatusCode, string(respBody))
413+
return nil, err
518414
}
519415

520416
var comments issueCommentsResponse

0 commit comments

Comments
 (0)