Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pass empty Locations and PrivateLocations to backend [sc-23129] #125

Merged
merged 4 commits into from
Feb 3, 2025
Merged

fix: pass empty Locations and PrivateLocations to backend [sc-23129] #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d19fe3b
fix: remove omitempty from Locations and PrivateLocations
sorccu Jan 31, 2025
50161a0
fix: always send empty [] for locations (public/private) even when no…
sorccu Feb 3, 2025
a3c9e4c
chore: update tests, these need a complete overhaul though
sorccu Feb 3, 2025
4ec9a48
fix: keep using /v1/checks for Create after all as it behaves differe…
sorccu Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 48 additions & 67 deletions checkly.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,27 +66,9 @@ func (c *client) Create(
ctx context.Context,
check Check,
) (*Check, error) {
data, err := json.Marshal(check)
if err != nil {
return nil, err
}
status, res, err := c.apiCall(
ctx,
http.MethodPost,
withAutoAssignAlertsFlag("checks"),
data,
)
if err != nil {
return nil, err
}
if status != http.StatusCreated {
return nil, fmt.Errorf("unexpected response status %d: %q", status, res)
}
var result Check
if err = json.NewDecoder(strings.NewReader(res)).Decode(&result); err != nil {
return nil, fmt.Errorf("decoding error for data %s: %v", res, err)
}
return &result, nil
// There are differences between /v1/checks and /v1/checks/<type>. Keep
// using /v1/checks here for backwards compatibility reasons.
return c.createCheck(ctx, check, "checks")
}

// Update updates an existing check with the specified details. It returns the
Expand All @@ -98,28 +80,7 @@ func (c *client) Update(
ctx context.Context,
ID string, check Check,
) (*Check, error) {
data, err := json.Marshal(check)
if err != nil {
return nil, err
}
status, res, err := c.apiCall(
ctx,
http.MethodPut,
withAutoAssignAlertsFlag(fmt.Sprintf("checks/%s", ID)),
data,
)
if err != nil {
return nil, err
}
if status != http.StatusOK {
return nil, fmt.Errorf("unexpected response status %d: %q", status, res)
}
var result Check
err = json.NewDecoder(strings.NewReader(res)).Decode(&result)
if err != nil {
return nil, fmt.Errorf("decoding error for data %s: %v", res, err)
}
return &result, nil
return c.UpdateCheck(ctx, ID, check)
}

// Delete deletes the check with the specified ID.
Expand All @@ -130,19 +91,7 @@ func (c *client) Delete(
ctx context.Context,
ID string,
) error {
status, res, err := c.apiCall(
ctx,
http.MethodDelete,
fmt.Sprintf("checks/%s", ID),
nil,
)
if err != nil {
return err
}
if status != http.StatusNoContent {
return fmt.Errorf("unexpected response status %d: %q", status, res)
}
return nil
return c.DeleteCheck(ctx, ID)
}

// Get takes the ID of an existing check, and returns the check parameters, or
Expand Down Expand Up @@ -180,29 +129,37 @@ func (c *client) CreateCheck(
ctx context.Context,
check Check,
) (*Check, error) {
data, err := json.Marshal(check)
if err != nil {
return nil, err
}
var checkType string
var endpoint string
switch check.Type {
case "BROWSER":
checkType = "checks/browser"
endpoint = "checks/browser"
case "API":
checkType = "checks/api"
endpoint = "checks/api"
case "HEARTBEAT":
checkType = "checks/heartbeat"
endpoint = "checks/heartbeat"
case "MULTI_STEP":
checkType = "checks/multistep"
endpoint = "checks/multistep"
case "TCP":
return nil, fmt.Errorf("user error: use CreateTCPCheck to create TCP checks")
default:
return nil, fmt.Errorf("unknown check type: %s", checkType)
return nil, fmt.Errorf("unknown check type: %s", endpoint)
}
return c.createCheck(ctx, check, endpoint)
}

func (c *client) createCheck(
ctx context.Context,
check Check,
endpoint string,
) (*Check, error) {
data, err := json.Marshal(check)
if err != nil {
return nil, err
}
status, res, err := c.apiCall(
ctx,
http.MethodPost,
withAutoAssignAlertsFlag(checkType),
withAutoAssignAlertsFlag(endpoint),
data,
)
if err != nil {
Expand Down Expand Up @@ -278,6 +235,14 @@ func (c *client) UpdateCheck(
ctx context.Context,
ID string, check Check,
) (*Check, error) {
// A nil value for a list will cause the backend to not update the value.
// We must send empty lists instead.
if check.Locations == nil {
check.Locations = []string{}
}
if check.PrivateLocations == nil {
check.PrivateLocations = &[]string{}
}
data, err := json.Marshal(check)
if err != nil {
return nil, err
Expand Down Expand Up @@ -337,6 +302,14 @@ func (c *client) UpdateTCPCheck(
ID string,
check TCPCheck,
) (*TCPCheck, error) {
// A nil value for a list will cause the backend to not update the value.
// We must send empty lists instead.
if check.Locations == nil {
check.Locations = []string{}
}
if check.PrivateLocations == nil {
check.PrivateLocations = &[]string{}
}
// Unfortunately `checkType` is required for this endpoint, so sneak it in
// using an anonymous struct.
payload := struct {
Expand Down Expand Up @@ -532,6 +505,14 @@ func (c *client) UpdateGroup(
ID int64,
group Group,
) (*Group, error) {
// A nil value for a list will cause the backend to not update the value.
// We must send empty lists instead.
if group.Locations == nil {
group.Locations = []string{}
}
if group.PrivateLocations == nil {
group.PrivateLocations = &[]string{}
}
data, err := json.Marshal(group)
if err != nil {
return nil, err
Expand Down
42 changes: 23 additions & 19 deletions checkly_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ import (
var wantCheckID = "73d29e72-6540-4bb5-967e-e07fa2c9465e"

var wantCheck = checkly.Check{
Name: "test",
Type: checkly.TypeAPI,
Frequency: 10,
Activated: true,
Muted: false,
DoubleCheck: true,
ShouldFail: false,
Locations: []string{"eu-west-1"},
Name: "test",
Type: checkly.TypeAPI,
Frequency: 10,
Activated: true,
Muted: false,
DoubleCheck: true,
ShouldFail: false,
Locations: []string{"eu-west-1"},
PrivateLocations: &[]string{},
Request: checkly.Request{
Method: http.MethodGet,
URL: "https://example.com",
Expand Down Expand Up @@ -161,14 +162,16 @@ func TestAPIError(t *testing.T) {
t.Parallel()
ts := cannedResponseServer(t,
http.MethodPost,
"/v1/checks?autoAssignAlerts=false",
"/v1/checks/api?autoAssignAlerts=false",
validateAnything,
http.StatusBadRequest,
"BadRequest.json",
)
defer ts.Close()
client := checkly.NewClient(ts.URL, "dummy-key", ts.Client(), nil)
_, err := client.Create(context.Background(), checkly.Check{})
_, err := client.CreateCheck(context.Background(), checkly.Check{
Type: checkly.TypeAPI,
})
if err == nil {
t.Error("want error when API returns 'bad request' status, got nil")
}
Expand Down Expand Up @@ -334,12 +337,13 @@ func TestDeleteCheck(t *testing.T) {
var wantGroupID int64 = 135

var wantGroup = checkly.Group{
Name: "test",
Activated: true,
Muted: false,
Tags: []string{"auto"},
Locations: []string{"eu-west-1"},
Concurrency: 3,
Name: "test",
Activated: true,
Muted: false,
Tags: []string{"auto"},
Locations: []string{"eu-west-1"},
PrivateLocations: &[]string{},
Concurrency: 3,
APICheckDefaults: checkly.APICheckDefaults{
BaseURL: "example.com/api/test",
Headers: []checkly.KeyValue{
Expand Down Expand Up @@ -429,7 +433,7 @@ func TestCreateGroup(t *testing.T) {
if err != nil {
t.Error(err)
}
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions")
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions", "PrivateLocations")
if !cmp.Equal(wantGroup, *gotGroup, ignored) {
t.Error(cmp.Diff(wantGroup, *gotGroup, ignoreGroupFields))
}
Expand All @@ -450,7 +454,7 @@ func TestGetGroup(t *testing.T) {
if err != nil {
t.Error(err)
}
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions")
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions", "PrivateLocations")
if !cmp.Equal(wantGroup, *gotGroup, ignored) {
t.Error(cmp.Diff(wantGroup, *gotGroup, ignored))
}
Expand All @@ -471,7 +475,7 @@ func TestUpdateGroup(t *testing.T) {
if err != nil {
t.Error(err)
}
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions")
ignored := cmpopts.IgnoreFields(checkly.Group{}, "ID", "AlertChannelSubscriptions", "PrivateLocations")
if !cmp.Equal(wantGroup, *gotGroup, ignored) {
t.Error(cmp.Diff(wantGroup, *gotGroup, ignoreGroupFields))
}
Expand Down
8 changes: 4 additions & 4 deletions types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ type Check struct {
Muted bool `json:"muted"`
ShouldFail bool `json:"shouldFail"`
RunParallel bool `json:"runParallel"`
Locations []string `json:"locations,omitempty"`
Locations []string `json:"locations"`
DegradedResponseTime int `json:"degradedResponseTime"`
MaxResponseTime int `json:"maxResponseTime"`
Script string `json:"script,omitempty"`
Expand Down Expand Up @@ -517,7 +517,7 @@ type MultiStepCheck struct {
Muted bool `json:"muted"`
ShouldFail bool `json:"shouldFail"`
RunParallel bool `json:"runParallel"`
Locations []string `json:"locations,omitempty"`
Locations []string `json:"locations"`
Script string `json:"script,omitempty"`
EnvironmentVariables []EnvironmentVariable `json:"environmentVariables"`
Tags []string `json:"tags,omitempty"`
Expand Down Expand Up @@ -559,7 +559,7 @@ type TCPCheck struct {
Muted bool `json:"muted"`
ShouldFail bool `json:"shouldFail"`
RunParallel bool `json:"runParallel"`
Locations []string `json:"locations,omitempty"`
Locations []string `json:"locations"`
DegradedResponseTime int `json:"degradedResponseTime,omitempty"`
MaxResponseTime int `json:"maxResponseTime,omitempty"`
Tags []string `json:"tags,omitempty"`
Expand All @@ -569,7 +569,7 @@ type TCPCheck struct {
GroupID int64 `json:"groupId,omitempty"`
GroupOrder int `json:"groupOrder,omitempty"`
AlertChannelSubscriptions []AlertChannelSubscription `json:"alertChannelSubscriptions,omitempty"`
PrivateLocations *[]string `json:"privateLocations,omitempty"`
PrivateLocations *[]string `json:"privateLocations"`
RuntimeID *string `json:"runtimeId"`
RetryStrategy *RetryStrategy `json:"retryStrategy,omitempty"`
CreatedAt time.Time `json:"created_at,omitempty"`
Expand Down