WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit 2a13eb9

Browse files
manusamanusa
authored
refactor(api): add parameter extraction utilities for tool handlers (#564)
* refactor(api): add parameter extraction utilities for tool handlers Add RequiredString, OptionalString, and OptionalBool helper functions to reduce code duplication across toolsets. OptionalString and OptionalBool accept default values for cleaner call sites. - Remove duplicate getRequiredString/getOptionalString/getOptionalBool from kubevirt create and lifecycle tools - Fix hardcoded enum values in lifecycle tool to use Action constants - Add comprehensive tests for new parameter utilities Signed-off-by: Marc Nuri <[email protected]> * refactor(api): add parameter extraction utilities for tool handlers (review comments) Signed-off-by: Marc Nuri <[email protected]> --------- Signed-off-by: Marc Nuri <[email protected]>
1 parent 5c6c1de commit 2a13eb9

File tree

4 files changed

+164
-78
lines changed

4 files changed

+164
-78
lines changed

‎pkg/api/params.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,49 @@ func ParseInt64(value interface{}) (int64, error) {
2626
return 0, &ErrInvalidInt64Type{Value: value}
2727
}
2828
}
29+
30+
// RequiredString extracts a required string parameter from tool arguments.
31+
// Returns the string value and nil error on success.
32+
// Returns an error if the parameter is missing or not a string.
33+
func RequiredString(params ToolHandlerParams, key string) (string, error) {
34+
args := params.GetArguments()
35+
val, ok := args[key]
36+
if !ok {
37+
return "", fmt.Errorf("%s parameter required", key)
38+
}
39+
str, ok := val.(string)
40+
if !ok {
41+
return "", fmt.Errorf("%s parameter must be a string", key)
42+
}
43+
return str, nil
44+
}
45+
46+
// OptionalString extracts an optional string parameter from tool arguments.
47+
// Returns the string value if present and valid, or defaultVal if missing or not a string.
48+
func OptionalString(params ToolHandlerParams, key, defaultVal string) string {
49+
args := params.GetArguments()
50+
val, ok := args[key]
51+
if !ok {
52+
return defaultVal
53+
}
54+
str, ok := val.(string)
55+
if !ok {
56+
return defaultVal
57+
}
58+
return str
59+
}
60+
61+
// OptionalBool extracts an optional boolean parameter from tool arguments.
62+
// Returns the boolean value if present and valid, or defaultVal if missing or not a boolean.
63+
func OptionalBool(params ToolHandlerParams, key string, defaultVal bool) bool {
64+
args := params.GetArguments()
65+
val, ok := args[key]
66+
if !ok {
67+
return defaultVal
68+
}
69+
b, ok := val.(bool)
70+
if !ok {
71+
return defaultVal
72+
}
73+
return b
74+
}

‎pkg/api/params_test.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,108 @@ func (s *ParamsSuite) TestErrInvalidInt64Type() {
103103
s.Equal("invalid", typeErr.Value)
104104
})
105105
}
106+
107+
// mockToolCallRequest implements ToolCallRequest for testing
108+
type mockToolCallRequest struct {
109+
args map[string]any
110+
}
111+
112+
func (m *mockToolCallRequest) GetArguments() map[string]any {
113+
return m.args
114+
}
115+
116+
func (s *ParamsSuite) TestRequiredString() {
117+
s.Run("returns string value when present", func() {
118+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": "test-value"}}}
119+
result, err := RequiredString(params, "name")
120+
s.NoError(err)
121+
s.Equal("test-value", result)
122+
})
123+
124+
s.Run("returns error when key is missing", func() {
125+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{}}}
126+
result, err := RequiredString(params, "name")
127+
s.Error(err)
128+
s.Equal("", result)
129+
s.Contains(err.Error(), "name parameter required")
130+
})
131+
132+
s.Run("returns error when value is not a string", func() {
133+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": 123}}}
134+
result, err := RequiredString(params, "name")
135+
s.Error(err)
136+
s.Equal("", result)
137+
s.Contains(err.Error(), "name parameter must be a string")
138+
})
139+
140+
s.Run("returns empty string when value is empty string", func() {
141+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": ""}}}
142+
result, err := RequiredString(params, "name")
143+
s.NoError(err)
144+
s.Equal("", result)
145+
})
146+
}
147+
148+
func (s *ParamsSuite) TestOptionalString() {
149+
s.Run("returns string value when present", func() {
150+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": "test-value"}}}
151+
result := OptionalString(params, "name", "default")
152+
s.Equal("test-value", result)
153+
})
154+
155+
s.Run("returns default when key is missing", func() {
156+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{}}}
157+
result := OptionalString(params, "name", "default-value")
158+
s.Equal("default-value", result)
159+
})
160+
161+
s.Run("returns default when value is not a string", func() {
162+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": 123}}}
163+
result := OptionalString(params, "name", "fallback")
164+
s.Equal("fallback", result)
165+
})
166+
167+
s.Run("returns empty string when value is empty string", func() {
168+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"name": ""}}}
169+
result := OptionalString(params, "name", "default")
170+
s.Equal("", result)
171+
})
172+
173+
s.Run("returns empty string when default is empty and key is missing", func() {
174+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{}}}
175+
result := OptionalString(params, "name", "")
176+
s.Equal("", result)
177+
})
178+
}
179+
180+
func (s *ParamsSuite) TestOptionalBool() {
181+
s.Run("returns true when value is true", func() {
182+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"enabled": true}}}
183+
result := OptionalBool(params, "enabled", false)
184+
s.True(result)
185+
})
186+
187+
s.Run("returns false when value is false", func() {
188+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"enabled": false}}}
189+
result := OptionalBool(params, "enabled", true)
190+
s.False(result)
191+
})
192+
193+
s.Run("returns default when key is missing", func() {
194+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{}}}
195+
result := OptionalBool(params, "enabled", true)
196+
s.True(result)
197+
})
198+
199+
s.Run("returns default when value is not a bool", func() {
200+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{"enabled": "true"}}}
201+
result := OptionalBool(params, "enabled", true)
202+
s.True(result)
203+
})
204+
205+
s.Run("returns false default when key is missing", func() {
206+
params := ToolHandlerParams{ToolCallRequest: &mockToolCallRequest{args: map[string]any{}}}
207+
result := OptionalBool(params, "enabled", false)
208+
s.False(result)
209+
})
210+
}

‎pkg/toolsets/kubevirt/vm/create/tool.go‎

Lines changed: 9 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -160,39 +160,26 @@ type createParameters struct {
160160

161161
// parseCreateParameters parses and validates input parameters
162162
func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, error) {
163-
namespace, err := getRequiredString(params, "namespace")
163+
namespace, err := api.RequiredString(params, "namespace")
164164
if err != nil {
165165
return nil, err
166166
}
167167

168-
name, err := getRequiredString(params, "name")
168+
name, err := api.RequiredString(params, "name")
169169
if err != nil {
170170
return nil, err
171171
}
172172

173-
workload := getOptionalString(params, "workload")
174-
if workload == "" {
175-
workload = "fedora" // Default to fedora if not specified
176-
}
177-
178-
performance := normalizePerformance(getOptionalString(params, "performance"))
179-
autostart := getOptionalBool(params, "autostart")
180-
181-
storage := getOptionalString(params, "storage")
182-
if storage == "" {
183-
storage = "30Gi" // Default storage size
184-
}
185-
186173
return &createParameters{
187174
Namespace: namespace,
188175
Name: name,
189-
Workload: workload,
190-
Instancetype: getOptionalString(params, "instancetype"),
191-
Preference: getOptionalString(params, "preference"),
192-
Size: getOptionalString(params, "size"),
193-
Performance: performance,
194-
Storage: storage,
195-
Autostart: autostart,
176+
Workload: api.OptionalString(params, "workload", "fedora"),
177+
Instancetype: api.OptionalString(params, "instancetype", ""),
178+
Preference: api.OptionalString(params, "preference", ""),
179+
Size: api.OptionalString(params, "size", ""),
180+
Performance: normalizePerformance(api.OptionalString(params, "performance", "")),
181+
Storage: api.OptionalString(params, "storage", "30Gi"),
182+
Autostart: api.OptionalBool(params, "autostart", false),
196183
}, nil
197184
}
198185

@@ -293,45 +280,6 @@ func normalizePerformance(performance string) string {
293280
return "u1"
294281
}
295282

296-
func getRequiredString(params api.ToolHandlerParams, key string) (string, error) {
297-
args := params.GetArguments()
298-
val, ok := args[key]
299-
if !ok {
300-
return "", fmt.Errorf("%s parameter required", key)
301-
}
302-
str, ok := val.(string)
303-
if !ok {
304-
return "", fmt.Errorf("%s parameter must be a string", key)
305-
}
306-
return str, nil
307-
}
308-
309-
func getOptionalString(params api.ToolHandlerParams, key string) string {
310-
args := params.GetArguments()
311-
val, ok := args[key]
312-
if !ok {
313-
return ""
314-
}
315-
str, ok := val.(string)
316-
if !ok {
317-
return ""
318-
}
319-
return str
320-
}
321-
322-
func getOptionalBool(params api.ToolHandlerParams, key string) bool {
323-
args := params.GetArguments()
324-
val, ok := args[key]
325-
if !ok {
326-
return false
327-
}
328-
b, ok := val.(bool)
329-
if !ok {
330-
return false
331-
}
332-
return b
333-
}
334-
335283
// resolveContainerDisk resolves OS names to container disk images from quay.io/containerdisks
336284
func resolveContainerDisk(input string) string {
337285
// If input already looks like a container image, return as-is

‎pkg/toolsets/kubevirt/vm/lifecycle/tool.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func Tools() []api.ServerTool {
3939
},
4040
"action": {
4141
Type: "string",
42-
Enum: []any{"start", "stop", "restart"},
42+
Enum: []any{string(ActionStart), string(ActionStop), string(ActionRestart)},
4343
Description: "The lifecycle action to perform: 'start' (changes runStrategy to Always), 'stop' (changes runStrategy to Halted), or 'restart' (stops then starts the VM)",
4444
},
4545
},
@@ -60,17 +60,17 @@ func Tools() []api.ServerTool {
6060

6161
func lifecycle(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
6262
// Parse input parameters
63-
namespace, err := getRequiredString(params, "namespace")
63+
namespace, err := api.RequiredString(params, "namespace")
6464
if err != nil {
6565
return api.NewToolCallResult("", err), nil
6666
}
6767

68-
name, err := getRequiredString(params, "name")
68+
name, err := api.RequiredString(params, "name")
6969
if err != nil {
7070
return api.NewToolCallResult("", err), nil
7171
}
7272

73-
action, err := getRequiredString(params, "action")
73+
action, err := api.RequiredString(params, "action")
7474
if err != nil {
7575
return api.NewToolCallResult("", err), nil
7676
}
@@ -124,16 +124,3 @@ func lifecycle(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
124124

125125
return api.NewToolCallResult(message+marshalledYaml, nil), nil
126126
}
127-
128-
func getRequiredString(params api.ToolHandlerParams, key string) (string, error) {
129-
args := params.GetArguments()
130-
val, ok := args[key]
131-
if !ok {
132-
return "", fmt.Errorf("%s parameter required", key)
133-
}
134-
str, ok := val.(string)
135-
if !ok {
136-
return "", fmt.Errorf("%s parameter must be a string", key)
137-
}
138-
return str, nil
139-
}

0 commit comments

Comments
 (0)