opsgenie / opsgenie-go-sdk-v2

Opsgenie GO SDK v2
Apache License 2.0
35 stars 64 forks source link

An empty TimeRestriction struct does not result in an empty JSON object "{}" #42

Closed foax closed 4 years ago

foax commented 4 years ago

The TimeRestriction struct is defined in og/entity.go as:

type TimeRestriction struct {
    Type            RestrictionType `json:"type,omitempty"`
    RestrictionList []Restriction   `json:"restrictions,omitempty"`
    Restriction     Restriction     `json:"restriction,omitempty"`
}

When an empty TimeRestriction is created and encoding/json is used to encode the struct into JSON, the resulting JSON is an object with a restriction: {} field. A very basic example is:

package main

import (
    "fmt"
    "encoding/json"

    "github.com/opsgenie/opsgenie-go-sdk-v2/og"
)

func main() {
    time_restriction := og.TimeRestriction{}
    b, _ := json.Marshal(time_restriction)
    fmt.Println(string(b))
}

Result:

{"restriction":{}}

This expected result should be an empty JSON object (i.e., {}).

This is preventing me from using the OpsGenie Go SDK to remove a time restriction from an existing schedule rotation. I'm wanting to use schedule.UpdateRotation() with an UpdateRotationRequest that includes an empty time restriction within the rotation definition. The UpdateRotationRequest.Rotation.TimeRestriction field cannot be a nil pointer, as this would result in the time restriction not being updated at all. It needs to have a zero value so that it results in an empty map when it's converted to JSON in the HTTP request body.

I think the solution is to redefine the TimeRestriction struct with the TimeRestriction.Restriction field as a pointer:

type TimeRestriction struct {
    Type            RestrictionType `json:"type,omitempty"`
    RestrictionList []Restriction   `json:"restrictions,omitempty"`
    Restriction     *Restriction     `json:"restriction,omitempty"`
}

A zero-value TimeRestriction in this case results in an empty JSON object, which would allow for a time restriction to be removed using schedule.UpdateRotation().

I don't know what implications this has on the rest of the code base though.

MeralBusraTekinsen commented 4 years ago

In our Schedule Rotation API, it seems that the time restriction field is not mandatory for update rotation requests. But if you define a time restriction field you need to define inner fields of time restriction. They are mandatory fields. So you can not create an empty time restriction for update roration request. It seems that your request can not be done because of our API structure. You can follow this link for a detailed explanation.

cemkucuk commented 4 years ago

I'm closing this issue as no response from owner