diff --git a/cmd/check.go b/cmd/check.go index 33a3a2eb3..9e9c2ad43 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -26,9 +26,6 @@ var checkCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create check pipeline - pipeline := pipelines.NewCheckPipeline() - // Create output function outputFunc := func(output string) { fmt.Fprintln(cmd.OutOrStdout(), output) @@ -38,9 +35,10 @@ var checkCmd = &cobra.Command{ ctx := context.WithValue(cmd.Context(), "operation", "tools") ctx = context.WithValue(ctx, "output", outputFunc) - // Initialize the pipeline - if err := pipeline.Initialize(injector, ctx); err != nil { - return fmt.Errorf("Error initializing: %w", err) + // Set up the check pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "checkPipeline") + if err != nil { + return fmt.Errorf("failed to set up check pipeline: %w", err) } // Execute the pipeline @@ -61,9 +59,6 @@ var checkNodeHealthCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create check pipeline - pipeline := pipelines.NewCheckPipeline() - // Require nodes to be specified if len(nodeHealthNodes) == 0 { return fmt.Errorf("No nodes specified. Use --nodes flag to specify nodes to check") @@ -86,9 +81,10 @@ var checkNodeHealthCmd = &cobra.Command{ ctx = context.WithValue(ctx, "version", nodeHealthVersion) ctx = context.WithValue(ctx, "output", outputFunc) - // Initialize the pipeline - if err := pipeline.Initialize(injector, ctx); err != nil { - return fmt.Errorf("Error initializing: %w", err) + // Set up the check pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "checkPipeline") + if err != nil { + return fmt.Errorf("failed to set up check pipeline: %w", err) } // Execute the pipeline diff --git a/cmd/context.go b/cmd/context.go index 6607bce8c..32528778a 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -19,9 +19,6 @@ var getContextCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create context pipeline - pipeline := pipelines.NewContextPipeline() - // Create output function outputFunc := func(output string) { fmt.Fprintln(cmd.OutOrStdout(), output) @@ -31,9 +28,10 @@ var getContextCmd = &cobra.Command{ ctx := context.WithValue(cmd.Context(), "operation", "get") ctx = context.WithValue(ctx, "output", outputFunc) - // Initialize the pipeline - if err := pipeline.Initialize(injector, ctx); err != nil { - return fmt.Errorf("Error initializing: %w", err) + // Set up the context pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "contextPipeline") + if err != nil { + return fmt.Errorf("failed to set up context pipeline: %w", err) } // Execute the pipeline @@ -55,9 +53,6 @@ var setContextCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create context pipeline - pipeline := pipelines.NewContextPipeline() - // Create output function outputFunc := func(output string) { fmt.Fprintln(cmd.OutOrStdout(), output) @@ -68,9 +63,10 @@ var setContextCmd = &cobra.Command{ ctx = context.WithValue(ctx, "contextName", args[0]) ctx = context.WithValue(ctx, "output", outputFunc) - // Initialize the pipeline - if err := pipeline.Initialize(injector, ctx); err != nil { - return fmt.Errorf("Error initializing: %w", err) + // Set up the context pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "contextPipeline") + if err != nil { + return fmt.Errorf("failed to set up context pipeline: %w", err) } // Execute the pipeline diff --git a/cmd/env.go b/cmd/env.go index d6e589bf2..61e7cda9f 100644 --- a/cmd/env.go +++ b/cmd/env.go @@ -18,9 +18,6 @@ var envCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create env pipeline - pipeline := pipelines.NewEnvPipeline() - // Get flags hook, _ := cmd.Flags().GetBool("hook") decrypt, _ := cmd.Flags().GetBool("decrypt") @@ -37,9 +34,10 @@ var envCmd = &cobra.Command{ ctx = context.WithValue(ctx, "hook", true) } - // Initialize the pipeline with appropriate configuration - if err := pipeline.Initialize(injector, ctx); err != nil { - return fmt.Errorf("Error initializing: %w", err) + // Set up the env pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "envPipeline") + if err != nil { + return fmt.Errorf("failed to set up env pipeline: %w", err) } // Execute the pipeline diff --git a/cmd/exec.go b/cmd/exec.go index c55475a01..a65270161 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -25,15 +25,9 @@ var execCmd = &cobra.Command{ injector := cmd.Context().Value(injectorKey).(di.Injector) // First, run the env pipeline in quiet mode to set up environment variables - var envPipeline pipelines.Pipeline - if existing := injector.Resolve("envPipeline"); existing != nil { - envPipeline = existing.(pipelines.Pipeline) - } else { - envPipeline = pipelines.NewEnvPipeline() - if err := envPipeline.Initialize(injector, cmd.Context()); err != nil { - return fmt.Errorf("failed to initialize env pipeline: %w", err) - } - injector.Register("envPipeline", envPipeline) + envPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") + if err != nil { + return fmt.Errorf("failed to set up env pipeline: %w", err) } // Execute env pipeline in quiet mode (inject environment variables without printing) @@ -44,15 +38,9 @@ var execCmd = &cobra.Command{ } // Then, run the exec pipeline to execute the command - var execPipeline pipelines.Pipeline - if existing := injector.Resolve("execPipeline"); existing != nil { - execPipeline = existing.(pipelines.Pipeline) - } else { - execPipeline = pipelines.NewExecPipeline() - if err := execPipeline.Initialize(injector, cmd.Context()); err != nil { - return fmt.Errorf("failed to initialize exec pipeline: %w", err) - } - injector.Register("execPipeline", execPipeline) + execPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "execPipeline") + if err != nil { + return fmt.Errorf("failed to set up exec pipeline: %w", err) } // Create execution context with command and arguments diff --git a/cmd/exec_test.go b/cmd/exec_test.go index 239e24a23..d2ce1e026 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -17,6 +17,7 @@ func TestExecCmd(t *testing.T) { Use: "exec -- [command]", Short: "Execute a shell command with environment variables", Long: "Execute a shell command with environment variables set for the application.", + Args: cobra.MinimumNArgs(1), SilenceUsage: true, RunE: execCmd.RunE, } @@ -79,7 +80,7 @@ func TestExecCmd(t *testing.T) { t.Error("Expected error, got nil") } - expectedError := "no command provided" + expectedError := "requires at least 1 arg(s), only received 0" if err.Error() != expectedError { t.Errorf("Expected error %q, got %q", expectedError, err.Error()) } diff --git a/cmd/hook.go b/cmd/hook.go index bc4031a96..01382b48c 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -14,28 +14,23 @@ var hookCmd = &cobra.Command{ Short: "Prints out shell hook information per platform (zsh,bash,fish,tcsh,powershell).", Long: "Prints out shell hook information for each platform (zsh,bash,fish,tcsh,powershell).", SilenceUsage: true, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return fmt.Errorf("No shell name provided") - } - // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create hook pipeline - pipeline := pipelines.NewHookPipeline() - - // Initialize the pipeline - if err := pipeline.Initialize(injector, cmd.Context()); err != nil { - return fmt.Errorf("Error initializing: %w", err) - } - // Create execution context with shell type ctx := context.WithValue(cmd.Context(), "shellType", args[0]) if verbose { ctx = context.WithValue(ctx, "verbose", true) } + // Set up the hook pipeline + pipeline, err := pipelines.WithPipeline(injector, ctx, "hookPipeline") + if err != nil { + return fmt.Errorf("failed to set up hook pipeline: %w", err) + } + // Execute the pipeline if err := pipeline.Execute(ctx); err != nil { return fmt.Errorf("Error executing hook pipeline: %w", err) diff --git a/cmd/hook_test.go b/cmd/hook_test.go index ce0b5a7f9..dde24344c 100644 --- a/cmd/hook_test.go +++ b/cmd/hook_test.go @@ -49,7 +49,7 @@ func TestHookCmd(t *testing.T) { } // And error should contain usage message - expectedError := "No shell name provided" + expectedError := "accepts 1 arg(s), received 0" if err.Error() != expectedError { t.Errorf("Expected error %q, got %q", expectedError, err.Error()) } diff --git a/cmd/init.go b/cmd/init.go index e97593e1a..1c7c75d51 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -36,23 +36,24 @@ var initCmd = &cobra.Command{ Long: "Initialize the application environment with the specified context configuration", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - var injector di.Injector - if contextInjector := cmd.Context().Value(injectorKey); contextInjector != nil { - injector = contextInjector.(di.Injector) - } else { - injector = di.NewInjector() + // Get shared dependency injector from context + injector := cmd.Context().Value(injectorKey).(di.Injector) + + // First, run the env pipeline in quiet mode to set up environment variables + envPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") + if err != nil { + return fmt.Errorf("failed to set up env pipeline: %w", err) + } + envCtx := context.WithValue(cmd.Context(), "quiet", true) + envCtx = context.WithValue(envCtx, "decrypt", true) + if err := envPipeline.Execute(envCtx); err != nil { + return fmt.Errorf("failed to set up environment: %w", err) } - // Check if init pipeline exists in injector, create if not - var pipeline pipelines.Pipeline - if existing := injector.Resolve("initPipeline"); existing != nil { - pipeline = existing.(pipelines.Pipeline) - } else { - pipeline = pipelines.NewInitPipeline() - if err := pipeline.Initialize(injector, cmd.Context()); err != nil { - return fmt.Errorf("failed to initialize pipeline: %w", err) - } - injector.Register("initPipeline", pipeline) + // Set up the init pipeline + initPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "initPipeline") + if err != nil { + return fmt.Errorf("failed to set up init pipeline: %w", err) } ctx := cmd.Context() @@ -142,7 +143,7 @@ var initCmd = &cobra.Command{ } } - return pipeline.Execute(ctx) + return initPipeline.Execute(ctx) }, } diff --git a/cmd/init_test.go b/cmd/init_test.go index e31c48234..9cdfe32c6 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -42,11 +42,17 @@ func setupInitTest(t *testing.T, opts ...*SetupOptions) *InitMocks { baseMocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { return nil } baseMocks.Shell.WriteResetTokenFunc = func() (string, error) { return "test-token", nil } - // Register mock pipeline in injector (following exec_test.go pattern) - mockPipeline := pipelines.NewMockBasePipeline() - mockPipeline.InitializeFunc = func(injector di.Injector, ctx context.Context) error { return nil } - mockPipeline.ExecuteFunc = func(ctx context.Context) error { return nil } - baseMocks.Injector.Register("initPipeline", mockPipeline) + // Register mock env pipeline in injector (needed since init now runs env pipeline first) + mockEnvPipeline := pipelines.NewMockBasePipeline() + mockEnvPipeline.InitializeFunc = func(injector di.Injector, ctx context.Context) error { return nil } + mockEnvPipeline.ExecuteFunc = func(ctx context.Context) error { return nil } + baseMocks.Injector.Register("envPipeline", mockEnvPipeline) + + // Register mock init pipeline in injector (following exec_test.go pattern) + mockInitPipeline := pipelines.NewMockBasePipeline() + mockInitPipeline.InitializeFunc = func(injector di.Injector, ctx context.Context) error { return nil } + mockInitPipeline.ExecuteFunc = func(ctx context.Context) error { return nil } + baseMocks.Injector.Register("initPipeline", mockInitPipeline) return &InitMocks{ Injector: baseMocks.Injector, diff --git a/pkg/pipelines/pipeline.go b/pkg/pipelines/pipeline.go index f7338e4ee..1efd291b1 100644 --- a/pkg/pipelines/pipeline.go +++ b/pkg/pipelines/pipeline.go @@ -42,6 +42,50 @@ type Pipeline interface { Execute(ctx context.Context) error } +// PipelineConstructor defines a function that creates a new pipeline instance +type PipelineConstructor func() Pipeline + +// ============================================================================= +// Pipeline Factory +// ============================================================================= + +// pipelineConstructors maps pipeline names to their constructor functions +var pipelineConstructors = map[string]PipelineConstructor{ + "envPipeline": func() Pipeline { return NewEnvPipeline() }, + "initPipeline": func() Pipeline { return NewInitPipeline() }, + "execPipeline": func() Pipeline { return NewExecPipeline() }, + "contextPipeline": func() Pipeline { return NewContextPipeline() }, + "hookPipeline": func() Pipeline { return NewHookPipeline() }, + "checkPipeline": func() Pipeline { return NewCheckPipeline() }, +} + +// WithPipeline resolves or creates a pipeline instance from the DI container by name. +// If the pipeline already exists in the injector, it is returned directly. Otherwise, a new instance is constructed, +// initialized with the provided injector and context, registered in the DI container, and then returned. +// Returns an error if the pipeline name is unknown or initialization fails. +func WithPipeline(injector di.Injector, ctx context.Context, pipelineName string) (Pipeline, error) { + if existing := injector.Resolve(pipelineName); existing != nil { + if pipeline, ok := existing.(Pipeline); ok { + return pipeline, nil + } + } + + constructor, exists := pipelineConstructors[pipelineName] + if !exists { + return nil, fmt.Errorf("unknown pipeline: %s", pipelineName) + } + + pipeline := constructor() + + if err := pipeline.Initialize(injector, ctx); err != nil { + return nil, fmt.Errorf("failed to initialize %s: %w", pipelineName, err) + } + + injector.Register(pipelineName, pipeline) + + return pipeline, nil +} + // BasePipeline provides common pipeline functionality including config loading // Specific pipelines should embed this and add their own dependencies type BasePipeline struct { diff --git a/pkg/pipelines/pipeline_test.go b/pkg/pipelines/pipeline_test.go index c74adb9c1..4a98bbf2e 100644 --- a/pkg/pipelines/pipeline_test.go +++ b/pkg/pipelines/pipeline_test.go @@ -202,7 +202,7 @@ func TestBasePipeline_Initialize(t *testing.T) { // Then no error should be returned if err != nil { - t.Errorf("Expected no error, got %v", err) + t.Errorf("Expected no error, got: %v", err) } }) } @@ -215,22 +215,271 @@ func TestBasePipeline_Execute(t *testing.T) { } t.Run("ExecuteReturnsNilByDefault", func(t *testing.T) { - // Given a base pipeline - pipeline, mocks := setup(t) + pipeline, _ := setup(t) - // When initializing and executing the pipeline - err := pipeline.Initialize(mocks.Injector, context.Background()) + // When executing the pipeline + err := pipeline.Execute(context.Background()) + + // Then no error should be returned if err != nil { - t.Fatalf("Initialize failed: %v", err) + t.Errorf("Expected no error, got: %v", err) + } + }) +} + +func TestWithPipeline(t *testing.T) { + t.Run("Success", func(t *testing.T) { + testCases := []struct { + name string + pipelineType string + }{ + {"EnvPipeline", "envPipeline"}, + {"InitPipeline", "initPipeline"}, + {"ExecPipeline", "execPipeline"}, + {"ContextPipeline", "contextPipeline"}, + {"HookPipeline", "hookPipeline"}, + {"CheckPipeline", "checkPipeline"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating a pipeline with WithPipeline + pipeline, err := WithPipeline(injector, ctx, tc.pipelineType) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And a pipeline should be returned + if pipeline == nil { + t.Error("Expected non-nil pipeline") + } + }) + } + }) + + t.Run("UnknownPipelineType", func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating a pipeline with unknown type + pipeline, err := WithPipeline(injector, ctx, "unknownPipeline") + + // Then an error should be returned + if err == nil { + t.Error("Expected error for unknown pipeline type, got nil") + } + if !strings.Contains(err.Error(), "unknown pipeline") { + t.Errorf("Expected 'unknown pipeline' error, got: %v", err) } + // And no pipeline should be returned + if pipeline != nil { + t.Error("Expected nil pipeline for unknown type") + } + }) + + t.Run("ExistingPipelineInInjector", func(t *testing.T) { + // Given an injector with existing pipeline + injector := di.NewInjector() + existingPipeline := NewMockBasePipeline() + injector.Register("envPipeline", existingPipeline) ctx := context.Background() - err = pipeline.Execute(ctx) + + // When creating a pipeline with WithPipeline + pipeline, err := WithPipeline(injector, ctx, "envPipeline") // Then no error should be returned if err != nil { - t.Errorf("Expected no error, got %v", err) + t.Errorf("Expected no error, got: %v", err) + } + + // And the existing pipeline should be returned + if pipeline != existingPipeline { + t.Error("Expected existing pipeline to be returned") + } + }) + + t.Run("ExistingNonPipelineInInjector", func(t *testing.T) { + // Given an injector with non-pipeline object + injector := di.NewInjector() + nonPipeline := "not a pipeline" + injector.Register("envPipeline", nonPipeline) + ctx := context.Background() + + // When creating a pipeline with WithPipeline + pipeline, err := WithPipeline(injector, ctx, "envPipeline") + + // Then no error should be returned (it creates a new pipeline) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And a new pipeline should be returned + if pipeline == nil { + t.Error("Expected non-nil pipeline") + } + }) + + t.Run("ContextPropagation", func(t *testing.T) { + // Given an injector and context with values + injector := di.NewInjector() + ctx := context.WithValue(context.Background(), "testKey", "testValue") + + // When creating a pipeline with WithPipeline + pipeline, err := WithPipeline(injector, ctx, "envPipeline") + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And pipeline should be created successfully + if pipeline == nil { + t.Error("Expected non-nil pipeline") + } + }) + + t.Run("NilInjector", func(t *testing.T) { + // Given a nil injector + ctx := context.Background() + + // When creating a pipeline with nil injector, it should panic + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic for nil injector, but no panic occurred") + } + }() + + // This should panic due to nil pointer dereference + WithPipeline(nil, ctx, "envPipeline") + + // If we reach here, the test should fail + t.Error("Expected panic for nil injector, but function returned normally") + }) + + t.Run("NilContext", func(t *testing.T) { + // Given an injector with nil context + injector := di.NewInjector() + + // When creating a pipeline with nil context, it should panic + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic for nil context, but no panic occurred") + } + }() + + // This should panic due to nil pointer dereference during initialization + WithPipeline(injector, nil, "envPipeline") + + // If we reach here, the test should fail + t.Error("Expected panic for nil context, but function returned normally") + }) + + t.Run("EmptyPipelineType", func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating a pipeline with empty type + pipeline, err := WithPipeline(injector, ctx, "") + + // Then an error should be returned + if err == nil { + t.Error("Expected error for empty pipeline type, got nil") + } + if !strings.Contains(err.Error(), "unknown pipeline") { + t.Errorf("Expected 'unknown pipeline' error, got: %v", err) + } + + // And no pipeline should be returned + if pipeline != nil { + t.Error("Expected nil pipeline for empty type") + } + }) + + t.Run("AllSupportedTypes", func(t *testing.T) { + supportedTypes := []string{ + "envPipeline", + "initPipeline", + "execPipeline", + "contextPipeline", + "hookPipeline", + "checkPipeline", + } + + for _, pipelineType := range supportedTypes { + t.Run(pipelineType, func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating the pipeline + pipeline, err := WithPipeline(injector, ctx, pipelineType) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error for %s, got: %v", pipelineType, err) + } + + // And a pipeline should be returned + if pipeline == nil { + t.Errorf("Expected non-nil pipeline for %s", pipelineType) + } + }) + } + }) + + t.Run("PipelineRegistration", func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating a pipeline with WithPipeline + pipeline, err := WithPipeline(injector, ctx, "envPipeline") + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And the pipeline should be registered in the injector + registered := injector.Resolve("envPipeline") + if registered == nil { + t.Error("Expected pipeline to be registered in injector") + } + if registered != pipeline { + t.Error("Expected registered pipeline to match returned pipeline") + } + }) + + t.Run("FactoryFunctionality", func(t *testing.T) { + // Given an injector and context + injector := di.NewInjector() + ctx := context.Background() + + // When creating multiple pipelines of the same type + pipeline1, err1 := WithPipeline(injector, ctx, "envPipeline") + pipeline2, err2 := WithPipeline(injector, ctx, "envPipeline") + + // Then both calls should succeed + if err1 != nil { + t.Errorf("Expected no error for first call, got: %v", err1) + } + if err2 != nil { + t.Errorf("Expected no error for second call, got: %v", err2) + } + + // And the same pipeline instance should be returned + if pipeline1 != pipeline2 { + t.Error("Expected same pipeline instance to be returned from factory") } }) }