mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
refactor: deduplicate utility helpers across the codebase (#23338)
Audited exported helpers in `coderd/util/*`, `testutil`, `cryptorand`, and friends, then replaced duplicated implementations with canonical versions. - **fix: `maps.SortedKeys` generic signature** — value type was hardcoded to `any`, making it impossible to actually call. Added second type parameter `V any`. Added table-driven tests with `cmp.Diff`. - **refactor: replace ad-hoc ptr helpers with `ptr.Ref`** — removed `int64Ptr`, `stringPtr`, `boolPtr`, `i64ptr`, `strPtr`, `PtrInt32` across 6 files. - **refactor: replace local `sortedKeys`/`sortKeys` with `maps.SortedKeys`** — now that the signature is fixed, scripts can use it. - **refactor: replace hand-rolled `capitalize` with `strings.Capitalize`** — the typegen version was also not UTF-8 safe. > 🤖 This PR was created with the help of Coder Agents, and was reviewed by my human. 🧑💻
This commit is contained in:
@@ -30,6 +30,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
"github.com/coder/coder/v2/coderd/database/pubsub"
|
||||
coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/coderd/webpush"
|
||||
"github.com/coder/coder/v2/coderd/workspacestats"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
@@ -2916,19 +2917,19 @@ func (p *Server) runChat(
|
||||
var usageForCost codersdk.ChatMessageUsage
|
||||
if hasUsage {
|
||||
if step.Usage.InputTokens != 0 {
|
||||
usageForCost.InputTokens = int64Ptr(step.Usage.InputTokens)
|
||||
usageForCost.InputTokens = ptr.Ref(step.Usage.InputTokens)
|
||||
}
|
||||
if step.Usage.OutputTokens != 0 {
|
||||
usageForCost.OutputTokens = int64Ptr(step.Usage.OutputTokens)
|
||||
usageForCost.OutputTokens = ptr.Ref(step.Usage.OutputTokens)
|
||||
}
|
||||
if step.Usage.ReasoningTokens != 0 {
|
||||
usageForCost.ReasoningTokens = int64Ptr(step.Usage.ReasoningTokens)
|
||||
usageForCost.ReasoningTokens = ptr.Ref(step.Usage.ReasoningTokens)
|
||||
}
|
||||
if step.Usage.CacheCreationTokens != 0 {
|
||||
usageForCost.CacheCreationTokens = int64Ptr(step.Usage.CacheCreationTokens)
|
||||
usageForCost.CacheCreationTokens = ptr.Ref(step.Usage.CacheCreationTokens)
|
||||
}
|
||||
if step.Usage.CacheReadTokens != 0 {
|
||||
usageForCost.CacheReadTokens = int64Ptr(step.Usage.CacheReadTokens)
|
||||
usageForCost.CacheReadTokens = ptr.Ref(step.Usage.CacheReadTokens)
|
||||
}
|
||||
}
|
||||
totalCostMicros := chatcost.CalculateTotalCostMicros(usageForCost, callConfig.Cost)
|
||||
@@ -3532,10 +3533,6 @@ func (p *Server) resolveModelConfig(
|
||||
return defaultConfig, nil
|
||||
}
|
||||
|
||||
func int64Ptr(value int64) *int64 {
|
||||
return &value
|
||||
}
|
||||
|
||||
func refreshChatWorkspaceSnapshot(
|
||||
ctx context.Context,
|
||||
chat database.Chat,
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/chatd/chatprovider"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
@@ -25,37 +26,37 @@ func TestReasoningEffortFromChat(t *testing.T) {
|
||||
{
|
||||
name: "OpenAICaseInsensitive",
|
||||
provider: "openai",
|
||||
input: stringPtr(" HIGH "),
|
||||
want: stringPtr(string(fantasyopenai.ReasoningEffortHigh)),
|
||||
input: ptr.Ref(" HIGH "),
|
||||
want: ptr.Ref(string(fantasyopenai.ReasoningEffortHigh)),
|
||||
},
|
||||
{
|
||||
name: "AnthropicEffort",
|
||||
provider: "anthropic",
|
||||
input: stringPtr("max"),
|
||||
want: stringPtr(string(fantasyanthropic.EffortMax)),
|
||||
input: ptr.Ref("max"),
|
||||
want: ptr.Ref(string(fantasyanthropic.EffortMax)),
|
||||
},
|
||||
{
|
||||
name: "OpenRouterEffort",
|
||||
provider: "openrouter",
|
||||
input: stringPtr("medium"),
|
||||
want: stringPtr(string(fantasyopenrouter.ReasoningEffortMedium)),
|
||||
input: ptr.Ref("medium"),
|
||||
want: ptr.Ref(string(fantasyopenrouter.ReasoningEffortMedium)),
|
||||
},
|
||||
{
|
||||
name: "VercelEffort",
|
||||
provider: "vercel",
|
||||
input: stringPtr("xhigh"),
|
||||
want: stringPtr(string(fantasyvercel.ReasoningEffortXHigh)),
|
||||
input: ptr.Ref("xhigh"),
|
||||
want: ptr.Ref(string(fantasyvercel.ReasoningEffortXHigh)),
|
||||
},
|
||||
{
|
||||
name: "InvalidEffortReturnsNil",
|
||||
provider: "openai",
|
||||
input: stringPtr("unknown"),
|
||||
input: ptr.Ref("unknown"),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "UnsupportedProviderReturnsNil",
|
||||
provider: "bedrock",
|
||||
input: stringPtr("high"),
|
||||
input: ptr.Ref("high"),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
@@ -83,7 +84,7 @@ func TestMergeMissingProviderOptions_OpenRouterNested(t *testing.T) {
|
||||
options := &codersdk.ChatModelProviderOptions{
|
||||
OpenRouter: &codersdk.ChatModelOpenRouterProviderOptions{
|
||||
Reasoning: &codersdk.ChatModelReasoningOptions{
|
||||
Enabled: boolPtr(true),
|
||||
Enabled: ptr.Ref(true),
|
||||
},
|
||||
Provider: &codersdk.ChatModelOpenRouterProvider{
|
||||
Order: []string{"openai"},
|
||||
@@ -93,21 +94,21 @@ func TestMergeMissingProviderOptions_OpenRouterNested(t *testing.T) {
|
||||
defaults := &codersdk.ChatModelProviderOptions{
|
||||
OpenRouter: &codersdk.ChatModelOpenRouterProviderOptions{
|
||||
Reasoning: &codersdk.ChatModelReasoningOptions{
|
||||
Enabled: boolPtr(false),
|
||||
Exclude: boolPtr(true),
|
||||
MaxTokens: int64Ptr(123),
|
||||
Effort: stringPtr("high"),
|
||||
Enabled: ptr.Ref(false),
|
||||
Exclude: ptr.Ref(true),
|
||||
MaxTokens: ptr.Ref[int64](123),
|
||||
Effort: ptr.Ref("high"),
|
||||
},
|
||||
IncludeUsage: boolPtr(true),
|
||||
IncludeUsage: ptr.Ref(true),
|
||||
Provider: &codersdk.ChatModelOpenRouterProvider{
|
||||
Order: []string{"anthropic"},
|
||||
AllowFallbacks: boolPtr(true),
|
||||
RequireParameters: boolPtr(false),
|
||||
DataCollection: stringPtr("allow"),
|
||||
AllowFallbacks: ptr.Ref(true),
|
||||
RequireParameters: ptr.Ref(false),
|
||||
DataCollection: ptr.Ref("allow"),
|
||||
Only: []string{"openai"},
|
||||
Ignore: []string{"foo"},
|
||||
Quantizations: []string{"int8"},
|
||||
Sort: stringPtr("latency"),
|
||||
Sort: ptr.Ref("latency"),
|
||||
},
|
||||
},
|
||||
}
|
||||
@@ -136,15 +137,3 @@ func TestMergeMissingProviderOptions_OpenRouterNested(t *testing.T) {
|
||||
require.Equal(t, []string{"int8"}, options.OpenRouter.Provider.Quantizations)
|
||||
require.Equal(t, "latency", *options.OpenRouter.Provider.Sort)
|
||||
}
|
||||
|
||||
func stringPtr(value string) *string {
|
||||
return &value
|
||||
}
|
||||
|
||||
func boolPtr(value bool) *bool {
|
||||
return &value
|
||||
}
|
||||
|
||||
func int64Ptr(value int64) *int64 {
|
||||
return &value
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/notifications"
|
||||
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
|
||||
"github.com/coder/coder/v2/coderd/promoauth"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/cryptorand"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
@@ -405,7 +406,7 @@ func TestUserOAuth2Github(t *testing.T) {
|
||||
AuthenticatedUser: func(ctx context.Context, _ *http.Client) (*github.User, error) {
|
||||
return &github.User{
|
||||
AvatarURL: github.String("/hello-world"),
|
||||
ID: i64ptr(1234),
|
||||
ID: ptr.Ref[int64](1234),
|
||||
Login: github.String("kyle"),
|
||||
Name: github.String("Kylium Carbonate"),
|
||||
}, nil
|
||||
@@ -473,7 +474,7 @@ func TestUserOAuth2Github(t *testing.T) {
|
||||
AuthenticatedUser: func(_ context.Context, _ *http.Client) (*github.User, error) {
|
||||
return &github.User{
|
||||
AvatarURL: github.String("/hello-world"),
|
||||
ID: i64ptr(1234),
|
||||
ID: ptr.Ref[int64](1234),
|
||||
Login: github.String("kyle"),
|
||||
Name: github.String(" " + strings.Repeat("a", 129) + " "),
|
||||
}, nil
|
||||
@@ -2525,10 +2526,6 @@ func oauth2Callback(t *testing.T, client *codersdk.Client, opts ...func(*http.Re
|
||||
return res
|
||||
}
|
||||
|
||||
func i64ptr(i int64) *int64 {
|
||||
return &i
|
||||
}
|
||||
|
||||
func authCookieValue(cookies []*http.Cookie) string {
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name == codersdk.SessionTokenCookie {
|
||||
|
||||
@@ -31,7 +31,7 @@ func Subset[T, U comparable](a, b map[T]U) bool {
|
||||
}
|
||||
|
||||
// SortedKeys returns the keys of m in sorted order.
|
||||
func SortedKeys[T constraints.Ordered](m map[T]any) (keys []T) {
|
||||
func SortedKeys[K constraints.Ordered, V any](m map[K]V) (keys []K) {
|
||||
for k := range m {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
|
||||
@@ -4,9 +4,53 @@ import (
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/util/maps"
|
||||
)
|
||||
|
||||
func TestSortedKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for idx, tc := range []struct {
|
||||
name string
|
||||
input map[string]int
|
||||
expected []string
|
||||
}{
|
||||
{
|
||||
name: "SortsAlphabetically",
|
||||
input: map[string]int{
|
||||
"banana": 1,
|
||||
"apple": 2,
|
||||
"cherry": 3,
|
||||
},
|
||||
expected: []string{"apple", "banana", "cherry"},
|
||||
},
|
||||
{
|
||||
name: "AlreadySorted",
|
||||
input: map[string]int{
|
||||
"alpha": 1,
|
||||
"mango": 2,
|
||||
"zebra": 3,
|
||||
},
|
||||
expected: []string{"alpha", "mango", "zebra"},
|
||||
},
|
||||
{
|
||||
name: "EmptyMap",
|
||||
input: map[string]int{},
|
||||
expected: nil,
|
||||
},
|
||||
} {
|
||||
t.Run("#"+strconv.Itoa(idx)+"_"+tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := maps.SortedKeys(tc.input)
|
||||
if diff := cmp.Diff(tc.expected, got); diff != "" {
|
||||
t.Fatalf("unexpected result (-want +got):\n%s", diff)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSubset(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user