From df5dcfad96f1cd998413570be243c9e029e5397f Mon Sep 17 00:00:00 2001 From: Philip Wernersbach Date: Tue, 4 Feb 2025 20:28:16 -0500 Subject: [PATCH] fix: Ignore ScheduledAction's omitempty when SupportHours is specified Fixes https://github.com/PagerDuty/go-pagerduty/issues/468. When `Service.SupportHours` is specified, `Service.ScheduledActions` is required, otherwise the PagerDuty API returns the following error: ``` HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001): Scheduled actions is required. ``` This implements a custom `MarshalJSON` function for `Service`, which ignores `ScheduledActions`'s `omitempty` when `SupportHours` is specified, and enforces it otherwise. --- service.go | 82 +++++++++++++++++++++++++++++++++++++++ service_test.go | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/service.go b/service.go index 497b4c1d..97c1538f 100644 --- a/service.go +++ b/service.go @@ -2,6 +2,7 @@ package pagerduty import ( "context" + "encoding/json" "fmt" "net/http" @@ -76,6 +77,7 @@ type ServiceRuleActions struct { } // Service represents something you monitor (like a web service, email service, or database service). +// Do not update this struct without also updating serviceToMarshal! type Service struct { APIObject Name string `json:"name,omitempty"` @@ -100,6 +102,86 @@ type Service struct { AutoPauseNotificationsParameters *AutoPauseNotificationsParameters `json:"auto_pause_notifications_parameters,omitempty"` } +// A Service that is going to be marshaled. +// Do not update this struct without also updating Service! +type serviceToMarshal struct { + APIObject + Name string `json:"name,omitempty"` + Description string `json:"description,omitempty"` + AutoResolveTimeout *uint `json:"auto_resolve_timeout,omitempty"` + AcknowledgementTimeout *uint `json:"acknowledgement_timeout,omitempty"` + CreateAt string `json:"created_at,omitempty"` + Status string `json:"status,omitempty"` + LastIncidentTimestamp string `json:"last_incident_timestamp,omitempty"` + Integrations []Integration `json:"integrations,omitempty"` + EscalationPolicy EscalationPolicy `json:"escalation_policy,omitempty"` + Teams []Team `json:"teams,omitempty"` + IncidentUrgencyRule *IncidentUrgencyRule `json:"incident_urgency_rule,omitempty"` + SupportHours *SupportHours `json:"support_hours,omitempty"` + // Because of the logic in MarshalJSON, ScheduledActions cannot have omitempty. + ScheduledActions []ScheduledAction `json:"scheduled_actions"` + AlertCreation string `json:"alert_creation,omitempty"` + AlertGrouping string `json:"alert_grouping,omitempty"` + AlertGroupingTimeout *uint `json:"alert_grouping_timeout,omitempty"` + AlertGroupingParameters *AlertGroupingParameters `json:"alert_grouping_parameters,omitempty"` + ResponsePlay *APIObject `json:"response_play,omitempty"` + Addons []Addon `json:"addons,omitempty"` + AutoPauseNotificationsParameters *AutoPauseNotificationsParameters `json:"auto_pause_notifications_parameters,omitempty"` +} + +func (s Service) MarshalJSON() ([]byte, error) { + // Marshal and unmarshal the Service so that we can get the data that would be written to the wire. + sm := serviceToMarshal{ + APIObject: s.APIObject, + Name: s.Name, + Description: s.Description, + AutoResolveTimeout: s.AutoResolveTimeout, + AcknowledgementTimeout: s.AcknowledgementTimeout, + CreateAt: s.CreateAt, + Status: s.Status, + LastIncidentTimestamp: s.LastIncidentTimestamp, + Integrations: s.Integrations, + EscalationPolicy: s.EscalationPolicy, + Teams: s.Teams, + IncidentUrgencyRule: s.IncidentUrgencyRule, + SupportHours: s.SupportHours, + ScheduledActions: s.ScheduledActions, + AlertCreation: s.AlertCreation, + AlertGrouping: s.AlertGrouping, + AlertGroupingTimeout: s.AlertGroupingTimeout, + AlertGroupingParameters: s.AlertGroupingParameters, + ResponsePlay: s.ResponsePlay, + Addons: s.Addons, + AutoPauseNotificationsParameters: s.AutoPauseNotificationsParameters, + } + + sb, err := json.Marshal(sm) + if err != nil { + return nil, err + } + + sd := map[string]interface{}{} + err = json.Unmarshal(sb, &sd) + if err != nil { + return nil, err + } + + // If support_hours is specified, scheduled_actions also has to be specified. + if sd["support_hours"] != nil { + if sd["scheduled_actions"] == nil { + // This marshalls to JSON's "null". + sd["scheduled_actions"] = interface{}(nil) + } + // Otherwise, enforce Service.ScheduledActions's omitempty behavior. + } else if sd["scheduled_actions"] == nil { + delete(sd, "scheduled_actions") + } else if v, ok := sd["scheduled_actions"].([]interface{}); ok && len(v) == 0 { + delete(sd, "scheduled_actions") + } + + return json.Marshal(sd) +} + // AutoPauseNotificationsParameters defines how alerts on the service will be automatically paused type AutoPauseNotificationsParameters struct { Enabled bool `json:"enabled"` diff --git a/service_test.go b/service_test.go index 79d19922..fd6bfdb4 100644 --- a/service_test.go +++ b/service_test.go @@ -3,6 +3,7 @@ package pagerduty import ( "context" "fmt" + "io" "net/http" "strconv" "testing" @@ -141,12 +142,54 @@ func TestService_Create(t *testing.T) { mux.HandleFunc("/services", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "POST") + + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + testEqual(t, nil, err) + testEqual(t, []uint8(`{"service":{"escalation_policy":{"teams":null},"name":"foo"}}`), body) + + _, _ = w.Write([]byte(`{"service": {"id": "1","name":"foo"}}`)) + }) + + client := defaultTestClient(server.URL, "foo") + input := Service{ + Name: "foo", + } + res, err := client.CreateService(input) + + want := &Service{ + APIObject: APIObject{ + ID: "1", + }, + Name: "foo", + } + + if err != nil { + t.Fatal(err) + } + testEqual(t, want, res) +} + +// Create Service with empty ScheduledActions +func TestService_CreateWithEmptyScheduledActions(t *testing.T) { + setup() + defer teardown() + + mux.HandleFunc("/services", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + testEqual(t, nil, err) + testEqual(t, []uint8(`{"service":{"escalation_policy":{"teams":null},"name":"foo"}}`), body) + _, _ = w.Write([]byte(`{"service": {"id": "1","name":"foo"}}`)) }) client := defaultTestClient(server.URL, "foo") input := Service{ Name: "foo", + ScheduledActions: []ScheduledAction{}, } res, err := client.CreateService(input) @@ -163,6 +206,63 @@ func TestService_Create(t *testing.T) { testEqual(t, want, res) } +// Create Service with SupportHours +func TestService_CreateWithSupportHours(t *testing.T) { + setup() + defer teardown() + + mux.HandleFunc("/services", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + testEqual(t, nil, err) + testEqual(t, []uint8(`{"service":{"escalation_policy":{"teams":null},"name":"foo","scheduled_actions":null,"support_hours":{"days_of_week":[1]}}}`), body) + + _, _ = w.Write([]byte(`{"service": {"id": "1","name":"foo"}}`)) + }) + + client := defaultTestClient(server.URL, "foo") + input := Service{ + Name: "foo", + SupportHours: &SupportHours{DaysOfWeek: []uint{1}}, + } + _, err := client.CreateService(input) + + if err != nil { + t.Fatal(err) + } +} + +// Create Service with SupportHours and empty ScheduledActions +func TestService_CreateWithSupportHoursAndEmptyScheduledActions(t *testing.T) { + setup() + defer teardown() + + mux.HandleFunc("/services", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + defer r.Body.Close() + + body, err := io.ReadAll(r.Body) + testEqual(t, nil, err) + testEqual(t, []uint8(`{"service":{"escalation_policy":{"teams":null},"name":"foo","scheduled_actions":[],"support_hours":{"days_of_week":[1]}}}`), body) + + _, _ = w.Write([]byte(`{"service": {"id": "1","name":"foo"}}`)) + }) + + client := defaultTestClient(server.URL, "foo") + input := Service{ + Name: "foo", + SupportHours: &SupportHours{DaysOfWeek: []uint{1}}, + ScheduledActions: []ScheduledAction{}, + } + _, err := client.CreateService(input) + + if err != nil { + t.Fatal(err) + } +} + // Create Service with AlertGroupingParameters of type time func TestService_CreateWithAlertGroupParamsTime(t *testing.T) { setup()