From 4521606d7b64234eb8377c3edb8b15fbc4ed97d7 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sat, 28 Feb 2026 09:38:15 +0200 Subject: refactor: extract report config, flags, and rendering into internal package Add ReportConfig, ReportFlags, RegisterReportFlags, and WriteReports to the internal goprecords package. Both runQuery and runReportFromFiles now use these shared functions, eliminating duplicated flag definitions and report-rendering loops. main.go reduced from 313 to 220 lines. Task: d8f7af80-1aca-4dea-9a20-b8f95640acb7 Amp-Thread-ID: https://ampcode.com/threads/T-019ca323-dde1-73ac-97f0-cebfae5922a5 Co-authored-by: Amp --- cmd/goprecords/main.go | 105 +++---------------------------------- internal/goprecords/report.go | 98 ++++++++++++++++++++++++++++++++++ internal/goprecords/report_test.go | 79 ++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 99 deletions(-) diff --git a/cmd/goprecords/main.go b/cmd/goprecords/main.go index c608f78..1bec690 100644 --- a/cmd/goprecords/main.go +++ b/cmd/goprecords/main.go @@ -68,13 +68,7 @@ func runImport(args []string) { func runQuery(args []string) { fs := flag.NewFlagSet("query", flag.ExitOnError) dbPath := fs.String("db", defaultDB, "SQLite database path") - category := fs.String("category", "Host", "Category: Host, Kernel, KernelMajor, KernelName") - metric := fs.String("metric", "Uptime", "Metric: Boots, Uptime, Score, Downtime, Lifespan") - limit := fs.Uint("limit", 20, "Limit output to num of entries") - outputFormat := fs.String("output-format", "Plaintext", "Output format: Plaintext, Markdown, Gemtext") - all := fs.Bool("all", false, "Generate all possible stats but Kernel") - includeKernel := fs.Bool("include-kernel", false, "Also include Kernel when using -all") - statsOrder := fs.String("stats-order", "", "Comma-separated Category:Metric order for -all") + rf := goprecords.RegisterReportFlags(fs) fs.Parse(args) db, err := goprecords.OpenDB(*dbPath) @@ -91,56 +85,15 @@ func runQuery(args []string) { os.Exit(1) } - cat, err := goprecords.ParseCategory(*category) + cfg, err := rf.Parse() if err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } - met, err := goprecords.ParseMetric(*metric) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - outFmt, err := goprecords.ParseOutputFormat(*outputFormat) - if err != nil { + if err := goprecords.WriteReports(os.Stdout, aggregates, cfg); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } - - if !*all { - if cat != goprecords.CategoryHost && (met == goprecords.MetricDowntime || met == goprecords.MetricLifespan) { - fmt.Fprintf(os.Stderr, "Category %s only supports: Boots, Uptime, Score\n", *category) - os.Exit(1) - } - if cat == goprecords.CategoryHost { - os.Stdout.WriteString(goprecords.NewHostReporter(aggregates, *limit, met, outFmt, 1).Report()) - } else { - os.Stdout.WriteString(goprecords.NewReporter(aggregates, cat, *limit, met, outFmt, 1).Report()) - } - return - } - - order, err := goprecords.StatsOrderList(*statsOrder) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - headerIndent := uint(2) - for _, pair := range order { - c, m := pair.Category, pair.Metric - if !*includeKernel && c == goprecords.CategoryKernel { - continue - } - if c != goprecords.CategoryHost && (m == goprecords.MetricDowntime || m == goprecords.MetricLifespan) { - continue - } - if c == goprecords.CategoryHost { - os.Stdout.WriteString(goprecords.NewHostReporter(aggregates, *limit, m, outFmt, headerIndent).Report()) - } else { - os.Stdout.WriteString(goprecords.NewReporter(aggregates, c, *limit, m, outFmt, headerIndent).Report()) - } - os.Stdout.WriteString("\n") - } } func runReportFromFiles(args []string) { @@ -149,13 +102,7 @@ func runReportFromFiles(args []string) { } fs := flag.NewFlagSet("goprecords", flag.ExitOnError) statsDir := fs.String("stats-dir", "", "The uptimed raw record input dir (required)") - category := fs.String("category", "Host", "Category: Host, Kernel, KernelMajor, KernelName") - metric := fs.String("metric", "Uptime", "Metric: Boots, Uptime, Score, Downtime, Lifespan") - limit := fs.Uint("limit", 20, "Limit output to num of entries") - outputFormat := fs.String("output-format", "Plaintext", "Output format: Plaintext, Markdown, Gemtext") - all := fs.Bool("all", false, "Generate all possible stats but Kernel") - includeKernel := fs.Bool("include-kernel", false, "Also include Kernel when using -all") - statsOrder := fs.String("stats-order", "", "Comma-separated Category:Metric order for -all") + rf := goprecords.RegisterReportFlags(fs) fs.Parse(args) if *statsDir == "" { @@ -164,17 +111,7 @@ func runReportFromFiles(args []string) { os.Exit(1) } - cat, err := goprecords.ParseCategory(*category) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - met, err := goprecords.ParseMetric(*metric) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - outFmt, err := goprecords.ParseOutputFormat(*outputFormat) + cfg, err := rf.Parse() if err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) @@ -188,40 +125,10 @@ func runReportFromFiles(args []string) { os.Exit(1) } - if !*all { - if cat != goprecords.CategoryHost && (met == goprecords.MetricDowntime || met == goprecords.MetricLifespan) { - fmt.Fprintf(os.Stderr, "Category %s only supports: Boots, Uptime, Score\n", *category) - os.Exit(1) - } - if cat == goprecords.CategoryHost { - os.Stdout.WriteString(goprecords.NewHostReporter(aggregates, *limit, met, outFmt, 1).Report()) - } else { - os.Stdout.WriteString(goprecords.NewReporter(aggregates, cat, *limit, met, outFmt, 1).Report()) - } - return - } - - order, err := goprecords.StatsOrderList(*statsOrder) - if err != nil { + if err := goprecords.WriteReports(os.Stdout, aggregates, cfg); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } - headerIndent := uint(2) - for _, pair := range order { - c, m := pair.Category, pair.Metric - if !*includeKernel && c == goprecords.CategoryKernel { - continue - } - if c != goprecords.CategoryHost && (m == goprecords.MetricDowntime || m == goprecords.MetricLifespan) { - continue - } - if c == goprecords.CategoryHost { - os.Stdout.WriteString(goprecords.NewHostReporter(aggregates, *limit, m, outFmt, headerIndent).Report()) - } else { - os.Stdout.WriteString(goprecords.NewReporter(aggregates, c, *limit, m, outFmt, headerIndent).Report()) - } - os.Stdout.WriteString("\n") - } } func runTests() { diff --git a/internal/goprecords/report.go b/internal/goprecords/report.go index 01ca570..0b4454d 100644 --- a/internal/goprecords/report.go +++ b/internal/goprecords/report.go @@ -1,11 +1,109 @@ package goprecords import ( + "flag" "fmt" + "io" "sort" "strings" ) +// ReportConfig holds parsed report configuration. +type ReportConfig struct { + Category Category + Metric Metric + Limit uint + OutputFormat OutputFormat + All bool + IncludeKernel bool + StatsOrder string +} + +// ReportFlags holds flag pointers registered on a FlagSet. +type ReportFlags struct { + category *string + metric *string + limit *uint + outputFormat *string + all *bool + includeKernel *bool + statsOrder *string +} + +// RegisterReportFlags registers common report flags on the given FlagSet. +func RegisterReportFlags(fs *flag.FlagSet) *ReportFlags { + return &ReportFlags{ + category: fs.String("category", "Host", "Category: Host, Kernel, KernelMajor, KernelName"), + metric: fs.String("metric", "Uptime", "Metric: Boots, Uptime, Score, Downtime, Lifespan"), + limit: fs.Uint("limit", 20, "Limit output to num of entries"), + outputFormat: fs.String("output-format", "Plaintext", "Output format: Plaintext, Markdown, Gemtext"), + all: fs.Bool("all", false, "Generate all possible stats but Kernel"), + includeKernel: fs.Bool("include-kernel", false, "Also include Kernel when using -all"), + statsOrder: fs.String("stats-order", "", "Comma-separated Category:Metric order for -all"), + } +} + +// Parse converts flag values into a ReportConfig. +func (rf *ReportFlags) Parse() (ReportConfig, error) { + cat, err := ParseCategory(*rf.category) + if err != nil { + return ReportConfig{}, err + } + met, err := ParseMetric(*rf.metric) + if err != nil { + return ReportConfig{}, err + } + outFmt, err := ParseOutputFormat(*rf.outputFormat) + if err != nil { + return ReportConfig{}, err + } + return ReportConfig{ + Category: cat, + Metric: met, + Limit: *rf.limit, + OutputFormat: outFmt, + All: *rf.all, + IncludeKernel: *rf.includeKernel, + StatsOrder: *rf.statsOrder, + }, nil +} + +// WriteReports renders reports to w based on the given config. +func WriteReports(w io.Writer, aggregates *Aggregates, cfg ReportConfig) error { + if !cfg.All { + if cfg.Category != CategoryHost && (cfg.Metric == MetricDowntime || cfg.Metric == MetricLifespan) { + return fmt.Errorf("Category %s only supports: Boots, Uptime, Score", cfg.Category) + } + if cfg.Category == CategoryHost { + io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()) + } else { + io.WriteString(w, NewReporter(aggregates, cfg.Category, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()) + } + return nil + } + order, err := StatsOrderList(cfg.StatsOrder) + if err != nil { + return err + } + headerIndent := uint(2) + for _, pair := range order { + c, m := pair.Category, pair.Metric + if !cfg.IncludeKernel && c == CategoryKernel { + continue + } + if c != CategoryHost && (m == MetricDowntime || m == MetricLifespan) { + continue + } + if c == CategoryHost { + io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()) + } else { + io.WriteString(w, NewReporter(aggregates, c, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()) + } + io.WriteString(w, "\n") + } + return nil +} + // Reporter builds a single report (category + metric + format). type Reporter struct { aggregates *Aggregates diff --git a/internal/goprecords/report_test.go b/internal/goprecords/report_test.go index 651e887..f6bdab9 100644 --- a/internal/goprecords/report_test.go +++ b/internal/goprecords/report_test.go @@ -1,6 +1,7 @@ package goprecords import ( + "bytes" "strings" "testing" ) @@ -217,6 +218,84 @@ func TestReportLimit(t *testing.T) { } } +func TestWriteReportsSingle(t *testing.T) { + aggs := testAggregates() + var buf bytes.Buffer + cfg := ReportConfig{ + Category: CategoryHost, + Metric: MetricUptime, + Limit: 20, + OutputFormat: FormatPlaintext, + } + if err := WriteReports(&buf, aggs, cfg); err != nil { + t.Fatalf("WriteReports: %v", err) + } + if !strings.Contains(buf.String(), "host1") { + t.Error("expected output to contain host1") + } +} + +func TestWriteReportsAll(t *testing.T) { + aggs := testAggregates() + var buf bytes.Buffer + cfg := ReportConfig{ + Category: CategoryHost, + Metric: MetricUptime, + Limit: 20, + OutputFormat: FormatPlaintext, + All: true, + IncludeKernel: true, + } + if err := WriteReports(&buf, aggs, cfg); err != nil { + t.Fatalf("WriteReports: %v", err) + } + out := buf.String() + if !strings.Contains(out, "Uptime") { + t.Error("expected output to contain Uptime report") + } + if !strings.Contains(out, "Boots") { + t.Error("expected output to contain Boots report") + } +} + +func TestWriteReportsInvalidMetricForCategory(t *testing.T) { + aggs := testAggregates() + var buf bytes.Buffer + cfg := ReportConfig{ + Category: CategoryKernel, + Metric: MetricDowntime, + Limit: 20, + OutputFormat: FormatPlaintext, + } + err := WriteReports(&buf, aggs, cfg) + if err == nil { + t.Fatal("expected error for Downtime on Kernel category") + } + if !strings.Contains(err.Error(), "only supports") { + t.Errorf("unexpected error: %v", err) + } +} + +func testAggregates() *Aggregates { + aggs := &Aggregates{ + Host: make(map[string]*HostAggregate), + Kernel: make(map[string]*Aggregate), + KernelMajor: make(map[string]*Aggregate), + KernelName: make(map[string]*Aggregate), + } + hagg := NewHostAggregate("host1", "Linux 5.10") + hagg.Uptime = 86400000 + hagg.Boots = 10 + hagg.FirstBoot = 1000 + hagg.LastSeen = 86401000 + aggs.Host["host1"] = hagg + kernel := NewAggregate("Linux 5.10") + kernel.Uptime = 86400000 + kernel.Boots = 10 + aggs.Kernel["Linux 5.10"] = kernel + return aggs +} + func hostName(i int) string { switch i { case 0: -- cgit v1.2.3