Skip to content

Commit bd35f40

Browse files
github-actions[bot]leggetterclaude
authored
fix: --rule-filter-headers parses JSON as object; --rule-retry-response-status-codes stores integers (#262)
* fix: parse JSON filter flags as objects and status codes as integers * test(acceptance): add end-to-end tests for filter flag JSON parsing (issue #192) * Claude/review fix pr 262 ubt6o (#265) * fix: address PR #262 review feedback - parseJSONOrString: only parse JSON objects/arrays (starting with { or [), not bare primitives like "order", 123, true that could be JQ expressions - Add HTTP status code range validation (100-599) for --rule-retry-response-status-codes - Fix UpsertRetryResponseStatusCodesAsArray acceptance test: status codes are now integers (float64 after JSON round-trip), not strings - Upgrade assert.True to require.True for map type assertions to prevent nil-pointer panics on failure - Update filter flag help text to mention JSON object support alongside JQ - Remove unused strings import from acceptance test https://claude.ai/code/session_01No6Vv4niFtYoF56at7uL2P * fix: remove incorrect JQ expression references from flag descriptions and tests The CLI does not support JQ expressions — filter flags accept JSON objects using Hookdeck filter syntax. Updated flag help text to match the listen command's wording and removed misleading JQ references from test names and comments. https://claude.ai/code/session_01No6Vv4niFtYoF56at7uL2P * test: verify exact JSON values in filter, retry, and transform tests Strengthen unit and acceptance tests to check full structure down to exact values instead of only checking types and key presence. Add coverage for --rule-filter-query, --rule-filter-path, combined filter flags, --rule-transform-env, and retry status code exact ordering. https://claude.ai/code/session_01No6Vv4niFtYoF56at7uL2P * test: add exact-value tests for source/destination --config and connection --rules Add unit and acceptance tests verifying that JSON input via --config, --config-file, --rules, and --rules-file on sources, destinations, and connections is parsed correctly and the API returns resources with the exact same structure and values. Covers inline JSON strings and file input for all three resource types. https://claude.ai/code/session_01No6Vv4niFtYoF56at7uL2P * fix: acceptance tests - response_status_codes as strings, filter rule assertions - Send response_status_codes as []string per API (RetryRule schema) - normalizeRulesForAPI: convert numeric codes to strings for --rules/--rules-file - assertFilterRuleFieldMatches: accept body/headers as string or map from API - assertResponseStatusCodesMatch: accept codes as string or number in responses - Update comments to avoid referencing local-only OpenAPI file Made-with: Cursor * test: expect response_status_codes as []string in unit tests (match API schema) Made-with: Cursor --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: leggetter <leggetter@users.noreply.github.com> Co-authored-by: Phil Leggetter <phil@leggetter.co.uk> Co-authored-by: Claude <noreply@anthropic.com>
1 parent d6b2ab8 commit bd35f40

12 files changed

+1716
-95
lines changed

‎pkg/cmd/connection_common.go‎

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"os"
7+
"strconv"
78
"strings"
89

910
"github.com/spf13/cobra"
@@ -76,10 +77,10 @@ func addConnectionRuleFlags(cmd *cobra.Command, f *connectionRuleFlags) {
7677
cmd.Flags().IntVar(&f.RuleRetryInterval, "rule-retry-interval", 0, "Interval between retries in milliseconds")
7778
cmd.Flags().StringVar(&f.RuleRetryResponseStatusCode, "rule-retry-response-status-codes", "", "Comma-separated HTTP status codes to retry on")
7879

79-
cmd.Flags().StringVar(&f.RuleFilterBody, "rule-filter-body", "", "JQ expression to filter on request body")
80-
cmd.Flags().StringVar(&f.RuleFilterHeaders, "rule-filter-headers", "", "JQ expression to filter on request headers")
81-
cmd.Flags().StringVar(&f.RuleFilterQuery, "rule-filter-query", "", "JQ expression to filter on request query parameters")
82-
cmd.Flags().StringVar(&f.RuleFilterPath, "rule-filter-path", "", "JQ expression to filter on request path")
80+
cmd.Flags().StringVar(&f.RuleFilterBody, "rule-filter-body", "", "Filter on request body using Hookdeck filter syntax (JSON)")
81+
cmd.Flags().StringVar(&f.RuleFilterHeaders, "rule-filter-headers", "", "Filter on request headers using Hookdeck filter syntax (JSON)")
82+
cmd.Flags().StringVar(&f.RuleFilterQuery, "rule-filter-query", "", "Filter on request query parameters using Hookdeck filter syntax (JSON)")
83+
cmd.Flags().StringVar(&f.RuleFilterPath, "rule-filter-path", "", "Filter on request path using Hookdeck filter syntax (JSON)")
8384

8485
cmd.Flags().StringVar(&f.RuleTransformName, "rule-transform-name", "", "Name or ID of the transformation to apply")
8586
cmd.Flags().StringVar(&f.RuleTransformCode, "rule-transform-code", "", "Transformation code (if creating inline)")
@@ -102,7 +103,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
102103
if err := json.Unmarshal([]byte(f.Rules), &rules); err != nil {
103104
return nil, fmt.Errorf("invalid JSON for --rules: %w", err)
104105
}
105-
return rules, nil
106+
return normalizeRulesForAPI(rules), nil
106107
}
107108

108109
if f.RulesFile != "" {
@@ -114,7 +115,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
114115
if err := json.Unmarshal(data, &rules); err != nil {
115116
return nil, fmt.Errorf("invalid JSON in rules file: %w", err)
116117
}
117-
return rules, nil
118+
return normalizeRulesForAPI(rules), nil
118119
}
119120

120121
// Build each rule type (order matches create: deduplicate -> transform -> filter -> delay -> retry)
@@ -158,16 +159,16 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
158159
if f.RuleFilterBody != "" || f.RuleFilterHeaders != "" || f.RuleFilterQuery != "" || f.RuleFilterPath != "" {
159160
rule := hookdeck.Rule{"type": "filter"}
160161
if f.RuleFilterBody != "" {
161-
rule["body"] = f.RuleFilterBody
162+
rule["body"] = parseJSONOrString(f.RuleFilterBody)
162163
}
163164
if f.RuleFilterHeaders != "" {
164-
rule["headers"] = f.RuleFilterHeaders
165+
rule["headers"] = parseJSONOrString(f.RuleFilterHeaders)
165166
}
166167
if f.RuleFilterQuery != "" {
167-
rule["query"] = f.RuleFilterQuery
168+
rule["query"] = parseJSONOrString(f.RuleFilterQuery)
168169
}
169170
if f.RuleFilterPath != "" {
170-
rule["path"] = f.RuleFilterPath
171+
rule["path"] = parseJSONOrString(f.RuleFilterPath)
171172
}
172173
rules = append(rules, rule)
173174
}
@@ -190,15 +191,78 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
190191
if f.RuleRetryInterval > 0 {
191192
rule["interval"] = f.RuleRetryInterval
192193
}
194+
// API expects response_status_codes as []string (RetryRule schema)
193195
if f.RuleRetryResponseStatusCode != "" {
194-
codes := strings.Split(f.RuleRetryResponseStatusCode, ",")
195-
for i := range codes {
196-
codes[i] = strings.TrimSpace(codes[i])
196+
parts := strings.Split(f.RuleRetryResponseStatusCode, ",")
197+
strCodes := make([]string, 0, len(parts))
198+
for _, part := range parts {
199+
part = strings.TrimSpace(part)
200+
if part == "" {
201+
continue
202+
}
203+
n, err := strconv.Atoi(part)
204+
if err != nil {
205+
return nil, fmt.Errorf("invalid HTTP status code %q in --rule-retry-response-status-codes: must be an integer", part)
206+
}
207+
if n < 100 || n > 599 {
208+
return nil, fmt.Errorf("invalid HTTP status code %d in --rule-retry-response-status-codes: must be between 100 and 599", n)
209+
}
210+
strCodes = append(strCodes, part)
197211
}
198-
rule["response_status_codes"] = codes
212+
rule["response_status_codes"] = strCodes
199213
}
200214
rules = append(rules, rule)
201215
}
202216

203217
return rules, nil
204218
}
219+
220+
// normalizeRulesForAPI ensures rules match the API schema: RetryRule.response_status_codes
221+
// must be []string; FilterRule body/headers may be string or object.
222+
func normalizeRulesForAPI(rules []hookdeck.Rule) []hookdeck.Rule {
223+
out := make([]hookdeck.Rule, len(rules))
224+
for i, r := range rules {
225+
out[i] = make(hookdeck.Rule)
226+
for k, v := range r {
227+
out[i][k] = v
228+
}
229+
if r["type"] == "retry" {
230+
if codes, ok := r["response_status_codes"].([]interface{}); ok && len(codes) > 0 {
231+
strCodes := make([]string, 0, len(codes))
232+
for _, c := range codes {
233+
switch v := c.(type) {
234+
case string:
235+
strCodes = append(strCodes, v)
236+
case float64:
237+
strCodes = append(strCodes, strconv.Itoa(int(v)))
238+
case int:
239+
strCodes = append(strCodes, strconv.Itoa(v))
240+
default:
241+
strCodes = append(strCodes, fmt.Sprintf("%v", c))
242+
}
243+
}
244+
out[i]["response_status_codes"] = strCodes
245+
}
246+
}
247+
}
248+
return out
249+
}
250+
251+
// parseJSONOrString attempts to parse s as a JSON object or array. Only values
252+
// starting with '{' or '[' (after trimming whitespace) are candidates for
253+
// parsing; bare primitives like "order", 123, or true are returned as-is so
254+
// that plain strings are never misinterpreted.
255+
func parseJSONOrString(s string) interface{} {
256+
trimmed := strings.TrimSpace(s)
257+
if len(trimmed) == 0 {
258+
return s
259+
}
260+
if trimmed[0] != '{' && trimmed[0] != '[' {
261+
return s
262+
}
263+
var v interface{}
264+
if err := json.Unmarshal([]byte(s), &v); err == nil {
265+
return v
266+
}
267+
return s
268+
}

‎pkg/cmd/connection_rules_json_test.go‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package cmd
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// TestBuildConnectionRulesFromJSONString verifies that --rules (JSON string) parses
13+
// into a rules slice with exact values and structure preserved.
14+
func TestBuildConnectionRulesFromJSONString(t *testing.T) {
15+
t.Run("filter rule JSON with exact nested values", func(t *testing.T) {
16+
input := `[{"type":"filter","headers":{"x-shopify-topic":{"$startsWith":"order/"}},"body":{"event_type":"payment"}}]`
17+
flags := connectionRuleFlags{Rules: input}
18+
rules, err := buildConnectionRules(&flags)
19+
require.NoError(t, err)
20+
require.Len(t, rules, 1)
21+
22+
rule := rules[0]
23+
assert.Equal(t, "filter", rule["type"])
24+
25+
headersMap, ok := rule["headers"].(map[string]interface{})
26+
require.True(t, ok, "headers should be a map, got %T", rule["headers"])
27+
28+
topicMap, ok := headersMap["x-shopify-topic"].(map[string]interface{})
29+
require.True(t, ok, "x-shopify-topic should be a nested map")
30+
assert.Equal(t, "order/", topicMap["$startsWith"])
31+
32+
bodyMap, ok := rule["body"].(map[string]interface{})
33+
require.True(t, ok, "body should be a map")
34+
assert.Equal(t, "payment", bodyMap["event_type"])
35+
})
36+
37+
t.Run("retry rule JSON with exact numeric values", func(t *testing.T) {
38+
input := `[{"type":"retry","strategy":"exponential","count":5,"interval":30000,"response_status_codes":[500,502,503]}]`
39+
flags := connectionRuleFlags{Rules: input}
40+
rules, err := buildConnectionRules(&flags)
41+
require.NoError(t, err)
42+
require.Len(t, rules, 1)
43+
44+
rule := rules[0]
45+
assert.Equal(t, "retry", rule["type"])
46+
assert.Equal(t, "exponential", rule["strategy"])
47+
assert.Equal(t, float64(5), rule["count"])
48+
assert.Equal(t, float64(30000), rule["interval"])
49+
50+
statusCodes, ok := rule["response_status_codes"].([]string)
51+
require.True(t, ok, "response_status_codes should be []string (API schema), got %T", rule["response_status_codes"])
52+
assert.Equal(t, []string{"500", "502", "503"}, statusCodes)
53+
})
54+
55+
t.Run("multiple rules JSON preserves all rules with exact values", func(t *testing.T) {
56+
input := `[{"type":"filter","headers":{"content-type":"application/json"}},{"type":"delay","delay":5000},{"type":"retry","strategy":"linear","count":3,"interval":10000}]`
57+
flags := connectionRuleFlags{Rules: input}
58+
rules, err := buildConnectionRules(&flags)
59+
require.NoError(t, err)
60+
require.Len(t, rules, 3)
61+
62+
// Filter rule
63+
assert.Equal(t, "filter", rules[0]["type"])
64+
headersMap, ok := rules[0]["headers"].(map[string]interface{})
65+
require.True(t, ok)
66+
assert.Equal(t, "application/json", headersMap["content-type"])
67+
68+
// Delay rule
69+
assert.Equal(t, "delay", rules[1]["type"])
70+
assert.Equal(t, float64(5000), rules[1]["delay"])
71+
72+
// Retry rule
73+
assert.Equal(t, "retry", rules[2]["type"])
74+
assert.Equal(t, "linear", rules[2]["strategy"])
75+
assert.Equal(t, float64(3), rules[2]["count"])
76+
assert.Equal(t, float64(10000), rules[2]["interval"])
77+
})
78+
79+
t.Run("transform rule JSON preserves transformation config", func(t *testing.T) {
80+
input := `[{"type":"transform","transformation":{"name":"my-transform","env":{"API_KEY":"sk-123","MODE":"production"}}}]`
81+
flags := connectionRuleFlags{Rules: input}
82+
rules, err := buildConnectionRules(&flags)
83+
require.NoError(t, err)
84+
require.Len(t, rules, 1)
85+
86+
rule := rules[0]
87+
assert.Equal(t, "transform", rule["type"])
88+
89+
transformation, ok := rule["transformation"].(map[string]interface{})
90+
require.True(t, ok, "transformation should be a map")
91+
assert.Equal(t, "my-transform", transformation["name"])
92+
93+
env, ok := transformation["env"].(map[string]interface{})
94+
require.True(t, ok, "env should be a map")
95+
assert.Equal(t, "sk-123", env["API_KEY"])
96+
assert.Equal(t, "production", env["MODE"])
97+
})
98+
99+
t.Run("invalid JSON returns error", func(t *testing.T) {
100+
flags := connectionRuleFlags{Rules: `[{broken`}
101+
_, err := buildConnectionRules(&flags)
102+
require.Error(t, err)
103+
assert.Contains(t, err.Error(), "--rules")
104+
})
105+
}
106+
107+
// TestBuildConnectionRulesFromJSONFile verifies that --rules-file reads a JSON file
108+
// and produces rules with exact values and structure preserved.
109+
func TestBuildConnectionRulesFromJSONFile(t *testing.T) {
110+
t.Run("file with filter and retry rules preserves exact values", func(t *testing.T) {
111+
content := `[{"type":"filter","headers":{"x-event-type":{"$eq":"order.created"}},"body":{"amount":{"$gte":100}}},{"type":"retry","strategy":"linear","count":3,"interval":5000,"response_status_codes":[500,502]}]`
112+
tmpFile := filepath.Join(t.TempDir(), "rules.json")
113+
require.NoError(t, os.WriteFile(tmpFile, []byte(content), 0644))
114+
115+
flags := connectionRuleFlags{RulesFile: tmpFile}
116+
rules, err := buildConnectionRules(&flags)
117+
require.NoError(t, err)
118+
require.Len(t, rules, 2)
119+
120+
// Filter rule with exact nested values
121+
filterRule := rules[0]
122+
assert.Equal(t, "filter", filterRule["type"])
123+
124+
headersMap, ok := filterRule["headers"].(map[string]interface{})
125+
require.True(t, ok)
126+
eventTypeMap, ok := headersMap["x-event-type"].(map[string]interface{})
127+
require.True(t, ok)
128+
assert.Equal(t, "order.created", eventTypeMap["$eq"])
129+
130+
bodyMap, ok := filterRule["body"].(map[string]interface{})
131+
require.True(t, ok)
132+
amountMap, ok := bodyMap["amount"].(map[string]interface{})
133+
require.True(t, ok)
134+
assert.Equal(t, float64(100), amountMap["$gte"])
135+
136+
// Retry rule with exact values
137+
retryRule := rules[1]
138+
assert.Equal(t, "retry", retryRule["type"])
139+
assert.Equal(t, "linear", retryRule["strategy"])
140+
assert.Equal(t, float64(3), retryRule["count"])
141+
assert.Equal(t, float64(5000), retryRule["interval"])
142+
143+
statusCodes, ok := retryRule["response_status_codes"].([]string)
144+
require.True(t, ok, "response_status_codes should be []string (API schema)")
145+
assert.Equal(t, []string{"500", "502"}, statusCodes)
146+
})
147+
148+
t.Run("file with deduplicate rule preserves fields", func(t *testing.T) {
149+
content := `[{"type":"deduplicate","window":3600,"include_fields":["id","timestamp"]}]`
150+
tmpFile := filepath.Join(t.TempDir(), "dedup-rules.json")
151+
require.NoError(t, os.WriteFile(tmpFile, []byte(content), 0644))
152+
153+
flags := connectionRuleFlags{RulesFile: tmpFile}
154+
rules, err := buildConnectionRules(&flags)
155+
require.NoError(t, err)
156+
require.Len(t, rules, 1)
157+
158+
rule := rules[0]
159+
assert.Equal(t, "deduplicate", rule["type"])
160+
assert.Equal(t, float64(3600), rule["window"])
161+
162+
fields, ok := rule["include_fields"].([]interface{})
163+
require.True(t, ok, "include_fields should be an array")
164+
assert.Equal(t, []interface{}{"id", "timestamp"}, fields)
165+
})
166+
167+
t.Run("nonexistent file returns error", func(t *testing.T) {
168+
flags := connectionRuleFlags{RulesFile: "/nonexistent/rules.json"}
169+
_, err := buildConnectionRules(&flags)
170+
require.Error(t, err)
171+
assert.Contains(t, err.Error(), "rules file")
172+
})
173+
174+
t.Run("invalid JSON file returns error", func(t *testing.T) {
175+
tmpFile := filepath.Join(t.TempDir(), "bad-rules.json")
176+
require.NoError(t, os.WriteFile(tmpFile, []byte(`[{invalid`), 0644))
177+
178+
flags := connectionRuleFlags{RulesFile: tmpFile}
179+
_, err := buildConnectionRules(&flags)
180+
require.Error(t, err)
181+
assert.Contains(t, err.Error(), "rules file")
182+
})
183+
}

0 commit comments

Comments
 (0)