From e6485c0dd1ba25f7981e37a13a00835d43761d2d Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Thu, 25 Sep 2025 14:05:46 +0530 Subject: [PATCH 1/9] feat: raw json --- router/cmd/plan_generator.go | 1 + router/core/plan_generator.go | 10 +++++++++- router/pkg/plan_generator/plan_generator.go | 11 ++++++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index d88227e8fb..8d10980d9a 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -36,6 +36,7 @@ func PlanGenerator(args []string) { f.BoolVar(&cfg.FailOnPlanError, "fail-on-error", false, "if at least one plan fails, the command exit code will be 1") f.BoolVar(&cfg.FailFast, "fail-fast", false, "stop as soon as possible if a plan fails") f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") + f.BoolVar(&cfg.Raw, "raw", false, "get the raw json output of the plan generator") f.UintVar(&cfg.MaxDataSourceCollectorsConcurrency, "max-collectors", 0, "max number of concurrent data source collectors, if unset or 0, no limit will be enforced") if err := f.Parse(args[1:]); err != nil { diff --git a/router/core/plan_generator.go b/router/core/plan_generator.go index 3297a4a3bf..7947fc6e79 100644 --- a/router/core/plan_generator.go +++ b/router/core/plan_generator.go @@ -2,6 +2,7 @@ package core import ( "context" + "encoding/json" "errors" "fmt" "net/http" @@ -65,7 +66,7 @@ func NewPlanner(planConfiguration *plan.Configuration, definition *ast.Document, }, nil } -func (pl *Planner) PlanOperation(operationFilePath string) (string, error) { +func (pl *Planner) PlanOperation(operationFilePath string, rawQueryPlan bool) (string, error) { operation, err := pl.parseOperation(operationFilePath) if err != nil { return "", &PlannerOperationValidationError{err: err} @@ -91,6 +92,13 @@ func (pl *Planner) PlanOperation(operationFilePath string) (string, error) { return "", fmt.Errorf("failed to plan operation: %w", err) } + if rawQueryPlan { + marshal, err := json.Marshal(rawPlan) + if err != nil { + return "", fmt.Errorf("failed to marshal raw plan: %w", err) + } + return string(marshal), nil + } return rawPlan.PrettyPrint(), nil } diff --git a/router/pkg/plan_generator/plan_generator.go b/router/pkg/plan_generator/plan_generator.go index 8b14858431..729fdaf935 100644 --- a/router/pkg/plan_generator/plan_generator.go +++ b/router/pkg/plan_generator/plan_generator.go @@ -30,6 +30,7 @@ type QueryPlanConfig struct { OutputReport bool FailOnPlanError bool FailFast bool + Raw bool LogLevel string Logger *zap.Logger MaxDataSourceCollectorsConcurrency uint @@ -140,9 +141,13 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { queryFilePath := filepath.Join(queriesPath, queryFile.Name()) - outContent, err := planner.PlanOperation(queryFilePath) + outContent, err := planner.PlanOperation(queryFilePath, cfg.Raw) + fileName := queryFile.Name() + if cfg.Raw { + fileName += ".json" + } res := QueryPlanResult{ - FileName: queryFile.Name(), + FileName: fileName, Plan: outContent, } if err != nil { @@ -160,7 +165,7 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { } if cfg.OutputFiles { - outFileName := filepath.Join(outPath, queryFile.Name()) + outFileName := filepath.Join(outPath, fileName) err = os.WriteFile(outFileName, []byte(outContent), 0644) if err != nil { cancelError(fmt.Errorf("failed to write file: %v", err)) From 6b1185bf1314ba32a34b0976103e28c79df77eb0 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Thu, 25 Sep 2025 14:29:22 +0530 Subject: [PATCH 2/9] fix: add tests --- .../pkg/plan_generator/plan_generator_test.go | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/router/pkg/plan_generator/plan_generator_test.go b/router/pkg/plan_generator/plan_generator_test.go index e4b7ae9f78..b67552af2c 100644 --- a/router/pkg/plan_generator/plan_generator_test.go +++ b/router/pkg/plan_generator/plan_generator_test.go @@ -331,4 +331,107 @@ func TestPlanGenerator(t *testing.T) { assert.NoError(t, err) assert.Equal(t, errMsg, writtenResults.Error) }) + + t.Run("generates raw json plans when Raw is enabled", func(t *testing.T) { + tempDir, err := os.MkdirTemp("", "plans-") + require.NoError(t, err) + defer func() { + _ = os.RemoveAll(tempDir) + }() + + cfg := QueryPlanConfig{ + SourceDir: path.Join(getTestDataDir(), "queries", "base"), + OutDir: tempDir, + ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), + Timeout: "30s", + OutputFiles: true, + Raw: true, + } + + err = PlanGenerator(context.Background(), cfg) + assert.NoError(t, err) + + entriesInOutDir, err := os.ReadDir(tempDir) + assert.NoError(t, err) + assert.Len(t, entriesInOutDir, len(allFiles)) + + for _, de := range entriesInOutDir { + name := de.Name() + content, err := os.ReadFile(path.Join(tempDir, name)) + assert.NoError(t, err) + var m map[string]interface{} + + // One of the queries produces a failed result + if err := json.Unmarshal(content, &m); err != nil { + assert.True(t, strings.HasPrefix(string(content), "Warning:")) + } else { + assert.NotEmpty(t, m) + } + } + }) + + t.Run("report file uses .json filenames when Raw is enabled", func(t *testing.T) { + tempDir, err := os.MkdirTemp("", "plans-") + require.NoError(t, err) + defer func() { + _ = os.RemoveAll(tempDir) + }() + + cfg := QueryPlanConfig{ + SourceDir: path.Join(getTestDataDir(), "queries", "base"), + OutDir: tempDir, + ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), + Timeout: "30s", + OutputReport: true, + Raw: true, + } + + err = PlanGenerator(context.Background(), cfg) + assert.NoError(t, err) + + results, err := os.ReadFile(path.Join(tempDir, ReportFileName)) + assert.NoError(t, err) + var writtenResults QueryPlanResults + assert.NoError(t, json.Unmarshal(results, &writtenResults)) + assert.Len(t, writtenResults.Plans, len(allFiles)) + for _, pr := range writtenResults.Plans { + assert.True(t, strings.HasSuffix(pr.FileName, ".json")) + } + }) + + t.Run("generates non-raw textual plans when Raw is disabled", func(t *testing.T) { + tempDir, err := os.MkdirTemp("", "plans-") + require.NoError(t, err) + defer func() { + _ = os.RemoveAll(tempDir) + }() + + cfg := QueryPlanConfig{ + SourceDir: path.Join(getTestDataDir(), "queries", "base"), + OutDir: tempDir, + ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), + Timeout: "30s", + OutputFiles: true, + } + + err = PlanGenerator(context.Background(), cfg) + assert.NoError(t, err) + + entriesInOutDir, err := os.ReadDir(tempDir) + assert.NoError(t, err) + assert.Len(t, entriesInOutDir, len(allFiles)) + + for _, de := range entriesInOutDir { + name := de.Name() + assert.True(t, strings.HasSuffix(name, ".graphql")) + content, err := os.ReadFile(path.Join(tempDir, name)) + assert.NoError(t, err) + var m map[string]interface{} + assert.Error(t, json.Unmarshal(content, &m)) + // Should be textual query plan or warning + s := string(content) + assert.True(t, strings.HasPrefix(s, "QueryPlan {") || strings.HasPrefix(s, "Warning:")) + } + }) + } From bd2d2b7eb6d29ac7ec46b422b9cc3ac0e17a4626 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Thu, 25 Sep 2025 14:48:03 +0530 Subject: [PATCH 3/9] fix: default back to using same file name --- router/pkg/plan_generator/plan_generator.go | 8 ++--- .../pkg/plan_generator/plan_generator_test.go | 29 ------------------- 2 files changed, 2 insertions(+), 35 deletions(-) diff --git a/router/pkg/plan_generator/plan_generator.go b/router/pkg/plan_generator/plan_generator.go index 729fdaf935..245d056744 100644 --- a/router/pkg/plan_generator/plan_generator.go +++ b/router/pkg/plan_generator/plan_generator.go @@ -142,12 +142,8 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { queryFilePath := filepath.Join(queriesPath, queryFile.Name()) outContent, err := planner.PlanOperation(queryFilePath, cfg.Raw) - fileName := queryFile.Name() - if cfg.Raw { - fileName += ".json" - } res := QueryPlanResult{ - FileName: fileName, + FileName: queryFile.Name(), Plan: outContent, } if err != nil { @@ -165,7 +161,7 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { } if cfg.OutputFiles { - outFileName := filepath.Join(outPath, fileName) + outFileName := filepath.Join(outPath, queryFile.Name()) err = os.WriteFile(outFileName, []byte(outContent), 0644) if err != nil { cancelError(fmt.Errorf("failed to write file: %v", err)) diff --git a/router/pkg/plan_generator/plan_generator_test.go b/router/pkg/plan_generator/plan_generator_test.go index b67552af2c..475a203f34 100644 --- a/router/pkg/plan_generator/plan_generator_test.go +++ b/router/pkg/plan_generator/plan_generator_test.go @@ -370,35 +370,6 @@ func TestPlanGenerator(t *testing.T) { } }) - t.Run("report file uses .json filenames when Raw is enabled", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(tempDir) - }() - - cfg := QueryPlanConfig{ - SourceDir: path.Join(getTestDataDir(), "queries", "base"), - OutDir: tempDir, - ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), - Timeout: "30s", - OutputReport: true, - Raw: true, - } - - err = PlanGenerator(context.Background(), cfg) - assert.NoError(t, err) - - results, err := os.ReadFile(path.Join(tempDir, ReportFileName)) - assert.NoError(t, err) - var writtenResults QueryPlanResults - assert.NoError(t, json.Unmarshal(results, &writtenResults)) - assert.Len(t, writtenResults.Plans, len(allFiles)) - for _, pr := range writtenResults.Plans { - assert.True(t, strings.HasSuffix(pr.FileName, ".json")) - } - }) - t.Run("generates non-raw textual plans when Raw is disabled", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) From b7106e2e1293f687939b201c82d9927caac17a67 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Thu, 25 Sep 2025 18:14:13 +0530 Subject: [PATCH 4/9] fix: updates --- router/cmd/plan_generator.go | 19 ++++++- router/core/plan_generator.go | 17 ++++-- router/pkg/plan_generator/plan_generator.go | 8 ++- .../pkg/plan_generator/plan_generator_test.go | 52 ++++++++++++++----- 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index 8d10980d9a..e020987e50 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -7,6 +7,7 @@ import ( "log" "os" "os/signal" + "strings" "syscall" "github.com/KimMachineGun/automemlimit/memlimit" @@ -22,7 +23,9 @@ import ( func PlanGenerator(args []string) { var planHelp bool - cfg := plan_generator.QueryPlanConfig{} + cfg := plan_generator.QueryPlanConfig{ + OutputFormat: core.PlanOutputFormatText, + } f := flag.NewFlagSet("router "+args[0], flag.ExitOnError) f.BoolVar(&planHelp, "help", false, "Prints the help message") f.StringVar(&cfg.ExecutionConfig, "execution-config", "", "required, execution config file location") @@ -36,7 +39,19 @@ func PlanGenerator(args []string) { f.BoolVar(&cfg.FailOnPlanError, "fail-on-error", false, "if at least one plan fails, the command exit code will be 1") f.BoolVar(&cfg.FailFast, "fail-fast", false, "stop as soon as possible if a plan fails") f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") - f.BoolVar(&cfg.Raw, "raw", false, "get the raw json output of the plan generator") + f.Func("print-format", "output format (text|json)", func(s string) error { + v := core.PlanOutputFormat(strings.ToLower(s)) + // Set default + if v == "" { + v = core.PlanOutputFormatText + } + switch v { + case core.PlanOutputFormatText, core.PlanOutputFormatJSON: + cfg.OutputFormat = v + return nil + } + return fmt.Errorf("must be one of: text, json (got %q)", s) + }) f.UintVar(&cfg.MaxDataSourceCollectorsConcurrency, "max-collectors", 0, "max number of concurrent data source collectors, if unset or 0, no limit will be enforced") if err := f.Parse(args[1:]); err != nil { diff --git a/router/core/plan_generator.go b/router/core/plan_generator.go index 7947fc6e79..8826cd3f2d 100644 --- a/router/core/plan_generator.go +++ b/router/core/plan_generator.go @@ -53,6 +53,13 @@ type Planner struct { operationValidator *astvalidation.OperationValidator } +type PlanOutputFormat string + +const ( + PlanOutputFormatText PlanOutputFormat = "text" + PlanOutputFormatJSON PlanOutputFormat = "json" +) + func NewPlanner(planConfiguration *plan.Configuration, definition *ast.Document, clientDefinition *ast.Document) (*Planner, error) { planner, err := plan.NewPlanner(*planConfiguration) if err != nil { @@ -66,7 +73,7 @@ func NewPlanner(planConfiguration *plan.Configuration, definition *ast.Document, }, nil } -func (pl *Planner) PlanOperation(operationFilePath string, rawQueryPlan bool) (string, error) { +func (pl *Planner) PlanOperation(operationFilePath string, outputFormat PlanOutputFormat) (string, error) { operation, err := pl.parseOperation(operationFilePath) if err != nil { return "", &PlannerOperationValidationError{err: err} @@ -92,14 +99,18 @@ func (pl *Planner) PlanOperation(operationFilePath string, rawQueryPlan bool) (s return "", fmt.Errorf("failed to plan operation: %w", err) } - if rawQueryPlan { + switch outputFormat { + case PlanOutputFormatText: + return rawPlan.PrettyPrint(), nil + case PlanOutputFormatJSON: marshal, err := json.Marshal(rawPlan) if err != nil { return "", fmt.Errorf("failed to marshal raw plan: %w", err) } return string(marshal), nil } - return rawPlan.PrettyPrint(), nil + + return "", fmt.Errorf("invalid type specified: %w", err) } func (pl *Planner) PlanParsedOperation(operation *ast.Document) (*resolve.FetchTreeQueryPlanNode, error) { diff --git a/router/pkg/plan_generator/plan_generator.go b/router/pkg/plan_generator/plan_generator.go index 245d056744..40e2896dfa 100644 --- a/router/pkg/plan_generator/plan_generator.go +++ b/router/pkg/plan_generator/plan_generator.go @@ -30,7 +30,7 @@ type QueryPlanConfig struct { OutputReport bool FailOnPlanError bool FailFast bool - Raw bool + OutputFormat core.PlanOutputFormat LogLevel string Logger *zap.Logger MaxDataSourceCollectorsConcurrency uint @@ -53,6 +53,10 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { cfg.Concurrency = runtime.GOMAXPROCS(0) } + if cfg.OutputFormat == "" { + cfg.OutputFormat = core.PlanOutputFormatText + } + queriesPath, err := filepath.Abs(cfg.SourceDir) if err != nil { return fmt.Errorf("failed to get absolute path for queries: %v", err) @@ -141,7 +145,7 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { queryFilePath := filepath.Join(queriesPath, queryFile.Name()) - outContent, err := planner.PlanOperation(queryFilePath, cfg.Raw) + outContent, err := planner.PlanOperation(queryFilePath, cfg.OutputFormat) res := QueryPlanResult{ FileName: queryFile.Name(), Plan: outContent, diff --git a/router/pkg/plan_generator/plan_generator_test.go b/router/pkg/plan_generator/plan_generator_test.go index 475a203f34..6d95031187 100644 --- a/router/pkg/plan_generator/plan_generator_test.go +++ b/router/pkg/plan_generator/plan_generator_test.go @@ -3,6 +3,7 @@ package plan_generator import ( "context" "encoding/json" + "github.com/wundergraph/cosmo/router/core" "os" "path" "path/filepath" @@ -33,7 +34,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("checks queries path exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "notexistant"), @@ -64,7 +67,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("checks filter file exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -82,7 +87,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fail if execution config don't exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -99,7 +106,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fail with invalid execution config ", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -116,7 +125,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fails with wrong timeout duration", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -133,7 +144,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a plan for every file", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -167,7 +180,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a plan for every file filtered", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -195,7 +210,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a result file with every plan inside", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -227,7 +244,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("will not fail on warnings and results should return the warnings and generate results file", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -260,7 +279,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("will not fail on warnings and files should have warnings and generate files", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -293,7 +314,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("when reaching timeout an error should be returned", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -310,7 +333,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("when reaching timeout the report should contains the error", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + _ = os.RemoveAll(tempDir) + }() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -345,7 +370,7 @@ func TestPlanGenerator(t *testing.T) { ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), Timeout: "30s", OutputFiles: true, - Raw: true, + OutputFormat: core.PlanOutputFormatJSON, } err = PlanGenerator(context.Background(), cfg) @@ -383,6 +408,7 @@ func TestPlanGenerator(t *testing.T) { ExecutionConfig: path.Join(getTestDataDir(), "execution_config", "base.json"), Timeout: "30s", OutputFiles: true, + OutputFormat: core.PlanOutputFormatText, } err = PlanGenerator(context.Background(), cfg) From cb3924110c2b1f204f655242c7cd2ac06ef7904a Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 26 Sep 2025 02:00:57 +0530 Subject: [PATCH 5/9] fix: readd --- router/cmd/plan_generator.go | 12 +++++------- router/core/plan_generator.go | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index e020987e50..b3f29a795a 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -40,14 +40,12 @@ func PlanGenerator(args []string) { f.BoolVar(&cfg.FailFast, "fail-fast", false, "stop as soon as possible if a plan fails") f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") f.Func("print-format", "output format (text|json)", func(s string) error { - v := core.PlanOutputFormat(strings.ToLower(s)) - // Set default - if v == "" { - v = core.PlanOutputFormatText - } - switch v { + switch value := core.PlanOutputFormat(strings.ToLower(s)); value { + case "": + cfg.OutputFormat = core.PlanOutputFormatText + return nil case core.PlanOutputFormatText, core.PlanOutputFormatJSON: - cfg.OutputFormat = v + cfg.OutputFormat = value return nil } return fmt.Errorf("must be one of: text, json (got %q)", s) diff --git a/router/core/plan_generator.go b/router/core/plan_generator.go index 8826cd3f2d..c0d9c6a609 100644 --- a/router/core/plan_generator.go +++ b/router/core/plan_generator.go @@ -110,7 +110,7 @@ func (pl *Planner) PlanOperation(operationFilePath string, outputFormat PlanOutp return string(marshal), nil } - return "", fmt.Errorf("invalid type specified: %w", err) + return "", fmt.Errorf("invalid type specified: %q", outputFormat) } func (pl *Planner) PlanParsedOperation(operation *ast.Document) (*resolve.FetchTreeQueryPlanNode, error) { From 3f7ee594c2d425a80e3ea98ff92fded28d87a970 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 26 Sep 2025 02:03:45 +0530 Subject: [PATCH 6/9] fix: cleanup updates --- .../pkg/plan_generator/plan_generator_test.go | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/router/pkg/plan_generator/plan_generator_test.go b/router/pkg/plan_generator/plan_generator_test.go index 6d95031187..8e02dfa697 100644 --- a/router/pkg/plan_generator/plan_generator_test.go +++ b/router/pkg/plan_generator/plan_generator_test.go @@ -34,9 +34,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("checks queries path exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "notexistant"), @@ -67,9 +67,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("checks filter file exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -87,9 +87,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fail if execution config don't exists", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -106,9 +106,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fail with invalid execution config ", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -125,9 +125,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("fails with wrong timeout duration", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -144,9 +144,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a plan for every file", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -180,9 +180,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a plan for every file filtered", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -210,9 +210,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates a result file with every plan inside", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -244,9 +244,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("will not fail on warnings and results should return the warnings and generate results file", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -279,9 +279,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("will not fail on warnings and files should have warnings and generate files", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -314,9 +314,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("when reaching timeout an error should be returned", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -333,9 +333,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("when reaching timeout the report should contains the error", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -360,9 +360,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates raw json plans when Raw is enabled", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -398,9 +398,9 @@ func TestPlanGenerator(t *testing.T) { t.Run("generates non-raw textual plans when Raw is disabled", func(t *testing.T) { tempDir, err := os.MkdirTemp("", "plans-") require.NoError(t, err) - defer func() { + t.Cleanup(func() { _ = os.RemoveAll(tempDir) - }() + }) cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), From 14ca578de3ebf0dddd073ec25aa15e163f18ac53 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 26 Sep 2025 14:57:51 +0530 Subject: [PATCH 7/9] fix: tests --- router/cmd/plan_generator.go | 2 +- router/core/plan_generator.go | 5 +- .../pkg/plan_generator/plan_generator_test.go | 104 +++++------------- 3 files changed, 30 insertions(+), 81 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index b3f29a795a..8f09ad3d93 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -41,7 +41,7 @@ func PlanGenerator(args []string) { f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") f.Func("print-format", "output format (text|json)", func(s string) error { switch value := core.PlanOutputFormat(strings.ToLower(s)); value { - case "": + case core.PlanOutputFormatInvalid: cfg.OutputFormat = core.PlanOutputFormatText return nil case core.PlanOutputFormatText, core.PlanOutputFormatJSON: diff --git a/router/core/plan_generator.go b/router/core/plan_generator.go index c0d9c6a609..03261124db 100644 --- a/router/core/plan_generator.go +++ b/router/core/plan_generator.go @@ -56,8 +56,9 @@ type Planner struct { type PlanOutputFormat string const ( - PlanOutputFormatText PlanOutputFormat = "text" - PlanOutputFormatJSON PlanOutputFormat = "json" + PlanOutputFormatInvalid PlanOutputFormat = "" + PlanOutputFormatText PlanOutputFormat = "text" + PlanOutputFormatJSON PlanOutputFormat = "json" ) func NewPlanner(planConfiguration *plan.Configuration, definition *ast.Document, clientDefinition *ast.Document) (*Planner, error) { diff --git a/router/pkg/plan_generator/plan_generator_test.go b/router/pkg/plan_generator/plan_generator_test.go index 8e02dfa697..4247824b35 100644 --- a/router/pkg/plan_generator/plan_generator_test.go +++ b/router/pkg/plan_generator/plan_generator_test.go @@ -32,11 +32,7 @@ func TestPlanGenerator(t *testing.T) { filtered := "1.graphql" t.Run("checks queries path exists", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "notexistant"), @@ -46,7 +42,7 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorContains(t, err, "failed to read queries directory:") }) @@ -65,11 +61,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("checks filter file exists", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -80,7 +72,7 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorContains(t, err, "failed to read filter file:") }) @@ -104,11 +96,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("fail with invalid execution config ", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -118,16 +106,12 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorContains(t, err, "unexpected EOF") }) t.Run("fails with wrong timeout duration", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -137,16 +121,12 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorContains(t, err, "failed to parse timeout:") }) t.Run("generates a plan for every file", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -157,7 +137,7 @@ func TestPlanGenerator(t *testing.T) { Logger: zap.NewNop(), } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) queries, err := os.ReadDir(tempDir) @@ -178,11 +158,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("generates a plan for every file filtered", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -193,7 +169,7 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) queries, err := os.ReadDir(tempDir) @@ -208,11 +184,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("generates a result file with every plan inside", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -222,7 +194,7 @@ func TestPlanGenerator(t *testing.T) { OutputReport: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) queries, err := os.ReadDir(tempDir) @@ -242,11 +214,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("will not fail on warnings and results should return the warnings and generate results file", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -257,7 +225,7 @@ func TestPlanGenerator(t *testing.T) { OutputReport: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) queries, err := os.ReadDir(tempDir) @@ -277,11 +245,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("will not fail on warnings and files should have warnings and generate files", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -292,7 +256,7 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) queries, err := os.ReadDir(tempDir) @@ -312,11 +276,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("when reaching timeout an error should be returned", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -326,16 +286,12 @@ func TestPlanGenerator(t *testing.T) { OutputFiles: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorIs(t, err, context.DeadlineExceeded) }) t.Run("when reaching timeout the report should contains the error", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -345,7 +301,7 @@ func TestPlanGenerator(t *testing.T) { OutputReport: true, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.ErrorIs(t, err, context.DeadlineExceeded) results, err := os.ReadFile(path.Join(tempDir, ReportFileName)) @@ -358,11 +314,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("generates raw json plans when Raw is enabled", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -373,7 +325,7 @@ func TestPlanGenerator(t *testing.T) { OutputFormat: core.PlanOutputFormatJSON, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) entriesInOutDir, err := os.ReadDir(tempDir) @@ -396,11 +348,7 @@ func TestPlanGenerator(t *testing.T) { }) t.Run("generates non-raw textual plans when Raw is disabled", func(t *testing.T) { - tempDir, err := os.MkdirTemp("", "plans-") - require.NoError(t, err) - t.Cleanup(func() { - _ = os.RemoveAll(tempDir) - }) + tempDir := t.TempDir() cfg := QueryPlanConfig{ SourceDir: path.Join(getTestDataDir(), "queries", "base"), @@ -411,7 +359,7 @@ func TestPlanGenerator(t *testing.T) { OutputFormat: core.PlanOutputFormatText, } - err = PlanGenerator(context.Background(), cfg) + err := PlanGenerator(context.Background(), cfg) assert.NoError(t, err) entriesInOutDir, err := os.ReadDir(tempDir) From 678723cb5a395977baf730158da2db9f79fb18f1 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 26 Sep 2025 15:01:11 +0530 Subject: [PATCH 8/9] fix: refactoring --- router/cmd/plan_generator.go | 2 +- router/core/plan_generator.go | 6 +++--- router/pkg/plan_generator/plan_generator.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index 8f09ad3d93..eae3667d89 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -41,7 +41,7 @@ func PlanGenerator(args []string) { f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") f.Func("print-format", "output format (text|json)", func(s string) error { switch value := core.PlanOutputFormat(strings.ToLower(s)); value { - case core.PlanOutputFormatInvalid: + case core.PlanOutputFormatUnset: cfg.OutputFormat = core.PlanOutputFormatText return nil case core.PlanOutputFormatText, core.PlanOutputFormatJSON: diff --git a/router/core/plan_generator.go b/router/core/plan_generator.go index 03261124db..1026fbe592 100644 --- a/router/core/plan_generator.go +++ b/router/core/plan_generator.go @@ -56,9 +56,9 @@ type Planner struct { type PlanOutputFormat string const ( - PlanOutputFormatInvalid PlanOutputFormat = "" - PlanOutputFormatText PlanOutputFormat = "text" - PlanOutputFormatJSON PlanOutputFormat = "json" + PlanOutputFormatUnset PlanOutputFormat = "" + PlanOutputFormatText PlanOutputFormat = "text" + PlanOutputFormatJSON PlanOutputFormat = "json" ) func NewPlanner(planConfiguration *plan.Configuration, definition *ast.Document, clientDefinition *ast.Document) (*Planner, error) { diff --git a/router/pkg/plan_generator/plan_generator.go b/router/pkg/plan_generator/plan_generator.go index 40e2896dfa..a2668d3bc2 100644 --- a/router/pkg/plan_generator/plan_generator.go +++ b/router/pkg/plan_generator/plan_generator.go @@ -53,7 +53,7 @@ func PlanGenerator(ctx context.Context, cfg QueryPlanConfig) error { cfg.Concurrency = runtime.GOMAXPROCS(0) } - if cfg.OutputFormat == "" { + if cfg.OutputFormat == core.PlanOutputFormatUnset { cfg.OutputFormat = core.PlanOutputFormatText } From 304e47afdc84d7d2e0dfce8a53f9c72b320124c0 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 26 Sep 2025 15:11:21 +0530 Subject: [PATCH 9/9] fix: updates --- router/cmd/plan_generator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/router/cmd/plan_generator.go b/router/cmd/plan_generator.go index eae3667d89..98b67b6002 100644 --- a/router/cmd/plan_generator.go +++ b/router/cmd/plan_generator.go @@ -38,8 +38,8 @@ func PlanGenerator(args []string) { f.BoolVar(&cfg.OutputReport, "print-report", false, "write a report.json file, with all the query plans and errors sorted by file name") f.BoolVar(&cfg.FailOnPlanError, "fail-on-error", false, "if at least one plan fails, the command exit code will be 1") f.BoolVar(&cfg.FailFast, "fail-fast", false, "stop as soon as possible if a plan fails") - f.StringVar(&cfg.LogLevel, "log-level", "warn", "log level to use (debug, info, warn, error, panic, fatal)") - f.Func("print-format", "output format (text|json)", func(s string) error { + f.StringVar(&cfg.LogLevel, "log-level", "warn", "the log level to use (debug, info, warn, error, panic, fatal)") + f.Func("print-format", "output format of the query plan (text, json) (default \"text\")", func(s string) error { switch value := core.PlanOutputFormat(strings.ToLower(s)); value { case core.PlanOutputFormatUnset: cfg.OutputFormat = core.PlanOutputFormatText