From 8d2ac163ae1725c93b1b4d9973bf598439f7a0f9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 20 Jun 2026 02:55:06 +0000 Subject: [PATCH] Use report-specific distributor paths --- internal/app/app.go | 35 ++- internal/app/app_test.go | 339 +++++++++++++++++++++++++++-- internal/app/batch_notification.go | 4 +- 3 files changed, 351 insertions(+), 27 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index a7976a8..405fec5 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -934,7 +934,7 @@ func buildNotificationRequest(cfg config.Config, resolved report.Resolved, repor if err != nil { return NotificationRequest{}, err } - bundlePaths, err := config.RenderDistributorReportPaths("notify.distributor.report_path_templates", cfg.Notify.Distributor.ReportPathTemplates, values) + bundlePaths, err := renderDistributorReportBundlePaths(cfg, resolved, metadata.RunID, reportPath, values) if err != nil { return NotificationRequest{}, err } @@ -970,6 +970,39 @@ func distributorTemplateValuesForReport(cfg config.Config, resolved report.Resol return values, nil } +func renderDistributorReportBundlePaths(cfg config.Config, resolved report.Resolved, runID string, sourcePath string, values config.DistributorTemplateValues) ([]string, error) { + templates, name, err := distributorReportPathTemplates(cfg, resolved.Definition) + if err != nil { + return nil, distributorReportPathError(resolved.Definition.ID, runID, sourcePath, err) + } + paths, err := config.RenderDistributorReportPaths(name, templates, values) + if err != nil { + return nil, distributorReportPathError(resolved.Definition.ID, runID, sourcePath, err) + } + return paths, nil +} + +func distributorReportPathTemplates(cfg config.Config, definition report.Definition) ([]string, string, error) { + overrides, err := cfg.ReportDistributorPathOverrides() + if err != nil { + return nil, "", err + } + if templates, ok := overrides[definition.ID]; ok { + return append([]string(nil), templates...), fmt.Sprintf("reports.%s.distributor.path_templates", definition.ID), nil + } + if len(definition.DistributorPathTemplates) > 0 { + return append([]string(nil), definition.DistributorPathTemplates...), fmt.Sprintf("report.%s.distributor_path_templates", definition.ID), nil + } + return nil, "", fmt.Errorf("no distributor path templates configured") +} + +func distributorReportPathError(id report.ID, runID string, sourcePath string, err error) error { + if sourcePath != "" { + return fmt.Errorf("report %q run %q source path %q: %w", id, runID, sourcePath, err) + } + return fmt.Errorf("report %q run %q: %w", id, runID, err) +} + func addDistributorValidPeriodValues(values *config.DistributorTemplateValues, period timeutil.Period, timezone string) error { location, err := timeutil.LoadLocation(timezone) if err != nil { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 49a9aae..2bcff92 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -739,7 +739,7 @@ func TestGenerateHourlyReportCopiesOutputAndNotifiesManagedReport(t *testing.T) if req.ReportPath == outputPath { t.Fatalf("notification used output copy %q, want managed report path", outputPath) } - wantBundlePaths := []string{"2026-05-29/hourly/hourly.md"} + wantBundlePaths := []string{"hourly/index.md"} if strings.Join(req.BundlePaths, "\n") != strings.Join(wantBundlePaths, "\n") { t.Fatalf("notification BundlePaths = %#v, want %#v", req.BundlePaths, wantBundlePaths) } @@ -778,7 +778,6 @@ func TestGenerateTodayReportCopiesOutputAndNotifiesTodayTemplateValues(t *testin cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{report_id}.{artifact_group}" cfg.Notify.Distributor.BundleIDTemplate = "{artifact_group}.{batch_output_name}.{report_id}" cfg.Notify.Distributor.IdempotencyKeyTemplate = "{bundle_id}.{run_id}" - cfg.Notify.Distributor.ReportPathTemplates = []string{"{valid_start_date}/{artifact_group}/{batch_output_name}"} resolved := resolveGenerateForTest(t, cfg, GenerateRequest{ Report: ReportToday, Date: mustParse("2026-05-29T12:00:00-05:00"), @@ -834,7 +833,11 @@ func TestGenerateTodayReportCopiesOutputAndNotifiesTodayTemplateValues(t *testin if req.BundleID != "today.today.md.today" { t.Fatalf("BundleID = %q, want Today artifact group, output name, and report id", req.BundleID) } - wantBundlePaths := []string{"2026-05-29/today/today.md"} + wantBundlePaths := []string{ + "daily/2026-05-29/" + result.Metadata.RunID + ".md", + "daily/2026-05-29/index.md", + "today/index.md", + } if strings.Join(req.BundlePaths, "\n") != strings.Join(wantBundlePaths, "\n") { t.Fatalf("BundlePaths = %#v, want %#v", req.BundlePaths, wantBundlePaths) } @@ -981,7 +984,6 @@ func TestGenerateTomorrowReportNotificationUsesTomorrowTemplateValues(t *testing cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{report_id}.{artifact_group}" cfg.Notify.Distributor.BundleIDTemplate = "{artifact_group}.{batch_output_name}.{report_id}" cfg.Notify.Distributor.IdempotencyKeyTemplate = "{bundle_id}.{run_id}" - cfg.Notify.Distributor.ReportPathTemplates = []string{"{valid_start_date}/{artifact_group}/{batch_output_name}"} resolved := resolveGenerateForTest(t, cfg, GenerateRequest{ Report: ReportTomorrow, }, "2026-05-29T18:00:00-05:00") @@ -1011,7 +1013,11 @@ func TestGenerateTomorrowReportNotificationUsesTomorrowTemplateValues(t *testing if req.BundleID != "tomorrow.tomorrow.md.tomorrow" { t.Fatalf("BundleID = %q, want artifact group, batch output name, and report id", req.BundleID) } - wantBundlePaths := []string{"2026-05-30/tomorrow/tomorrow.md"} + wantBundlePaths := []string{ + "daily/2026-05-30/" + result.Metadata.RunID + ".md", + "daily/2026-05-30/index.md", + "tomorrow/index.md", + } if strings.Join(req.BundlePaths, "\n") != strings.Join(wantBundlePaths, "\n") { t.Fatalf("BundlePaths = %#v, want %#v", req.BundlePaths, wantBundlePaths) } @@ -1234,7 +1240,8 @@ func TestGenerateReportNotifiesManagedReportPath(t *testing.T) { t.Fatalf("decode notification artifact: %v", err) } wantBundlePaths := []string{ - "2026-05-29/daily/2026-05-29-daily-" + result.Metadata.RunID + ".md", + "daily/2026-05-29/" + result.Metadata.RunID + ".md", + "daily/2026-05-29/index.md", } if notificationArtifact.PipelineID != "weatherreporter.daily" || strings.Join(notificationArtifact.BundlePaths, "\n") != strings.Join(wantBundlePaths, "\n") || notificationArtifact.BundleCreated.IsZero() || notificationArtifact.RunStatus == nil || !strings.Contains(string(notificationArtifact.RunStatus.Report), "replace_older") { t.Fatalf("notification artifact = %#v, want requested pipeline, status report, and created timestamp", notificationArtifact) @@ -2263,6 +2270,205 @@ func TestDistributorTemplateValuesLeaveStormIDEmptyForOtherReports(t *testing.T) } } +func TestBuildNotificationRequestUsesReportDefaultBundlePaths(t *testing.T) { + cfg := config.Defaults() + cfg.Location.ID = "home" + cfg.WeatherAPI.Timezone = "America/Chicago" + cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{report_id}" + location := mustLoadTestLocation(t, cfg.WeatherAPI.Timezone) + now := mustParse("2026-05-29T12:00:00-05:00") + registry := report.DefaultRegistry() + + tests := []struct { + id report.ID + req report.ResolveRequest + want func(state.Metadata) []string + source string + }{ + { + id: report.Hourly, + req: report.ResolveRequest{ + Now: now, + Location: location, + }, + want: func(metadata state.Metadata) []string { + return []string{"hourly/index.md"} + }, + source: "/managed/hourly.md", + }, + { + id: report.Daily, + req: report.ResolveRequest{ + Now: now, + Location: location, + Date: mustParse("2026-05-29T12:00:00-05:00"), + }, + want: func(metadata state.Metadata) []string { + return []string{ + "daily/2026-05-29/" + metadata.RunID + ".md", + "daily/2026-05-29/index.md", + } + }, + source: "/managed/daily.md", + }, + { + id: report.Today, + req: report.ResolveRequest{ + Now: now, + Location: location, + }, + want: func(metadata state.Metadata) []string { + return []string{ + "daily/2026-05-29/" + metadata.RunID + ".md", + "daily/2026-05-29/index.md", + "today/index.md", + } + }, + source: "/managed/today.md", + }, + { + id: report.Tomorrow, + req: report.ResolveRequest{ + Now: now, + Location: location, + }, + want: func(metadata state.Metadata) []string { + return []string{ + "daily/2026-05-30/" + metadata.RunID + ".md", + "daily/2026-05-30/index.md", + "tomorrow/index.md", + } + }, + source: "/managed/tomorrow.md", + }, + { + id: report.ThreeDay, + req: report.ResolveRequest{ + Now: now, + Location: location, + }, + want: func(metadata state.Metadata) []string { + return []string{ + "three-day/2026-05-29/" + metadata.RunID + ".md", + "three-day/2026-05-29/index.md", + } + }, + source: "/managed/three-day.md", + }, + { + id: report.Weekend, + req: report.ResolveRequest{ + Now: mustParse("2026-05-29T05:00:00-05:00"), + Location: location, + }, + want: func(metadata state.Metadata) []string { + return []string{ + "weekend/2026-05-29/" + metadata.RunID + ".md", + "weekend/2026-05-29/index.md", + } + }, + source: "/managed/weekend.md", + }, + { + id: report.Storm, + req: report.ResolveRequest{ + Now: now, + Location: location, + StormStart: mustParse("2026-05-29T18:00:00-05:00"), + StormEnd: mustParse("2026-05-30T06:00:00-05:00"), + }, + want: func(metadata state.Metadata) []string { + return []string{ + "storm/2026-05-29T1800-2026-05-30T0600/" + metadata.RunID + ".md", + "storm/2026-05-29T1800-2026-05-30T0600/index.md", + } + }, + source: "/managed/storm.md", + }, + } + + for _, tt := range tests { + t.Run(string(tt.id), func(t *testing.T) { + resolved, err := registry.Resolve(tt.id, tt.req) + if err != nil { + t.Fatalf("Resolve() error = %v", err) + } + metadata := notificationMetadataForTest(resolved) + req, err := buildNotificationRequest(cfg, resolved, tt.source, metadata) + if err != nil { + t.Fatalf("buildNotificationRequest() error = %v", err) + } + want := tt.want(metadata) + if strings.Join(req.BundlePaths, "\n") != strings.Join(want, "\n") { + t.Fatalf("BundlePaths = %#v, want %#v", req.BundlePaths, want) + } + if req.ReportPath != tt.source { + t.Fatalf("ReportPath = %q, want %q", req.ReportPath, tt.source) + } + }) + } +} + +func TestBuildNotificationRequestUsesReportDistributorPathOverride(t *testing.T) { + cfg := config.Defaults() + cfg.Location.ID = "home" + cfg.WeatherAPI.Timezone = "America/Chicago" + cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{report_id}" + applyReportDistributorPathOverrides(t, &cfg, ` +reports: + daily: + distributor: + path_templates: + - "custom/{report_id}/{run_id}.md" +`) + location := mustLoadTestLocation(t, cfg.WeatherAPI.Timezone) + resolved, err := report.Resolve(report.Daily, report.ResolveRequest{ + Now: mustParse("2026-05-29T12:00:00-05:00"), + Location: location, + Date: mustParse("2026-05-29T12:00:00-05:00"), + }) + if err != nil { + t.Fatalf("Resolve() error = %v", err) + } + metadata := notificationMetadataForTest(resolved) + + req, err := buildNotificationRequest(cfg, resolved, "/managed/daily.md", metadata) + if err != nil { + t.Fatalf("buildNotificationRequest() error = %v", err) + } + want := []string{"custom/daily/" + metadata.RunID + ".md"} + if strings.Join(req.BundlePaths, "\n") != strings.Join(want, "\n") { + t.Fatalf("BundlePaths = %#v, want override %#v", req.BundlePaths, want) + } +} + +func TestBuildNotificationRequestRequiresReportBundlePaths(t *testing.T) { + cfg := config.Defaults() + cfg.Location.ID = "home" + cfg.WeatherAPI.Timezone = "America/Chicago" + cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{report_id}" + location := mustLoadTestLocation(t, cfg.WeatherAPI.Timezone) + resolved, err := report.Resolve(report.Hourly, report.ResolveRequest{ + Now: mustParse("2026-05-29T12:00:00-05:00"), + Location: location, + }) + if err != nil { + t.Fatalf("Resolve() error = %v", err) + } + resolved.Definition.DistributorPathTemplates = nil + metadata := notificationMetadataForTest(resolved) + + _, err = buildNotificationRequest(cfg, resolved, "/managed/hourly.md", metadata) + if err == nil { + t.Fatal("buildNotificationRequest() error = nil, want missing path templates error") + } + for _, want := range []string{`report "hourly"`, metadata.RunID, `/managed/hourly.md`, "no distributor path templates"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error = %q, want %q", err.Error(), want) + } + } +} + func TestBatchRunIDUsesUTCStartAndBatchName(t *testing.T) { tests := []struct { name string @@ -2382,10 +2588,6 @@ func TestBatchResultJSONIncludesNotification(t *testing.T) { func TestBuildBatchNotificationRequestIncludesEveningReports(t *testing.T) { server := dailyBundleServer(t) cfg := dailyNotificationConfig(t, server) - cfg.Notify.Distributor.ReportPathTemplates = []string{ - "archive/{valid_start_date}/{artifact_group}/{run_id}.md", - "latest/{batch_output_name}", - } cfg.Notify.Distributor.Batch.PipelineIDTemplate = "weatherreporter.{batch}.{batch_started_date}" cfg.Notify.Distributor.Batch.BundleIDTemplate = "weatherreporter.{location_id}.{batch}" cfg.Notify.Distributor.Batch.IdempotencyKeyTemplate = "{bundle_id}.{batch_run_id}" @@ -2417,20 +2619,26 @@ func TestBuildBatchNotificationRequestIncludesEveningReports(t *testing.T) { if len(req.IncludedReports) != 3 { t.Fatalf("IncludedReports = %d, want 3", len(req.IncludedReports)) } - if len(req.Files) != 6 { - t.Fatalf("Files = %d, want two mappings per report", len(req.Files)) + if len(req.Files) != 7 { + t.Fatalf("Files = %d, want report-specific mappings", len(req.Files)) } wantBundlePaths := map[string]struct{}{} for _, plannedReport := range planned { resolved := plannedReport.Resolved runID := resolved.Metadata().RunID - outputName := plannedReport.OutputCopyName - if outputName == "" { - outputName = resolved.Definition.BatchOutputName + validStart := resolved.ValidPeriod.Start.In(mustLoadTestLocation(t, cfg.WeatherAPI.Timezone)).Format(timeutil.DateLayout) + switch resolved.Definition.ID { + case report.Tomorrow: + wantBundlePaths["daily/"+validStart+"/"+runID+".md"] = struct{}{} + wantBundlePaths["daily/"+validStart+"/index.md"] = struct{}{} + wantBundlePaths["tomorrow/index.md"] = struct{}{} + case report.Daily: + wantBundlePaths["daily/"+validStart+"/"+runID+".md"] = struct{}{} + wantBundlePaths["daily/"+validStart+"/index.md"] = struct{}{} + default: + t.Fatalf("unexpected planned report %s", resolved.Definition.ID) } - wantBundlePaths[fmt.Sprintf("archive/%s/%s/%s.md", resolved.ValidPeriod.Start.In(mustLoadTestLocation(t, cfg.WeatherAPI.Timezone)).Format(timeutil.DateLayout), resolved.Definition.ArtifactGroup, runID)] = struct{}{} - wantBundlePaths["latest/"+outputName] = struct{}{} } gotBundlePaths := map[string]struct{}{} gotSourcePaths := map[string]struct{}{} @@ -2458,10 +2666,67 @@ func TestBuildBatchNotificationRequestIncludesEveningReports(t *testing.T) { } } +func TestBuildBatchNotificationRequestUsesReportOverridesAndDefaults(t *testing.T) { + server := dailyBundleServer(t) + cfg := dailyNotificationConfig(t, server) + applyReportDistributorPathOverrides(t, &cfg, ` +reports: + daily: + distributor: + path_templates: + - "custom-daily/{valid_start_date}/{run_id}.md" +`) + startedAt := mustParse("2026-05-29T18:00:00-05:00") + planned, reports := plannedBatchNotificationReports(t, cfg, BatchEvening, startedAt, "2026-05-31") + runID := batchRunID(startedAt, BatchEvening) + + req, err := buildBatchNotificationRequest(cfg, BatchEvening, runID, startedAt, reports, planned) + if err != nil { + t.Fatalf("buildBatchNotificationRequest() error = %v", err) + } + gotBundlePaths := map[string]struct{}{} + for _, file := range req.Files { + gotBundlePaths[file.BundlePath] = struct{}{} + } + for _, plannedReport := range planned { + reportRunID := plannedReport.Resolved.Metadata().RunID + switch plannedReport.Resolved.Definition.ID { + case report.Tomorrow: + for _, want := range []string{ + "daily/2026-05-30/" + reportRunID + ".md", + "daily/2026-05-30/index.md", + "tomorrow/index.md", + } { + if _, ok := gotBundlePaths[want]; !ok { + t.Fatalf("bundle paths = %#v, missing default path %q", gotBundlePaths, want) + } + } + case report.Daily: + want := "custom-daily/2026-05-31/" + reportRunID + ".md" + if _, ok := gotBundlePaths[want]; !ok { + t.Fatalf("bundle paths = %#v, missing override path %q", gotBundlePaths, want) + } + if _, ok := gotBundlePaths["daily/2026-05-31/index.md"]; ok { + t.Fatalf("bundle paths = %#v, want daily defaults replaced by override", gotBundlePaths) + } + } + } +} + func TestBuildBatchNotificationRequestRejectsDuplicateBundlePaths(t *testing.T) { server := dailyBundleServer(t) cfg := dailyNotificationConfig(t, server) - cfg.Notify.Distributor.ReportPathTemplates = []string{"index.md"} + applyReportDistributorPathOverrides(t, &cfg, ` +reports: + tomorrow: + distributor: + path_templates: + - "index.md" + daily: + distributor: + path_templates: + - "index.md" +`) startedAt := mustParse("2026-05-29T18:00:00-05:00") planned, reports := plannedBatchNotificationReports(t, cfg, BatchEvening, startedAt, "2026-05-31") @@ -2586,16 +2851,13 @@ func TestRunBatchMorningSendsOneBatchNotification(t *testing.T) { if req.Batch != BatchMorning || req.RunID != "20260529T100000.000000000Z_morning" { t.Fatalf("batch request identity = %s/%s, want morning run id", req.Batch, req.RunID) } - if len(req.IncludedReports) != 3 || len(req.Files) != 3 { - t.Fatalf("batch request reports/files = %d/%d, want 3/3", len(req.IncludedReports), len(req.Files)) + if len(req.IncludedReports) != 3 || len(req.Files) != 8 { + t.Fatalf("batch request reports/files = %d/%d, want 3/8", len(req.IncludedReports), len(req.Files)) } for _, file := range req.Files { if file.SourcePath == "" || file.BundlePath == "" { t.Fatalf("batch file = %#v, want source and bundle path", file) } - if !strings.Contains(file.BundlePath, file.RunID) { - t.Fatalf("bundle path %q does not include report run id %q", file.BundlePath, file.RunID) - } } if result.Notification == nil || result.Notification.Status != "succeeded" || result.Notification.RunID != "batch-distributor-run" || result.Notification.Path == "" { t.Fatalf("batch notification = %#v, want succeeded result with artifact path", result.Notification) @@ -3047,6 +3309,36 @@ func dailyNotificationConfig(t *testing.T, server *httptest.Server) config.Confi return cfg } +func notificationMetadataForTest(resolved report.Resolved) state.Metadata { + metadata := resolved.Metadata() + return state.Metadata{ + RunID: metadata.RunID, + ReportID: metadata.ReportID, + PromptID: metadata.PromptID, + GeneratedAt: metadata.GeneratedAt, + Timezone: metadata.Timezone, + ValidPeriod: metadata.ValidPeriod, + } +} + +func applyReportDistributorPathOverrides(t *testing.T, cfg *config.Config, data string) { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(path, []byte(data), 0o600); err != nil { + t.Fatalf("write config fixture: %v", err) + } + loaded, err := config.LoadFile(path) + if err != nil { + t.Fatalf("LoadFile() error = %v", err) + } + if cfg.Reports == nil { + cfg.Reports = map[string]config.ReportConfig{} + } + for key, reportCfg := range loaded.Reports { + cfg.Reports[key] = reportCfg + } +} + func plannedBatchNotificationReports(t *testing.T, cfg config.Config, batch BatchKind, now time.Time, futureDailyDates ...string) ([]plannedBatchReport, []BatchReportResult) { t.Helper() collection := collectionWithFutureDailyForTest(t, cfg, futureDailyDates...) @@ -3189,7 +3481,6 @@ func applyHourlyGeneratedTextSettings(cfg *config.Config, t *testing.T, server * cfg.Notify.Distributor.PipelineIDTemplate = "weatherreporter.{artifact_group}" cfg.Notify.Distributor.BundleIDTemplate = "weatherreporter.{location_id}.{report_id}" cfg.Notify.Distributor.IdempotencyKeyTemplate = "weatherreporter.{location_id}.{report_id}.{run_id}" - cfg.Notify.Distributor.ReportPathTemplates = []string{"{valid_start_date}/{artifact_group}/{batch_output_name}"} } func resolveHourlyGeneratedTextFixture(t *testing.T, cfg config.Config) (report.Resolved, *recordingStore, *recordingNotifier, string) { diff --git a/internal/app/batch_notification.go b/internal/app/batch_notification.go index cdec654..c82ee35 100644 --- a/internal/app/batch_notification.go +++ b/internal/app/batch_notification.go @@ -161,9 +161,9 @@ func buildBatchNotificationRequest(cfg config.Config, batch BatchKind, runID str if err != nil { return batchNotificationRequest{}, fmt.Errorf("batch notification report %q run %q source path %q: %w", item.ReportID, item.RunID, item.ReportPath, err) } - bundlePaths, err := config.RenderDistributorReportPaths("notify.distributor.report_path_templates", cfg.Notify.Distributor.ReportPathTemplates, values) + bundlePaths, err := renderDistributorReportBundlePaths(cfg, plannedReport.Resolved, item.RunID, item.ReportPath, values) if err != nil { - return batchNotificationRequest{}, fmt.Errorf("batch notification report %q run %q source path %q: %w", item.ReportID, item.RunID, item.ReportPath, err) + return batchNotificationRequest{}, err } included := BatchNotificationReport{