From 2af6a5cfd20ef218a132cbfbd7f81c65aa68b3eb Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 2 Aug 2026 01:38:06 +0000 Subject: [PATCH] Apply configured output directories --- internal/app/app.go | 104 +--------------------- internal/app/batch_generation_test.go | 94 ++++++++++++++++++++ internal/app/generation_test.go | 109 ++++++++++++++++++++++- internal/app/output.go | 123 ++++++++++++++++++++++++++ 4 files changed, 327 insertions(+), 103 deletions(-) create mode 100644 internal/app/output.go diff --git a/internal/app/app.go b/internal/app/app.go index b270bcb..278a43f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -4,9 +4,7 @@ package app import ( "context" "fmt" - "os" "path/filepath" - "strings" "time" distributoradapter "gitea.maximumdirect.net/eric/weatherreporter/internal/adapters/distributor" @@ -251,7 +249,7 @@ func GenerateDetailed(ctx context.Context, req GenerateRequest) (*ReportResult, return nil, err } result := initialReportResult(req, resolved, PromptInspectionResult{}) - outputPath, err := resolveReportOutputPath(req.WorkingDir, req.OutputPath, resolved) + outputPath, err := resolveReportOutputPath(req.WorkingDir, req.OutputPath, req.Config.Output.Directory, resolved) if err != nil { return result, err } @@ -302,7 +300,7 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro if _, err := report.BatchForCommandName(string(req.Batch)); err != nil { return nil, err } - outputDir, err := resolveOutputDir(req.WorkingDir, req.OutputDir) + outputDir, err := resolveOutputDirWithConfigured(req.WorkingDir, req.OutputDir, req.Config.Output.Directory) if err != nil { return nil, err } @@ -431,104 +429,6 @@ func batchReportResult(planned plannedBatchReport) BatchReportResult { } } -func plannedBatchOutputPath(outputDir string, planned plannedBatchReport) (string, error) { - outputName, err := planned.Resolved.OutputName() - if err != nil { - return "", err - } - return validateOutputPath(filepath.Join(outputDir, outputName)) -} - -func prepareBatchOutputs(outputDir string, plannedReports []plannedBatchReport) error { - for index := range plannedReports { - outputPath, err := plannedBatchOutputPath(outputDir, plannedReports[index]) - if err != nil { - return err - } - plannedReports[index].OutputPath = outputPath - } - return nil -} - -func resolveReportOutputPath(workingDir, override string, resolved report.Resolved) (string, error) { - outputName, err := resolved.OutputName() - if err != nil { - return "", err - } - return resolveOutputPath(workingDir, override, outputName) -} - -func resolveOutputDir(workingDir, override string) (string, error) { - workingDir, err := validateWorkingDir(workingDir) - if err != nil { - return "", err - } - if override == "" { - return workingDir, nil - } - if strings.TrimSpace(override) == "" { - return "", fmt.Errorf("output directory is required") - } - directory := override - if !filepath.IsAbs(directory) { - directory = filepath.Join(workingDir, directory) - } - directory = filepath.Clean(directory) - if info, err := os.Stat(directory); err == nil && !info.IsDir() { - return "", fmt.Errorf("output directory %q is not a directory", directory) - } else if err != nil && !os.IsNotExist(err) { - return "", fmt.Errorf("inspect output directory %q: %w", directory, err) - } - return directory, nil -} - -func resolveOutputPath(workingDir, override, defaultName string) (string, error) { - workingDir, err := validateWorkingDir(workingDir) - if err != nil { - return "", err - } - path := override - if path == "" { - path = defaultName - } - if strings.TrimSpace(path) == "" { - return "", fmt.Errorf("final output path is required") - } - if !filepath.IsAbs(path) { - path = filepath.Join(workingDir, path) - } - return validateOutputPath(path) -} - -func validateWorkingDir(workingDir string) (string, error) { - if strings.TrimSpace(workingDir) == "" { - return "", fmt.Errorf("working directory is required") - } - if !filepath.IsAbs(workingDir) { - return "", fmt.Errorf("working directory %q must be absolute", workingDir) - } - return filepath.Clean(workingDir), nil -} - -func validateOutputPath(path string) (string, error) { - if strings.TrimSpace(path) == "" { - return "", fmt.Errorf("final output path is required") - } - path = filepath.Clean(path) - if !filepath.IsAbs(path) { - return "", fmt.Errorf("final output path %q must be absolute", path) - } - if filepath.Dir(path) == path { - return "", fmt.Errorf("final output path %q must not be a filesystem root", path) - } - if info, err := os.Stat(path); err == nil && info.IsDir() { - return "", fmt.Errorf("final output path %q is a directory", path) - } else if err != nil && !os.IsNotExist(err) { - return "", fmt.Errorf("inspect final output path %q: %w", path, err) - } - return path, nil -} - func ResolveGenerate(req GenerateRequest, now time.Time) (report.Resolved, error) { location, err := timeutil.LoadLocation(req.Config.WeatherAPI.Timezone) if err != nil { diff --git a/internal/app/batch_generation_test.go b/internal/app/batch_generation_test.go index b478f5e..85d5fdf 100644 --- a/internal/app/batch_generation_test.go +++ b/internal/app/batch_generation_test.go @@ -61,6 +61,100 @@ func TestRunBatchDetailedNotifiesOnlyAfterAllOutputsExist(t *testing.T) { } } +func TestRunBatchDetailedUsesDefaultAndConfiguredOutputDirectories(t *testing.T) { + tests := []struct { + name string + directory func(t *testing.T, workingDir string) string + wantDir func(t *testing.T, workingDir string, configuredDir string) string + }{ + { + name: "working directory default", + directory: func(_ *testing.T, _ string) string { + return "" + }, + wantDir: func(_ *testing.T, workingDir string, _ string) string { + return workingDir + }, + }, + { + name: "absolute directory", + directory: func(t *testing.T, _ string) string { + return filepath.Join(t.TempDir(), "reports") + }, + wantDir: func(_ *testing.T, _ string, configuredDir string) string { + return configuredDir + }, + }, + { + name: "relative directory", + directory: func(_ *testing.T, _ string) string { + return "configured/../reports" + }, + wantDir: func(_ *testing.T, workingDir string, _ string) string { + return filepath.Join(workingDir, "reports") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workingDir := t.TempDir() + configuredDir := tt.directory(t, workingDir) + cfg := generationDistributorConfig() + cfg.Output.Directory = configuredDir + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + notifier := &generationNotifier{} + + result, err := RunBatchDetailed(context.Background(), BatchRequest{ + Config: cfg, Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: workingDir, + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier, + }) + wantDir := tt.wantDir(t, workingDir, configuredDir) + if err != nil || result == nil || result.Succeeded != len(result.Reports) || notifier.batchCalls != 1 { + t.Fatalf("RunBatchDetailed() result/error/notifier = %#v/%v/%#v", result, err, notifier) + } + for _, item := range result.Reports { + if filepath.Dir(item.OutputPath) != wantDir { + t.Fatalf("report output %q, want directory %q", item.OutputPath, wantDir) + } + } + for _, file := range notifier.batchRequest.Files { + if filepath.Dir(file.SourcePath) != wantDir { + t.Fatalf("notification source %q, want directory %q", file.SourcePath, wantDir) + } + } + }) + } +} + +func TestRunBatchDetailedExplicitOutputDirectoryIgnoresConfiguredDirectory(t *testing.T) { + configuredPath := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(configuredPath, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + explicitDir := t.TempDir() + cfg := generationDistributorConfig() + cfg.Output.Directory = configuredPath + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + + result, err := RunBatchDetailed(context.Background(), BatchRequest{ + Config: cfg, Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: explicitDir, + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: &generationNotifier{}, + }) + if err != nil || result == nil || result.Succeeded != len(result.Reports) { + t.Fatalf("RunBatchDetailed() result/error = %#v/%v", result, err) + } + for _, item := range result.Reports { + if filepath.Dir(item.OutputPath) != explicitDir { + t.Fatalf("report output %q, want directory %q", item.OutputPath, explicitDir) + } + } +} + func TestRunBatchDetailedPreflightsAllOutputPaths(t *testing.T) { bundle := generationBundle(t) bundle.Hourly.Periods = bundle.Hourly.Periods[:1] diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index c2aea11..9facaec 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -113,6 +113,108 @@ func TestGenerateDetailedPublishesOnlySelectedOutput(t *testing.T) { } } +func TestGenerateDetailedUsesConfiguredOutputDirectory(t *testing.T) { + tests := []struct { + name string + directory func(t *testing.T, workingDir string) string + wantDir func(t *testing.T, workingDir string, configuredDir string) string + }{ + { + name: "absolute directory", + directory: func(t *testing.T, _ string) string { + return filepath.Join(t.TempDir(), "reports") + }, + wantDir: func(_ *testing.T, _ string, configuredDir string) string { + return configuredDir + }, + }, + { + name: "relative directory", + directory: func(_ *testing.T, _ string) string { + return "configured/../reports" + }, + wantDir: func(_ *testing.T, workingDir string, _ string) string { + return filepath.Join(workingDir, "reports") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workingDir := t.TempDir() + configuredDir := tt.directory(t, workingDir) + cfg := generationDistributorConfig() + cfg.Output.Directory = configuredDir + bundle := generationBundle(t) + notifier := &generationNotifier{} + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: cfg, Report: ReportDaily, + Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), + WorkingDir: workingDir, Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier, + }) + wantPath := filepath.Join(tt.wantDir(t, workingDir, configuredDir), "daily-2026-05-29.md") + if err != nil || result == nil || result.OutputPath != wantPath || notifier.request.ReportPath != wantPath { + t.Fatalf("GenerateDetailed() result/error/notification = %#v/%v/%#v", result, err, notifier.request) + } + if info, statErr := os.Stat(filepath.Dir(wantPath)); statErr != nil || !info.IsDir() { + t.Fatalf("configured output directory info/error = %#v/%v", info, statErr) + } + if _, statErr := os.Stat(wantPath); statErr != nil { + t.Fatalf("output %q: %v", wantPath, statErr) + } + }) + } +} + +func TestGenerateDetailedExplicitOutputPathIgnoresConfiguredDirectory(t *testing.T) { + configuredPath := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(configuredPath, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + explicitPath := filepath.Join(t.TempDir(), "explicit.md") + cfg := generationConfig() + cfg.Output.Directory = configuredPath + bundle := generationBundle(t) + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: cfg, Report: ReportDaily, + Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), + WorkingDir: t.TempDir(), OutputPath: explicitPath, Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, + }) + if err != nil || result == nil || result.OutputPath != explicitPath { + t.Fatalf("GenerateDetailed() result/error = %#v/%v", result, err) + } + if _, statErr := os.Stat(explicitPath); statErr != nil { + t.Fatalf("explicit output %q: %v", explicitPath, statErr) + } +} + +func TestGenerateDetailedRejectsConfiguredNonDirectoryBeforeWork(t *testing.T) { + configuredPath := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(configuredPath, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + cfg := generationDistributorConfig() + cfg.Output.Directory = configuredPath + bundle := generationBundle(t) + collector := &generationCollector{bundle: &bundle} + executor := &generationExecutor{} + notifier := &generationNotifier{} + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: cfg, Report: ReportDaily, + Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), + WorkingDir: t.TempDir(), Collector: collector, Executor: executor, Notifier: notifier, + }) + if err == nil || result == nil || collector.called || executor.promptInspections != 0 || executor.called || notifier.calls != 0 { + t.Fatalf("GenerateDetailed() result/error/collector/executor/notifier = %#v/%v/%t/%#v/%#v", result, err, collector.called, executor, notifier) + } + if data, readErr := os.ReadFile(configuredPath); readErr != nil || string(data) != "not a directory" { + t.Fatalf("configured path = %q, error = %v", data, readErr) + } +} + func TestGenerateDetailedReturnsResolvedResultWhenCollectionFails(t *testing.T) { cfg := config.Defaults() cfg.WeatherAPI.Timezone, cfg.Location.ID = "America/Chicago", "home" @@ -244,14 +346,19 @@ func generationBundlePointer(t *testing.T) *weatherdata.Bundle { type generationNotifier struct { err error batchErr error + calls int request NotificationRequest batchRequest batchNotificationRequest batchCalls int } func (n *generationNotifier) Notify(_ context.Context, request NotificationRequest) (*NotificationResult, error) { + n.calls++ n.request = request - return nil, n.err + if n.err != nil { + return nil, n.err + } + return &NotificationResult{Status: "succeeded"}, nil } func (n *generationNotifier) NotifyBatch(_ context.Context, request batchNotificationRequest) (*NotificationResult, error) { diff --git a/internal/app/output.go b/internal/app/output.go new file mode 100644 index 0000000..8ce4f1b --- /dev/null +++ b/internal/app/output.go @@ -0,0 +1,123 @@ +package app + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "gitea.maximumdirect.net/eric/weatherreporter/internal/report" +) + +func plannedBatchOutputPath(outputDir string, planned plannedBatchReport) (string, error) { + outputName, err := planned.Resolved.OutputName() + if err != nil { + return "", err + } + return validateOutputPath(filepath.Join(outputDir, outputName)) +} + +func prepareBatchOutputs(outputDir string, plannedReports []plannedBatchReport) error { + for index := range plannedReports { + outputPath, err := plannedBatchOutputPath(outputDir, plannedReports[index]) + if err != nil { + return err + } + plannedReports[index].OutputPath = outputPath + } + return nil +} + +func resolveReportOutputPath(workingDir, override, configuredDir string, resolved report.Resolved) (string, error) { + outputName, err := resolved.OutputName() + if err != nil { + return "", err + } + if override != "" { + return resolveOutputPath(workingDir, override, outputName) + } + outputDir, err := resolveOutputDir(workingDir, configuredDir) + if err != nil { + return "", err + } + return validateOutputPath(filepath.Join(outputDir, outputName)) +} + +func resolveOutputDirWithConfigured(workingDir, override, configuredDir string) (string, error) { + directory := configuredDir + if override != "" { + directory = override + } + return resolveOutputDir(workingDir, directory) +} + +func resolveOutputDir(workingDir, override string) (string, error) { + workingDir, err := validateWorkingDir(workingDir) + if err != nil { + return "", err + } + if override == "" { + return workingDir, nil + } + if strings.TrimSpace(override) == "" { + return "", fmt.Errorf("output directory is required") + } + directory := override + if !filepath.IsAbs(directory) { + directory = filepath.Join(workingDir, directory) + } + directory = filepath.Clean(directory) + if info, err := os.Stat(directory); err == nil && !info.IsDir() { + return "", fmt.Errorf("output directory %q is not a directory", directory) + } else if err != nil && !os.IsNotExist(err) { + return "", fmt.Errorf("inspect output directory %q: %w", directory, err) + } + return directory, nil +} + +func resolveOutputPath(workingDir, override, defaultName string) (string, error) { + workingDir, err := validateWorkingDir(workingDir) + if err != nil { + return "", err + } + path := override + if path == "" { + path = defaultName + } + if strings.TrimSpace(path) == "" { + return "", fmt.Errorf("final output path is required") + } + if !filepath.IsAbs(path) { + path = filepath.Join(workingDir, path) + } + return validateOutputPath(path) +} + +func validateWorkingDir(workingDir string) (string, error) { + if strings.TrimSpace(workingDir) == "" { + return "", fmt.Errorf("working directory is required") + } + if !filepath.IsAbs(workingDir) { + return "", fmt.Errorf("working directory %q must be absolute", workingDir) + } + return filepath.Clean(workingDir), nil +} + +func validateOutputPath(path string) (string, error) { + if strings.TrimSpace(path) == "" { + return "", fmt.Errorf("final output path is required") + } + path = filepath.Clean(path) + if !filepath.IsAbs(path) { + return "", fmt.Errorf("final output path %q must be absolute", path) + } + if filepath.Dir(path) == path { + return "", fmt.Errorf("final output path %q must not be a filesystem root", path) + } + if info, err := os.Stat(path); err == nil && info.IsDir() { + return "", fmt.Errorf("final output path %q is a directory", path) + } else if err != nil && !os.IsNotExist(err) { + return "", fmt.Errorf("inspect final output path %q: %w", path, err) + } + return path, nil +}