mapic: Fix parsing of human_issues field (#646)

It is undocumented and I assumed it was a string just like
the issues. Turns out it is a string array.

If I just changed the types it would make the parsing logic
much more convoluted, as the array is actually parsed as a
[]interface{} and I'd need to check each field. So I changed
it once again, making yet another parsing of the input object.

mapic/misttriggers/stream_buffer.go

@@ -88,9 +88,9 @@ type StreamHealthPayload struct {
 	SessionID  string `json:"session_id"`
 	IsActive   bool   `json:"is_active"`
 
-	IsHealthy   bool   `json:"is_healthy"`
-	Issues      string `json:"issues,omitempty"`
-	HumanIssues string `json:"human_issues,omitempty"`
+	IsHealthy   bool     `json:"is_healthy"`
+	Issues      string   `json:"issues,omitempty"`
+	HumanIssues []string `json:"human_issues,omitempty"`
 
 	Tracks map[string]TrackDetails `json:"tracks,omitempty"`
 	Extra  map[string]any          `json:"extra,omitempty"`
@@ -165,45 +165,47 @@ func ParseStreamBufferPayload(lines []string) (*StreamBufferPayload, error) {
 }
 
 type MistStreamDetails struct {
-	Tracks              map[string]TrackDetails
-	Issues, HumanIssues string
-	Extra               map[string]any
+	Tracks      map[string]TrackDetails
+	Issues      string
+	HumanIssues []string
+	Extra       map[string]any
 }
 
-// Mists saves the tracks and issues in the same JSON object, so we need to
-// parse them separately. e.g. {track-id-1: {...}, issues: "..."}
+// Mists sends the track detail objects in the same JSON object as other
+// non-object fields (string and array issues and numeric metrics). So we need
+// to parse them separately and do a couple of JSON juggling here.
+// e.g. {track-id-1: {...}, issues: "a string", human_issues: ["a", "b"], "jitter": 32}
 func ParseMistStreamDetails(streamState string, data []byte) (*MistStreamDetails, error) {
 	if streamState == "EMPTY" {
 		return nil, nil
 	}
 
+	var issues struct {
+		Issues      string   `json:"issues"`
+		HumanIssues []string `json:"human_issues"`
+	}
+	err := json.Unmarshal(data, &issues)
+	if err != nil {
+		return nil, fmt.Errorf("error unmarshalling issues JSON: %w", err)
+	}
+
 	var tracksAndIssues map[string]any
-	err := json.Unmarshal(data, &tracksAndIssues)
+	err = json.Unmarshal(data, &tracksAndIssues)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing stream details JSON: %w", err)
+		return nil, fmt.Errorf("error unmarshalling JSON: %w", err)
 	}
+	delete(tracksAndIssues, "issues")
+	delete(tracksAndIssues, "human_issues")
 
-	var (
-		issues, humanIssues string
-		extra               = map[string]any{}
-	)
+	extra := map[string]any{}
 	for key, val := range tracksAndIssues {
-		switch tval := val.(type) {
-		case map[string]any:
+		if _, isObj := val.(map[string]any); isObj {
 			// this is a track, it will be parsed from the serialized obj below
 			continue
-		case string:
-			if key == "issues" {
-				issues = tval
-			} else if key == "human_issues" {
-				humanIssues = tval
-			} else {
-				extra[key] = val
-			}
-		default:
+		} else {
 			extra[key] = val
+			delete(tracksAndIssues, key)
 		}
-		delete(tracksAndIssues, key)
 	}
 
 	tracksJSON, err := json.Marshal(tracksAndIssues) // only tracks now
@@ -216,5 +218,5 @@ func ParseMistStreamDetails(streamState string, data []byte) (*MistStreamDetails
 		return nil, fmt.Errorf("error parsing stream details tracks: %w", err)
 	}
 
-	return &MistStreamDetails{tracks, issues, humanIssues, extra}, nil
+	return &MistStreamDetails{tracks, issues.Issues, issues.HumanIssues, extra}, nil
 }

mapic/misttriggers/stream_buffer_test.go

@@ -16,7 +16,7 @@ var (
 		"stream1", "FULL", `{"track1":{"codec":"h264","kbits":1000,"keys":{"B":"1"},"fpks":30,"height":720,"width":1280},"jitter":420}`,
 	}
 	streamBufferPayloadIssues = []string{
-		"stream1", "RECOVER", `{"track1":{"codec":"h264","kbits":1000,"keys":{"B":"1"},"fpks":30,"height":720,"width":1280},"issues":"The aqueous linear entity, in a manner pertaining to its metaphorical state of existence, appears to be experiencing an ostensibly suboptimal condition that is reminiscent of an individual's disposition when subjected to an unfavorable meteorological phenomenon","human_issues":"Stream is feeling under the weather"}`,
+		"stream1", "RECOVER", `{"track1":{"codec":"h264","kbits":1000,"keys":{"B":"1"},"fpks":30,"height":720,"width":1280},"issues":"The aqueous linear entity, in a manner pertaining to its metaphorical state of existence, appears to be experiencing an ostensibly suboptimal condition that is reminiscent of an individual's disposition when subjected to an unfavorable meteorological phenomenon","human_issues":["Stream is feeling under the weather"]}`,
 	}
 	streamBufferPayloadInvalid = []string{
 		"stream1", "FULL", `{"track1":{},"notatrack":{"codec":2}}`,
@@ -42,7 +42,7 @@ func TestItCanParseAStreamBufferPayloadWithStreamIssues(t *testing.T) {
 	require.Equal(t, p.StreamName, "stream1")
 	require.Equal(t, p.State, "RECOVER")
 	require.NotNil(t, p.Details)
-	require.Equal(t, p.Details.HumanIssues, "Stream is feeling under the weather")
+	require.Equal(t, p.Details.HumanIssues, []string{"Stream is feeling under the weather"})
 	require.Contains(t, p.Details.Issues, "unfavorable meteorological phenomenon")
 	require.Len(t, p.Details.Tracks, 1)
 	require.Contains(t, p.Details.Tracks, "track1")
@@ -142,5 +142,5 @@ func TestTriggerStreamBufferE2E(t *testing.T) {
 	require.Equal(t, receivedPayload.IsHealthy, false)
 	require.Len(t, receivedPayload.Tracks, 1)
 	require.Contains(t, receivedPayload.Tracks, "track1")
-	require.Equal(t, receivedPayload.HumanIssues, "Stream is feeling under the weather")
+	require.Equal(t, receivedPayload.HumanIssues, []string{"Stream is feeling under the weather"})
 }