|
| 1 | +package cmd |
| 2 | + |
| 3 | +import ( |
| 4 | + "encoding/json" |
| 5 | + "testing" |
| 6 | + |
| 7 | + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" |
| 8 | + "github.com/stretchr/testify/assert" |
| 9 | + "github.com/stretchr/testify/require" |
| 10 | +) |
| 11 | + |
| 12 | +func strPtr(s string) *string { |
| 13 | + return &s |
| 14 | +} |
| 15 | + |
| 16 | +// TestBuildConnectionRulesRetryStatusCodesArray verifies that buildConnectionRules |
| 17 | +// produces response_status_codes as a []string array, not a single string. |
| 18 | +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 3. |
| 19 | +func TestBuildConnectionRulesRetryStatusCodesArray(t *testing.T) { |
| 20 | + tests := []struct { |
| 21 | + name string |
| 22 | + flags connectionRuleFlags |
| 23 | + wantCodes []string |
| 24 | + wantCodeCount int |
| 25 | + wantRuleCount int |
| 26 | + }{ |
| 27 | + { |
| 28 | + name: "comma-separated status codes should produce array", |
| 29 | + flags: connectionRuleFlags{ |
| 30 | + RuleRetryStrategy: "linear", |
| 31 | + RuleRetryCount: 3, |
| 32 | + RuleRetryInterval: 5000, |
| 33 | + RuleRetryResponseStatusCode: "500,502,503,504", |
| 34 | + }, |
| 35 | + wantCodes: []string{"500", "502", "503", "504"}, |
| 36 | + wantCodeCount: 4, |
| 37 | + wantRuleCount: 1, |
| 38 | + }, |
| 39 | + { |
| 40 | + name: "single status code should produce single-element array", |
| 41 | + flags: connectionRuleFlags{ |
| 42 | + RuleRetryStrategy: "exponential", |
| 43 | + RuleRetryResponseStatusCode: "500", |
| 44 | + }, |
| 45 | + wantCodes: []string{"500"}, |
| 46 | + wantCodeCount: 1, |
| 47 | + wantRuleCount: 1, |
| 48 | + }, |
| 49 | + { |
| 50 | + name: "status codes with spaces should be trimmed", |
| 51 | + flags: connectionRuleFlags{ |
| 52 | + RuleRetryStrategy: "linear", |
| 53 | + RuleRetryResponseStatusCode: "500, 502, 503", |
| 54 | + }, |
| 55 | + wantCodes: []string{"500", "502", "503"}, |
| 56 | + wantCodeCount: 3, |
| 57 | + wantRuleCount: 1, |
| 58 | + }, |
| 59 | + { |
| 60 | + name: "no status codes should not include response_status_codes", |
| 61 | + flags: connectionRuleFlags{ |
| 62 | + RuleRetryStrategy: "linear", |
| 63 | + RuleRetryCount: 3, |
| 64 | + }, |
| 65 | + wantCodes: nil, |
| 66 | + wantCodeCount: 0, |
| 67 | + wantRuleCount: 1, |
| 68 | + }, |
| 69 | + } |
| 70 | + |
| 71 | + for _, tt := range tests { |
| 72 | + t.Run(tt.name, func(t *testing.T) { |
| 73 | + rules, err := buildConnectionRules(&tt.flags) |
| 74 | + require.NoError(t, err, "buildConnectionRules should not error") |
| 75 | + require.Len(t, rules, tt.wantRuleCount, "Expected %d rule(s)", tt.wantRuleCount) |
| 76 | + |
| 77 | + if tt.wantRuleCount == 0 { |
| 78 | + return |
| 79 | + } |
| 80 | + |
| 81 | + retryRule := rules[len(rules)-1] |
| 82 | + assert.Equal(t, "retry", retryRule["type"], "Last rule should be retry") |
| 83 | + |
| 84 | + if tt.wantCodes == nil { |
| 85 | + _, exists := retryRule["response_status_codes"] |
| 86 | + assert.False(t, exists, "response_status_codes should not be present when not specified") |
| 87 | + return |
| 88 | + } |
| 89 | + |
| 90 | + statusCodes, ok := retryRule["response_status_codes"] |
| 91 | + require.True(t, ok, "response_status_codes should be present") |
| 92 | + |
| 93 | + codesSlice, ok := statusCodes.([]string) |
| 94 | + require.True(t, ok, "response_status_codes should be []string, got %T", statusCodes) |
| 95 | + assert.Equal(t, tt.wantCodeCount, len(codesSlice)) |
| 96 | + assert.Equal(t, tt.wantCodes, codesSlice) |
| 97 | + |
| 98 | + // Verify it serializes to a JSON array |
| 99 | + jsonBytes, err := json.Marshal(retryRule) |
| 100 | + require.NoError(t, err) |
| 101 | + |
| 102 | + var parsed map[string]interface{} |
| 103 | + err = json.Unmarshal(jsonBytes, &parsed) |
| 104 | + require.NoError(t, err) |
| 105 | + |
| 106 | + jsonCodes, ok := parsed["response_status_codes"].([]interface{}) |
| 107 | + require.True(t, ok, "JSON response_status_codes should be an array, got %T", parsed["response_status_codes"]) |
| 108 | + assert.Len(t, jsonCodes, tt.wantCodeCount) |
| 109 | + }) |
| 110 | + } |
| 111 | +} |
| 112 | + |
| 113 | +// TestUpsertBuildRequestRulesOnlyPreservesDestinationByID verifies that when |
| 114 | +// upserting with only rule flags, the request uses destination_id (not a full |
| 115 | +// destination object that could include incomplete auth config). |
| 116 | +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 1. |
| 117 | +func TestUpsertBuildRequestRulesOnlyPreservesDestinationByID(t *testing.T) { |
| 118 | + cu := &connectionUpsertCmd{ |
| 119 | + connectionCreateCmd: &connectionCreateCmd{}, |
| 120 | + } |
| 121 | + cu.name = "test-conn" |
| 122 | + // Only set rule flags, no source/destination flags |
| 123 | + cu.RuleRetryStrategy = "linear" |
| 124 | + cu.RuleRetryCount = 3 |
| 125 | + |
| 126 | + existing := &hookdeck.Connection{ |
| 127 | + ID: "conn_123", |
| 128 | + Name: strPtr("test-conn"), |
| 129 | + Source: &hookdeck.Source{ |
| 130 | + ID: "src_123", |
| 131 | + Name: "test-source", |
| 132 | + Type: "WEBHOOK", |
| 133 | + }, |
| 134 | + Destination: &hookdeck.Destination{ |
| 135 | + ID: "dst_123", |
| 136 | + Name: "test-dest", |
| 137 | + Type: "HTTP", |
| 138 | + Config: map[string]interface{}{ |
| 139 | + "url": "https://api.example.com", |
| 140 | + "auth_type": "AWS_SIGNATURE", |
| 141 | + }, |
| 142 | + }, |
| 143 | + } |
| 144 | + |
| 145 | + req, err := cu.buildUpsertRequest(existing, true) |
| 146 | + require.NoError(t, err) |
| 147 | + |
| 148 | + // Should reference existing destination by ID, not recreate it |
| 149 | + assert.NotNil(t, req.DestinationID, "Should use DestinationID") |
| 150 | + assert.Equal(t, "dst_123", *req.DestinationID) |
| 151 | + assert.Nil(t, req.Destination, "Should NOT send full Destination object") |
| 152 | + |
| 153 | + // Source should also be preserved by ID |
| 154 | + assert.NotNil(t, req.SourceID, "Should use SourceID") |
| 155 | + assert.Equal(t, "src_123", *req.SourceID) |
| 156 | + assert.Nil(t, req.Source, "Should NOT send full Source object") |
| 157 | + |
| 158 | + // Rules should be present |
| 159 | + assert.NotEmpty(t, req.Rules) |
| 160 | +} |
| 161 | + |
| 162 | +// TestUpsertHasAnyDestinationFlagIgnoresDefault verifies that hasAnyDestinationFlag |
| 163 | +// returns false when no destination flags are explicitly set (the cli-path default |
| 164 | +// of "/" was previously causing this to always return true). |
| 165 | +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 1. |
| 166 | +func TestUpsertHasAnyDestinationFlagIgnoresDefault(t *testing.T) { |
| 167 | + cu := &connectionUpsertCmd{ |
| 168 | + connectionCreateCmd: &connectionCreateCmd{}, |
| 169 | + } |
| 170 | + // No destination flags set at all (cli-path default is "" for upsert) |
| 171 | + |
| 172 | + result := cu.hasAnyDestinationFlag() |
| 173 | + assert.False(t, result, "hasAnyDestinationFlag should be false with no destination flags set") |
| 174 | +} |
| 175 | + |
| 176 | +// TestUpsertValidateSourceFlagsAllowsNameOnly verifies that validateSourceFlags |
| 177 | +// allows --source-name without --source-type for the upsert command. |
| 178 | +// Regression test for https://github.com/hookdeck/hookdeck-cli/issues/209 Bug 2. |
| 179 | +func TestUpsertValidateSourceFlagsAllowsNameOnly(t *testing.T) { |
| 180 | + cu := &connectionUpsertCmd{ |
| 181 | + connectionCreateCmd: &connectionCreateCmd{}, |
| 182 | + } |
| 183 | + cu.sourceName = "my-source" |
| 184 | + // sourceType intentionally empty |
| 185 | + |
| 186 | + err := cu.validateSourceFlags() |
| 187 | + assert.NoError(t, err, "validateSourceFlags should allow --source-name alone for upsert") |
| 188 | +} |
| 189 | + |
| 190 | +// TestUpsertValidateDestinationFlagsAllowsNameOnly verifies the same relaxation |
| 191 | +// for destination flags. |
| 192 | +func TestUpsertValidateDestinationFlagsAllowsNameOnly(t *testing.T) { |
| 193 | + cu := &connectionUpsertCmd{ |
| 194 | + connectionCreateCmd: &connectionCreateCmd{}, |
| 195 | + } |
| 196 | + cu.destinationName = "my-dest" |
| 197 | + // destinationType intentionally empty |
| 198 | + |
| 199 | + err := cu.validateDestinationFlags() |
| 200 | + assert.NoError(t, err, "validateDestinationFlags should allow --destination-name alone for upsert") |
| 201 | +} |
| 202 | + |
| 203 | +// TestUpsertBuildRequestFillsSourceTypeFromExisting verifies that when |
| 204 | +// --source-name is provided without --source-type during an update, |
| 205 | +// the existing source type is used. |
| 206 | +func TestUpsertBuildRequestFillsSourceTypeFromExisting(t *testing.T) { |
| 207 | + cu := &connectionUpsertCmd{ |
| 208 | + connectionCreateCmd: &connectionCreateCmd{}, |
| 209 | + } |
| 210 | + cu.name = "test-conn" |
| 211 | + cu.sourceName = "new-source-name" |
| 212 | + // sourceType intentionally empty - should be filled from existing |
| 213 | + |
| 214 | + existing := &hookdeck.Connection{ |
| 215 | + ID: "conn_123", |
| 216 | + Name: strPtr("test-conn"), |
| 217 | + Source: &hookdeck.Source{ |
| 218 | + ID: "src_123", |
| 219 | + Name: "old-source-name", |
| 220 | + Type: "WEBHOOK", |
| 221 | + }, |
| 222 | + Destination: &hookdeck.Destination{ |
| 223 | + ID: "dst_123", |
| 224 | + Name: "test-dest", |
| 225 | + Type: "HTTP", |
| 226 | + Config: map[string]interface{}{ |
| 227 | + "url": "https://api.example.com", |
| 228 | + }, |
| 229 | + }, |
| 230 | + } |
| 231 | + |
| 232 | + req, err := cu.buildUpsertRequest(existing, true) |
| 233 | + require.NoError(t, err) |
| 234 | + |
| 235 | + // Source should be an inline input (not just ID) since name was changed |
| 236 | + require.NotNil(t, req.Source, "Should have Source input for name change") |
| 237 | + assert.Equal(t, "new-source-name", req.Source.Name) |
| 238 | + assert.Equal(t, "WEBHOOK", req.Source.Type, "Should fill type from existing source") |
| 239 | +} |
0 commit comments