From 589ebbd5d37a5c9e013033ef52a8356fb06a9b79 Mon Sep 17 00:00:00 2001 From: Ryan Thorn Date: Fri, 14 Apr 2023 16:18:55 -0700 Subject: [PATCH 1/4] chore(install): Refactor re.setOutput to use bufio scanner; fix invalid JSON message on empty string --- .../execution/go_task_recipe_executor.go | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/install/execution/go_task_recipe_executor.go b/internal/install/execution/go_task_recipe_executor.go index a0c02690c..b5fe044de 100644 --- a/internal/install/execution/go_task_recipe_executor.go +++ b/internal/install/execution/go_task_recipe_executor.go @@ -1,6 +1,7 @@ package execution import ( + "bufio" "bytes" "context" "encoding/json" @@ -184,33 +185,32 @@ func createOutputJSONFile(r types.OpenInstallationRecipe, recipeVars types.Recip func (re *GoTaskRecipeExecutor) setOutput(outputFileName string) { outputFile, err := os.Open(outputFileName) if err != nil { - log.Debugf("error openning json output file %s", outputFileName) + log.Debugf("error opening JSON output file %s", outputFileName) return } defer outputFile.Close() - outputBytes, _ := ioutil.ReadAll(outputFile) - if len(outputBytes) > 0 { - var result map[string]interface{} - lines := strings.Split(string(outputBytes), "\n") - for _, line := range lines { - if json.Valid([]byte(line)) { - var jsonFromString map[string]interface{} - if err := json.Unmarshal([]byte(line), &jsonFromString); err == nil { - if mergoErr := mergo.Merge(&result, jsonFromString, mergo.WithOverride); mergoErr != nil { - log.Debugf("error while merging JSON output resuls, details:%s", mergoErr.Error()) - } - } else { - log.Debugf("error while unmarshaling JSON output, details:%s", err.Error()) - } - } else { - log.Debugf(fmt.Sprintf("Invalid JSON string found in output file, skipping: %v", line)) + output := map[string]interface{}{} + s := bufio.NewScanner(outputFile) + for s.Scan() { + bs := s.Bytes() + if json.Valid(bs) { + var jsonFromString map[string]interface{} + if err = json.Unmarshal(bs, &jsonFromString); err != nil { + log.Debugf("error unmarshaling JSON output: %s", err.Error()) } + if err = mergo.Merge(&output, jsonFromString, mergo.WithOverride); err != nil { + log.Debugf("error merging JSON output: %s", err.Error()) + } + } else { + log.Debugf("Invalid JSON string found in output file, skipping: %s", s.Text()) } - re.Output = NewOutputParser(result) - } else { - re.Output = NewOutputParser(map[string]interface{}{}) } + if err = s.Err(); err != nil { + log.Debugf("error scanning output file %s: %w", outputFileName, err) + } + + re.Output = NewOutputParser(output) } func (re *GoTaskRecipeExecutor) GetOutput() *OutputParser { From ab81bd200034d8298da0b3c886d51279a856a523 Mon Sep 17 00:00:00 2001 From: Ryan Thorn Date: Sun, 16 Apr 2023 09:52:23 -0700 Subject: [PATCH 2/4] chore(install): Fix unsupported verb %w --- internal/install/execution/go_task_recipe_executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/install/execution/go_task_recipe_executor.go b/internal/install/execution/go_task_recipe_executor.go index b5fe044de..294251307 100644 --- a/internal/install/execution/go_task_recipe_executor.go +++ b/internal/install/execution/go_task_recipe_executor.go @@ -207,7 +207,7 @@ func (re *GoTaskRecipeExecutor) setOutput(outputFileName string) { } } if err = s.Err(); err != nil { - log.Debugf("error scanning output file %s: %w", outputFileName, err) + log.Debugf("error scanning output file %s: %s", outputFileName, err.Error()) } re.Output = NewOutputParser(output) From d31d7892cbc7353fbfc3916364fc1c6da8121fb9 Mon Sep 17 00:00:00 2001 From: Ryan Thorn Date: Tue, 18 Apr 2023 13:17:59 -0700 Subject: [PATCH 3/4] chore(install): Extract mergeStringJSON, test --- .../execution/go_task_recipe_executor.go | 20 ++++-- .../execution/go_task_recipe_executor_test.go | 68 ++++++++++++++++++- 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/internal/install/execution/go_task_recipe_executor.go b/internal/install/execution/go_task_recipe_executor.go index 294251307..ccb6be99f 100644 --- a/internal/install/execution/go_task_recipe_executor.go +++ b/internal/install/execution/go_task_recipe_executor.go @@ -195,12 +195,8 @@ func (re *GoTaskRecipeExecutor) setOutput(outputFileName string) { for s.Scan() { bs := s.Bytes() if json.Valid(bs) { - var jsonFromString map[string]interface{} - if err = json.Unmarshal(bs, &jsonFromString); err != nil { - log.Debugf("error unmarshaling JSON output: %s", err.Error()) - } - if err = mergo.Merge(&output, jsonFromString, mergo.WithOverride); err != nil { - log.Debugf("error merging JSON output: %s", err.Error()) + if err = mergeStringJSON(&output, bs); err != nil { + log.Debug(err.Error()) } } else { log.Debugf("Invalid JSON string found in output file, skipping: %s", s.Text()) @@ -213,6 +209,18 @@ func (re *GoTaskRecipeExecutor) setOutput(outputFileName string) { re.Output = NewOutputParser(output) } +func mergeStringJSON(output *map[string]interface{}, bs []byte) error { + var jsonFromString map[string]interface{} + var err error + if err = json.Unmarshal(bs, &jsonFromString); err != nil { + return fmt.Errorf("error unmarshaling JSON output: %w", err) + } + if err = mergo.Merge(output, jsonFromString, mergo.WithOverride); err != nil { + return fmt.Errorf("error merging JSON output: %w", err) + } + return nil +} + func (re *GoTaskRecipeExecutor) GetOutput() *OutputParser { return re.Output } diff --git a/internal/install/execution/go_task_recipe_executor_test.go b/internal/install/execution/go_task_recipe_executor_test.go index d64cf454c..fdce616d3 100644 --- a/internal/install/execution/go_task_recipe_executor_test.go +++ b/internal/install/execution/go_task_recipe_executor_test.go @@ -9,11 +9,9 @@ import ( "strings" "testing" + "github.com/newrelic/newrelic-cli/internal/install/types" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/newrelic/newrelic-cli/internal/install/types" ) func TestExecute_SystemVariableInterpolation(t *testing.T) { @@ -486,3 +484,67 @@ tasks: assert.Empty(t, e.GetOutput().Metadata()["something"]) assert.Equal(t, "very", e.GetOutput().Metadata()["clean"]) } + +func Test_mergeStringJSON(t *testing.T) { + type args struct { + output *map[string]interface{} + bs []byte + } + outputNewMap := map[string]interface{}{} + argsNewMap := args{ + output: &outputNewMap, + bs: []byte("{\"Metadata\":{\"some\":\"thing\"}}\n"), + } + outputExistingMap := map[string]interface{}{ + "Metadata": map[string]interface{}{"some": "thing"}, + } + argsExistingMap := args{ + output: &outputExistingMap, + bs: []byte("{\"Metadata\":{\"another\":\"thing\"}}\n"), + } + outputInvalidJson := map[string]interface{}{} + argsInvalidJson := args{ + output: &outputInvalidJson, + bs: []byte("Invalid JSON"), + } + outputEmptyString := map[string]interface{}{} + argsEmptyString := args{ + output: &outputEmptyString, + bs: []byte(""), + } + tests := []struct { + name string + args args + wantErr bool + wantLen int + }{ + {"new map", argsNewMap, false, 1}, + {"existing map", argsExistingMap, false, 2}, + {"invalid JSON", argsInvalidJson, true, 0}, + {"empty string", argsEmptyString, true, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := mergeStringJSON(tt.args.output, tt.args.bs); (err != nil) != tt.wantErr { + t.Errorf("mergeStringJSON() error = %v, wantErr %v", err, tt.wantErr) + } + output := *tt.args.output + if output == nil { + t.Errorf("expected output map, got nil") + } + if tt.wantLen == 0 { + if len(output) > 0 { + t.Errorf("unexpected number of results -- expected 0, got %d", len(output)) + } + } else { + metadata := output["Metadata"].(map[string]interface{}) + if metadata == nil { + t.Errorf("expected 'Metadata' field in output map, got nil") + } + if len(metadata) != tt.wantLen { + t.Errorf("unexpected number of results -- expected %d, got %d\n results: %v", tt.wantLen, len(metadata), tt.args.output) + } + } + }) + } +} From 452f1a132f16b5f4d22856429bef0a21b2acfdac Mon Sep 17 00:00:00 2001 From: Ryan Thorn Date: Tue, 18 Apr 2023 13:26:27 -0700 Subject: [PATCH 4/4] chore(install): Fix lint error --- .../install/execution/go_task_recipe_executor_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/install/execution/go_task_recipe_executor_test.go b/internal/install/execution/go_task_recipe_executor_test.go index fdce616d3..fed9cb39a 100644 --- a/internal/install/execution/go_task_recipe_executor_test.go +++ b/internal/install/execution/go_task_recipe_executor_test.go @@ -502,9 +502,9 @@ func Test_mergeStringJSON(t *testing.T) { output: &outputExistingMap, bs: []byte("{\"Metadata\":{\"another\":\"thing\"}}\n"), } - outputInvalidJson := map[string]interface{}{} - argsInvalidJson := args{ - output: &outputInvalidJson, + outputInvalidJSON := map[string]interface{}{} + argsInvalidJSON := args{ + output: &outputInvalidJSON, bs: []byte("Invalid JSON"), } outputEmptyString := map[string]interface{}{} @@ -520,7 +520,7 @@ func Test_mergeStringJSON(t *testing.T) { }{ {"new map", argsNewMap, false, 1}, {"existing map", argsExistingMap, false, 2}, - {"invalid JSON", argsInvalidJson, true, 0}, + {"invalid JSON", argsInvalidJSON, true, 0}, {"empty string", argsEmptyString, true, 0}, } for _, tt := range tests {