Skip to content

Fix missing User-Agent header in Upload API client#42

Merged
alxgsv merged 1 commit into
masterfrom
phase7/upload-api-user-agent-v2
May 27, 2026
Merged

Fix missing User-Agent header in Upload API client#42
alxgsv merged 1 commit into
masterfrom
phase7/upload-api-user-agent-v2

Conversation

@alxgsv

@alxgsv alxgsv commented May 19, 2026

Copy link
Copy Markdown
Contributor
  • Adds User-Agent construction to the Upload API client with format: UploadcareGo/<version>/<public key>.
  • Appends custom Config.UserAgent values when provided via WithUserAgent.

Summary by CodeRabbit

  • New Features

    • Upload API client now automatically includes a User-Agent header in HTTP requests, constructed from client version information, public key, and optional configuration parameters.
  • Tests

    • Refactored test suite to use table-driven test cases for improved coverage and validation of User-Agent header behavior.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Walkthrough

The upload API client now computes and applies a User-Agent header during initialization. The client struct adds a userAgent field built from configuration prefix, client version, and public key credentials, with optional custom user agent suffix. HTTP requests set this computed value as the User-Agent header. Tests are refactored to table-driven format to validate the header computation for default and custom configurations.

Changes

User-Agent Header Support

Layer / File(s) Summary
User-Agent field and request header setup
ucare/uploadapi.go
The uploadAPIClient struct gains a userAgent field computed during client initialization from UserAgentPrefix, ClientVersion, PublicKey, and optional UserAgent config values. HTTP request creation sets this computed user agent value as the User-Agent header.
User-Agent test coverage
ucare/uploadapi_test.go
TestUploadAPIClient is refactored to use table-driven test cases that validate user agent behavior, including default composition and custom user agent override scenarios. Each case specifies its own configuration options and request validation logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix missing User-Agent header in Upload API client' directly and clearly describes the main change—adding User-Agent header support to the Upload API client, which matches the core objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch phase7/upload-api-user-agent-v2

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

@alxgsv alxgsv requested a review from ckcnik May 19, 2026 18:38
@alxgsv alxgsv marked this pull request as ready for review May 19, 2026 18:38

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ucare/uploadapi_test.go (1)

59-77: ⚡ Quick win

Strengthen User-Agent assertions to exact expected value.

These checks only validate substring presence, so malformed ordering/format could still pass. Assert the full expected header value per case.

Proposed test tightening
@@
 import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"sync/atomic"
 	"testing"
@@
 		{
 			test:     "user_agent",
@@
 			checkReq: func(r *http.Request) error {
-				if !strings.Contains(r.Header.Get("User-Agent"), config.UserAgentPrefix+"/") {
-					return errors.New("expected User-Agent to contain UploadcareGo/")
+				expected := fmt.Sprintf("%s/%s/%s", config.UserAgentPrefix, config.ClientVersion, testCreds().PublicKey)
+				if r.Header.Get("User-Agent") != expected {
+					return errors.New("unexpected User-Agent value")
 				}
 				return nil
 			},
 		},
@@
 			checkReq: func(r *http.Request) error {
-				userAgent := r.Header.Get("User-Agent")
-				if !strings.Contains(userAgent, config.UserAgentPrefix+"/") {
-					return errors.New("expected User-Agent to contain UploadcareGo/")
-				}
-				if !strings.Contains(userAgent, "MyApp/1.0") {
-					return errors.New("expected User-Agent to contain custom value")
+				expected := fmt.Sprintf("%s/%s/%s MyApp/1.0", config.UserAgentPrefix, config.ClientVersion, testCreds().PublicKey)
+				if r.Header.Get("User-Agent") != expected {
+					return errors.New("unexpected User-Agent value")
 				}
 				return nil
 			},
 		},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ucare/uploadapi_test.go` around lines 59 - 77, Replace the loose substring
checks in the checkReq funcs with exact equality assertions for the full
User-Agent header: in the "default_user_agent" case compute an explicit
expectedUserAgent (e.g. config.UserAgentPrefix + "/" + "<expected-version>") and
assert r.Header.Get("User-Agent") == expectedUserAgent; in the
"custom_user_agent" case compute expectedUserAgent as the default value plus the
custom suffix (e.g. config.UserAgentPrefix + "/" + "<expected-version>" + "
MyApp/1.0") and assert equality instead of using strings.Contains; update the
two checkReq closures to fail when the header does not exactly match the
constructed expectedUserAgent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ucare/uploadapi_test.go`:
- Around line 59-77: Replace the loose substring checks in the checkReq funcs
with exact equality assertions for the full User-Agent header: in the
"default_user_agent" case compute an explicit expectedUserAgent (e.g.
config.UserAgentPrefix + "/" + "<expected-version>") and assert
r.Header.Get("User-Agent") == expectedUserAgent; in the "custom_user_agent" case
compute expectedUserAgent as the default value plus the custom suffix (e.g.
config.UserAgentPrefix + "/" + "<expected-version>" + " MyApp/1.0") and assert
equality instead of using strings.Contains; update the two checkReq closures to
fail when the header does not exactly match the constructed expectedUserAgent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 66469446-0ab2-452a-97d5-9d41995b106c

📥 Commits

Reviewing files that changed from the base of the PR and between 6433c35 and 3d91588.

📒 Files selected for processing (2)
  • ucare/uploadapi.go
  • ucare/uploadapi_test.go

@alxgsv alxgsv merged commit fb27a89 into master May 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants