commit a17d3f9dc6856d0cb60bf176e66844dc2752782d
from: Oliver Lowe <o@olowe.co>
date: Fri Feb 11 23:48:25 2022 UTC

Simplify service JSON marshalling

We don't need to have a MarshalJSON method just so that it can be
created ok. It's clearer to do stuff necessart for object creation at
the time of object creation, not way earlier in some unrelated bit of
the code.
While here simplify the tests to actually test what we're worried about.

commit - 8f08828e8134e1160b4be74544d3b6020059366e
commit + a17d3f9dc6856d0cb60bf176e66844dc2752782d
blob - c9d34273251c4488a2de996b0fefc36114881964
blob + 8489d3be87abb80b5dfdf4791a962e05133740dd
--- object.go
+++ object.go
@@ -14,6 +14,38 @@ type object interface {
 	path() string
 }
 
+// jsonForCreate marshals obj into the required JSON object to be sent
+// in the body of a PUT request to Icinga. Some fields of obj must not be set for
+// Icinga to create the object. Since some of those fields are structs
+// (and not pointers to structs), they are always included, even if unset.
+// jsonForCreate overrides those fields to always be empty. Other fields are left
+// alone to let Icinga report an error for us.
+func jsonForCreate(obj object) ([]byte, error) {
+	m := make(map[string]interface{})
+	switch v := obj.(type) {
+	case User, HostGroup:
+		m["attrs"] = v
+	case Host:
+		aux := &struct {
+			// fields not added to Host yet
+			// LastCheck struct{}
+			// LastCheckResult struct{}
+			Host
+		}{Host: v}
+		m["attrs"] = aux
+	case Service:
+		aux := &struct {
+			LastCheck       *struct{} `json:",omitempty"`
+			LastCheckResult *struct{} `json:"last_check_result,omitempty"`
+			Service
+		}{Service: v}
+		m["attrs"] = aux
+	default:
+		return nil, fmt.Errorf("marshal %T for creation unsupported", v)
+	}
+	return json.Marshal(m)
+}
+
 //go:generate ./crud.sh -o crud.go
 
 func (c *Client) lookupObject(objpath string) (object, error) {
@@ -56,18 +88,11 @@ func (c *Client) filterObjects(objpath, expr string) (
 }
 
 func (c *Client) createObject(obj object) error {
-	buf := &bytes.Buffer{}
-	switch v := obj.(type) {
-	case Host, Service, User, HostGroup:
-		m := make(map[string]interface{})
-		m["attrs"] = v
-		if err := json.NewEncoder(buf).Encode(m); err != nil {
-			return err
-		}
-	default:
-		return fmt.Errorf("create type %T unsupported", v)
+	b, err := jsonForCreate(obj)
+	if err != nil {
+		return fmt.Errorf("marshal into json: %v", err)
 	}
-	resp, err := c.put(obj.path(), buf)
+	resp, err := c.put(obj.path(), bytes.NewReader(b))
 	if err != nil {
 		return err
 	}
blob - 27ad6d6ed237df31a19942fa9698a7817eb54162
blob + 2efeac2254523381f0045edf0dbe2e1c796cc489
--- service.go
+++ service.go
@@ -23,7 +23,7 @@ type Service struct {
 	CheckCommand    string       `json:"check_command"`
 	DisplayName     string       `json:"display_name,omitempty"`
 	LastCheck       time.Time    `json:",omitempty"`
-	LastCheckResult *CheckResult `json:"last_check_result,omitempty"`
+	LastCheckResult CheckResult  `json:"last_check_result,omitempty"`
 	Acknowledgement bool         `json:",omitempty"`
 }
 
blob - f73240cfb1b4e93b27b8dcd523f2ed520ceae03c
blob + 8dea0673a98854ab0bb9955c1d30f5793b1a5b1e
--- service_test.go
+++ service_test.go
@@ -2,10 +2,11 @@ package icinga
 
 import (
 	"os"
-	"reflect"
 	"testing"
+	"time"
 )
 
+// Tests the trickier parts of the custom Unmarshaller functionality.
 func TestServiceUnmarshal(t *testing.T) {
 	f, err := os.Open("testdata/objects/services/9p.io!http")
 	if err != nil {
@@ -16,19 +17,35 @@ func TestServiceUnmarshal(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	want := Service{
-		Name:         "9p.io!http",
-		Groups:       []string{},
-		State:        ServiceOK,
-		StateType:    StateHard,
-		CheckCommand: "http",
-		DisplayName:  "http",
-		LastCheckResult: &CheckResult{
-			Output: "HTTP OK: HTTP/1.1 200 OK - 1714 bytes in 1.083 second response time ",
+	svc := resp.Results[0].(Service)
+	if svc.LastCheck.IsZero() {
+		t.Error("zero time")
+	}
+	if !svc.Acknowledgement {
+		t.Error("should be acknowledged")
+	}
+	if t.Failed() {
+		t.Log(svc)
+	}
+}
+
+func TestServiceMarshalForCreate(t *testing.T) {
+	want := `{"attrs":{"check_command":"dummy","display_name":"test"}}`
+	service := Service{
+		CheckCommand: "dummy",
+		DisplayName:  "test",
+		LastCheck:    time.Now(),
+		LastCheckResult: CheckResult{
+			Output:      "xxx",
+			CheckSource: "xxx",
+			Command:     nil,
 		},
 	}
-	got := resp.Results[0].(Service)
-	if !reflect.DeepEqual(want, got) {
-		t.Errorf("want %+v, got %+v", want, got)
+	got, err := jsonForCreate(service)
+	if err != nil {
+		t.Fatal(err)
 	}
+	if want != string(got) {
+		t.Error("not matching", string(got))
+	}
 }
blob - ac22c0888275fc037a4a55e539a81940cf5a34e4
blob + dea97f1e5d5449a1b0ed2f8eafc8a6472d389e4e
--- testdata/objects/services/9p.io!http
+++ testdata/objects/services/9p.io!http
@@ -3,7 +3,7 @@
         {
             "attrs": {
                 "__name": "9p.io!http",
-                "acknowledgement": 0,
+                "acknowledgement": 1,
                 "acknowledgement_expiry": 0,
                 "acknowledgement_last_change": 0,
                 "action_url": "",