diff --git a/cmd/jobsearch/main_test.go b/cmd/jobsearch/main_test.go index bde0dce..b7d83d1 100644 --- a/cmd/jobsearch/main_test.go +++ b/cmd/jobsearch/main_test.go @@ -37,11 +37,16 @@ func TestVersion(t *testing.T) { // flags and hooks, and the subcommand beneath it. func TestRootRunsCommands(t *testing.T) { cmdtest.Setup(t, "logging:\n level: info\n") - cmdtest.Contains(t, cmdtest.MustRun(t, newRootCmd, "config", "get", "logging.level"), - "info ") - cmdtest.Contains(t, + cmdtest.Contains( + t, + cmdtest.MustRun(t, newRootCmd, "config", "get", "logging.level"), + "info ", + ) + cmdtest.Contains( + t, cmdtest.MustRun(t, newRootCmd, "config", "get", "logging.level", "--debug"), - "debug ") + "debug ", + ) } func TestUnknownCommand(t *testing.T) { diff --git a/cmd/mock-fts/main.go b/cmd/mock-fts/main.go index 09edf93..8521fd1 100644 --- a/cmd/mock-fts/main.go +++ b/cmd/mock-fts/main.go @@ -107,7 +107,7 @@ func main() { } func newRootCmd() *cobra.Command { - o := &options{} + opts := &options{} cmd := &cobra.Command{ Use: "fts-mock", @@ -120,53 +120,68 @@ func newRootCmd() *cobra.Command { fts-mock --data-dir ./mock-data --reset fts-mock --min-gap 5s --max-gap 10s --rate-limit 1000`, PreRunE: func(cmd *cobra.Command, args []string) error { - return o.validate() + return opts.validate() }, RunE: func(cmd *cobra.Command, args []string) error { - return run(cmd.Context(), o) + return run(cmd.Context(), opts) }, } - f := cmd.Flags() - f.SortFlags = false // show flags in the order defined here + flags := cmd.Flags() + flags.SortFlags = false // show flags in the order defined here - f.StringVar(&o.addr, "addr", ":8080", "listen address") - f.StringVar( - &o.baseURL, + flags.StringVar(&opts.addr, "addr", ":8080", "listen address") + flags.StringVar( + &opts.baseURL, "base-url", "http://localhost:8080", "public base URL used in uri and links.next", ) - f.StringVar( - &o.dataDir, + flags.StringVar( + &opts.dataDir, "data-dir", "./mock-data", "directory where releases.jsonl will be stored", ) - f.BoolVar( - &o.reset, + flags.BoolVar( + &opts.reset, "reset", false, "delete stored releases and regenerate the --backfill history", ) - f.DurationVar( - &o.backfill, + flags.DurationVar( + &opts.backfill, "backfill", 72*time.Hour, "history to generate when the data directory is empty", ) - f.DurationVar(&o.minGap, "min-gap", 30*time.Second, "minimum time between generated releases") - f.DurationVar(&o.maxGap, "max-gap", 5*time.Minute, "maximum time between generated releases") + flags.DurationVar( + &opts.minGap, + "min-gap", + 30*time.Second, + "minimum time between generated releases", + ) + flags.DurationVar( + &opts.maxGap, + "max-gap", + 5*time.Minute, + "maximum time between generated releases", + ) - f.IntVar(&o.rateLimit, "rate-limit", 12, "requests allowed per window per client") - f.DurationVar(&o.rateWindow, "rate-window", 5*time.Minute, "rate limit window length") + flags.IntVar(&opts.rateLimit, "rate-limit", 12, "requests allowed per window per client") + flags.DurationVar(&opts.rateWindow, "rate-window", 5*time.Minute, "rate limit window length") - f.StringVar(&o.accessLogPath, "access-log", "-", `access log file, or "-" for stdout`) - f.StringVar(&o.logFormat, "log-format", "combined", `access log format: "combined" or "common"`) - f.StringVar( - &o.tz, + flags.StringVar(&opts.accessLogPath, "access-log", "-", `access log file, or "-" for stdout`) + flags.StringVar( + &opts.logFormat, + "log-format", + "combined", + `access log format: "combined" or "common"`, + ) + flags.StringVar( + &opts.tz, "tz", "Europe/London", `time zone for updatedFrom/updatedTo without an offset ("Local" for this machine's)`, @@ -193,32 +208,32 @@ func (o *options) validate() error { } // run starts the mock and blocks until ctx is cancelled or the server fails. -func run(ctx context.Context, o *options) (err error) { - loc, err := time.LoadLocation(o.tz) // already validated +func run(ctx context.Context, opts *options) (err error) { + location, err := time.LoadLocation(opts.tz) // already validated if err != nil { return fmt.Errorf("--tz: %w", err) } - if err := os.MkdirAll(o.dataDir, 0o750); err != nil { + if err := os.MkdirAll(opts.dataDir, 0o750); err != nil { return err } - // ----- catalog ----- - cat, err := mock.LoadCatalog() + // ---------- catalog ---------- + catalog, err := mock.LoadCatalog() if err != nil { return err } slog.Info( "catalog loaded", - "buyers", len(cat.Buyers), - "suppliers", len(cat.Suppliers), - "subjects", len(cat.Subjects), - "methods", len(cat.Methods), - "classifications", len(cat.Classifications), + "buyers", len(catalog.Buyers), + "suppliers", len(catalog.Suppliers), + "subjects", len(catalog.Subjects), + "methods", len(catalog.Methods), + "classifications", len(catalog.Classifications), ) - // ----- store ----- - storePath := filepath.Join(o.dataDir, mock.ReleasesFile) - if o.reset { + // ---------- store ---------- + storePath := filepath.Join(opts.dataDir, mock.ReleasesFile) + if opts.reset { if err := os.Remove(storePath); err != nil && !errors.Is(err, os.ErrNotExist) { return err } @@ -228,16 +243,18 @@ func run(ctx context.Context, o *options) (err error) { if err != nil { return err } - defer func() { err = errors.Join(err, store.Close()) }() + defer func() { + err = errors.Join(err, store.Close()) + }() - // ----- generator ----- - gen := mock.NewGenerator(cat, store, time.Now().UnixNano()) - n, err := gen.CatchUp(time.Now(), o.backfill, o.minGap, o.maxGap) + // ---------- generator ---------- + generator := mock.NewGenerator(catalog, store, time.Now().UnixNano()) + generated, err := generator.CatchUp(time.Now(), opts.backfill, opts.minGap, opts.maxGap) if err != nil { return err } - slog.Info("store ready", "releases", store.Len(), "generated", n) + slog.Info("store ready", "releases", store.Len(), "generated", generated) // The generator and server share a context that also ends if the server // fails, so the generator is never left running after run returns. @@ -247,34 +264,36 @@ func run(ctx context.Context, o *options) (err error) { workerDone := make(chan struct{}) go func() { defer close(workerDone) - gen.Run(ctx, o.minGap, o.maxGap) + generator.Run(ctx, opts.minGap, opts.maxGap) }() - // ----- HTTP ----- + // ---------- HTTP ---------- out := os.Stdout - if o.accessLogPath != "-" { - f, err := os.OpenFile( - o.accessLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600, + if opts.accessLogPath != "-" { + logFile, err := os.OpenFile( + opts.accessLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600, ) if err != nil { cancel() <-workerDone return fmt.Errorf("opening access log: %w", err) } - defer func() { err = errors.Join(err, f.Close()) }() - out = f + defer func() { + err = errors.Join(err, logFile.Close()) + }() + out = logFile } accessLogger := log.New(out, "", 0) - rl := mock.NewRateLimiter(o.rateLimit, o.rateWindow) + rateLimiter := mock.NewRateLimiter(opts.rateLimit, opts.rateWindow) mux := http.NewServeMux() mux.HandleFunc( - mock.BasePath, rl.Middleware(mock.NewHandler(store, o.baseURL, loc)), + mock.BasePath, rateLimiter.Middleware(mock.NewHandler(store, opts.baseURL, location)), ) httpSrv := &http.Server{ - Addr: o.addr, - Handler: logging.AccessLog(accessLogger, o.logFormat == "combined", mux), + Addr: opts.addr, + Handler: logging.AccessLog(accessLogger, opts.logFormat == "combined", mux), ReadHeaderTimeout: 10 * time.Second, } go func() { @@ -289,7 +308,12 @@ func run(ctx context.Context, o *options) (err error) { } }() - slog.Info("mock listening", "addr", o.addr, "path", mock.BasePath, "time_zone", loc.String()) + slog.Info( + "mock listening", + "addr", opts.addr, + "path", mock.BasePath, + "time_zone", location.String(), + ) serveErr := httpSrv.ListenAndServe() cancel() // stop the worker (and the shutdown goroutine) if the server failed diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 95ad006..66a18eb 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -48,7 +48,9 @@ func writeFile(t *testing.T, path, contents string) { // root command has, after parsing args. func newCommand(t *testing.T, opts *Options, args ...string) *cobra.Command { t.Helper() - cmd := &cobra.Command{Use: "test"} + cmd := &cobra.Command{ + Use: "test", + } opts.Bind(cmd) if err := cmd.ParseFlags(args); err != nil { t.Fatal(err) @@ -203,7 +205,9 @@ func confirmWithInput(t *testing.T, input string) bool { oldIn, oldOut := os.Stdin, os.Stdout os.Stdin, os.Stdout = stdin, stdout - defer func() { os.Stdin, os.Stdout = oldIn, oldOut }() + defer func() { + os.Stdin, os.Stdout = oldIn, oldOut + }() return Confirm("Proceed?") } diff --git a/internal/cli/config/config_test.go b/internal/cli/config/config_test.go index 32ea395..ec11cee 100644 --- a/internal/cli/config/config_test.go +++ b/internal/cli/config/config_test.go @@ -63,8 +63,12 @@ func TestConfigShow(t *testing.T) { cmdtest.Setup(t, appConfig) out := mustRun(t, "config", "show") - cmdtest.Contains(t, out, - "logging.level = info ", "sources.sleeper.interval = 2m0s ") + cmdtest.Contains( + t, + out, + "logging.level = info ", + "sources.sleeper.interval = 2m0s ", + ) // Keys are listed in order, the same every time. if again := mustRun(t, "config", "show"); again != out { t.Errorf("config show output changed between runs:\n%s\n---\n%s", out, again) @@ -158,8 +162,11 @@ func TestConfigSetUnset(t *testing.T) { t.Errorf("config.yaml settings = %v; want %v", got, want) } // Back to the default. - cmdtest.Contains(t, mustRun(t, "config", "get", "sources.sleeper.interval"), - "10m0s ") + cmdtest.Contains( + t, + mustRun(t, "config", "get", "sources.sleeper.interval"), + "10m0s ", + ) } func TestConfigSetWithoutConfigFile(t *testing.T) { diff --git a/internal/cli/config/get.go b/internal/cli/config/get.go index 9eb1d0d..a636c14 100644 --- a/internal/cli/config/get.go +++ b/internal/cli/config/get.go @@ -32,12 +32,12 @@ func commandGet(opts *cli.Options) *cobra.Command { return fmt.Errorf("no configuration loaded (%w)", err) } - v, err := conf.ValueOf(args[0]) + value, err := conf.ValueOf(args[0]) if err != nil { return err } - t, _ := conf.TypeOf(args[0]) - fmt.Println(v, "<"+t.String()+">") + kind, _ := conf.TypeOf(args[0]) + fmt.Println(value, "<"+kind.String()+">") return nil }, diff --git a/internal/cli/config/init.go b/internal/cli/config/init.go index 4fbd850..5b89e69 100644 --- a/internal/cli/config/init.go +++ b/internal/cli/config/init.go @@ -74,7 +74,8 @@ func commandInit() *cobra.Command { fmt.Print( "writing default configuration to file: " + viper.ConfigFileUsed() + - "\n") + "\n", + ) if err := viper.WriteConfig(); err != nil { return err diff --git a/internal/cli/confirm_action.go b/internal/cli/confirm_action.go index 186e552..a282625 100644 --- a/internal/cli/confirm_action.go +++ b/internal/cli/confirm_action.go @@ -8,14 +8,14 @@ import ( "strings" ) -// Confirm asks the user the yes/no question s on stdin and reports whether +// Confirm asks the user the yes/no question prompt on stdin and reports whether // they answered yes. "y" and "yes" confirm, "n" and "no" refuse, in any case; // anything else asks again. It exits the program if stdin can't be read. -func Confirm(s string) bool { +func Confirm(prompt string) bool { reader := bufio.NewReader(os.Stdin) for { - fmt.Printf("%s [y/n]: ", s) + fmt.Printf("%s [y/n]: ", prompt) response, err := reader.ReadString('\n') if err != nil { diff --git a/internal/cli/fetch/once.go b/internal/cli/fetch/once.go index b132eac..b111887 100644 --- a/internal/cli/fetch/once.go +++ b/internal/cli/fetch/once.go @@ -29,23 +29,23 @@ var stageTags = map[string][]string{ // filter keeps the opportunities whose latest release is at stage. Releases // are oldest first, so the last one is the procedure's current stage. -func filter(opps []*types.Opportunity, stage string) []*types.Opportunity { - var resp []*types.Opportunity +func filter(opportunities []*types.Opportunity, stage string) []*types.Opportunity { + var matching []*types.Opportunity - for _, opp := range opps { - if len(opp.Releases) == 0 { + for _, opportunity := range opportunities { + if len(opportunity.Releases) == 0 { continue } - latest := opp.Releases[len(opp.Releases)-1] + latest := opportunity.Releases[len(opportunity.Releases)-1] for _, tag := range latest.Tags { if slices.Contains(stageTags[stage], tag.Tag) { - resp = append(resp, opp) + matching = append(matching, opportunity) break } } } - return resp + return matching } func commandOnce(opts *cli.Options) *cobra.Command { @@ -71,12 +71,12 @@ func commandOnce(opts *cli.Options) *cobra.Command { return sinceErr } - ft := findatender.New(conf, conf.Sources.FindATender.APIRoot) - defer ft.Shutdown() + findATender := findatender.New(conf, conf.Sources.FindATender.APIRoot) + defer findATender.Shutdown() ignoreRateLimit, _ := cmd.Flags().GetBool("ignore-rate-limit") - results, err := ft.FetchAll(since, !ignoreRateLimit) + results, err := findATender.FetchAll(since, !ignoreRateLimit) if err != nil { return err } diff --git a/internal/cli/fetch/once_test.go b/internal/cli/fetch/once_test.go index 9399e09..d078bd7 100644 --- a/internal/cli/fetch/once_test.go +++ b/internal/cli/fetch/once_test.go @@ -12,7 +12,9 @@ import ( // opp makes an opportunity whose releases, oldest first, have these tags. func opp(uuid string, tags ...string) *types.Opportunity { - o := &types.Opportunity{UUID: uuid} + o := &types.Opportunity{ + UUID: uuid, + } for _, tag := range tags { o.Releases = append(o.Releases, types.OpportunityRelease{ Tags: []types.OpportunityReleaseTag{{Tag: tag}}, diff --git a/internal/cli/migrate/common.go b/internal/cli/migrate/common.go index ec58df0..e2b4c84 100644 --- a/internal/cli/migrate/common.go +++ b/internal/cli/migrate/common.go @@ -59,12 +59,12 @@ func openMigrator( return nil, nil, err } - m, err := store.NewMigrator(db) + migrator, err := store.NewMigrator(db) if err != nil { return nil, nil, errors.Join(err, db.Close()) } - return m, db, nil + return migrator, db, nil } // closeDB closes db when deferred, adding any error to *err. @@ -80,61 +80,72 @@ type errWriter struct { } // printf writes like fmt.Fprintf, unless an earlier write failed. -func (e *errWriter) printf(format string, a ...any) { +func (e *errWriter) printf(format string, args ...any) { if e.err == nil { - _, e.err = fmt.Fprintf(e.w, format, a...) + _, e.err = fmt.Fprintf(e.w, format, args...) } } // printResults lists migrations that ran, e.g. // // applied 00003_create_release_tags_table.sql (2ms) -func printResults(w io.Writer, results []*goose.MigrationResult) error { - ew := &errWriter{w: w} - for _, r := range results { +func printResults(out io.Writer, results []*goose.MigrationResult) error { + output := &errWriter{ + w: out, + } + for _, result := range results { verb := "applied" - if r.Direction == "down" { + if result.Direction == "down" { verb = "reverted" } - ew.printf(" %-9s %s (%s)\n", - verb, filepath.Base(r.Source.Path), r.Duration.Round(time.Millisecond)) + output.printf( + " %-9s %s (%s)\n", + verb, + filepath.Base(result.Source.Path), + result.Duration.Round(time.Millisecond), + ) } - return ew.err + return output.err } // failure prints what ran before a failed migration and describes the // failure. Each migration runs in a transaction, so the failed one has been // rolled back and the database is at the last version that succeeded. -func failure(w io.Writer, err error) error { - var pe *goose.PartialError - if !errors.As(err, &pe) { +func failure(out io.Writer, err error) error { + var partialErr *goose.PartialError + if !errors.As(err, &partialErr) { return err } - printErr := printResults(w, pe.Applied) + printErr := printResults(out, partialErr.Applied) name := "unknown migration" - if pe.Failed != nil && pe.Failed.Source != nil { - name = filepath.Base(pe.Failed.Source.Path) + if partialErr.Failed != nil && partialErr.Failed.Source != nil { + name = filepath.Base(partialErr.Failed.Source.Path) } - return errors.Join(fmt.Errorf( - "migration %s failed and was rolled back; the database is at the last "+ - "version that succeeded.\nMigration error: %w", - name, pe.Err), printErr) + return errors.Join( + fmt.Errorf( + "migration %s failed and was rolled back; the database is at the last "+ + "version that succeeded.\nMigration error: %w", + name, + partialErr.Err, + ), + printErr, + ) } // printVersion reports where the database ended up. -func printVersion(ctx context.Context, w io.Writer, m *goose.Provider) error { - v, err := m.GetDBVersion(ctx) +func printVersion(ctx context.Context, out io.Writer, migrator *goose.Provider) error { + dbVersion, err := migrator.GetDBVersion(ctx) if err != nil { return err } - if v == 0 { - _, err = fmt.Fprintln(w, "Database cleared") + if dbVersion == 0 { + _, err = fmt.Fprintln(out, "Database cleared") } else { - _, err = fmt.Fprintf(w, "Database migrated to version %d\n", v) + _, err = fmt.Fprintf(out, "Database migrated to version %d\n", dbVersion) } return err } diff --git a/internal/cli/migrate/common_test.go b/internal/cli/migrate/common_test.go index ef4daa4..ac53364 100644 --- a/internal/cli/migrate/common_test.go +++ b/internal/cli/migrate/common_test.go @@ -16,7 +16,9 @@ import ( func result(path, direction string) *goose.MigrationResult { return &goose.MigrationResult{ - Source: &goose.Source{Path: "migrations/" + path}, + Source: &goose.Source{ + Path: "migrations/" + path, + }, Direction: direction, Duration: 1500 * time.Microsecond, } @@ -45,7 +47,9 @@ func TestFailurePartial(t *testing.T) { } func TestFailureWithoutFailedSource(t *testing.T) { - err := failure(&bytes.Buffer{}, &goose.PartialError{Err: errors.New("boom")}) + err := failure(&bytes.Buffer{}, &goose.PartialError{ + Err: errors.New("boom"), + }) if err == nil || !strings.Contains(err.Error(), "unknown migration") { t.Errorf("err = %v, want unknown migration", err) } diff --git a/internal/cli/migrate/down.go b/internal/cli/migrate/down.go index 6d395de..11387ad 100644 --- a/internal/cli/migrate/down.go +++ b/internal/cli/migrate/down.go @@ -32,13 +32,13 @@ confirmed by providing -yes.`, yes, _ := cmd.Flags().GetBool("yes") steps, _ := cmd.Flags().GetUint("steps") - m, db, err := openMigrator(cmd, opts, false, false) + migrator, db, err := openMigrator(cmd, opts, false, false) if err != nil { return err } defer closeDB(db, &err) - results, err := store.StepDown(cmd.Context(), m, int(steps), yes) + results, err := store.StepDown(cmd.Context(), migrator, int(steps), yes) if len(results) == 0 && err == nil { fmt.Println("No changes made to database (empty)") @@ -58,7 +58,7 @@ confirmed by providing -yes.`, return failure(cmd.OutOrStdout(), err) } - return printVersion(cmd.Context(), cmd.OutOrStdout(), m) + return printVersion(cmd.Context(), cmd.OutOrStdout(), migrator) }, } cmd.Flags().Uint( diff --git a/internal/cli/migrate/migrate_test.go b/internal/cli/migrate/migrate_test.go index e4053a2..d936864 100644 --- a/internal/cli/migrate/migrate_test.go +++ b/internal/cli/migrate/migrate_test.go @@ -55,7 +55,9 @@ func TestMigrateUpCreateThenStatus(t *testing.T) { latest := latestVersion(t) out := mustRun(t, "migrate", "up", "--create") - cmdtest.Contains(t, out, + cmdtest.Contains( + t, + out, "applied 00001_create_opportunities_table.sql", fmt.Sprintf("Database migrated to version %d", latest), ) @@ -64,7 +66,9 @@ func TestMigrateUpCreateThenStatus(t *testing.T) { } out = mustRun(t, "migrate", "status") - cmdtest.Contains(t, out, + cmdtest.Contains( + t, + out, fmt.Sprintf("Database version %d of %d (up-to-date)", latest, latest), "00001_create_opportunities_table.sql", ) @@ -87,8 +91,11 @@ func TestMigrateUpAsksBeforeCreating(t *testing.T) { out, err := run(t, "n\n", "migrate", "up") cmdtest.ErrContains(t, err, "no database file") - cmdtest.Contains(t, out, - "Database file './test.db' does not exist. Would you like to create it?") + cmdtest.Contains( + t, + out, + "Database file './test.db' does not exist. Would you like to create it?", + ) if cmdtest.Exists("test.db") { t.Error("test.db was created after answering no") } @@ -110,8 +117,11 @@ func TestMigrateUpBySteps(t *testing.T) { cmdtest.Contains(t, out, "Database migrated to version 2") out = mustRun(t, "migrate", "status") - cmdtest.Contains(t, out, - fmt.Sprintf("Database version 2 of %d (%d pending)", latest, latest-2)) + cmdtest.Contains( + t, + out, + fmt.Sprintf("Database version 2 of %d (%d pending)", latest, latest-2), + ) } func TestMigrateDown(t *testing.T) { @@ -130,8 +140,11 @@ func TestMigrateDown(t *testing.T) { _, err := run(t, "", args...) cmdtest.ErrContains(t, err, "pass the --yes flag") } - cmdtest.Contains(t, mustRun(t, "migrate", "status"), - fmt.Sprintf("Database version %d of %d", latest-1, latest)) + cmdtest.Contains( + t, + mustRun(t, "migrate", "status"), + fmt.Sprintf("Database version %d of %d", latest-1, latest), + ) out = mustRun(t, "migrate", "down", "--steps", "0", "--yes") cmdtest.Contains(t, out, "Database cleared") diff --git a/internal/cli/migrate/status.go b/internal/cli/migrate/status.go index e11b763..e155424 100644 --- a/internal/cli/migrate/status.go +++ b/internal/cli/migrate/status.go @@ -22,51 +22,63 @@ import ( // 1 applied 2026-10-07 10:12:01 00001_create_opportunities_table.sql // ... // 5 pending 00005_create_opportunity_classifications_table.sql -func printStatus(ctx context.Context, w io.Writer, m *goose.Provider) error { - statuses, err := m.Status(ctx) +func printStatus(ctx context.Context, out io.Writer, migrator *goose.Provider) error { + statuses, err := migrator.Status(ctx) if err != nil { return err } - current, err := m.GetDBVersion(ctx) + current, err := migrator.GetDBVersion(ctx) if err != nil { return err } - latest := store.LatestVersion(m) + latest := store.LatestVersion(migrator) pending := 0 - for _, s := range statuses { - if s.State == goose.StatePending { + for _, status := range statuses { + if status.State == goose.StatePending { pending++ } } - ew := &errWriter{w: w} + output := &errWriter{ + w: out, + } switch { case current > latest: - ew.printf("Database version %d is newer than this build (latest %d)\n", - current, latest) + output.printf( + "Database version %d is newer than this build (latest %d)\n", + current, + latest, + ) case pending == 0: - ew.printf("Database version %d of %d (up-to-date)\n", current, latest) + output.printf("Database version %d of %d (up-to-date)\n", current, latest) default: - ew.printf("Database version %d of %d (%d pending)\n", current, latest, pending) + output.printf("Database version %d of %d (%d pending)\n", current, latest, pending) } - if ew.err != nil { - return ew.err + if output.err != nil { + return output.err } - tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) - ew = &errWriter{w: tw} - for _, s := range statuses { - applied := "" - if s.State == goose.StateApplied { - applied = s.AppliedAt.Local().Format("2006-01-02 15:04:05") - } - ew.printf(" %d\t%s\t%s\t%s\n", - s.Source.Version, s.State, applied, filepath.Base(s.Source.Path)) + table := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) + output = &errWriter{ + w: table, } - return errors.Join(ew.err, tw.Flush()) + for _, status := range statuses { + applied := "" + if status.State == goose.StateApplied { + applied = status.AppliedAt.Local().Format("2006-01-02 15:04:05") + } + output.printf( + " %d\t%s\t%s\t%s\n", + status.Source.Version, + status.State, + applied, + filepath.Base(status.Source.Path), + ) + } + return errors.Join(output.err, table.Flush()) } func commandStatus(opts *cli.Options) *cobra.Command { @@ -83,13 +95,13 @@ func commandStatus(opts *cli.Options) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) (err error) { slog.Debug("command run", "command", "migrate.status") - m, db, err := openMigrator(cmd, opts, false, false) + migrator, db, err := openMigrator(cmd, opts, false, false) if err != nil { return err } defer closeDB(db, &err) - return printStatus(cmd.Context(), cmd.OutOrStdout(), m) + return printStatus(cmd.Context(), cmd.OutOrStdout(), migrator) }, } } diff --git a/internal/cli/migrate/up.go b/internal/cli/migrate/up.go index dfe493b..71961b9 100644 --- a/internal/cli/migrate/up.go +++ b/internal/cli/migrate/up.go @@ -16,13 +16,13 @@ import ( func runStepUp( cmd *cobra.Command, opts *cli.Options, steps int, create bool, ) (err error) { - m, db, err := openMigrator(cmd, opts, true, create) + migrator, db, err := openMigrator(cmd, opts, true, create) if err != nil { return err } defer closeDB(db, &err) - results, err := store.StepUp(cmd.Context(), m, int(steps)) + results, err := store.StepUp(cmd.Context(), migrator, int(steps)) if len(results) == 0 && err == nil { _, err := fmt.Fprintln(cmd.OutOrStdout(), "No changes made to database (up-to-date)") @@ -37,7 +37,7 @@ func runStepUp( return failure(cmd.OutOrStdout(), err) } - return printVersion(cmd.Context(), cmd.OutOrStdout(), m) + return printVersion(cmd.Context(), cmd.OutOrStdout(), migrator) } func commandUp(opts *cli.Options) *cobra.Command { diff --git a/internal/cli/options.go b/internal/cli/options.go index bb80f9c..1c76243 100644 --- a/internal/cli/options.go +++ b/internal/cli/options.go @@ -20,10 +20,10 @@ type Options struct { // Bind registers the shared flags as persistent flags on the root command cmd. func (o *Options) Bind(cmd *cobra.Command) { - pf := cmd.PersistentFlags() - pf.StringVarP(&o.configFile, "config", "c", "", "configuration file") - pf.BoolVarP(&o.debug, "debug", "d", false, "enable debug messages") - pf.BoolVarP( + flags := cmd.PersistentFlags() + flags.StringVarP(&o.configFile, "config", "c", "", "configuration file") + flags.BoolVarP(&o.debug, "debug", "d", false, "enable debug messages") + flags.BoolVarP( &o.quiet, "quiet", "q", false, "don't report info or warning messages", ) } diff --git a/internal/cli/wsserve/command.go b/internal/cli/wsserve/command.go index 58d0b4a..237c30f 100644 --- a/internal/cli/wsserve/command.go +++ b/internal/cli/wsserve/command.go @@ -17,6 +17,8 @@ import ( "git.avsdev.uk/Go/job-search/internal/websocket/tcpserver" ) +// NewCommand returns the wsserve command, which runs a plain TCP server for +// websocket hub clients (see tcpserver). func NewCommand(opts *cli.Options) *cobra.Command { cmd := &cobra.Command{ Use: "wsserve", @@ -43,7 +45,7 @@ func NewCommand(opts *cli.Options) *cobra.Command { } i, err := strconv.Atoi(listenParts[1]) if err != nil { - return fmt.Errorf("Failed to parse --listen port: %w", err) + return fmt.Errorf("failed to parse --listen port: %w", err) } hostOverride = listenParts[0] portOverride = i @@ -75,9 +77,9 @@ func NewCommand(opts *cli.Options) *cobra.Command { }() hub := websocket.NewHub(conf.Server.WebSocket) - svr := tcpserver.New(&conf.Server, hub) + server := tcpserver.New(&conf.Server, hub) - return svr.Run(ctx) + return server.Run(ctx) }, } cmd.Flags().String("listen", "", "listen address (default from config, 127.0.0.1:9393)") diff --git a/internal/config/conf_logging.go b/internal/config/conf_logging.go index a03ad07..2cb4f80 100644 --- a/internal/config/conf_logging.go +++ b/internal/config/conf_logging.go @@ -50,7 +50,8 @@ func (c *JSCLogging) validate(key string, value any) error { case "debug", "info", "warn", "error": default: return errors.New( - "invalid log level. Must be one of: debug, info, warn, error") + "invalid log level. Must be one of: debug, info, warn, error", + ) } } return nil diff --git a/internal/config/conf_sections_test.go b/internal/config/conf_sections_test.go index 4ada0b5..0424e4d 100644 --- a/internal/config/conf_sections_test.go +++ b/internal/config/conf_sections_test.go @@ -25,8 +25,12 @@ func captureLogs(t *testing.T, level slog.Level) *bytes.Buffer { t.Helper() var buf bytes.Buffer def := slog.Default() - slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: level}))) - t.Cleanup(func() { slog.SetDefault(def) }) + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{ + Level: level, + }))) + t.Cleanup(func() { + slog.SetDefault(def) + }) return &buf } @@ -62,7 +66,10 @@ func TestDefaults(t *testing.T) { } func TestLoggingSection(t *testing.T) { - c := JSCLogging{Level: "info", File: "/tmp/x.log"} + c := JSCLogging{ + Level: "info", + File: "/tmp/x.log", + } if got, want := c.Keys(), []string{"level", "file"}; !reflect.DeepEqual(got, want) { t.Errorf("Keys = %v; want %v", got, want) @@ -142,7 +149,9 @@ func TestConfigLevelToLogLevel(t *testing.T) { if got := ConfigLevelToLogLevel(in); got != want { t.Errorf("ConfigLevelToLogLevel(%q) = %v; want %v", in, got, want) } - c := JSCLogging{Level: in} + c := JSCLogging{ + Level: in, + } if got := c.LogLevel(); got != want { t.Errorf("LogLevel() with %q = %v; want %v", in, got, want) } @@ -150,7 +159,10 @@ func TestConfigLevelToLogLevel(t *testing.T) { } func TestSourceSection(t *testing.T) { - c := JSCSourcesSource{Enabled: true, Interval: time.Minute} + c := JSCSourcesSource{ + Enabled: true, + Interval: time.Minute, + } if got, want := c.Keys(), []string{"enabled", "interval"}; !reflect.DeepEqual(got, want) { t.Errorf("Keys = %v; want %v", got, want) @@ -458,7 +470,9 @@ func TestLoadFromViperInvalidValues(t *testing.T) { } func TestDatabaseSection(t *testing.T) { - c := JSCDatabase{File: "./jobsearch.db"} + c := JSCDatabase{ + File: "./jobsearch.db", + } if got, want := c.Keys(), []string{"file"}; !reflect.DeepEqual(got, want) { t.Errorf("Keys = %v; want %v", got, want) diff --git a/internal/config/config.go b/internal/config/config.go index f8dbde9..731b92b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -62,8 +62,8 @@ func (c *JobSearchConfig) SetValue(key string, value any) error { // Unknown keys are skipped; invalid values are logged and ignored. func LoadFromViper(conf *JobSearchConfig) { keys := viper.AllKeys() - for idx := range keys { - key := keys[idx] + for i := range keys { + key := keys[i] keyType, _ := conf.TypeOf(key) if keyType == reflect.Invalid { continue @@ -94,15 +94,15 @@ func viperValue(key string, kind reflect.Kind) (value any, ok bool) { // SetValue to split on commas; GetStringSlice would split it on // spaces. A list of objects (a list of sections) is passed on as it // is, as GetStringSlice would flatten it. - switch v := viper.Get(key).(type) { + switch value := viper.Get(key).(type) { case string: - return v, true + return value, true case []map[string]any: // as LoadToViper sets it - return v, true + return value, true case []any: - if len(v) > 0 { - if _, ok := v[0].(map[string]any); ok { - return v, true + if len(value) > 0 { + if _, ok := value[0].(map[string]any); ok { + return value, true } } } @@ -121,26 +121,26 @@ func LoadToViper(conf JobSearchConfig) { // String formats the configuration one "key = value " line per key, with // dotted keys, sorted by key. -func (conf JobSearchConfig) String() string { - var sb strings.Builder - flat := conf.Flatten() - for _, k := range slices.Sorted(maps.Keys(flat)) { - typ, _ := conf.TypeOf(k) - fmt.Fprint(&sb, k, " = ", flat[k], " <", typ, ">\n") +func (c JobSearchConfig) String() string { + var builder strings.Builder + flat := c.Flatten() + for _, key := range slices.Sorted(maps.Keys(flat)) { + kind, _ := c.TypeOf(key) + fmt.Fprint(&builder, key, " = ", flat[key], " <", kind, ">\n") } - return sb.String() + return builder.String() } // JSON formats the configuration as indented JSON. -func (conf JobSearchConfig) JSON() string { - confMap := conf.Map() +func (c JobSearchConfig) JSON() string { + confMap := c.Map() jbytes, _ := json.MarshalIndent(confMap, "", " ") return string(jbytes) } // YAML formats the configuration as YAML. -func (conf JobSearchConfig) YAML() string { - confMap := conf.Map() +func (c JobSearchConfig) YAML() string { + confMap := c.Map() ybytes, _ := yaml.Marshal(confMap) return string(ybytes) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 13e8eb0..53b3316 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -70,7 +70,14 @@ func countSections(uses map[reflect.Type]int, section reflect.Type) { } } -func checkSections(t *testing.T, conf *JobSearchConfig, all map[string]any, uses map[reflect.Type]int, prefix string, section reflect.Value) { +func checkSections( + t *testing.T, + conf *JobSearchConfig, + all map[string]any, + uses map[reflect.Type]int, + prefix string, + section reflect.Value, +) { for _, f := range fieldsOf(section.Type()) { key := joinKey(prefix, f.key) if isSectionList(f.typ) { @@ -93,8 +100,12 @@ func checkSections(t *testing.T, conf *JobSearchConfig, all map[string]any, uses } for k := range keys { if _, ok := all[joinKey(keyPrefix, k)]; !ok { - t.Errorf("%s.Flatten() key %q does not match its cfg tag %q", - name, k, key) + t.Errorf( + "%s.Flatten() key %q does not match its cfg tag %q", + name, + k, + key, + ) } } @@ -116,6 +127,7 @@ func checkSections(t *testing.T, conf *JobSearchConfig, all map[string]any, uses // checkSectionList checks the items of the list of sections at key implement // Section and that their methods agree with the list's value in all. func checkSectionList(t *testing.T, all map[string]any, key string, list reflect.Value) { + t.Helper() name := list.Type().Elem().Name() if !reflect.PointerTo(list.Type().Elem()).Implements(sectionType) { t.Errorf("*%s (items of %s) does not implement Section", name, key) @@ -145,7 +157,13 @@ func checkSectionList(t *testing.T, all map[string]any, key string, list reflect // checkSectionMethods checks section's Map, TypeOf, ValueOf and SetValue // against want, its keys and values, and kinds, their kinds. // Setting each key to its own value must succeed and leave section unchanged. -func checkSectionMethods(t *testing.T, name string, section reflect.Value, want map[string]any, kinds map[string]reflect.Kind) { +func checkSectionMethods( + t *testing.T, + name string, + section reflect.Value, + want map[string]any, + kinds map[string]reflect.Kind, +) { s := section.Addr().Interface().(Section) if got := flattenMap("", s.Map()); !reflect.DeepEqual(got, want) { t.Errorf("%s.Map() =\n%v\nwant\n%v", name, got, want) @@ -184,14 +202,18 @@ func flattenMap(prefix string, m map[string]any) map[string]any { // checkEmbedded checks that structs embedded in a section, such as // JSCSourcesSource, implement Section too. func checkEmbedded(t *testing.T, section reflect.Type) { + t.Helper() for i := 0; i < section.NumField(); i++ { sf := section.Field(i) if !sf.Anonymous || sf.Type.Kind() != reflect.Struct { continue } if !reflect.PointerTo(sf.Type).Implements(sectionType) { - t.Errorf("*%s (embedded in %s) does not implement Section", - sf.Type.Name(), section.Name()) + t.Errorf( + "*%s (embedded in %s) does not implement Section", + sf.Type.Name(), + section.Name(), + ) } checkEmbedded(t, sf.Type) } diff --git a/internal/config/load_test.go b/internal/config/load_test.go index c8f1c86..757efd9 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -22,7 +22,9 @@ func initDir(t *testing.T, env []string, config string) string { // godotenv sets variables without cleaning up after the test. for _, kv := range env { key, _, _ := strings.Cut(kv, "=") - t.Cleanup(func() { os.Unsetenv(key) }) + t.Cleanup(func() { + os.Unsetenv(key) + }) } writeFile(t, filepath.Join(dir, ".env"), strings.Join(env, "\n")+"\n") } @@ -108,7 +110,11 @@ func TestInitKeepsCommandLineLevel(t *testing.T) { initDir(t, []string{}, "logging:\n level: info\n") logs := captureLogs(t, slog.LevelDebug) - conf := JobSearchConfig{Logging: JSCLogging{Level: level}} + conf := JobSearchConfig{ + Logging: JSCLogging{ + Level: level, + }, + } if err := Init("", &conf); err != nil { t.Fatal(err) } diff --git a/internal/config/modify.go b/internal/config/modify.go index adf5eed..7ae3212 100644 --- a/internal/config/modify.go +++ b/internal/config/modify.go @@ -21,8 +21,8 @@ func StoreValue( // storeValue is StoreValue for any section, so key types that // JobSearchConfig doesn't have yet can be tested. -func storeValue(s Section, key string, value string) (any, reflect.Kind, error) { - confType, err := s.TypeOf(key) +func storeValue(section Section, key string, value string) (any, reflect.Kind, error) { + confType, err := section.TypeOf(key) if err != nil { return nil, confType, err } @@ -38,10 +38,10 @@ func storeValue(s Section, key string, value string) (any, reflect.Kind, error) // SetValue parses the string into the field's own type (e.g. int8, // float32 or time.Duration) and rejects values the configuration itself // would not accept, e.g. an unknown log level or an unparseable duration. - if err := s.SetValue(key, value); err != nil { + if err := section.SetValue(key, value); err != nil { return nil, confType, err } - confValue, err := s.ValueOf(key) + confValue, err := section.ValueOf(key) if err != nil { return nil, confType, err } @@ -93,9 +93,9 @@ func PurgeValue(conf *JobSearchConfig, key string) error { } out := viper.New() out.SetConfigFile(file.ConfigFileUsed()) - for _, k := range file.AllKeys() { - if k != key { - out.Set(k, file.Get(k)) + for _, fileKey := range file.AllKeys() { + if fileKey != key { + out.Set(fileKey, file.Get(fileKey)) } } return out.WriteConfig() diff --git a/internal/config/print.go b/internal/config/print.go index 9c7eb2e..bd374b3 100644 --- a/internal/config/print.go +++ b/internal/config/print.go @@ -11,18 +11,18 @@ import ( ) // PrintTerminal prints the configuration to stdout as String formats it. -func (conf JobSearchConfig) PrintTerminal() { - fmt.Print(conf.String() + "\n") +func (c JobSearchConfig) PrintTerminal() { + fmt.Print(c.String() + "\n") } // PrintJson prints the configuration to stdout as JSON. -func (conf JobSearchConfig) PrintJson() { - fmt.Print(conf.JSON() + "\n") +func (c JobSearchConfig) PrintJson() { + fmt.Print(c.JSON() + "\n") } // PrintYaml prints the configuration to stdout as YAML. -func (conf JobSearchConfig) PrintYaml() { - fmt.Print(conf.YAML() + "\n") +func (c JobSearchConfig) PrintYaml() { + fmt.Print(c.YAML() + "\n") } // PrintLog logs each configuration key and value, sorted by key, at atLevel: @@ -55,4 +55,4 @@ func (conf JobSearchConfig) PrintLog(atLevel slog.Level) { for _, key := range keys { logger.Log(ctx, atLevel, "config value", "key", key, "value", flat[key]) } -} +} \ No newline at end of file diff --git a/internal/config/section.go b/internal/config/section.go index 366cbe6..28429ee 100644 --- a/internal/config/section.go +++ b/internal/config/section.go @@ -26,7 +26,8 @@ import ( // // time.Duration fields report reflect.String from TypeOf and accept either a // time.Duration or a duration string such as "90s" in SetValue. -// util.ByteSize fields work the same way, with size strings such as "32KiB". +// util.ByteSize fields work the same way, with size strings such as "32KiB" +// (or "-1" for no limit). // // []string fields report reflect.Slice and accept either a []string or a // comma-separated string such as "a, b" in SetValue (and so in default tags). @@ -42,16 +43,25 @@ import ( // // A section can check values before they are stored by implementing // validator on its pointer type. +// The struct tags section.go reads. const ( - keyTag = "cfg" + // keyTag holds a field's configuration key. + keyTag = "cfg" + // defaultTag holds a field's default value. defaultTag = "default" ) +// The unknown key error and the field types section.go handles specially. var ( - errUnknownKey = errors.New("unknown configuration key") - durationType = reflect.TypeOf(time.Duration(0)) - byteSizeType = reflect.TypeOf(util.ByteSize(0)) - timeType = reflect.TypeOf(time.Time{}) + // errUnknownKey is returned for a key that names no field. + errUnknownKey = errors.New("unknown configuration key") + // durationType is time.Duration, read and given out as a string. + durationType = reflect.TypeOf(time.Duration(0)) + // byteSizeType is util.ByteSize, read and given out as a string. + byteSizeType = reflect.TypeOf(util.ByteSize(0)) + // timeType is time.Time, a struct that is a value, not a section. + timeType = reflect.TypeOf(time.Time{}) + // stringSliceType is []string, set from a comma-separated list. stringSliceType = reflect.TypeOf([]string{}) ) @@ -100,67 +110,67 @@ type field struct { hasDefault bool } -// isSectionList reports whether t is a list of sections, []S for a section -// struct S. -func isSectionList(t reflect.Type) bool { - return t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Struct && - t.Elem() != timeType +// isSectionList reports whether listType is a list of sections, []S for a +// section struct S. +func isSectionList(listType reflect.Type) bool { + return listType.Kind() == reflect.Slice && listType.Elem().Kind() == reflect.Struct && + listType.Elem() != timeType } // fieldsOf lists the configuration fields of a section type in declaration // order, with the fields of embedded structs promoted into it. -func fieldsOf(t reflect.Type) []field { +func fieldsOf(structType reflect.Type) []field { var fields []field - for i := 0; i < t.NumField(); i++ { - sf := t.Field(i) - if sf.Anonymous && sf.Type.Kind() == reflect.Struct { - for _, f := range fieldsOf(sf.Type) { - f.index = append([]int{i}, f.index...) - fields = append(fields, f) + for i := 0; i < structType.NumField(); i++ { + structField := structType.Field(i) + if structField.Anonymous && structField.Type.Kind() == reflect.Struct { + for _, embeddedField := range fieldsOf(structField.Type) { + embeddedField.index = append([]int{i}, embeddedField.index...) + fields = append(fields, embeddedField) } continue } - key, ok := sf.Tag.Lookup(keyTag) - if !ok || key == "" || key == "-" || !sf.IsExported() { + key, ok := structField.Tag.Lookup(keyTag) + if !ok || key == "" || key == "-" || !structField.IsExported() { continue } - def, hasDefault := sf.Tag.Lookup(defaultTag) + defaultValue, hasDefault := structField.Tag.Lookup(defaultTag) fields = append(fields, field{ key: key, index: []int{i}, - typ: sf.Type, - section: sf.Type.Kind() == reflect.Struct && sf.Type != timeType, - def: def, + typ: structField.Type, + section: structField.Type.Kind() == reflect.Struct && structField.Type != timeType, + def: defaultValue, hasDefault: hasDefault, }) } return fields } -// structValue dereferences v down to the section struct it holds. -func structValue(v any) reflect.Value { - rv := reflect.ValueOf(v) - for rv.Kind() == reflect.Pointer { - rv = rv.Elem() +// structValue dereferences value down to the section struct it holds. +func structValue(value any) reflect.Value { + reflected := reflect.ValueOf(value) + for reflected.Kind() == reflect.Pointer { + reflected = reflected.Elem() } - if rv.Kind() != reflect.Struct { - panic(fmt.Sprintf("config: %T is not a configuration section", v)) + if reflected.Kind() != reflect.Struct { + panic(fmt.Sprintf("config: %T is not a configuration section", value)) } - return rv + return reflected } // lookup resolves a dotted key to the leaf field and the section holding it. func lookup(section reflect.Value, key string) (reflect.Value, field, error) { head, rest, nested := strings.Cut(key, ".") - for _, f := range fieldsOf(section.Type()) { - if f.key != head { + for _, configField := range fieldsOf(section.Type()) { + if configField.key != head { continue } switch { - case f.section && nested: - return lookup(section.FieldByIndex(f.index), rest) - case !f.section && !nested: - return section, f, nil + case configField.section && nested: + return lookup(section.FieldByIndex(configField.index), rest) + case !configField.section && !nested: + return section, configField, nil } // Key matched but the shape didn't: a section with no sub-key, or a // value with one. @@ -177,52 +187,52 @@ func joinKey(prefix, key string) string { } // keysOf returns the keys directly in a section. -func keysOf(v any) []string { - fields := fieldsOf(structValue(v).Type()) +func keysOf(value any) []string { + fields := fieldsOf(structValue(value).Type()) keys := make([]string, 0, len(fields)) - for _, f := range fields { - keys = append(keys, f.key) + for _, configField := range fields { + keys = append(keys, configField.key) } return keys } // mapOf converts a section to a map, with nested sections as nested maps. -func mapOf(v any) map[string]any { - return mapOfValue(structValue(v)) +func mapOf(value any) map[string]any { + return mapOfValue(structValue(value)) } // mapOfValue is mapOf for a section struct value. func mapOfValue(section reflect.Value) map[string]any { - m := map[string]any{} - for _, f := range fieldsOf(section.Type()) { - fv := section.FieldByIndex(f.index) - if f.section { - m[f.key] = mapOfValue(fv) + values := map[string]any{} + for _, configField := range fieldsOf(section.Type()) { + fieldValue := section.FieldByIndex(configField.index) + if configField.section { + values[configField.key] = mapOfValue(fieldValue) } else { - m[f.key] = exportValue(fv) + values[configField.key] = exportValue(fieldValue) } } - return m + return values } // flattenOf converts a section to a flat map of dotted keys, each prefixed // with prefix (if non-empty). -func flattenOf(prefix string, v any) map[string]any { - m := map[string]any{} - flattenInto(m, prefix, structValue(v)) - return m +func flattenOf(prefix string, value any) map[string]any { + flat := map[string]any{} + flattenInto(flat, prefix, structValue(value)) + return flat } // flattenInto adds section's keys to m, dotted and prefixed with prefix (if // non-empty), descending into nested sections. -func flattenInto(m map[string]any, prefix string, section reflect.Value) { - for _, f := range fieldsOf(section.Type()) { - fv := section.FieldByIndex(f.index) - key := joinKey(prefix, f.key) - if f.section { - flattenInto(m, key, fv) +func flattenInto(flat map[string]any, prefix string, section reflect.Value) { + for _, configField := range fieldsOf(section.Type()) { + fieldValue := section.FieldByIndex(configField.index) + key := joinKey(prefix, configField.key) + if configField.section { + flattenInto(flat, key, fieldValue) } else { - m[key] = exportValue(fv) + flat[key] = exportValue(fieldValue) } } } @@ -232,33 +242,33 @@ func flattenInto(m map[string]any, prefix string, section reflect.Value) { // so values agree with kindOf and can be passed back to SetValue; []string is // copied; a list of sections becomes a []map[string]any (nil if empty); // everything else is unchanged. -func exportValue(fv reflect.Value) any { - switch fv.Type() { +func exportValue(fieldValue reflect.Value) any { + switch fieldValue.Type() { case durationType: - return time.Duration(fv.Int()).String() + return time.Duration(fieldValue.Int()).String() case byteSizeType: - return util.ByteSize(fv.Int()).String() + return util.ByteSize(fieldValue.Int()).String() } - if isSectionList(fv.Type()) { + if isSectionList(fieldValue.Type()) { var items []map[string]any - for i := range fv.Len() { - items = append(items, mapOfValue(fv.Index(i))) + for i := range fieldValue.Len() { + items = append(items, mapOfValue(fieldValue.Index(i))) } return items } - v := fv.Interface() - if s, ok := v.([]string); ok { + value := fieldValue.Interface() + if list, ok := value.([]string); ok { // A copy, so changing it doesn't change the configuration. - return slices.Clone(s) + return slices.Clone(list) } - return v + return value } // splitList splits a comma-separated list, trimming spaces around each item // and dropping empty ones, so "" is an empty list. -func splitList(s string) []string { +func splitList(list string) []string { var items []string - for item := range strings.SplitSeq(s, ",") { + for item := range strings.SplitSeq(list, ",") { if item = strings.TrimSpace(item); item != "" { items = append(items, item) } @@ -268,115 +278,120 @@ func splitList(s string) []string { // kindOf is the kind reported by TypeOf, i.e. how the value is read from // Viper. Durations and byte sizes are read as strings and parsed by SetValue. -func kindOf(t reflect.Type) reflect.Kind { - if t == durationType || t == byteSizeType { +func kindOf(fieldType reflect.Type) reflect.Kind { + if fieldType == durationType || fieldType == byteSizeType { return reflect.String } - return t.Kind() + return fieldType.Kind() } -// typeOf returns the kind of the dotted key in section v (see kindOf), or -// errUnknownKey. -func typeOf(v any, key string) (reflect.Kind, error) { - _, f, err := lookup(structValue(v), key) +// typeOf returns the kind of the dotted key in the section value (see +// kindOf), or errUnknownKey. +func typeOf(value any, key string) (reflect.Kind, error) { + _, configField, err := lookup(structValue(value), key) if err != nil { return reflect.Invalid, err } - return kindOf(f.typ), nil + return kindOf(configField.typ), nil } -// valueOf returns the value of the dotted key in section v, or errUnknownKey. -func valueOf(v any, key string) (any, error) { - section, f, err := lookup(structValue(v), key) +// valueOf returns the value of the dotted key in the section value, or +// errUnknownKey. +func valueOf(value any, key string) (any, error) { + section, configField, err := lookup(structValue(value), key) if err != nil { return nil, err } - return exportValue(section.FieldByIndex(f.index)), nil + return exportValue(section.FieldByIndex(configField.index)), nil } // setValue sets a dotted key on the section that ptr points to. func setValue(ptr any, key string, value any) error { - section, f, err := lookup(structValue(ptr), key) + section, configField, err := lookup(structValue(ptr), key) if err != nil { return err } - return setField(section, f, value) + return setField(section, configField, value) } // setField converts value to the field's type and, if the section has a // validator, checks it, before setting the field. On error the field is left // unchanged. -func setField(section reflect.Value, f field, value any) error { - newValue, err := convert(value, f.typ) +func setField(section reflect.Value, configField field, value any) error { + newValue, err := convert(value, configField.typ) if err != nil { return err } - if v, ok := section.Addr().Interface().(validator); ok { - if err := v.validate(f.key, newValue.Interface()); err != nil { + if checker, ok := section.Addr().Interface().(validator); ok { + if err := checker.validate(configField.key, newValue.Interface()); err != nil { return err } } - section.FieldByIndex(f.index).Set(newValue) + section.FieldByIndex(configField.index).Set(newValue) return nil } -// convert turns value into a value of type t. Values already of the right +// convert turns value into a value of type targetType. Values already of the right // type are used as-is, except that an empty []string becomes nil, as an empty // comma-separated list does (see splitList); strings are parsed. Lists of // sections are converted by convertSectionList. -func convert(value any, t reflect.Type) (reflect.Value, error) { - if isSectionList(t) { - return convertSectionList(value, t) +func convert(value any, targetType reflect.Type) (reflect.Value, error) { + if isSectionList(targetType) { + return convertSectionList(value, targetType) } if value != nil { - rv := reflect.ValueOf(value) - if rv.Type().AssignableTo(t) { - if t == stringSliceType && rv.Len() == 0 { - return reflect.Zero(t), nil + reflected := reflect.ValueOf(value) + if reflected.Type().AssignableTo(targetType) { + if targetType == stringSliceType && reflected.Len() == 0 { + return reflect.Zero(targetType), nil } - return rv, nil + return reflected, nil } - if s, ok := value.(string); ok { - return parseString(s, t) + if text, ok := value.(string); ok { + return parseString(text, targetType) } } return reflect.Value{}, fmt.Errorf("unsupported type %T", value) } -// convertSectionList turns value into a list of sections of type t: a value -// of type t, a list of maps of keys to values, or a JSON array of objects. +// convertSectionList turns value into a list of sections of type listType: a +// value of type listType, a list of maps of keys to values, or a JSON array of objects. // nil and empty lists become a nil list. -func convertSectionList(value any, t reflect.Type) (reflect.Value, error) { - if s, ok := value.(string); ok { +func convertSectionList(value any, listType reflect.Type) (reflect.Value, error) { + if text, ok := value.(string); ok { var items []map[string]any - if strings.TrimSpace(s) != "" { - if err := json.Unmarshal([]byte(s), &items); err != nil { - return reflect.Value{}, fmt.Errorf("unable to convert to a list of %s: %w", t.Elem(), err) + if strings.TrimSpace(text) != "" { + if err := json.Unmarshal([]byte(text), &items); err != nil { + return reflect.Value{}, fmt.Errorf( + "unable to convert to a list of %s: %w", + listType.Elem(), + err, + ) } } value = items } if value == nil { - return reflect.Zero(t), nil + return reflect.Zero(listType), nil } - rv := reflect.ValueOf(value) - if rv.Kind() != reflect.Slice { + reflected := reflect.ValueOf(value) + if reflected.Kind() != reflect.Slice { return reflect.Value{}, fmt.Errorf("unsupported type %T", value) } - if rv.Len() == 0 { - return reflect.Zero(t), nil + if reflected.Len() == 0 { + return reflect.Zero(listType), nil } - if rv.Type().AssignableTo(t) { - for i := range rv.Len() { - if err := validateSection(rv.Index(i)); err != nil { + if reflected.Type().AssignableTo(listType) { + for i := range reflected.Len() { + if err := validateSection(reflected.Index(i)); err != nil { return reflect.Value{}, fmt.Errorf("item %d: %w", i, err) } } - return rv, nil + return reflected, nil } - list := reflect.MakeSlice(t, 0, rv.Len()) - for i := range rv.Len() { - item, err := sectionFromMap(rv.Index(i).Interface(), t.Elem()) + list := reflect.MakeSlice(listType, 0, reflected.Len()) + for i := range reflected.Len() { + item, err := sectionFromMap(reflected.Index(i).Interface(), listType.Elem()) if err != nil { return reflect.Value{}, fmt.Errorf("item %d: %w", i, err) } @@ -385,31 +400,31 @@ func convertSectionList(value any, t reflect.Type) (reflect.Value, error) { return list, nil } -// sectionFromMap builds a section of type t from a map of its keys to values, -// starting from its defaults. Values that are neither strings nor of the -// field's type (e.g. numbers read from YAML) are formatted and parsed. The -// finished section is validated as a whole (see validateSection), so a key -// left out of the map can't skip its validation. -func sectionFromMap(value any, t reflect.Type) (reflect.Value, error) { +// sectionFromMap builds a section of type structType from value, a map of its +// keys to values, starting from its defaults. Values that are neither strings +// nor of the field's type (e.g. numbers read from YAML) are formatted and +// parsed. The finished section is validated as a whole (see +// validateSection), so a key left out of the map can't skip its validation. +func sectionFromMap(value any, structType reflect.Type) (reflect.Value, error) { item, ok := value.(map[string]any) if !ok { return reflect.Value{}, fmt.Errorf("unsupported type %T", value) } - section := reflect.New(t).Elem() + section := reflect.New(structType).Elem() if err := applyDefaultsTo("", section); err != nil { return reflect.Value{}, err } - for k, v := range item { - s, f, err := lookup(section, k) + for itemKey, itemValue := range item { + holder, configField, err := lookup(section, itemKey) if err != nil { - return reflect.Value{}, fmt.Errorf("%w %q", err, k) + return reflect.Value{}, fmt.Errorf("%w %q", err, itemKey) } - if _, isString := v.(string); v != nil && !isString && - !reflect.TypeOf(v).AssignableTo(f.typ) { - v = fmt.Sprint(v) + if _, isString := itemValue.(string); itemValue != nil && !isString && + !reflect.TypeOf(itemValue).AssignableTo(configField.typ) { + itemValue = fmt.Sprint(itemValue) } - if err := setField(s, f, v); err != nil { - return reflect.Value{}, fmt.Errorf("%s: %w", k, err) + if err := setField(holder, configField, itemValue); err != nil { + return reflect.Value{}, fmt.Errorf("%s: %w", itemKey, err) } } if err := validateSection(section); err != nil { @@ -422,73 +437,76 @@ func sectionFromMap(value any, t reflect.Type) (reflect.Value, error) { // field directly in it, so values that were never set (defaults or zero // values) are checked too. func validateSection(section reflect.Value) error { - v, ok := reflect.New(section.Type()).Interface().(validator) + checker, ok := reflect.New(section.Type()).Interface().(validator) if !ok { return nil } - for _, f := range fieldsOf(section.Type()) { - if f.section { + for _, configField := range fieldsOf(section.Type()) { + if configField.section { continue } - if err := v.validate(f.key, section.FieldByIndex(f.index).Interface()); err != nil { - return fmt.Errorf("%s: %w", f.key, err) + if err := checker.validate( + configField.key, + section.FieldByIndex(configField.index).Interface(), + ); err != nil { + return fmt.Errorf("%s: %w", configField.key, err) } } return nil } -// parseString parses s as a value of type t. It supports strings, bools, +// parseString parses text as a value of type targetType. It supports strings, bools, // signed and unsigned integers, floats, time.Duration, ByteSize and []string // (from a comma-separated list, see splitList). -func parseString(s string, t reflect.Type) (reflect.Value, error) { - v := reflect.New(t).Elem() - switch t { +func parseString(text string, targetType reflect.Type) (reflect.Value, error) { + parsed := reflect.New(targetType).Elem() + switch targetType { case durationType: - d, err := time.ParseDuration(s) + duration, err := time.ParseDuration(text) if err != nil { return reflect.Value{}, fmt.Errorf("unable to convert to duration: %w", err) } - v.SetInt(int64(d)) - return v, nil + parsed.SetInt(int64(duration)) + return parsed, nil case byteSizeType: - b, err := util.ParseByteSize(s) + size, err := util.ParseByteSize(text) if err != nil { return reflect.Value{}, fmt.Errorf("unable to convert to byte size: %w", err) } - v.SetInt(int64(b)) - return v, nil + parsed.SetInt(int64(size)) + return parsed, nil case stringSliceType: - v.Set(reflect.ValueOf(splitList(s))) - return v, nil + parsed.Set(reflect.ValueOf(splitList(text))) + return parsed, nil } var err error - switch t.Kind() { + switch targetType.Kind() { case reflect.String: - v.SetString(s) + parsed.SetString(text) case reflect.Bool: - var b bool - b, err = strconv.ParseBool(s) - v.SetBool(b) + var flag bool + flag, err = strconv.ParseBool(text) + parsed.SetBool(flag) case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - var n int64 - n, err = strconv.ParseInt(s, 10, t.Bits()) - v.SetInt(n) + var number int64 + number, err = strconv.ParseInt(text, 10, targetType.Bits()) + parsed.SetInt(number) case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - var n uint64 - n, err = strconv.ParseUint(s, 10, t.Bits()) - v.SetUint(n) + var number uint64 + number, err = strconv.ParseUint(text, 10, targetType.Bits()) + parsed.SetUint(number) case reflect.Float32, reflect.Float64: - var n float64 - n, err = strconv.ParseFloat(s, t.Bits()) - v.SetFloat(n) + var number float64 + number, err = strconv.ParseFloat(text, targetType.Bits()) + parsed.SetFloat(number) default: - return reflect.Value{}, fmt.Errorf("unsupported type string for %s", t) + return reflect.Value{}, fmt.Errorf("unsupported type string for %s", targetType) } if err != nil { - return reflect.Value{}, fmt.Errorf("unable to convert %q to %s: %w", s, t, err) + return reflect.Value{}, fmt.Errorf("unable to convert %q to %s: %w", text, targetType, err) } - return v, nil + return parsed, nil } // applyDefaults sets every field that has a default tag, recursing into @@ -501,18 +519,18 @@ func applyDefaults(ptr any) error { // recursing into nested sections. prefix is the section's dotted key, used in // error messages. func applyDefaultsTo(prefix string, section reflect.Value) error { - for _, f := range fieldsOf(section.Type()) { - key := joinKey(prefix, f.key) - if f.section { - if err := applyDefaultsTo(key, section.FieldByIndex(f.index)); err != nil { + for _, configField := range fieldsOf(section.Type()) { + key := joinKey(prefix, configField.key) + if configField.section { + if err := applyDefaultsTo(key, section.FieldByIndex(configField.index)); err != nil { return err } continue } - if !f.hasDefault { + if !configField.hasDefault { continue } - if err := setField(section, f, f.def); err != nil { + if err := setField(section, configField, configField.def); err != nil { return fmt.Errorf("invalid default for %s: %w", key, err) } } diff --git a/internal/config/section_test.go b/internal/config/section_test.go index 3dd6443..3d90f72 100644 --- a/internal/config/section_test.go +++ b/internal/config/section_test.go @@ -119,7 +119,9 @@ func TestMapOf(t *testing.T) { } func TestFlattenOfPrefix(t *testing.T) { - m := flattenOf("p", testSection{Dur: 90 * time.Second}) + m := flattenOf("p", testSection{ + Dur: 90 * time.Second, + }) for _, k := range []string{"p.str", "p.on", "p.inner.name"} { if _, ok := m[k]; !ok { t.Errorf("flattenOf missing %q in %v", k, m) @@ -145,14 +147,18 @@ func TestApplyDefaults(t *testing.T) { t.Fatalf("applyDefaults: %v", err) } want := testSection{ - testEmbedded: testEmbedded{On: true}, - Str: "hello", - Int: -42, - Uint: 7, - Float: 1.5, - Dur: 90 * time.Second, - Size: 32 << 10, - Inner: testInner{Name: "inner"}, + testEmbedded: testEmbedded{ + On: true, + }, + Str: "hello", + Int: -42, + Uint: 7, + Float: 1.5, + Dur: 90 * time.Second, + Size: 32 << 10, + Inner: testInner{ + Name: "inner", + }, } if !reflect.DeepEqual(s, want) { t.Errorf("applyDefaults = %+v; want %+v", s, want) @@ -247,6 +253,7 @@ func TestSetValue(t *testing.T) { {"size", "1 mb", "1MB"}, // byte sizes are given out as strings {"size", "1.5KiB", "1536B"}, {"size", "4096", "4KiB"}, + {"size", "-1", "-1"}, // no limit {"slice", " a, b ,, c ", []string{"a", "b", "c"}}, {"slice", "", []string(nil)}, {"inner.name", "nested", "nested"}, @@ -286,8 +293,14 @@ func TestSetValue(t *testing.T) { } got, err := valueOf(s, tt.key) if err != nil || !reflect.DeepEqual(got, tt.want) { - t.Errorf("after setValue(%q, %#v): valueOf = %#v, %v; want %#v", - tt.key, tt.value, got, err, tt.want) + t.Errorf( + "after setValue(%q, %#v): valueOf = %#v, %v; want %#v", + tt.key, + tt.value, + got, + err, + tt.want, + ) } } } @@ -322,8 +335,13 @@ func TestSetValueErrors(t *testing.T) { before := s err := setValue(&s, tt.key, tt.value) if err == nil || !strings.Contains(err.Error(), tt.want) { - t.Errorf("setValue(%q, %#v) = %v; want error containing %q", - tt.key, tt.value, err, tt.want) + t.Errorf( + "setValue(%q, %#v) = %v; want error containing %q", + tt.key, + tt.value, + err, + tt.want, + ) } if !reflect.DeepEqual(s, before) { t.Errorf("setValue(%q, %#v) modified the section on error", tt.key, tt.value) @@ -334,7 +352,9 @@ func TestSetValueErrors(t *testing.T) { // A []string is given out as a copy, so changing it leaves the section as it // was. func TestValueOfSliceIsACopy(t *testing.T) { - s := testSection{Slice: []string{"a", "b"}} + s := testSection{ + Slice: []string{"a", "b"}, + } got, err := valueOf(s, "slice") if err != nil { t.Fatal(err) diff --git a/internal/fetcher/fetcher.go b/internal/fetcher/fetcher.go index a6a247c..bf57171 100644 --- a/internal/fetcher/fetcher.go +++ b/internal/fetcher/fetcher.go @@ -12,6 +12,8 @@ import ( "git.avsdev.uk/Go/job-search/internal/util" ) +// ErrNoNewReleases is returned by a source's task when there was nothing new +// to fetch; it wraps util.ErrWorkerNoWork. var ErrNoNewReleases = fmt.Errorf("%w: no new releases", util.ErrWorkerNoWork) // Option configures Run. diff --git a/internal/fetcher/fetcher_test.go b/internal/fetcher/fetcher_test.go index 3598690..54ce736 100644 --- a/internal/fetcher/fetcher_test.go +++ b/internal/fetcher/fetcher_test.go @@ -77,7 +77,9 @@ func TestFetchFunctions(t *testing.T) { "releases found", http.StatusOK, onePackage, - func(err error) bool { return err == nil }, + func(err error) bool { + return err == nil + }, "nil", }, {"no releases", http.StatusOK, emptyPackage, func(err error) bool { @@ -92,7 +94,10 @@ func TestFetchFunctions(t *testing.T) { t.Run(fname+"/"+tc.name, func(t *testing.T) { srv, hits := ftsServer(t, tc.status, tc.body) err := fetch( - context.Background(), slog.Default(), confFor(srv.URL), time.Now().Add(-time.Hour), + context.Background(), + slog.Default(), + confFor(srv.URL), + time.Now().Add(-time.Hour), ) if !tc.check(err) { t.Fatalf("err = %v, want %s", err, tc.wantErr) @@ -204,7 +209,9 @@ func TestRunLogsSourceErrors(t *testing.T) { var buf bytes.Buffer def := slog.Default() slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil))) - t.Cleanup(func() { slog.SetDefault(def) }) + t.Cleanup(func() { + slog.SetDefault(def) + }) // A zero interval makes each worker fail straight away. conf := &config.JobSearchConfig{} @@ -253,7 +260,9 @@ func TestRunShutsDownOnWorkerError(t *testing.T) { conf.Sources.Sleeper.Enabled = true done := make(chan error, 1) - go func() { done <- Run(context.Background(), conf, time.Now().Add(-time.Hour)) }() + go func() { + done <- Run(context.Background(), conf, time.Now().Add(-time.Hour)) + }() select { case err := <-done: diff --git a/internal/fetcher/worker_findatender.go b/internal/fetcher/worker_findatender.go index b77590c..eb2dfd1 100644 --- a/internal/fetcher/worker_findatender.go +++ b/internal/fetcher/worker_findatender.go @@ -15,9 +15,12 @@ import ( // runFindAOpportunity polls Find a Tender every configured interval until ctx // is cancelled. func runFindAOpportunity( - ctx context.Context, logger *slog.Logger, conf *config.JobSearchConfig, since time.Time, + ctx context.Context, + logger *slog.Logger, + conf *config.JobSearchConfig, + since time.Time, ) error { - w := util.Worker{ + worker := util.Worker{ Logger: logger, Name: "FindAOpportunity", Interval: conf.Sources.FindATender.Interval, @@ -25,19 +28,26 @@ func runFindAOpportunity( return fetchFindAOpportunity(ctx, logger, conf, since) }, } - return w.Run(ctx, since) + return worker.Run(ctx, since) } // fetchFindAOpportunity fetches everything updated since since from Find a // Tender. Finding nothing returns ErrNoNewReleases, so the worker still moves // its checkpoint on. func fetchFindAOpportunity( - ctx context.Context, logger *slog.Logger, conf *config.JobSearchConfig, since time.Time, + ctx context.Context, + logger *slog.Logger, + conf *config.JobSearchConfig, + since time.Time, ) error { opportunities, err := util.AwaitTask(ctx, func() ([]*types.Opportunity, error) { - ft := findatender.New(conf, conf.Sources.FindATender.APIRoot, findatender.WithLogger(logger)) - defer ft.Shutdown() - return ft.With(ctx).FetchAll(since, true) + findATender := findatender.New( + conf, + conf.Sources.FindATender.APIRoot, + findatender.WithLogger(logger), + ) + defer findATender.Shutdown() + return findATender.With(ctx).FetchAll(since, true) }) if errors.Is(err, findatender.ErrNoReleases) { return ErrNoNewReleases diff --git a/internal/fetcher/worker_ftsmock.go b/internal/fetcher/worker_ftsmock.go index 47e5fb5..c08f840 100644 --- a/internal/fetcher/worker_ftsmock.go +++ b/internal/fetcher/worker_ftsmock.go @@ -15,9 +15,12 @@ import ( // runFTSMock polls the mock Find a Tender server every configured interval // until ctx is cancelled. func runFTSMock( - ctx context.Context, logger *slog.Logger, conf *config.JobSearchConfig, since time.Time, + ctx context.Context, + logger *slog.Logger, + conf *config.JobSearchConfig, + since time.Time, ) error { - w := util.Worker{ + worker := util.Worker{ Logger: logger, Name: "FTSMock", Interval: conf.Sources.FTSMock.Interval, @@ -25,19 +28,26 @@ func runFTSMock( return fetchFTSMock(ctx, logger, conf, since) }, } - return w.Run(ctx, since) + return worker.Run(ctx, since) } // fetchFTSMock fetches everything updated since since from the mock Find a // Tender server. Finding nothing returns ErrNoNewReleases, so the worker still // moves its checkpoint on. func fetchFTSMock( - ctx context.Context, logger *slog.Logger, conf *config.JobSearchConfig, since time.Time, + ctx context.Context, + logger *slog.Logger, + conf *config.JobSearchConfig, + since time.Time, ) error { opportunities, err := util.AwaitTask(ctx, func() ([]*types.Opportunity, error) { - ft := findatender.New(conf, conf.Sources.FTSMock.APIRoot, findatender.WithLogger(logger)) - defer ft.Shutdown() - return ft.With(ctx).FetchAll(since, true) + findATender := findatender.New( + conf, + conf.Sources.FTSMock.APIRoot, + findatender.WithLogger(logger), + ) + defer findATender.Shutdown() + return findATender.With(ctx).FetchAll(since, true) }) if errors.Is(err, findatender.ErrNoReleases) { return ErrNoNewReleases diff --git a/internal/fetcher/worker_sleeper.go b/internal/fetcher/worker_sleeper.go index 092b22c..b72b272 100644 --- a/internal/fetcher/worker_sleeper.go +++ b/internal/fetcher/worker_sleeper.go @@ -15,10 +15,13 @@ const sleepDuration = 60 * time.Second // runSleeper runs the sleeper test source until ctx is cancelled: every // configured interval it simulates work by sleeping for sleepDuration. func runSleeper( - ctx context.Context, logger *slog.Logger, conf *config.JobSearchConfig, since time.Time, + ctx context.Context, + logger *slog.Logger, + conf *config.JobSearchConfig, + since time.Time, ) error { blockShutdown := conf.Sources.Sleeper.BlockShutdown - w := util.Worker{ + worker := util.Worker{ Logger: logger, Name: "Sleeper", Interval: conf.Sources.Sleeper.Interval, @@ -30,7 +33,7 @@ func runSleeper( } }, } - return w.Run(ctx, since) + return worker.Run(ctx, since) } // sleep simulates work taking duration, returning early with ctx.Err() if ctx diff --git a/internal/findatender/api.go b/internal/findatender/api.go index 67c93af..1ffb559 100644 --- a/internal/findatender/api.go +++ b/internal/findatender/api.go @@ -46,7 +46,7 @@ type RequestOptions struct { AllowRateLimit bool } -// ---------- Utility methods for handling API irregularities ----------- +// ---------- Utility methods for handling API irregularities ---------- // safeReleasePackage is ocds.ReleasePackage with the releases left undecoded, // so safeUnpackReleases can repair or skip each one. @@ -65,14 +65,14 @@ type safeReleasePackage struct { // dateOnly matches a calendar date with no time of day: "2006-10-02". var dateOnly = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}$`) -// unmarshalLenientDates decodes raw json into v like json.Unmarshal, after +// unmarshalLenientDates decodes raw json into target like json.Unmarshal, after // repairing date fields that time.Time cannot parse. -func unmarshalLenientDates(raw json.RawMessage, v any) error { +func unmarshalLenientDates(raw json.RawMessage, target any) error { fixed, err := repairDates(raw) if err != nil { return err } - return json.Unmarshal(fixed, v) + return json.Unmarshal(fixed, target) } // repairDates returns a copy of raw in which every date field holding a @@ -85,15 +85,15 @@ func unmarshalLenientDates(raw json.RawMessage, v any) error { // A date field is one whose name is "date" or starts with "date", contains // "Date", or ends with "Deadline". func repairDates(raw json.RawMessage) (json.RawMessage, error) { - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() // keep numbers exactly as written, not as float64 + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.UseNumber() // keep numbers exactly as written, not as float64 var doc any - if err := dec.Decode(&doc); err != nil { + if err := decoder.Decode(&doc); err != nil { return nil, fmt.Errorf("repairing dates: %w", err) } - if _, err := dec.Token(); !errors.Is(err, io.EOF) { + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { return nil, errors.New( "repairing dates: unexpected data after the JSON value", ) @@ -106,31 +106,31 @@ func repairDates(raw json.RawMessage) (json.RawMessage, error) { return json.Marshal(doc) } -// recursiveRepairDates fixes date fields in v in place and reports whether it +// recursiveRepairDates fixes date fields in value in place and reports whether it // changed anything. -func recursiveRepairDates(v any) bool { +func recursiveRepairDates(value any) bool { changed := false - switch v := v.(type) { + switch typed := value.(type) { case map[string]any: - for key, val := range v { - if s, ok := val.(string); ok && isDateField(key) { + for key, fieldValue := range typed { + if text, ok := fieldValue.(string); ok && isDateField(key) { switch { - case dateOnly.MatchString(s): - v[key] = s + "T00:00:00Z" + case dateOnly.MatchString(text): + typed[key] = text + "T00:00:00Z" changed = true - case s == "": - v[key] = nil + case text == "": + typed[key] = nil changed = true } continue } - if recursiveRepairDates(val) { + if recursiveRepairDates(fieldValue) { changed = true } } case []any: - for _, el := range v { - if recursiveRepairDates(el) { + for _, element := range typed { + if recursiveRepairDates(element) { changed = true } } @@ -152,35 +152,35 @@ func isDateField(name string) bool { // value that wouldn't fit the release struct) doesn't matter. Fields before // them only need to be valid JSON. func releaseIDs(raw json.RawMessage) (ocid, id string, err error) { - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() // a numeric id keeps its exact digits + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.UseNumber() // a numeric id keeps its exact digits - if tok, err := dec.Token(); err != nil { + if token, err := decoder.Token(); err != nil { return "", "", fmt.Errorf("reading release: %w", err) - } else if tok != json.Delim('{') { + } else if token != json.Delim('{') { return "", "", errors.New("reading release: not a JSON object") } var haveOCID, haveID bool - for dec.More() { - tok, err := dec.Token() + for decoder.More() { + token, err := decoder.Token() if err != nil { return ocid, id, fmt.Errorf("reading release: %w", err) } - switch key, _ := tok.(string); key { + switch key, _ := token.(string); key { case "ocid": - if err := dec.Decode(&ocid); err != nil { + if err := decoder.Decode(&ocid); err != nil { return ocid, id, fmt.Errorf("reading release ocid: %w", err) } haveOCID = true case "id": - if id, err = decodeReleaseID(dec); err != nil { + if id, err = decodeReleaseID(decoder); err != nil { return ocid, id, err } haveID = true default: var skip json.RawMessage // must be valid JSON, but is otherwise ignored - if err := dec.Decode(&skip); err != nil { + if err := decoder.Decode(&skip); err != nil { return ocid, id, fmt.Errorf("reading release field %q: %w", key, err) } } @@ -194,18 +194,18 @@ func releaseIDs(raw json.RawMessage) (ocid, id string, err error) { // decodeReleaseID decodes a release id. OCDS ids are strings, but a number is // tolerated and kept as written. -func decodeReleaseID(dec *json.Decoder) (string, error) { - var v any - if err := dec.Decode(&v); err != nil { +func decodeReleaseID(decoder *json.Decoder) (string, error) { + var decoded any + if err := decoder.Decode(&decoded); err != nil { return "", fmt.Errorf("reading release id: %w", err) } - switch v := v.(type) { + switch typed := decoded.(type) { case string: - return v, nil + return typed, nil case json.Number: - return v.String(), nil + return typed.String(), nil default: - return "", fmt.Errorf("reading release id: unexpected %T", v) + return "", fmt.Errorf("reading release id: unexpected %T", typed) } } @@ -227,48 +227,49 @@ func missingIDsError(haveOCID, haveID bool) error { // // This method takes ownership of the response body and closes it when done. func safeUnpackReleases( - logger *slog.Logger, response *resty.Response, + logger *slog.Logger, + response *resty.Response, ) (*ocds.ReleasePackage, error) { - var pkg safeReleasePackage + var releasePackage safeReleasePackage // Take ownership of the response body. defer response.Body.Close() //nolint:errcheck // read only decoder := json.NewDecoder(response.Body) - if err := decoder.Decode(&pkg); err != nil { + if err := decoder.Decode(&releasePackage); err != nil { return nil, err } out := &ocds.ReleasePackage{ - URI: pkg.URI, - Version: pkg.Version, - Extensions: pkg.Extensions, - Publisher: pkg.Publisher, - License: pkg.License, - PublicationPolicy: pkg.PublicationPolicy, - Links: pkg.Links, + URI: releasePackage.URI, + Version: releasePackage.Version, + Extensions: releasePackage.Extensions, + Publisher: releasePackage.Publisher, + License: releasePackage.License, + PublicationPolicy: releasePackage.PublicationPolicy, + Links: releasePackage.Links, } // Correct PublishedDate of "" and "2006-01-02" - if pkg.PublishedDate != nil && len(*pkg.PublishedDate) > 0 { - t, err := time.Parse(time.RFC3339, *pkg.PublishedDate) + if releasePackage.PublishedDate != nil && len(*releasePackage.PublishedDate) > 0 { + published, err := time.Parse(time.RFC3339, *releasePackage.PublishedDate) if err != nil { - t, err = time.Parse(time.DateOnly, *pkg.PublishedDate) + published, err = time.Parse(time.DateOnly, *releasePackage.PublishedDate) } if err == nil { - out.PublishedDate = &t + out.PublishedDate = &published } } - for _, rawRel := range pkg.Releases { + for _, rawRelease := range releasePackage.Releases { var release ocds.Release - err := unmarshalLenientDates(rawRel, &release) + err := unmarshalLenientDates(rawRelease, &release) if err != nil { - ocid, release_id, _ := releaseIDs(rawRel) + ocid, releaseID, _ := releaseIDs(rawRelease) logger.Warn( "skipping unreadable release", "ocid", ocid, - "release_id", release_id, + "release_id", releaseID, "error", err, ) continue @@ -279,7 +280,7 @@ func safeUnpackReleases( return out, nil } -// ---------- API interaction methods ----------- +// ---------- API interaction methods ---------- // rateSleep sleeps the fetch worker until either the context is cancelled // (shutdown), returning ctx.Err(), or the duration has expired. The duration @@ -303,21 +304,21 @@ func closeBody(res *resty.Response) { } } -// fetchURI performs a single GET of uri, using ft's context, and traces it at +// fetchURI performs a single GET of uri, using findATender's context, and traces it at // debug level. Unlike the method of the same name it does not check the // response status. -func fetchURI(ft *FindATender, uri string) (*resty.Response, error) { - withTrace := ft.config.Logging.LogLevel() <= slog.LevelDebug - logger := ft.logger() +func fetchURI(findATender *FindATender, uri string) (*resty.Response, error) { + withTrace := findATender.config.Logging.LogLevel() <= slog.LevelDebug + logger := findATender.logger() logger.Info("fetching", "uri", uri) - ctx := ft.ctx + ctx := findATender.ctx if ctx == nil { ctx = context.Background() } - req := ft.httpClient.R() + req := findATender.httpClient.R() req.SetContext(ctx) res, err := req.SetTrace(withTrace).Get(uri) @@ -373,13 +374,13 @@ func (ft *FindATender) fetchURI( return nil, ErrRateLimitExceeded } - retrySecStr := res.Header().Get("Retry-After") - retrySec, err := strconv.ParseInt(retrySecStr, 10, 32) + retryAfterText := res.Header().Get("Retry-After") + retryAfterSeconds, err := strconv.ParseInt(retryAfterText, 10, 32) if err != nil { return nil, fmt.Errorf("unknown retry-after value: %w", err) } - retryAfter := time.Duration(retrySec) * time.Second + retryAfter := time.Duration(retryAfterSeconds) * time.Second ft.logger().Warn("api rate limit hit, waiting as asked", "retry_after", retryAfter) if rateSleep(ctx, retryAfter) != nil { return nil, ErrFetchCancelled @@ -429,13 +430,13 @@ func (ft *FindATender) fetchWithOpts( // ErrNoNext if there is no "Next" link; the caller can decide whether asking // for it was an error. func (ft *FindATender) fetchNext( - pkg *ocds.ReleasePackage, + releasePackage *ocds.ReleasePackage, allowRateLimit bool, ) (*resty.Response, error) { - if pkg.Links == nil || len(pkg.Links.Next) == 0 { + if releasePackage.Links == nil || len(releasePackage.Links.Next) == 0 { return nil, ErrNoNext } - return ft.fetchURI(pkg.Links.Next, allowRateLimit) + return ft.fetchURI(releasePackage.Links.Next, allowRateLimit) } // fetchOnce makes one call on the API. It does not follow any "Next" links. @@ -445,16 +446,16 @@ func (ft *FindATender) fetchOnce(opts RequestOptions) ([]ocds.Release, error) { return nil, err } - pkg, err := safeUnpackReleases(ft.logger(), res) + releasePackage, err := safeUnpackReleases(ft.logger(), res) if err != nil { return nil, err } - if len(pkg.Releases) == 0 { + if len(releasePackage.Releases) == 0 { return nil, ErrNoReleases } - return pkg.Releases, nil + return releasePackage.Releases, nil } // fetchAll makes a call on the API. If there are any "Next" links, it calls @@ -467,25 +468,25 @@ func (ft *FindATender) fetchAll(opts RequestOptions) ([]ocds.Release, error) { return nil, err } - pkg, err := safeUnpackReleases(ft.logger(), res) + releasePackage, err := safeUnpackReleases(ft.logger(), res) if err != nil { return nil, err } - releases = append(releases, pkg.Releases...) + releases = append(releases, releasePackage.Releases...) // Next links already followed, so a page that links back to an earlier // one cannot loop forever. seen := make(map[string]bool) for { - if pkg.Links != nil && len(pkg.Links.Next) > 0 { - if seen[pkg.Links.Next] { - ft.logger().Warn("stopping at repeated next link", "link", pkg.Links.Next) + if releasePackage.Links != nil && len(releasePackage.Links.Next) > 0 { + if seen[releasePackage.Links.Next] { + ft.logger().Warn("stopping at repeated next link", "link", releasePackage.Links.Next) break } - seen[pkg.Links.Next] = true + seen[releasePackage.Links.Next] = true } - res, err := ft.fetchNext(pkg, opts.AllowRateLimit) + res, err := ft.fetchNext(releasePackage, opts.AllowRateLimit) if err == ErrNoNext { break } @@ -493,11 +494,11 @@ func (ft *FindATender) fetchAll(opts RequestOptions) ([]ocds.Release, error) { return nil, err } - pkg, err = safeUnpackReleases(ft.logger(), res) + releasePackage, err = safeUnpackReleases(ft.logger(), res) if err != nil { return nil, err } - releases = append(releases, pkg.Releases...) + releases = append(releases, releasePackage.Releases...) } if len(releases) == 0 { @@ -505,4 +506,4 @@ func (ft *FindATender) fetchAll(opts RequestOptions) ([]ocds.Release, error) { } return releases, nil -} +} \ No newline at end of file diff --git a/internal/findatender/api_test.go b/internal/findatender/api_test.go index 15fbf68..f09f522 100644 --- a/internal/findatender/api_test.go +++ b/internal/findatender/api_test.go @@ -22,7 +22,11 @@ import ( ) func testConfig(level string) *config.JobSearchConfig { - return &config.JobSearchConfig{Logging: config.JSCLogging{Level: level}} + return &config.JobSearchConfig{ + Logging: config.JSCLogging{ + Level: level, + }, + } } // quietLogs discards slog output for the duration of the test, or sends it @@ -34,7 +38,9 @@ func quietLogs(t *testing.T, h slog.Handler) { } prev := slog.Default() slog.SetDefault(slog.New(h)) - t.Cleanup(func() { slog.SetDefault(prev) }) + t.Cleanup(func() { + slog.SetDefault(prev) + }) } // newTestFT starts h on a test server and returns a FindATender pointed at it. @@ -92,7 +98,10 @@ func TestFetchWithOptsBuildsURI(t *testing.T) { }) since := time.Date(2026, 10, 8, 9, 30, 15, 0, time.UTC) - res, err := ft.fetchWithOpts(RequestOptions{Since: since, Limit: 7}) + res, err := ft.fetchWithOpts(RequestOptions{ + Since: since, + Limit: 7, + }) if err != nil { t.Fatal(err) } @@ -104,7 +113,9 @@ func TestFetchWithOptsBuildsURI(t *testing.T) { t.Errorf("query = %q", gotQuery) } - res, err = ft.fetchWithOpts(RequestOptions{Since: since}) + res, err = ft.fetchWithOpts(RequestOptions{ + Since: since, + }) if err != nil { t.Fatal(err) } @@ -268,7 +279,8 @@ func TestFetchURIRateLimitThenError(t *testing.T) { } func TestFetchURIConnectionError(t *testing.T) { - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) {}) + ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + }) srv.Close() ft.httpClient.SetRetryCount(0) if _, err := ft.fetchURI(srv.URL, true); err == nil { @@ -290,10 +302,14 @@ func TestFetchURICancelledContext(t *testing.T) { func TestFetchURIDebugTrace(t *testing.T) { handlers := map[string]func(io.Writer) slog.Handler{ "text": func(w io.Writer) slog.Handler { - return slog.NewTextHandler(w, &slog.HandlerOptions{Level: slog.LevelDebug}) + return slog.NewTextHandler(w, &slog.HandlerOptions{ + Level: slog.LevelDebug, + }) }, "json": func(w io.Writer) slog.Handler { - return slog.NewJSONHandler(w, &slog.HandlerOptions{Level: slog.LevelDebug}) + return slog.NewJSONHandler(w, &slog.HandlerOptions{ + Level: slog.LevelDebug, + }) }, } for name, mk := range handlers { @@ -305,7 +321,9 @@ func TestFetchURIDebugTrace(t *testing.T) { quietLogs(t, mk(&logs)) // The body must still be readable after the debug logging. - releases, err := ft.fetchOnce(RequestOptions{Since: time.Now()}) + releases, err := ft.fetchOnce(RequestOptions{ + Since: time.Now(), + }) if err != nil { t.Fatal(err) } @@ -325,7 +343,8 @@ func TestFetchURIDebugTrace(t *testing.T) { } func TestFetchURIDebugTraceConnectionError(t *testing.T) { - ft, srv := newTestFT(t, "debug", func(w http.ResponseWriter, r *http.Request) {}) + ft, srv := newTestFT(t, "debug", func(w http.ResponseWriter, r *http.Request) { + }) srv.Close() ft.httpClient.SetRetryCount(0) if _, err := fetchURI(ft, srv.URL); err == nil { @@ -341,8 +360,10 @@ func TestFetchNext(t *testing.T) { }) for name, pkg := range map[string]*ocds.ReleasePackage{ - "nil links": {}, - "empty next": {Links: &ocds.ReleasePackageLinks{Prev: srv.URL}}, + "nil links": {}, + "empty next": {Links: &ocds.ReleasePackageLinks{ + Prev: srv.URL, + }}, } { if _, err := ft.fetchNext(pkg, true); !errors.Is(err, ErrNoNext) { t.Errorf("%s: err = %v, want ErrNoNext", name, err) @@ -350,7 +371,11 @@ func TestFetchNext(t *testing.T) { } res, err := ft.fetchNext( - &ocds.ReleasePackage{Links: &ocds.ReleasePackageLinks{Next: srv.URL + "/page2"}}, + &ocds.ReleasePackage{ + Links: &ocds.ReleasePackageLinks{ + Next: srv.URL + "/page2", + }, + }, true, ) if err != nil { @@ -372,7 +397,9 @@ func responseFor(t *testing.T, body string) *resty.Response { if err != nil { t.Fatal(err) } - t.Cleanup(func() { res.Body.Close() }) + t.Cleanup(func() { + res.Body.Close() + }) return res } @@ -481,11 +508,14 @@ func pagedServer(t *testing.T, pages ...[]string) (*FindATender, *atomic.Int32) } func TestFetchOnce(t *testing.T) { - ft, hits := pagedServer(t, + ft, hits := pagedServer( + t, []string{releaseJSON("ocds-1", "1"), releaseJSON("ocds-2", "1")}, []string{releaseJSON("ocds-3", "1")}, ) - releases, err := ft.fetchOnce(RequestOptions{Since: time.Now()}) + releases, err := ft.fetchOnce(RequestOptions{ + Since: time.Now(), + }) if err != nil { t.Fatal(err) } @@ -499,14 +529,18 @@ func TestFetchOnce(t *testing.T) { func TestFetchOnceErrors(t *testing.T) { ft, _ := pagedServer(t, []string{}) - if _, err := ft.fetchOnce(RequestOptions{Since: time.Now()}); !errors.Is(err, ErrNoReleases) { + if _, err := ft.fetchOnce(RequestOptions{ + Since: time.Now(), + }); !errors.Is(err, ErrNoReleases) { t.Errorf("empty page: err = %v, want ErrNoReleases", err) } ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }) - if _, err := ft.fetchOnce(RequestOptions{Since: time.Now()}); err == nil { + if _, err := ft.fetchOnce(RequestOptions{ + Since: time.Now(), + }); err == nil { t.Error("bad body: expected an error") } @@ -514,7 +548,9 @@ func TestFetchOnceErrors(t *testing.T) { w.WriteHeader(http.StatusTooManyRequests) }) if _, err := ft.fetchOnce( - RequestOptions{Since: time.Now()}, + RequestOptions{ + Since: time.Now(), + }, ); !errors.Is( err, ErrRateLimitExceeded, @@ -524,12 +560,15 @@ func TestFetchOnceErrors(t *testing.T) { } func TestFetchAll(t *testing.T) { - ft, hits := pagedServer(t, + ft, hits := pagedServer( + t, []string{releaseJSON("ocds-1", "1"), releaseJSON("ocds-2", "1")}, []string{}, []string{releaseJSON("ocds-1", "2")}, ) - releases, err := ft.fetchAll(RequestOptions{Since: time.Now()}) + releases, err := ft.fetchAll(RequestOptions{ + Since: time.Now(), + }) if err != nil { t.Fatal(err) } @@ -547,7 +586,9 @@ func TestFetchAll(t *testing.T) { func TestFetchAllErrors(t *testing.T) { ft, _ := pagedServer(t, []string{}, []string{}) - if _, err := ft.fetchAll(RequestOptions{Since: time.Now()}); !errors.Is(err, ErrNoReleases) { + if _, err := ft.fetchAll(RequestOptions{ + Since: time.Now(), + }); !errors.Is(err, ErrNoReleases) { t.Errorf("all pages empty: err = %v, want ErrNoReleases", err) } @@ -556,7 +597,9 @@ func TestFetchAllErrors(t *testing.T) { w.WriteHeader(http.StatusTooManyRequests) }) if _, err := ft.fetchAll( - RequestOptions{Since: time.Now()}, + RequestOptions{ + Since: time.Now(), + }, ); !errors.Is( err, ErrRateLimitExceeded, @@ -567,14 +610,20 @@ func TestFetchAllErrors(t *testing.T) { ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }) - if _, err := ft.fetchAll(RequestOptions{Since: time.Now()}); err == nil { + if _, err := ft.fetchAll(RequestOptions{ + Since: time.Now(), + }); err == nil { t.Error("first page bad body: expected an error") } // A later page fails, either to fetch or to decode. for name, fail := range map[string]http.HandlerFunc{ - "rate limited": func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTooManyRequests) }, - "bad body": func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }, + "rate limited": func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + }, + "bad body": func(w http.ResponseWriter, r *http.Request) { + io.WriteString(w, "not json") + }, } { t.Run(name, func(t *testing.T) { var srvURL string @@ -586,7 +635,9 @@ func TestFetchAllErrors(t *testing.T) { fail(w, r) }) srvURL = srv.URL - if _, err := ft.fetchAll(RequestOptions{Since: time.Now()}); err == nil { + if _, err := ft.fetchAll(RequestOptions{ + Since: time.Now(), + }); err == nil { t.Error("expected an error from the second page") } }) @@ -597,7 +648,9 @@ func TestFetchAllErrors(t *testing.T) { func TestFetchAllRepeatedNextLink(t *testing.T) { tests := map[string]func(base string, page string) string{ // page 0 -> page 1 -> page 1 ... - "self link": func(base, page string) string { return base + "/next?page=1" }, + "self link": func(base, page string) string { + return base + "/next?page=1" + }, // page 0 -> page 1 -> page 2 -> page 1 ... "cycle": func(base, page string) string { if page == "1" { @@ -623,7 +676,9 @@ func TestFetchAllRepeatedNextLink(t *testing.T) { }) srvURL = srv.URL - releases, err := ft.fetchAll(RequestOptions{Since: time.Now()}) + releases, err := ft.fetchAll(RequestOptions{ + Since: time.Now(), + }) if err != nil { t.Fatal(err) } @@ -668,7 +723,9 @@ func TestUnmarshalLenientDates(t *testing.T) { t.Fatal(err) } - midnight := func(y int, m time.Month, d int) time.Time { return time.Date(y, m, d, 0, 0, 0, 0, time.UTC) } + midnight := func(y int, m time.Month, d int) time.Time { + return time.Date(y, m, d, 0, 0, 0, 0, time.UTC) + } if !got.Date.Equal(midnight(2006, 10, 2)) { t.Errorf("date = %v", got.Date) } diff --git a/internal/findatender/findatender.go b/internal/findatender/findatender.go index d4c55dd..c8c0638 100644 --- a/internal/findatender/findatender.go +++ b/internal/findatender/findatender.go @@ -35,17 +35,17 @@ func WithLogger(logger *slog.Logger) Option { // 60 seconds and are retried once. Call Shutdown when finished with it. It // logs to slog.Default() unless given WithLogger. func New(conf *config.JobSearchConfig, apiRoot string, opts ...Option) *FindATender { - ft := &FindATender{ + findATender := &FindATender{ config: conf, apiRoot: apiRoot, } for _, opt := range opts { - opt(ft) + opt(findATender) } - ft.httpClient = resty.New(). + findATender.httpClient = resty.New(). SetTimeout(60 * time.Duration(time.Second)). SetRetryCount(1) - return ft + return findATender } // logger returns the logger the client logs to, tagged with its component. diff --git a/internal/findatender/findatender_test.go b/internal/findatender/findatender_test.go index 2385475..3862135 100644 --- a/internal/findatender/findatender_test.go +++ b/internal/findatender/findatender_test.go @@ -79,7 +79,8 @@ func opportunityIDs(ops []*types.Opportunity) []string { } func TestFetchOncePublic(t *testing.T) { - ft, _ := pagedServer(t, + ft, _ := pagedServer( + t, []string{releaseJSON("ocds-1", "1"), releaseJSON("ocds-1", "2"), releaseJSON("ocds-2", "1")}, []string{releaseJSON("ocds-3", "1")}, ) @@ -98,7 +99,8 @@ func TestFetchOncePublic(t *testing.T) { } func TestFetchAllPublic(t *testing.T) { - ft, _ := pagedServer(t, + ft, _ := pagedServer( + t, []string{releaseJSON("ocds-1", "1"), releaseJSON("ocds-2", "1")}, []string{releaseJSON("ocds-1", "2"), releaseJSON("ocds-3", "1")}, ) diff --git a/internal/findatender/type_convert.go b/internal/findatender/type_convert.go index 2da1671..c2cc5c7 100644 --- a/internal/findatender/type_convert.go +++ b/internal/findatender/type_convert.go @@ -48,8 +48,8 @@ func ocdsGetClassifications(release *ocds.Release) []types.ClassificationCode { add := func(classif *ocds.Classification) { newClassif := ocdsConvertClassification(classif) found := slices.ContainsFunc( - classifications, func(r types.ClassificationCode) bool { - return r.Code == newClassif.Code + classifications, func(code types.ClassificationCode) bool { + return code.Code == newClassif.Code }, ) if !found { @@ -59,11 +59,11 @@ func ocdsGetClassifications(release *ocds.Release) []types.ClassificationCode { if release.Tender.Classification != nil { add(release.Tender.Classification) } - for _, val := range release.Tender.Items { - if val.Classification != nil { - add(val.Classification) + for _, item := range release.Tender.Items { + if item.Classification != nil { + add(item.Classification) } - for _, classif := range val.AdditionalClassifications { + for _, classif := range item.AdditionalClassifications { add(&classif) } } @@ -113,15 +113,15 @@ func ocdsIsVCSESuitable(release *ocds.Release) bool { // re-encoded copy of the whole release as JSON. func ocdsRelease(release *ocds.Release) types.OpportunityRelease { tags := make([]types.OpportunityReleaseTag, 0) - for _, val := range release.Tag { + for _, tag := range release.Tag { tags = append(tags, types.OpportunityReleaseTag{ UUID: release.OCID, ReleaseID: release.ID, - Tag: val, + Tag: tag, }) } - rel := types.OpportunityRelease{ + converted := types.OpportunityRelease{ UUID: release.OCID, ReleaseID: release.ID, Tags: tags, @@ -130,16 +130,16 @@ func ocdsRelease(release *ocds.Release) types.OpportunityRelease { reencoded, err := json.Marshal(release) if err == nil { - rel.JSON = string(reencoded) + converted.JSON = string(reencoded) } - return rel + return converted } // convertOne builds an opportunity from a single release. Convert passes it // each ocid's latest release, as that has the current state of the tender. func convertOne(release *ocds.Release) *types.Opportunity { - t := &types.Opportunity{ + opportunity := &types.Opportunity{ UUID: release.OCID, UUIDType: "ocid", BuyerId: ocdsGetBuyerId(release), @@ -148,29 +148,29 @@ func convertOne(release *ocds.Release) *types.Opportunity { } if release.Tender != nil { - t.Title = release.Tender.Title - t.Status = release.Tender.Status - t.Description = release.Tender.Description - t.Classifications = ocdsGetClassifications(release) - t.MainCategory = release.Tender.MainProcurementCategory - t.ProcurementMethod = release.Tender.ProcurementMethod - t.SMESuitable = ocdsIsSMESuitable(release) - t.VCSESuitable = ocdsIsVCSESuitable(release) + opportunity.Title = release.Tender.Title + opportunity.Status = release.Tender.Status + opportunity.Description = release.Tender.Description + opportunity.Classifications = ocdsGetClassifications(release) + opportunity.MainCategory = release.Tender.MainProcurementCategory + opportunity.ProcurementMethod = release.Tender.ProcurementMethod + opportunity.SMESuitable = ocdsIsSMESuitable(release) + opportunity.VCSESuitable = ocdsIsVCSESuitable(release) if release.Tender.TenderPeriod != nil { - t.StartDate = release.Tender.TenderPeriod.StartDate - t.EndDate = release.Tender.TenderPeriod.EndDate + opportunity.StartDate = release.Tender.TenderPeriod.StartDate + opportunity.EndDate = release.Tender.TenderPeriod.EndDate } if release.Tender.Value != nil && release.Tender.Value.Amount != nil { - t.GrossValue = *release.Tender.Value.Amount + opportunity.GrossValue = *release.Tender.Value.Amount } - t.HasDocuments = len(release.Tender.Documents) > 0 - t.MultiLot = len(release.Tender.Lots) > 1 + opportunity.HasDocuments = len(release.Tender.Documents) > 0 + opportunity.MultiLot = len(release.Tender.Lots) > 1 } - t.LatestReleaseDate = release.Date - t.Releases = append(t.Releases, ocdsRelease(release)) + opportunity.LatestReleaseDate = release.Date + opportunity.Releases = append(opportunity.Releases, ocdsRelease(release)) - return t + return opportunity } // Convert groups releases by ocid into opportunities, in the order each ocid @@ -181,8 +181,8 @@ func Convert(releases []ocds.Release) []*types.Opportunity { var order []string byOCID := make(map[string][]*ocds.Release) - for idx := range releases { - release := &releases[idx] + for i := range releases { + release := &releases[i] if _, found := byOCID[release.OCID]; !found { order = append(order, release.OCID) } @@ -193,8 +193,8 @@ func Convert(releases []ocds.Release) []*types.Opportunity { for _, ocid := range order { group := byOCID[ocid] - slices.SortStableFunc(group, func(a, b *ocds.Release) int { - return a.Date.Compare(b.Date) + slices.SortStableFunc(group, func(left, right *ocds.Release) int { + return left.Date.Compare(right.Date) }) opportunity := convertOne(group[len(group)-1]) diff --git a/internal/findatender/type_convert_test.go b/internal/findatender/type_convert_test.go index 9c270ab..e5006b5 100644 --- a/internal/findatender/type_convert_test.go +++ b/internal/findatender/type_convert_test.go @@ -13,7 +13,12 @@ import ( func boolPtr(b bool) *bool { return &b } -func lotWith(s *ocds.Suitability) ocds.Lot { return ocds.Lot{ID: "1", Suitability: s} } +func lotWith(s *ocds.Suitability) ocds.Lot { + return ocds.Lot{ + ID: "1", + Suitability: s, + } +} func TestOcdsGetBuyer(t *testing.T) { tests := []struct { @@ -23,11 +28,16 @@ func TestOcdsGetBuyer(t *testing.T) { }{ {"nil buyer", nil, "", ""}, {"empty buyer", &ocds.OrganizationReference{}, "", ""}, - {"full buyer", &ocds.OrganizationReference{ID: "GB-1", Name: "Council"}, "GB-1", "Council"}, + {"full buyer", &ocds.OrganizationReference{ + ID: "GB-1", + Name: "Council", + }, "GB-1", "Council"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := &ocds.Release{Buyer: tt.buyer} + r := &ocds.Release{ + Buyer: tt.buyer, + } if got := ocdsGetBuyerId(r); got != tt.wantID { t.Errorf("buyer id = %q, want %q", got, tt.wantID) } @@ -44,12 +54,34 @@ func TestOcdsConvertClassification(t *testing.T) { in ocds.Classification want types.ClassificationCode }{ - {"CPV", ocds.Classification{Scheme: "CPV", ID: "72000000", Description: "IT services"}, - types.ClassificationCode{IsCPV: true, Code: "72000000", Classification: "IT services"}}, - {"non-CPV", ocds.Classification{Scheme: "UNSPSC", ID: "43000000", Description: "IT"}, - types.ClassificationCode{IsCPV: false, Code: "43000000", Classification: "IT"}}, - {"scheme is case sensitive", ocds.Classification{Scheme: "cpv", ID: "1"}, - types.ClassificationCode{IsCPV: false, Code: "1"}}, + {"CPV", ocds.Classification{ + Scheme: "CPV", + ID: "72000000", + Description: "IT services", + }, + types.ClassificationCode{ + IsCPV: true, + Code: "72000000", + Classification: "IT services", + }}, + {"non-CPV", ocds.Classification{ + Scheme: "UNSPSC", + ID: "43000000", + Description: "IT", + }, + types.ClassificationCode{ + IsCPV: false, + Code: "43000000", + Classification: "IT", + }}, + {"scheme is case sensitive", ocds.Classification{ + Scheme: "cpv", + ID: "1", + }, + types.ClassificationCode{ + IsCPV: false, + Code: "1", + }}, {"empty", ocds.Classification{}, types.ClassificationCode{}}, } for _, tt := range tests { @@ -63,7 +95,11 @@ func TestOcdsConvertClassification(t *testing.T) { func TestOcdsGetClassifications(t *testing.T) { cpv := func(id string) ocds.Classification { - return ocds.Classification{Scheme: "CPV", ID: ocds.ID(id), Description: "d" + id} + return ocds.Classification{ + Scheme: "CPV", + ID: ocds.ID(id), + Description: "d" + id, + } } codes := func(cs []types.ClassificationCode) []string { out := []string{} @@ -80,7 +116,9 @@ func TestOcdsGetClassifications(t *testing.T) { }{ {"nil tender", nil, []string{}}, {"no classifications", &ocds.Tender{}, []string{}}, - {"main only", &ocds.Tender{Classification: &main}, []string{"1"}}, + {"main only", &ocds.Tender{ + Classification: &main, + }, []string{"1"}}, {"main and item additionals, deduplicated", &ocds.Tender{ Classification: &main, Items: []ocds.Item{ @@ -95,7 +133,9 @@ func TestOcdsGetClassifications(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ocdsGetClassifications(&ocds.Release{Tender: tt.tender}) + got := ocdsGetClassifications(&ocds.Release{ + Tender: tt.tender, + }) if got == nil { t.Fatal("got nil slice, want non-nil") } @@ -107,7 +147,10 @@ func TestOcdsGetClassifications(t *testing.T) { } func TestOcdsGetClassificationsIncludesItemClassification(t *testing.T) { - item := ocds.Classification{Scheme: "CPV", ID: "9"} + item := ocds.Classification{ + Scheme: "CPV", + ID: "9", + } got := ocdsGetClassifications(&ocds.Release{Tender: &ocds.Tender{ Items: []ocds.Item{{ID: "a", Classification: &item}}, }}) @@ -125,31 +168,49 @@ func TestOcdsSuitability(t *testing.T) { }{ {"nil tender", nil, false, false}, {"no lots", &ocds.Tender{}, false, false}, - {"lot without suitability", &ocds.Tender{Lots: []ocds.Lot{lotWith(nil)}}, false, false}, + {"lot without suitability", &ocds.Tender{ + Lots: []ocds.Lot{lotWith(nil)}, + }, false, false}, { "suitability fields nil", - &ocds.Tender{Lots: []ocds.Lot{lotWith(&ocds.Suitability{})}}, + &ocds.Tender{ + Lots: []ocds.Lot{lotWith(&ocds.Suitability{})}, + }, false, false, }, {"explicitly false", &ocds.Tender{Lots: []ocds.Lot{ - lotWith(&ocds.Suitability{SME: boolPtr(false), Vcse: boolPtr(false)}), + lotWith(&ocds.Suitability{ + SME: boolPtr(false), + Vcse: boolPtr(false), + }), }}, false, false}, {"SME only", &ocds.Tender{Lots: []ocds.Lot{ - lotWith(&ocds.Suitability{SME: boolPtr(true)}), + lotWith(&ocds.Suitability{ + SME: boolPtr(true), + }), }}, true, false}, {"VCSE only", &ocds.Tender{Lots: []ocds.Lot{ - lotWith(&ocds.Suitability{Vcse: boolPtr(true)}), + lotWith(&ocds.Suitability{ + Vcse: boolPtr(true), + }), }}, false, true}, {"any lot suffices", &ocds.Tender{Lots: []ocds.Lot{ lotWith(nil), - lotWith(&ocds.Suitability{SME: boolPtr(false)}), - lotWith(&ocds.Suitability{SME: boolPtr(true), Vcse: boolPtr(true)}), + lotWith(&ocds.Suitability{ + SME: boolPtr(false), + }), + lotWith(&ocds.Suitability{ + SME: boolPtr(true), + Vcse: boolPtr(true), + }), }}, true, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := &ocds.Release{Tender: tt.tender} + r := &ocds.Release{ + Tender: tt.tender, + } if got := ocdsIsSMESuitable(r); got != tt.wantSME { t.Errorf("SME = %v, want %v", got, tt.wantSME) } @@ -162,7 +223,12 @@ func TestOcdsSuitability(t *testing.T) { func TestOcdsRelease(t *testing.T) { date := time.Date(2026, 10, 8, 12, 0, 0, 0, time.UTC) - r := &ocds.Release{OCID: "ocds-x-1", ID: "001", Date: date, Tag: []string{"planning", "tender"}} + r := &ocds.Release{ + OCID: "ocds-x-1", + ID: "001", + Date: date, + Tag: []string{"planning", "tender"}, + } got := ocdsRelease(r) if got.UUID != "ocds-x-1" || got.ReleaseID != "001" || !got.ReleaseDate.Equal(date) { @@ -193,12 +259,19 @@ func TestConvertOne(t *testing.T) { start := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC) end := time.Date(2026, 11, 1, 0, 0, 0, 0, time.UTC) amount := 125000.5 - main := ocds.Classification{Scheme: "CPV", ID: "72000000", Description: "IT"} + main := ocds.Classification{ + Scheme: "CPV", + ID: "72000000", + Description: "IT", + } r := &ocds.Release{ - OCID: "ocds-x-1", - ID: "001", - Tag: []string{"tender"}, - Buyer: &ocds.OrganizationReference{ID: "GB-1", Name: "Council"}, + OCID: "ocds-x-1", + ID: "001", + Tag: []string{"tender"}, + Buyer: &ocds.OrganizationReference{ + ID: "GB-1", + Name: "Council", + }, Tender: &ocds.Tender{ Title: "Website", Status: "active", @@ -206,10 +279,17 @@ func TestConvertOne(t *testing.T) { Classification: &main, MainProcurementCategory: "services", ProcurementMethod: "open", - Lots: []ocds.Lot{lotWith(&ocds.Suitability{SME: boolPtr(true)})}, - TenderPeriod: &ocds.Period{StartDate: &start, EndDate: &end}, - Value: &ocds.Value{Amount: &amount}, - Documents: []ocds.Document{{ID: "doc"}}, + Lots: []ocds.Lot{lotWith(&ocds.Suitability{ + SME: boolPtr(true), + })}, + TenderPeriod: &ocds.Period{ + StartDate: &start, + EndDate: &end, + }, + Value: &ocds.Value{ + Amount: &amount, + }, + Documents: []ocds.Document{{ID: "doc"}}, }, } @@ -250,12 +330,20 @@ func TestConvertOneMinimal(t *testing.T) { }{ {"nil tender", nil}, {"empty tender", &ocds.Tender{}}, - {"value without amount", &ocds.Tender{Value: &ocds.Value{}}}, - {"empty tender period", &ocds.Tender{TenderPeriod: &ocds.Period{}}}, + {"value without amount", &ocds.Tender{ + Value: &ocds.Value{}, + }}, + {"empty tender period", &ocds.Tender{ + TenderPeriod: &ocds.Period{}, + }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := convertOne(&ocds.Release{OCID: "ocds-x-2", ID: "1", Tender: tt.tender}) + got := convertOne(&ocds.Release{ + OCID: "ocds-x-2", + ID: "1", + Tender: tt.tender, + }) if got.UUID != "ocds-x-2" || got.BuyerId != "" || got.BuyerName != "" { t.Errorf("got %+v", got) } @@ -274,14 +362,22 @@ func TestConvert(t *testing.T) { t.Errorf("Convert(nil) = %v, want empty", got) } - day := func(d int) time.Time { return time.Date(2026, 1, d, 0, 0, 0, 0, time.UTC) } + day := func(d int) time.Time { + return time.Date(2026, 1, d, 0, 0, 0, 0, time.UTC) + } releases := []ocds.Release{ - {OCID: "ocds-b", ID: "1", Date: day(1), Tender: &ocds.Tender{Title: "B"}}, + {OCID: "ocds-b", ID: "1", Date: day(1), Tender: &ocds.Tender{ + Title: "B", + }}, {OCID: "ocds-a", ID: "2", Date: day(3), Tender: &ocds.Tender{ Title: "A latest", Lots: []ocds.Lot{{ID: "1"}, {ID: "2"}}, }}, - {OCID: "ocds-a", ID: "1", Date: day(2), Tender: &ocds.Tender{Title: "A first"}}, - {OCID: "ocds-a", ID: "3", Date: day(1), Tender: &ocds.Tender{Title: "A oldest"}}, + {OCID: "ocds-a", ID: "1", Date: day(2), Tender: &ocds.Tender{ + Title: "A first", + }}, + {OCID: "ocds-a", ID: "3", Date: day(1), Tender: &ocds.Tender{ + Title: "A oldest", + }}, } got := Convert(releases) @@ -321,8 +417,12 @@ func TestConvert(t *testing.T) { func TestConvertSameDate(t *testing.T) { date := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) got := Convert([]ocds.Release{ - {OCID: "ocds-a", ID: "1", Date: date, Tender: &ocds.Tender{Title: "first"}}, - {OCID: "ocds-a", ID: "2", Date: date, Tender: &ocds.Tender{Title: "second"}}, + {OCID: "ocds-a", ID: "1", Date: date, Tender: &ocds.Tender{ + Title: "first", + }}, + {OCID: "ocds-a", ID: "2", Date: date, Tender: &ocds.Tender{ + Title: "second", + }}, }) if len(got) != 1 || got[0].Title != "second" { t.Fatalf("got %+v, want one opportunity titled %q", got, "second") @@ -335,8 +435,10 @@ func TestConvertSetsSource(t *testing.T) { const want = "FindATender" for name, release := range map[string]*ocds.Release{ - "with tender": {OCID: "ocds-a", ID: "1", Tender: &ocds.Tender{Title: "A"}}, - "nil tender": {OCID: "ocds-b", ID: "1"}, + "with tender": {OCID: "ocds-a", ID: "1", Tender: &ocds.Tender{ + Title: "A", + }}, + "nil tender": {OCID: "ocds-b", ID: "1"}, } { if got := convertOne(release).Source; got != want { t.Errorf("convertOne (%s): Source = %q, want %q", name, got, want) @@ -344,8 +446,12 @@ func TestConvertSetsSource(t *testing.T) { } for _, o := range Convert([]ocds.Release{ - {OCID: "ocds-a", ID: "1", Tender: &ocds.Tender{Title: "A"}}, - {OCID: "ocds-a", ID: "2", Tender: &ocds.Tender{Title: "A amended"}}, + {OCID: "ocds-a", ID: "1", Tender: &ocds.Tender{ + Title: "A", + }}, + {OCID: "ocds-a", ID: "2", Tender: &ocds.Tender{ + Title: "A amended", + }}, {OCID: "ocds-b", ID: "1"}, }) { if o.Source != want { diff --git a/internal/httpx/problem.go b/internal/httpx/problem.go index abbe97e..b5eb610 100644 --- a/internal/httpx/problem.go +++ b/internal/httpx/problem.go @@ -18,17 +18,17 @@ type Problem struct { // WriteProblem writes a problem+json error response. func WriteProblem(w http.ResponseWriter, r *http.Request, status int, detail string) { - p := Problem{ + problem := Problem{ Type: "about:blank", Title: http.StatusText(status), Status: status, Detail: detail, } if r != nil { - p.RequestID = RequestID(r.Context()) + problem.RequestID = RequestID(r.Context()) } w.Header().Set("Content-Type", "application/problem+json") w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(status) - json.NewEncoder(w).Encode(p) //nolint:errcheck,gosec // the client may have gone; nothing to do + json.NewEncoder(w).Encode(problem) //nolint:errcheck,gosec // the client may have gone } diff --git a/internal/logging/accesslog.go b/internal/logging/accesslog.go index f12b4d9..204010d 100644 --- a/internal/logging/accesslog.go +++ b/internal/logging/accesslog.go @@ -26,32 +26,32 @@ func (r *statusRecorder) WriteHeader(code int) { r.ResponseWriter.WriteHeader(code) } -func (r *statusRecorder) Write(b []byte) (int, error) { +func (r *statusRecorder) Write(data []byte) (int, error) { if r.status == 0 { r.status = http.StatusOK } - n, err := r.ResponseWriter.Write(b) - r.bytes += n - return n, err + written, err := r.ResponseWriter.Write(data) + r.bytes += written + return written, err } // logField escapes a value for a quoted log field the way nginx does // (quotes, backslashes and control characters become \xHH) and uses "-" // for empty values. -func logField(s string) string { - if s == "" { +func logField(value string) string { + if value == "" { return "-" } - var b strings.Builder - for i := 0; i < len(s); i++ { - c := s[i] - if c == '"' || c == '\\' || c < 0x20 || c == 0x7f { - fmt.Fprintf(&b, "\\x%02X", c) + var builder strings.Builder + for i := 0; i < len(value); i++ { + char := value[i] + if char == '"' || char == '\\' || char < 0x20 || char == 0x7f { + fmt.Fprintf(&builder, "\\x%02X", char) } else { - b.WriteByte(c) + builder.WriteByte(char) } } - return b.String() + return builder.String() } // clientKey attempts to produce a client host name as a key identifier to split @@ -71,26 +71,29 @@ func clientKey(req *http.Request) string { func AccessLog(logger *log.Logger, combined bool, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { start := time.Now() - rec := &statusRecorder{ResponseWriter: w} - next.ServeHTTP(rec, req) - if rec.status == 0 { - rec.status = http.StatusOK + recorder := &statusRecorder{ + ResponseWriter: w, + } + next.ServeHTTP(recorder, req) + if recorder.status == 0 { + recorder.status = http.StatusOK } user := "-" - if u, _, ok := req.BasicAuth(); ok && u != "" { - user = logField(u) + if username, _, ok := req.BasicAuth(); ok && username != "" { + user = logField(username) } size := "-" // CLF uses "-" for an empty body - if rec.bytes > 0 { - size = strconv.Itoa(rec.bytes) + if recorder.bytes > 0 { + size = strconv.Itoa(recorder.bytes) } - line := fmt.Sprintf(`%s - %s [%s] "%s" %d %s`, + line := fmt.Sprintf( + `%s - %s [%s] "%s" %d %s`, clientKey(req), user, start.Format("02/Jan/2006:15:04:05 -0700"), logField(req.Method+" "+req.RequestURI+" "+req.Proto), - rec.status, + recorder.status, size, ) if combined { diff --git a/internal/logging/accesslog_test.go b/internal/logging/accesslog_test.go index 0a1f668..b70fb3a 100644 --- a/internal/logging/accesslog_test.go +++ b/internal/logging/accesslog_test.go @@ -40,7 +40,9 @@ func TestClientKey(t *testing.T) { {"", ""}, } for _, tc := range tests { - req := &http.Request{RemoteAddr: tc.remote} + req := &http.Request{ + RemoteAddr: tc.remote, + } if got := clientKey(req); got != tc.want { t.Errorf("clientKey(%q) = %q, want %q", tc.remote, got, tc.want) } @@ -49,7 +51,9 @@ func TestClientKey(t *testing.T) { func TestStatusRecorder(t *testing.T) { t.Run("first WriteHeader wins", func(t *testing.T) { - rec := &statusRecorder{ResponseWriter: httptest.NewRecorder()} + rec := &statusRecorder{ + ResponseWriter: httptest.NewRecorder(), + } rec.WriteHeader(http.StatusTeapot) rec.WriteHeader(http.StatusInternalServerError) if rec.status != http.StatusTeapot { @@ -58,7 +62,9 @@ func TestStatusRecorder(t *testing.T) { }) t.Run("Write implies 200 and counts bytes", func(t *testing.T) { inner := httptest.NewRecorder() - rec := &statusRecorder{ResponseWriter: inner} + rec := &statusRecorder{ + ResponseWriter: inner, + } rec.Write([]byte("hello")) rec.Write([]byte(" world")) if rec.status != http.StatusOK || rec.bytes != 11 { @@ -81,9 +87,10 @@ func TestAccessLog(t *testing.T) { want string // regexp for the whole line }{ { - name: "no body defaults to 200 and -", - handler: func(http.ResponseWriter, *http.Request) {}, - want: `^192\.0\.2\.1 - - ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 -$`, + name: "no body defaults to 200 and -", + handler: func(http.ResponseWriter, *http.Request) { + }, + want: `^192\.0\.2\.1 - - ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 -$`, }, { name: "status and size", @@ -94,15 +101,20 @@ func TestAccessLog(t *testing.T) { want: `^192\.0\.2\.1 - - ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 404 4$`, }, { - name: "basic auth user is escaped", - handler: func(w http.ResponseWriter, _ *http.Request) { w.Write([]byte("x")) }, - setup: func(r *http.Request) { r.SetBasicAuth(`bo"b`, "pw") }, - want: `^192\.0\.2\.1 - bo\\x22b ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 1$`, + name: "basic auth user is escaped", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("x")) + }, + setup: func(r *http.Request) { + r.SetBasicAuth(`bo"b`, "pw") + }, + want: `^192\.0\.2\.1 - bo\\x22b ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 1$`, }, { name: "combined adds referer and user agent", combined: true, - handler: func(http.ResponseWriter, *http.Request) {}, + handler: func(http.ResponseWriter, *http.Request) { + }, setup: func(r *http.Request) { r.Header.Set("Referer", "http://ref/") r.Header.Set("User-Agent", `ua "1"`) @@ -112,9 +124,12 @@ func TestAccessLog(t *testing.T) { { name: "combined with missing headers", combined: true, - handler: func(http.ResponseWriter, *http.Request) {}, - setup: func(r *http.Request) { r.Header.Del("User-Agent") }, - want: `^192\.0\.2\.1 - - ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 - "-" "-"$`, + handler: func(http.ResponseWriter, *http.Request) { + }, + setup: func(r *http.Request) { + r.Header.Del("User-Agent") + }, + want: `^192\.0\.2\.1 - - ` + clfTime + ` "GET /path\?q=1 HTTP/1\.1" 200 - "-" "-"$`, }, } for _, tc := range tests { diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 47d7b53..56c753f 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -135,9 +135,9 @@ func (h terminalHandler) WithGroup(name string) slog.Handler { } } -// IsJSONHandler reports whether h writes JSON, as the file logger does. -func IsJSONHandler(h slog.Handler) bool { - _, ok := h.(*slog.JSONHandler) +// IsJSONHandler reports whether handler writes JSON, as the file logger does. +func IsJSONHandler(handler slog.Handler) bool { + _, ok := handler.(*slog.JSONHandler) return ok } diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index 484d7dc..44c88cf 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -67,9 +67,12 @@ func TestIsJSONHandler(t *testing.T) { {"tint", tint.NewTextHandler(io.Discard, nil), false}, {"nil", nil, false}, {"json with attrs", slog.NewJSONHandler(io.Discard, nil).WithAttrs( - []slog.Attr{slog.String("k", "v")}), true}, + []slog.Attr{slog.String("k", "v")}, + ), true}, // Only slog's own JSON handler is recognised, not one wrapped in another. - {"wrapped json", wrapper{slog.NewJSONHandler(io.Discard, nil)}, false}, + {"wrapped json", wrapper{ + Handler: slog.NewJSONHandler(io.Discard, nil), + }, false}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/middleware/ratelimit.go b/internal/middleware/ratelimit.go index dd8ad80..054c4d0 100644 --- a/internal/middleware/ratelimit.go +++ b/internal/middleware/ratelimit.go @@ -24,6 +24,7 @@ type Limiter interface { // LimiterFunc adapts a function to the Limiter interface. type LimiterFunc func(key string) (bool, time.Duration) +// Allow implements Limiter by calling f. func (f LimiterFunc) Allow(key string) (bool, time.Duration) { return f(key) } // KeyFunc derives the rate limit key from a request. @@ -34,22 +35,24 @@ func KeyByIP(r *http.Request) string { return ClientIP(r) } // RateLimit turns a Limiter into middleware. Rejected requests get 429 with // a Retry-After header. A nil limiter yields a pass-through middleware. -func RateLimit(l Limiter, key KeyFunc) func(http.Handler) http.Handler { - if l == nil { - return func(next http.Handler) http.Handler { return next } +func RateLimit(limiter Limiter, key KeyFunc) func(http.Handler) http.Handler { + if limiter == nil { + return func(next http.Handler) http.Handler { + return next + } } if key == nil { key = KeyByIP } return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ok, retry := l.Allow(key(r)) + ok, retry := limiter.Allow(key(r)) if !ok { - secs := int(math.Ceil(retry.Seconds())) - if secs < 1 { - secs = 1 + seconds := int(math.Ceil(retry.Seconds())) + if seconds < 1 { + seconds = 1 } - w.Header().Set("Retry-After", strconv.Itoa(secs)) + w.Header().Set("Retry-After", strconv.Itoa(seconds)) httpx.WriteProblem(w, r, http.StatusTooManyRequests, "rate limit exceeded") return } @@ -74,22 +77,32 @@ type limiterShard struct { } type limiterEntry struct { - lim *rate.Limiter - last time.Time + limiter *rate.Limiter + last time.Time } -// NewKeyedLimiter creates a limiter allowing rps sustained requests per -// second with the given burst. A janitor goroutine evicts entries idle for -// longer than ttl until ctx is cancelled. -func NewKeyedLimiter(ctx context.Context, rps float64, burst int, ttl time.Duration) *KeyedLimiter { - k := &KeyedLimiter{limit: rate.Limit(rps), burst: burst, ttl: ttl, seed: maphash.MakeSeed()} - for i := range k.shards { - k.shards[i].entries = make(map[string]*limiterEntry) +// NewKeyedLimiter creates a limiter allowing requestsPerSecond sustained +// requests per second with the given burst. A janitor goroutine evicts +// entries idle for longer than ttl until ctx is cancelled. +func NewKeyedLimiter( + ctx context.Context, + requestsPerSecond float64, + burst int, + ttl time.Duration, +) *KeyedLimiter { + limiter := &KeyedLimiter{ + limit: rate.Limit(requestsPerSecond), + burst: burst, + ttl: ttl, + seed: maphash.MakeSeed(), + } + for i := range limiter.shards { + limiter.shards[i].entries = make(map[string]*limiterEntry) } if ttl > 0 { - go k.janitor(ctx) + go limiter.janitor(ctx) } - return k + return limiter } func (k *KeyedLimiter) shard(key string) *limiterShard { @@ -99,45 +112,47 @@ func (k *KeyedLimiter) shard(key string) *limiterShard { // Allow implements Limiter. func (k *KeyedLimiter) Allow(key string) (bool, time.Duration) { now := time.Now() - s := k.shard(key) - s.mu.Lock() - e := s.entries[key] - if e == nil { - e = &limiterEntry{lim: rate.NewLimiter(k.limit, k.burst)} - s.entries[key] = e + shard := k.shard(key) + shard.mu.Lock() + entry := shard.entries[key] + if entry == nil { + entry = &limiterEntry{ + limiter: rate.NewLimiter(k.limit, k.burst), + } + shard.entries[key] = entry } - e.last = now - lim := e.lim - s.mu.Unlock() + entry.last = now + limiter := entry.limiter + shard.mu.Unlock() - res := lim.ReserveN(now, 1) + res := limiter.ReserveN(now, 1) if !res.OK() { return false, time.Second } - if d := res.DelayFrom(now); d > 0 { + if delay := res.DelayFrom(now); delay > 0 { res.CancelAt(now) - return false, d + return false, delay } return true, 0 } func (k *KeyedLimiter) janitor(ctx context.Context) { - t := time.NewTicker(k.ttl / 2) - defer t.Stop() + ticker := time.NewTicker(k.ttl / 2) + defer ticker.Stop() for { select { case <-ctx.Done(): return - case now := <-t.C: + case now := <-ticker.C: for i := range k.shards { - s := &k.shards[i] - s.mu.Lock() - for key, e := range s.entries { - if now.Sub(e.last) > k.ttl { - delete(s.entries, key) + shard := &k.shards[i] + shard.mu.Lock() + for key, entry := range shard.entries { + if now.Sub(entry.last) > k.ttl { + delete(shard.entries, key) } } - s.mu.Unlock() + shard.mu.Unlock() } } } diff --git a/internal/middleware/ratelimit_test.go b/internal/middleware/ratelimit_test.go index 06f127f..e74c6a0 100644 --- a/internal/middleware/ratelimit_test.go +++ b/internal/middleware/ratelimit_test.go @@ -8,7 +8,8 @@ import ( func TestRateLimit(t *testing.T) { l := NewKeyedLimiter(t.Context(), 1, 2, 0) - h := RateLimit(l, nil)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + h := RateLimit(l, nil)(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + })) var last int for i := 0; i < 4; i++ { rec := httptest.NewRecorder() diff --git a/internal/middleware/real_ip.go b/internal/middleware/real_ip.go index 55430fe..a93e0ce 100644 --- a/internal/middleware/real_ip.go +++ b/internal/middleware/real_ip.go @@ -20,8 +20,8 @@ type ClientInfo struct { // Client returns the ClientInfo stored by RealIP. Without RealIP it falls back // to the connection itself (see peerInfo) and ignores forwarding headers. func Client(r *http.Request) ClientInfo { - if ci, ok := r.Context().Value(clientInfoKey).(ClientInfo); ok { - return ci + if info, ok := r.Context().Value(clientInfoKey).(ClientInfo); ok { + return info } return peerInfo(r) } @@ -62,10 +62,14 @@ func peerInfo(r *http.Request) ClientInfo { // RealIPOptions configures RealIP. type RealIPOptions struct { + // Trusted are the proxies whose forwarding headers are believed. Trusted []netip.Prefix - // Headers is processed in increasing priority order - the last header in the - // list that is found is the header used to determine a client IP address. - Headers []string // e.g. X-Forwarded-For, Forwarded, X-Real-IP + // Headers are the headers the client IP is read from, in priority order: + // the first of them that the request has and that gives an address is + // used. E.g. X-Forwarded-For, Forwarded, X-Real-IP. + Headers []string + // StripUntrusted removes the forwarding headers from requests that are + // not from a trusted proxy. StripUntrusted bool } @@ -91,9 +95,9 @@ var forwardingHeaders = []string{ // PROXY protocol is handled at the listener (see server package), so by the // time this runs r.RemoteAddr already holds the address from the PROXY header. func RealIP(opts RealIPOptions) func(http.Handler) http.Handler { - trusted := func(a netip.Addr) bool { - for _, p := range opts.Trusted { - if p.Contains(a) { + trusted := func(addr netip.Addr) bool { + for _, prefix := range opts.Trusted { + if prefix.Contains(addr) { return true } } @@ -105,19 +109,19 @@ func RealIP(opts RealIPOptions) func(http.Handler) http.Handler { info := peerInfo(r) if info.Peer.IsValid() && trusted(info.Peer) { - for _, h := range opts.Headers { - if ip, ok := clientFromHeader(r.Header, h, trusted); ok { + for _, name := range opts.Headers { + if ip, ok := clientFromHeader(r.Header, name, trusted); ok { info.IP = ip info.ViaProxy = true break } } - if p := forwardedProto(r.Header); p == "http" || p == "https" { - info.Scheme = p + if proto := forwardedProto(r.Header); proto == "http" || proto == "https" { + info.Scheme = proto } } else if opts.StripUntrusted { - for _, h := range forwardingHeaders { - r.Header.Del(h) + for _, name := range forwardingHeaders { + r.Header.Del(name) } } @@ -128,17 +132,17 @@ func RealIP(opts RealIPOptions) func(http.Handler) http.Handler { } func clientFromHeader( - h http.Header, name string, trusted func(netip.Addr) bool, + header http.Header, name string, trusted func(netip.Addr) bool, ) (netip.Addr, bool) { switch http.CanonicalHeaderKey(name) { case "X-Forwarded-For": - return walkChain(splitList(h.Values("X-Forwarded-For")), trusted) + return walkChain(splitList(header.Values("X-Forwarded-For")), trusted) case "Forwarded": var chain []string - for _, el := range splitList(h.Values("Forwarded")) { - if v, ok := forwardedParam(el, "for"); ok { - chain = append(chain, v) + for _, element := range splitList(header.Values("Forwarded")) { + if value, ok := forwardedParam(element, "for"); ok { + chain = append(chain, value) } else { chain = append(chain, "") // element without for= breaks the chain } @@ -146,8 +150,8 @@ func clientFromHeader( return walkChain(chain, trusted) case "X-Real-Ip": - if v := strings.TrimSpace(h.Get("X-Real-IP")); v != "" { - return parseIPToken(v) + if value := strings.TrimSpace(header.Get("X-Real-IP")); value != "" { + return parseIPToken(value) } } @@ -174,15 +178,15 @@ func walkChain( return last, last.IsValid() } -func forwardedProto(h http.Header) string { - if v := h.Get("X-Forwarded-Proto"); v != "" { - first, _, _ := strings.Cut(v, ",") +func forwardedProto(header http.Header) string { + if value := header.Get("X-Forwarded-Proto"); value != "" { + first, _, _ := strings.Cut(value, ",") return strings.ToLower(strings.TrimSpace(first)) } - if els := splitList(h.Values("Forwarded")); len(els) > 0 { - if v, ok := forwardedParam(els[0], "proto"); ok { - return strings.ToLower(v) + if elements := splitList(header.Values("Forwarded")); len(elements) > 0 { + if value, ok := forwardedParam(elements[0], "proto"); ok { + return strings.ToLower(value) } } @@ -193,9 +197,9 @@ func forwardedProto(h http.Header) string { // element (RFC 7239). func forwardedParam(element, key string) (string, bool) { for _, pair := range strings.Split(element, ";") { - k, v, ok := strings.Cut(strings.TrimSpace(pair), "=") - if ok && strings.EqualFold(k, key) { - return strings.Trim(strings.TrimSpace(v), `"`), true + name, value, ok := strings.Cut(strings.TrimSpace(pair), "=") + if ok && strings.EqualFold(name, key) { + return strings.Trim(strings.TrimSpace(value), `"`), true } } return "", false @@ -203,8 +207,8 @@ func forwardedParam(element, key string) (string, bool) { func splitList(values []string) []string { var out []string - for _, v := range values { - for _, part := range strings.Split(v, ",") { + for _, value := range values { + for _, part := range strings.Split(value, ",") { if part = strings.TrimSpace(part); part != "" { out = append(out, part) } @@ -214,23 +218,23 @@ func splitList(values []string) []string { } // parseIPToken accepts "1.2.3.4", "1.2.3.4:80", "::1", "[::1]" and "[::1]:80". -func parseIPToken(s string) (netip.Addr, bool) { - s = strings.Trim(strings.TrimSpace(s), `"`) - if ap, err := netip.ParseAddrPort(s); err == nil { - return ap.Addr().Unmap(), true +func parseIPToken(token string) (netip.Addr, bool) { + token = strings.Trim(strings.TrimSpace(token), `"`) + if addrPort, err := netip.ParseAddrPort(token); err == nil { + return addrPort.Addr().Unmap(), true } - s = strings.TrimSuffix(strings.TrimPrefix(s, "["), "]") - if a, err := netip.ParseAddr(s); err == nil { - return a.Unmap().WithZone(""), true + token = strings.TrimSuffix(strings.TrimPrefix(token, "["), "]") + if addr, err := netip.ParseAddr(token); err == nil { + return addr.Unmap().WithZone(""), true } return netip.Addr{}, false } func addrFromRemote(remote string) netip.Addr { - if a, ok := parseIPToken(remote); ok { - return a + if addr, ok := parseIPToken(remote); ok { + return addr } return netip.Addr{} } diff --git a/internal/middleware/real_ip_test.go b/internal/middleware/real_ip_test.go index 896751c..8254724 100644 --- a/internal/middleware/real_ip_test.go +++ b/internal/middleware/real_ip_test.go @@ -12,7 +12,9 @@ import ( func serveRealIP(t *testing.T, opts RealIPOptions, peer string, h map[string]string) ClientInfo { t.Helper() var got ClientInfo - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { got = Client(r) }) + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = Client(r) + }) req := httptest.NewRequest("GET", "/", nil) req.RemoteAddr = peer for k, v := range h { diff --git a/internal/mock/findatender/catalog.go b/internal/mock/findatender/catalog.go index 25a0b73..30215e2 100644 --- a/internal/mock/findatender/catalog.go +++ b/internal/mock/findatender/catalog.go @@ -62,15 +62,15 @@ func LoadCatalog() (*Catalog, error) { } // parseCatalog decodes and validates catalog JSON; name labels any error. -func parseCatalog(b []byte, name string) (*Catalog, error) { - var c Catalog - if err := json.Unmarshal(b, &c); err != nil { +func parseCatalog(data []byte, name string) (*Catalog, error) { + var catalog Catalog + if err := json.Unmarshal(data, &catalog); err != nil { return nil, fmt.Errorf("decoding %s: %w", name, err) } - if err := c.validate(); err != nil { + if err := catalog.validate(); err != nil { return nil, fmt.Errorf("invalid %s: %w", name, err) } - return &c, nil + return &catalog, nil } // validate checks that every pool has at least one entry and that every @@ -83,11 +83,26 @@ func (c *Catalog) validate() error { name string n int }{ - {"buyers", len(c.Buyers)}, - {"suppliers", len(c.Suppliers)}, - {"subjects", len(c.Subjects)}, - {"methods", len(c.Methods)}, - {"classifications", len(c.Classifications)}, + { + name: "buyers", + n: len(c.Buyers), + }, + { + name: "suppliers", + n: len(c.Suppliers), + }, + { + name: "subjects", + n: len(c.Subjects), + }, + { + name: "methods", + n: len(c.Methods), + }, + { + name: "classifications", + n: len(c.Classifications), + }, } { if pool.n == 0 { errs = append(errs, fmt.Errorf("no %s", pool.name)) @@ -96,25 +111,25 @@ func (c *Catalog) validate() error { missing := map[string]bool{} check := func(keys ...string) { - for _, k := range keys { - if _, ok := c.Classifications[k]; k != "" && !ok { - missing[k] = true + for _, key := range keys { + if _, ok := c.Classifications[key]; key != "" && !ok { + missing[key] = true } } } - for _, s := range c.Subjects { - check(s.Classification) - check(s.AdditionalClassifications...) - for _, it := range s.Items { - check(it.AdditionalClassifications...) + for _, subject := range c.Subjects { + check(subject.Classification) + check(subject.AdditionalClassifications...) + for _, item := range subject.Items { + check(item.AdditionalClassifications...) } } if len(missing) > 0 { keys := make([]string, 0, len(missing)) - for k := range missing { - keys = append(keys, k) + for key := range missing { + keys = append(keys, key) } sort.Strings(keys) if len(keys) > 5 { @@ -131,39 +146,39 @@ func (c *Catalog) validate() error { // ---------- picking (always returns a copy the caller may modify) ---------- -func (c *Catalog) buyer(r *rand.Rand) ocds.Organization { - return clone(c.Buyers[r.Intn(len(c.Buyers))]) +func (c *Catalog) buyer(rng *rand.Rand) ocds.Organization { + return clone(c.Buyers[rng.Intn(len(c.Buyers))]) } -func (c *Catalog) supplier(r *rand.Rand) ocds.Organization { - return clone(c.Suppliers[r.Intn(len(c.Suppliers))]) +func (c *Catalog) supplier(rng *rand.Rand) ocds.Organization { + return clone(c.Suppliers[rng.Intn(len(c.Suppliers))]) } -func (c *Catalog) method(r *rand.Rand) Method { - return c.Methods[r.Intn(len(c.Methods))] +func (c *Catalog) method(rng *rand.Rand) Method { + return c.Methods[rng.Intn(len(c.Methods))] } // subject picks a subject and returns it as an ocds.Tender, with its // classification keys resolved to full classifications. -func (c *Catalog) subject(r *rand.Rand) ocds.Tender { - s := clone(c.Subjects[r.Intn(len(c.Subjects))]) - t := s.Tender - t.Classification = c.resolveOne(s.Classification) - t.AdditionalClassifications = c.resolve(s.AdditionalClassifications) - t.Items = nil - for _, it := range s.Items { - item := it.Item - item.AdditionalClassifications = c.resolve(it.AdditionalClassifications) - t.Items = append(t.Items, item) +func (c *Catalog) subject(rng *rand.Rand) ocds.Tender { + subject := clone(c.Subjects[rng.Intn(len(c.Subjects))]) + tender := subject.Tender + tender.Classification = c.resolveOne(subject.Classification) + tender.AdditionalClassifications = c.resolve(subject.AdditionalClassifications) + tender.Items = nil + for _, subjectItem := range subject.Items { + item := subjectItem.Item + item.AdditionalClassifications = c.resolve(subjectItem.AdditionalClassifications) + tender.Items = append(tender.Items, item) } - return t + return tender } // resolveOne returns a copy of the classification with key, or nil if there // is none. func (c *Catalog) resolveOne(key string) *ocds.Classification { - if cl, ok := c.Classifications[key]; ok { - return &cl + if classification, ok := c.Classifications[key]; ok { + return &classification } return nil } @@ -172,20 +187,20 @@ func (c *Catalog) resolveOne(key string) *ocds.Classification { // that have none. func (c *Catalog) resolve(keys []string) []ocds.Classification { var out []ocds.Classification - for _, k := range keys { - if cl, ok := c.Classifications[k]; ok { - out = append(out, cl) + for _, key := range keys { + if classification, ok := c.Classifications[key]; ok { + out = append(out, classification) } } return out } // clone deep-copies a value through JSON, which is all the generated types need. -func clone[T any](v T) T { +func clone[T any](value T) T { var out T - b, err := json.Marshal(v) + data, err := json.Marshal(value) if err == nil { - json.Unmarshal(b, &out) //nolint:errcheck,gosec // b was just marshalled from a T + json.Unmarshal(data, &out) //nolint:errcheck,gosec // b was just marshalled from a T } return out } diff --git a/internal/mock/findatender/catalog_test.go b/internal/mock/findatender/catalog_test.go index 0c72b2d..fa0e717 100644 --- a/internal/mock/findatender/catalog_test.go +++ b/internal/mock/findatender/catalog_test.go @@ -33,8 +33,13 @@ func TestEmbeddedCatalogIsValid(t *testing.T) { } if len(cat.Buyers) == 0 || len(cat.Suppliers) == 0 || len(cat.Subjects) == 0 || len(cat.Methods) == 0 { - t.Fatalf("embedded catalog has an empty pool: %d buyers, %d suppliers, %d subjects, %d methods", - len(cat.Buyers), len(cat.Suppliers), len(cat.Subjects), len(cat.Methods)) + t.Fatalf( + "embedded catalog has an empty pool: %d buyers, %d suppliers, %d subjects, %d methods", + len(cat.Buyers), + len(cat.Suppliers), + len(cat.Subjects), + len(cat.Methods), + ) } } @@ -42,8 +47,13 @@ func TestParseCatalog(t *testing.T) { cat := loadTestCatalog(t) if len(cat.Buyers) != 2 || len(cat.Suppliers) != 1 || len(cat.Subjects) != 2 || len(cat.Methods) != 2 { - t.Fatalf("got %d buyers, %d suppliers, %d subjects, %d methods", - len(cat.Buyers), len(cat.Suppliers), len(cat.Subjects), len(cat.Methods)) + t.Fatalf( + "got %d buyers, %d suppliers, %d subjects, %d methods", + len(cat.Buyers), + len(cat.Suppliers), + len(cat.Subjects), + len(cat.Methods), + ) } if d := cat.Buyers[0].Details; d == nil || len(d.Classifications) != 1 || d.Classifications[0].Scheme != "UK_CA_TYPE" { diff --git a/internal/mock/findatender/doc.go b/internal/mock/findatender/doc.go index cf25fbc..513e9ed 100644 --- a/internal/mock/findatender/doc.go +++ b/internal/mock/findatender/doc.go @@ -6,8 +6,7 @@ package mock // Files kept in the mock-data directory. -// -// ReleasesFile is created and appended to by the mock. const ( + // ReleasesFile is created and appended to by the mock. ReleasesFile = "releases.jsonl" ) diff --git a/internal/mock/findatender/generator.go b/internal/mock/findatender/generator.go index 2d23b94..8c89d7c 100644 --- a/internal/mock/findatender/generator.go +++ b/internal/mock/findatender/generator.go @@ -23,12 +23,12 @@ type Generator struct { opts options } -// NewGenerator returns a generator drawing from cat and storing releases in +// NewGenerator returns a generator drawing from catalog and storing releases in // store. The same seed and store contents give the same releases. It logs to // slog.Default() unless given WithLogger. -func NewGenerator(cat *Catalog, store *Store, seed int64, opts ...Option) *Generator { +func NewGenerator(catalog *Catalog, store *Store, seed int64, opts ...Option) *Generator { return &Generator{ - cat: cat, + cat: catalog, store: store, rnd: rand.New(rand.NewSource(seed)), //nolint:gosec // seeded for repeatable mock data opts: applyOptions(opts), @@ -56,7 +56,7 @@ func (g *Generator) newProcedure(at time.Time) ocds.Release { buyer := g.cat.buyer(g.rnd) ensurePartyID(&buyer) buyer.Roles = []string{"buyer"} - m := g.cat.method(g.rnd) + method := g.cat.method(g.rnd) tag, status := "tender", "active" if g.rnd.Float64() < 0.2 { @@ -64,38 +64,38 @@ func (g *Generator) newProcedure(at time.Time) ocds.Release { } ocid := g.newOCID() - t := g.cat.subject(g.rnd) - t.ID = ocds.ID(ocid) - t.ProcurementMethod = m.Method - t.ProcurementMethodDetails = m.Details + tender := g.cat.subject(g.rnd) + tender.ID = ocds.ID(ocid) + tender.ProcurementMethod = method.Method + tender.ProcurementMethodDetails = method.Details factor := 0.5 + g.rnd.Float64()*1.5 - if t.Value == nil { + if tender.Value == nil { amount := float64((g.rnd.Intn(5000) + 10) * 1000) - t.Value = &ocds.Value{ + tender.Value = &ocds.Value{ Amount: &amount, Currency: "GBP", } } else { - scaleValue(t.Value, factor) + scaleValue(tender.Value, factor) } - for i := range t.Lots { - scaleValue(t.Lots[i].Value, factor) + for i := range tender.Lots { + scaleValue(tender.Lots[i].Value, factor) } - if len(t.Lots) == 0 { - v := clone(*t.Value) - t.Lots = []ocds.Lot{{ + if len(tender.Lots) == 0 { + value := clone(*tender.Value) + tender.Lots = []ocds.Lot{{ ID: "1", - Title: t.Title, - Value: &v, + Title: tender.Title, + Value: &value, }} } - setStatus(&t, status) + setStatus(&tender, status) if status == "active" { - t.TenderPeriod = &ocds.Period{ + tender.TenderPeriod = &ocds.Period{ EndDate: ptr(at.AddDate(0, 0, 30+g.rnd.Intn(30))), } } @@ -111,7 +111,7 @@ func (g *Generator) newProcedure(at time.Time) ocds.Release { ID: ocds.ID(buyer.ID), Name: buyer.Name, }, - Tender: &t, + Tender: &tender, Language: "en", } } @@ -120,13 +120,13 @@ func (g *Generator) newProcedure(at time.Time) ocds.Release { // tender, and a tender either has its deadline extended (tenderUpdate) or is // awarded to a supplier from the catalog. func (g *Generator) advance(prev ocds.Release, at time.Time) ocds.Release { - r := clone(prev) - r.ID = g.store.NextNoticeID(at.Year()) - r.Date = at - r.Awards = nil - if r.Tender == nil { - r.Tender = &ocds.Tender{ - ID: ocds.ID(r.OCID), + release := clone(prev) + release.ID = g.store.NextNoticeID(at.Year()) + release.Date = at + release.Awards = nil + if release.Tender == nil { + release.Tender = &ocds.Tender{ + ID: ocds.ID(release.OCID), } } @@ -139,60 +139,60 @@ func (g *Generator) advance(prev ocds.Release, at time.Time) ocds.Release { next = "tenderUpdate" } } - r.Tag = []string{next} + release.Tag = []string{next} switch next { case "tender": - setStatus(r.Tender, "active") - r.Tender.TenderPeriod = &ocds.Period{ + setStatus(release.Tender, "active") + release.Tender.TenderPeriod = &ocds.Period{ EndDate: ptr(at.AddDate(0, 0, 30+g.rnd.Intn(30))), } case "tenderUpdate": // A typical update: the deadline is extended by two weeks. end := at.AddDate(0, 0, 30) - if tp := r.Tender.TenderPeriod; tp != nil && tp.EndDate != nil { - end = *tp.EndDate + if period := release.Tender.TenderPeriod; period != nil && period.EndDate != nil { + end = *period.EndDate } - r.Tender.TenderPeriod = &ocds.Period{ + release.Tender.TenderPeriod = &ocds.Period{ EndDate: ptr(end.AddDate(0, 0, 14)), } case "award": - setStatus(r.Tender, "complete") - sup := g.cat.supplier(g.rnd) - ensurePartyID(&sup) - sup.Roles = []string{"supplier"} - r.Parties = append(r.Parties, sup) + setStatus(release.Tender, "complete") + supplier := g.cat.supplier(g.rnd) + ensurePartyID(&supplier) + supplier.Roles = []string{"supplier"} + release.Parties = append(release.Parties, supplier) var value *ocds.Value - if r.Tender.Value != nil { - v := clone(*r.Tender.Value) - scaleValue(&v, 0.8+g.rnd.Float64()*0.2) - value = &v + if release.Tender.Value != nil { + scaled := clone(*release.Tender.Value) + scaleValue(&scaled, 0.8+g.rnd.Float64()*0.2) + value = &scaled } var lotIDs []string - for _, lot := range r.Tender.Lots { + for _, lot := range release.Tender.Lots { if lot.ID != "" { lotIDs = append(lotIDs, lot.ID) } } - r.Awards = []ocds.Award{{ + release.Awards = []ocds.Award{{ ID: "1", Status: "active", Date: ptr(at), Value: value, Suppliers: []ocds.OrganizationReference{ { - ID: ocds.ID(sup.ID), - Name: sup.Name, + ID: ocds.ID(supplier.ID), + Name: supplier.Name, }, }, RelatedLots: lotIDs, }} } - return r + return release } // newOCID returns a random ocid not already in the store. @@ -224,25 +224,25 @@ func (g *Generator) CatchUp( start = last } - n := 0 - for t := start.Add(g.gap(min, max)); !t.After(now); t = t.Add(g.gap(min, max)) { - if err := g.store.AppendNoSync(g.Make(t)); err != nil { - return n, err + count := 0 + for when := start.Add(g.gap(min, max)); !when.After(now); when = when.Add(g.gap(min, max)) { + if err := g.store.AppendNoSync(g.Make(when)); err != nil { + return count, err } - n++ + count++ } - return n, g.store.Sync() + return count, g.store.Sync() } // Run publishes a new release after a random gap between min and max until ctx ends. func (g *Generator) Run(ctx context.Context, min, max time.Duration) { logger := g.opts.logger("generator") for { - d := g.gap(min, max) - logger.Info("next release", "in", d.Round(time.Second)) + gap := g.gap(min, max) + logger.Info("next release", "in", gap.Round(time.Second)) - timer := time.NewTimer(d) + timer := time.NewTimer(gap) select { case <-ctx.Done(): timer.Stop() @@ -250,46 +250,51 @@ func (g *Generator) Run(ctx context.Context, min, max time.Duration) { case <-timer.C: } - r := g.Make(time.Now()) - if err := g.store.Append(r); err != nil { + release := g.Make(time.Now()) + if err := g.store.Append(release); err != nil { logger.Error("storing release failed", "error", err) continue } - logger.Info("published release", "release_id", r.ID, "tag", r.Tag[0], "ocid", r.OCID) + logger.Info( + "published release", + "release_id", release.ID, + "tag", release.Tag[0], + "ocid", release.OCID, + ) } } // ---------- helpers ---------- -func ptr[T any](v T) *T { return &v } +func ptr[T any](value T) *T { return &value } -func setStatus(t *ocds.Tender, status string) { - t.Status = status - for i := range t.Lots { - t.Lots[i].Status = status +func setStatus(tender *ocds.Tender, status string) { + tender.Status = status + for i := range tender.Lots { + tender.Lots[i].Status = status } } -// scaleValue multiplies v's amounts by f, rounded to the nearest 100. -func scaleValue(v *ocds.Value, f float64) { - if v == nil { +// scaleValue multiplies value's amounts by factor, rounded to the nearest 100. +func scaleValue(value *ocds.Value, factor float64) { + if value == nil { return } - for _, a := range []**float64{&v.Amount, &v.AmountGross} { - if *a != nil { - *a = ptr(math.Round(**a*f/100) * 100) + for _, amount := range []**float64{&value.Amount, &value.AmountGross} { + if *amount != nil { + *amount = ptr(math.Round(**amount*factor/100) * 100) } } } -// ensurePartyID gives p an id derived from its name if it has none, so the +// ensurePartyID gives party an id derived from its name if it has none, so the // same organisation always gets the same id. -func ensurePartyID(p *ocds.Organization) { - if p.ID != "" { +func ensurePartyID(party *ocds.Organization) { + if party.ID != "" { return } - h := fnv.New32a() - h.Write([]byte(p.Name)) //nolint:gosec // never fails - p.ID = fmt.Sprintf("GB-FTS-%d", h.Sum32()%1000000) + hasher := fnv.New32a() + hasher.Write([]byte(party.Name)) //nolint:gosec // never fails + party.ID = fmt.Sprintf("GB-FTS-%d", hasher.Sum32()%1000000) } diff --git a/internal/mock/findatender/generator_test.go b/internal/mock/findatender/generator_test.go index 77beb1c..92918af 100644 --- a/internal/mock/findatender/generator_test.go +++ b/internal/mock/findatender/generator_test.go @@ -17,12 +17,16 @@ func quietLog(t *testing.T) { t.Helper() w := log.Writer() log.SetOutput(io.Discard) - t.Cleanup(func() { log.SetOutput(w) }) + t.Cleanup(func() { + log.SetOutput(w) + }) } func TestGap(t *testing.T) { //nolint:gosec // G404: seeded math/rand keeps the test deterministic - g := &Generator{rnd: rand.New(rand.NewSource(1))} + g := &Generator{ + rnd: rand.New(rand.NewSource(1)), + } if d := g.gap(time.Second, time.Second); d != time.Second { t.Errorf("min == max: %s", d) } @@ -39,7 +43,10 @@ func TestGap(t *testing.T) { func TestScaleValue(t *testing.T) { scaleValue(nil, 2) // must not panic - v := &ocds.Value{Amount: ptr(12345.0), Currency: "GBP"} + v := &ocds.Value{ + Amount: ptr(12345.0), + Currency: "GBP", + } scaleValue(v, 1.5) if *v.Amount != 18500 { // 18517.5 rounded to the nearest 100 t.Errorf("amount = %v, want 18500", *v.Amount) @@ -48,7 +55,10 @@ func TestScaleValue(t *testing.T) { t.Error("nil gross amount was set") } - v = &ocds.Value{Amount: ptr(1000.0), AmountGross: ptr(1200.0)} + v = &ocds.Value{ + Amount: ptr(1000.0), + AmountGross: ptr(1200.0), + } scaleValue(v, 0.5) if *v.Amount != 500 || *v.AmountGross != 600 { t.Errorf("amounts = %v / %v, want 500 / 600", *v.Amount, *v.AmountGross) @@ -56,15 +66,24 @@ func TestScaleValue(t *testing.T) { } func TestEnsurePartyID(t *testing.T) { - p := ocds.Organization{ID: "GB-EXISTING", Name: "Council"} + p := ocds.Organization{ + ID: "GB-EXISTING", + Name: "Council", + } ensurePartyID(&p) if p.ID != "GB-EXISTING" { t.Errorf("existing id replaced: %s", p.ID) } - a := ocds.Organization{Name: "Somewhere Council"} - b := ocds.Organization{Name: "Somewhere Council"} - c := ocds.Organization{Name: "Elsewhere Council"} + a := ocds.Organization{ + Name: "Somewhere Council", + } + b := ocds.Organization{ + Name: "Somewhere Council", + } + c := ocds.Organization{ + Name: "Elsewhere Council", + } ensurePartyID(&a) ensurePartyID(&b) ensurePartyID(&c) @@ -90,7 +109,13 @@ func TestAdvance(t *testing.T) { at := time.Date(2026, 10, 8, 12, 0, 0, 0, time.UTC) // A planning notice without a tender becomes an active tender. - r := g.advance(ocds.Release{OCID: "ocds-x", Tag: []string{"planning"}}, at) + r := g.advance( + ocds.Release{ + OCID: "ocds-x", + Tag: []string{"planning"}, + }, + at, + ) if r.Tag[0] != "tender" || r.Tender == nil || r.Tender.Status != "active" || r.Tender.TenderPeriod == nil || r.Tender.TenderPeriod.EndDate == nil { t.Fatalf("planning -> %v, tender %+v", r.Tag, r.Tender) @@ -105,10 +130,15 @@ func TestAdvance(t *testing.T) { OCID: "ocds-y", Tag: []string{"tender"}, Tender: &ocds.Tender{ - ID: "ocds-y", - Value: &ocds.Value{Amount: ptr(10000.0), Currency: "GBP"}, - Lots: []ocds.Lot{{ID: "1"}, {ID: ""}}, - TenderPeriod: &ocds.Period{EndDate: &end}, + ID: "ocds-y", + Value: &ocds.Value{ + Amount: ptr(10000.0), + Currency: "GBP", + }, + Lots: []ocds.Lot{{ID: "1"}, {ID: ""}}, + TenderPeriod: &ocds.Period{ + EndDate: &end, + }, }, } seen := map[string]bool{} @@ -147,7 +177,13 @@ func TestAdvance(t *testing.T) { // An update without a deadline gets 30 + 14 days; a tender with no value // is awarded with no value. - prev = ocds.Release{OCID: "ocds-z", Tag: []string{"tender"}, Tender: &ocds.Tender{ID: "ocds-z"}} + prev = ocds.Release{ + OCID: "ocds-z", + Tag: []string{"tender"}, + Tender: &ocds.Tender{ + ID: "ocds-z", + }, + } for i := 0; i < 50; i++ { r := g.advance(prev, at) switch r.Tag[0] { diff --git a/internal/mock/findatender/handler.go b/internal/mock/findatender/handler.go index a50e7b5..ea14610 100644 --- a/internal/mock/findatender/handler.go +++ b/internal/mock/findatender/handler.go @@ -14,11 +14,17 @@ import ( "git.avsdev.uk/Go/job-search/internal/mock/findatender/ocds" ) +// Paging and request parameters of the release package endpoint. const ( + // defaultLimit is the page size when the request sets no limit. defaultLimit = 100 - maxLimit = 100 - timeLayout = "2006-01-02T15:04:05" - BasePath = "/api/1.0/ocdsReleasePackages" + // maxLimit is the largest page size a request may ask for. + maxLimit = 100 + // timeLayout is the documented layout of updatedFrom and updatedTo; see + // parseTime for the others accepted. + timeLayout = "2006-01-02T15:04:05" + // BasePath is the path the endpoint is served at. + BasePath = "/api/1.0/ocdsReleasePackages" ) // server serves release packages from a Store; see NewHandler. @@ -36,10 +42,18 @@ type server struct { // Stored release dates are UTC; comparisons are on absolute instants. It // logs to slog.Default() unless given WithLogger. func NewHandler( - store *Store, baseURL string, loc *time.Location, opts ...Option, + store *Store, + baseURL string, + location *time.Location, + opts ...Option, ) http.HandlerFunc { - s := &server{store: store, baseURL: baseURL, loc: loc, opts: applyOptions(opts)} - return s.handle + handler := &server{ + store: store, + baseURL: baseURL, + loc: location, + opts: applyOptions(opts), + } + return handler.handle } // encodeCursor makes the opaque cursor for a next link that resumes at store @@ -50,18 +64,18 @@ func encodeCursor(i int) string { // decodeCursor returns the release index in a cursor made by encodeCursor. // Anything else, including a negative index, is an error. -func decodeCursor(c string) (int, error) { - b, err := base64.RawURLEncoding.DecodeString(c) - if err != nil || !strings.HasPrefix(string(b), "idx:") { +func decodeCursor(cursor string) (int, error) { + decoded, err := base64.RawURLEncoding.DecodeString(cursor) + if err != nil || !strings.HasPrefix(string(decoded), "idx:") { return 0, fmt.Errorf("invalid cursor") } - n, err := strconv.Atoi(strings.TrimPrefix(string(b), "idx:")) - if err != nil || n < 0 { + index, err := strconv.Atoi(strings.TrimPrefix(string(decoded), "idx:")) + if err != nil || index < 0 { return 0, fmt.Errorf("invalid cursor") } - return n, nil + return index, nil } func badRequest(w http.ResponseWriter, msg string) { @@ -72,20 +86,20 @@ func badRequest(w http.ResponseWriter, msg string) { } // parseTime reads a query timestamp. Values without an offset (the documented -// yyyy-MM-ddTHH:mm:ss, or a plain date) are local time in loc; values with an +// yyyy-MM-ddTHH:mm:ss, or a plain date) are local time in location; values with an // offset or Z (RFC 3339) mean exactly what they say. -func parseTime(v string, loc *time.Location) (time.Time, error) { - if loc == nil { - loc = time.UTC +func parseTime(value string, location *time.Location) (time.Time, error) { + if location == nil { + location = time.UTC } - if t, err := time.Parse(time.RFC3339, v); err == nil { - return t, nil + if parsed, err := time.Parse(time.RFC3339, value); err == nil { + return parsed, nil } for _, layout := range []string{timeLayout, "2006-01-02"} { - if t, err := time.ParseInLocation(layout, v, loc); err == nil { - return t, nil + if parsed, err := time.ParseInLocation(layout, value, location); err == nil { + return parsed, nil } } @@ -107,15 +121,15 @@ func (s *server) handle(w http.ResponseWriter, req *http.Request) { return } - q := req.URL.Query() - pq, err := s.parseQuery(q) + query := req.URL.Query() + page, err := s.parseQuery(query) if err != nil { badRequest(w, err.Error()) return } - releases, next, more := s.store.Page(pq.from, pq.upper, pq.start, pq.limit) - pkg := ocds.ReleasePackage{ + releases, next, more := s.store.Page(page.from, page.upper, page.start, page.limit) + releasePackage := ocds.ReleasePackage{ URI: s.baseURL + req.URL.RequestURI(), Version: "1.1", Extensions: ocds.DeclaredExtensions, @@ -127,73 +141,78 @@ func (s *server) handle(w http.ResponseWriter, req *http.Request) { Releases: releases, } if more { - pkg.Links = &ocds.ReleasePackageLinks{Next: s.nextLink(q, next)} + releasePackage.Links = &ocds.ReleasePackageLinks{ + Next: s.nextLink(query, next), + } } w.Header().Set("Content-Type", "application/json; charset=utf-8") w.Header().Set("Access-Control-Allow-Origin", "*") - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - enc.SetEscapeHTML(false) - if err := enc.Encode(pkg); err != nil { + encoder := json.NewEncoder(w) + encoder.SetIndent("", " ") + encoder.SetEscapeHTML(false) + if err := encoder.Encode(releasePackage); err != nil { s.opts.logger("handler").Warn("writing response failed", "error", err) } } // parseQuery reads and checks the query parameters. Its errors are meant for // the client. -func (s *server) parseQuery(q url.Values) (pageQuery, error) { - pq := pageQuery{limit: defaultLimit, upper: time.Now().UTC()} - - if v := q.Get("limit"); v != "" { - n, err := strconv.Atoi(v) - if err != nil || n < 1 || n > maxLimit { - return pq, fmt.Errorf("limit must be an integer between 1 and %d", maxLimit) - } - pq.limit = n +func (s *server) parseQuery(query url.Values) (pageQuery, error) { + page := pageQuery{ + limit: defaultLimit, + upper: time.Now().UTC(), } - if v := q.Get("updatedFrom"); v != "" { - t, err := parseTime(v, s.loc) + if value := query.Get("limit"); value != "" { + number, err := strconv.Atoi(value) + if err != nil || number < 1 || number > maxLimit { + return page, fmt.Errorf("limit must be an integer between 1 and %d", maxLimit) + } + page.limit = number + } + + if value := query.Get("updatedFrom"); value != "" { + from, err := parseTime(value, s.loc) if err != nil { - return pq, fmt.Errorf("updatedFrom %w", err) + return page, fmt.Errorf("updatedFrom %w", err) } - pq.from = t + page.from = from } - if v := q.Get("updatedTo"); v != "" { - t, err := parseTime(v, s.loc) + if value := query.Get("updatedTo"); value != "" { + to, err := parseTime(value, s.loc) if err != nil { - return pq, fmt.Errorf("updatedTo %w", err) + return page, fmt.Errorf("updatedTo %w", err) } - if q.Get("updatedFrom") != "" && t.Before(pq.from) { - return pq, errors.New("updatedTo must not be before updatedFrom") + if query.Get("updatedFrom") != "" && to.Before(page.from) { + return page, errors.New("updatedTo must not be before updatedFrom") } - if t.Before(pq.upper) { - pq.upper = t + if to.Before(page.upper) { + page.upper = to } } - if c := q.Get("cursor"); c != "" { - n, err := decodeCursor(c) + if cursor := query.Get("cursor"); cursor != "" { + index, err := decodeCursor(cursor) if err != nil { - return pq, err + return page, err } - pq.start = n + page.start = index } - return pq, nil + return page, nil } // nextLink is the URL of the page starting at release index next: the same // query with a new cursor. -func (s *server) nextLink(q url.Values, next int) string { - nq := url.Values{} - for k, vs := range q { - if k != "cursor" { - nq[k] = vs +func (s *server) nextLink(query url.Values, next int) string { + nextQuery := url.Values{} + for key, values := range query { + if key != "cursor" { + nextQuery[key] = values } } - nq.Set("cursor", encodeCursor(next)) - return s.baseURL + BasePath + "?" + nq.Encode() + nextQuery.Set("cursor", encodeCursor(next)) + return s.baseURL + BasePath + "?" + nextQuery.Encode() } diff --git a/internal/mock/findatender/handler_test.go b/internal/mock/findatender/handler_test.go index cd3b1ab..7152191 100644 --- a/internal/mock/findatender/handler_test.go +++ b/internal/mock/findatender/handler_test.go @@ -79,7 +79,9 @@ func openTestStore(t *testing.T) *Store { if err != nil { t.Fatal(err) } - t.Cleanup(func() { store.Close() }) + t.Cleanup(func() { + store.Close() + }) return store } @@ -92,7 +94,9 @@ func TestHandlerRejectsBadRequests(t *testing.T) { t.Errorf("POST: status %d, want 405", rec.Code) } - cursor := func(s string) string { return base64.RawURLEncoding.EncodeToString([]byte(s)) } + cursor := func(s string) string { + return base64.RawURLEncoding.EncodeToString([]byte(s)) + } cases := map[string]string{ "limit=0": "limit must be an integer between 1 and 100", "limit=101": "limit must be an integer between 1 and 100", diff --git a/internal/mock/findatender/ratelimit.go b/internal/mock/findatender/ratelimit.go index da7f8d7..17354c0 100644 --- a/internal/mock/findatender/ratelimit.go +++ b/internal/mock/findatender/ratelimit.go @@ -50,46 +50,50 @@ func (rl *RateLimiter) allow(key string) (bool, int) { now := rl.now().UTC() hour := now.Truncate(time.Hour) - c, ok := rl.clients[key] - if !ok || !c.hour.Equal(hour) { + client, ok := rl.clients[key] + if !ok || !client.hour.Equal(hour) { // First request of this clock hour: it anchors the windows. if !ok && len(rl.clients) > 10000 { rl.prune(hour) } - c = &clientWindow{hour: hour, anchor: now, windowStart: now} - rl.clients[key] = c + client = &clientWindow{ + hour: hour, + anchor: now, + windowStart: now, + } + rl.clients[key] = client } // Move to whichever fixed window, counted from the anchor, now falls in. - idx := now.Sub(c.anchor) / rl.window - if ws := c.anchor.Add(idx * rl.window); !ws.Equal(c.windowStart) { - c.windowStart = ws - c.count = 0 + index := now.Sub(client.anchor) / rl.window + if windowStart := client.anchor.Add(index * rl.window); !windowStart.Equal(client.windowStart) { + client.windowStart = windowStart + client.count = 0 } - if c.count < rl.limit { - c.count++ + if client.count < rl.limit { + client.count++ return true, 0 } // The next window opens at the end of this one, or at the top of the // next hour if that comes first (because a new hour re-anchors). - next := c.windowStart.Add(rl.window) - if nh := hour.Add(time.Hour); nh.Before(next) { - next = nh + next := client.windowStart.Add(rl.window) + if nextHour := hour.Add(time.Hour); nextHour.Before(next) { + next = nextHour } - secs := int((next.Sub(now) + time.Second - 1) / time.Second) // round up - if secs < 1 { - secs = 1 + seconds := int((next.Sub(now) + time.Second - 1) / time.Second) // round up + if seconds < 1 { + seconds = 1 } - return false, secs + return false, seconds } // prune drops clients whose state belongs to an earlier hour. func (rl *RateLimiter) prune(hour time.Time) { - for k, c := range rl.clients { - if c.hour.Before(hour) { - delete(rl.clients, k) + for key, client := range rl.clients { + if client.hour.Before(hour) { + delete(rl.clients, key) } } } diff --git a/internal/mock/findatender/ratelimit_test.go b/internal/mock/findatender/ratelimit_test.go index 37946a5..bf2521d 100644 --- a/internal/mock/findatender/ratelimit_test.go +++ b/internal/mock/findatender/ratelimit_test.go @@ -12,7 +12,9 @@ import ( func TestRateLimitWindows(t *testing.T) { now := time.Date(2026, 10, 6, 10, 3, 20, 0, time.UTC) // first request of the hour rl := NewRateLimiter(12, 5*time.Minute) - rl.now = func() time.Time { return now } + rl.now = func() time.Time { + return now + } for i := 0; i < 12; i++ { if ok, _ := rl.allow("a"); !ok { @@ -63,7 +65,9 @@ func TestClientKey(t *testing.T) { func TestRateLimitMiddleware(t *testing.T) { now := time.Date(2026, 10, 6, 10, 0, 0, 0, time.UTC) rl := NewRateLimiter(2, 5*time.Minute) - rl.now = func() time.Time { return now } + rl.now = func() time.Time { + return now + } calls := 0 h := rl.Middleware(func(w http.ResponseWriter, req *http.Request) { @@ -108,7 +112,9 @@ func TestRateLimitMiddleware(t *testing.T) { func TestRateLimitPrunesStaleClients(t *testing.T) { now := time.Date(2026, 10, 6, 10, 30, 0, 0, time.UTC) rl := NewRateLimiter(1, time.Minute) - rl.now = func() time.Time { return now } + rl.now = func() time.Time { + return now + } for i := 0; i <= 10000; i++ { rl.allow(strconv.Itoa(i)) diff --git a/internal/mock/findatender/store.go b/internal/mock/findatender/store.go index 15693e8..dee16e1 100644 --- a/internal/mock/findatender/store.go +++ b/internal/mock/findatender/store.go @@ -43,7 +43,7 @@ type Store struct { // left incomplete by a crash is ended so new ones start on a fresh line. // It logs to slog.Default() unless given WithLogger. func OpenStore(path string, opts ...Option) (*Store, error) { - s := &Store{ + store := &Store{ latest: map[string]int{}, lastNum: map[int]int{}, openPos: map[string]int{}, @@ -53,57 +53,57 @@ func OpenStore(path string, opts ...Option) (*Store, error) { return nil, err } - releases, err := readReleases(s.opts.logger("store"), path) + releases, err := readReleases(store.opts.logger("store"), path) if err != nil { return nil, err } // The file is written in order, but sort anyway in case it was hand-edited. - sort.SliceStable(releases, func(i, j int) bool { - return releases[i].Date.Before(releases[j].Date) + sort.SliceStable(releases, func(i, other int) bool { + return releases[i].Date.Before(releases[other].Date) }) - for _, r := range releases { - s.releases = append(s.releases, r) - s.times = append(s.times, r.Date) + for _, release := range releases { + store.releases = append(store.releases, release) + store.times = append(store.times, release.Date) } - for i, r := range s.releases { - s.index(i, r) + for i, release := range store.releases { + store.index(i, release) } - if s.f, err = openForAppend(path); err != nil { + if store.f, err = openForAppend(path); err != nil { return nil, err } - return s, nil + return store, nil } // readReleases reads the releases stored at path, skipping blank lines and // lines that can't be decoded (which are logged to logger). A missing file // holds no releases. func readReleases(logger *slog.Logger, path string) ([]ocds.Release, error) { - in, err := os.Open(filepath.Clean(path)) + input, err := os.Open(filepath.Clean(path)) if os.IsNotExist(err) { return nil, nil } if err != nil { return nil, err } - defer in.Close() //nolint:errcheck // read only + defer input.Close() //nolint:errcheck // read only var releases []ocds.Release - sc := bufio.NewScanner(in) - sc.Buffer(make([]byte, 1<<20), 64<<20) - for line := 1; sc.Scan(); line++ { - b := bytes.TrimSpace(sc.Bytes()) - if len(b) == 0 { + scanner := bufio.NewScanner(input) + scanner.Buffer(make([]byte, 1<<20), 64<<20) + for line := 1; scanner.Scan(); line++ { + data := bytes.TrimSpace(scanner.Bytes()) + if len(data) == 0 { continue } - var r ocds.Release - if err := json.Unmarshal(b, &r); err != nil { + var release ocds.Release + if err := json.Unmarshal(data, &release); err != nil { logger.Warn("skipping unreadable line", "line", line, "error", err) continue } - releases = append(releases, r) + releases = append(releases, release) } - if err := sc.Err(); err != nil { + if err := scanner.Err(); err != nil { return nil, fmt.Errorf("reading %s: %w", path, err) } return releases, nil @@ -113,60 +113,60 @@ func readReleases(logger *slog.Logger, path string) ([]ocds.Release, error) { // mid-write, it ends the partial record so the next one starts on a fresh // line. func openForAppend(path string) (*os.File, error) { - f, err := os.OpenFile(filepath.Clean(path), os.O_CREATE|os.O_RDWR|os.O_APPEND, 0o600) + file, err := os.OpenFile(filepath.Clean(path), os.O_CREATE|os.O_RDWR|os.O_APPEND, 0o600) if err != nil { return nil, err } - if st, err := f.Stat(); err == nil && st.Size() > 0 { + if info, err := file.Stat(); err == nil && info.Size() > 0 { last := make([]byte, 1) - if _, err := f.ReadAt(last, st.Size()-1); err == nil && last[0] != '\n' { - if _, err := f.Write([]byte("\n")); err != nil { - return nil, errors.Join(fmt.Errorf("repairing %s: %w", path, err), f.Close()) + if _, err := file.ReadAt(last, info.Size()-1); err == nil && last[0] != '\n' { + if _, err := file.Write([]byte("\n")); err != nil { + return nil, errors.Join(fmt.Errorf("repairing %s: %w", path, err), file.Close()) } } } - return f, nil + return file, nil } -// index records release r, at position i, in the lookups: the ocid's latest +// index records release, at position i, in the lookups: the ocid's latest // release, whether its procedure is still open (an award closes it), and the // highest notice number used each year. -func (s *Store) index(i int, r ocds.Release) { - s.latest[r.OCID] = i - if len(r.Tag) > 0 && r.Tag[0] == "award" { - s.closeProcedure(r.OCID) - } else if _, ok := s.openPos[r.OCID]; !ok { - s.openPos[r.OCID] = len(s.open) - s.open = append(s.open, r.OCID) +func (s *Store) index(i int, release ocds.Release) { + s.latest[release.OCID] = i + if len(release.Tag) > 0 && release.Tag[0] == "award" { + s.closeProcedure(release.OCID) + } else if _, ok := s.openPos[release.OCID]; !ok { + s.openPos[release.OCID] = len(s.open) + s.open = append(s.open, release.OCID) } - if num, year, ok := strings.Cut(r.ID, "-"); ok { - n, err1 := strconv.Atoi(num) - y, err2 := strconv.Atoi(year) - if err1 == nil && err2 == nil && n > s.lastNum[y] { - s.lastNum[y] = n + if numberText, year, ok := strings.Cut(release.ID, "-"); ok { + number, numberErr := strconv.Atoi(numberText) + yearNumber, yearErr := strconv.Atoi(year) + if numberErr == nil && yearErr == nil && number > s.lastNum[yearNumber] { + s.lastNum[yearNumber] = number } } } // closeProcedure removes ocid from the open procedures, if it is there. func (s *Store) closeProcedure(ocid string) { - pos, ok := s.openPos[ocid] + position, ok := s.openPos[ocid] if !ok { return } last := len(s.open) - 1 - s.open[pos] = s.open[last] - s.openPos[s.open[pos]] = pos + s.open[position] = s.open[last] + s.openPos[s.open[position]] = position s.open = s.open[:last] delete(s.openPos, ocid) } // Append persists a release and flushes it to disk. -func (s *Store) Append(r ocds.Release) error { return s.append(r, true) } +func (s *Store) Append(release ocds.Release) error { return s.append(release, true) } // AppendNoSync persists a release without an fsync; call Sync afterwards. // Used for bulk catch-up, where a sync per record would be very slow. -func (s *Store) AppendNoSync(r ocds.Release) error { return s.append(r, false) } +func (s *Store) AppendNoSync(release ocds.Release) error { return s.append(release, false) } // Sync flushes appended releases to disk. Call it after AppendNoSync. func (s *Store) Sync() error { @@ -175,22 +175,22 @@ func (s *Store) Sync() error { return s.f.Sync() } -// append stores r, syncing the file to disk if sync is true. A date earlier +// append stores release, syncing the file to disk if sync is true. A date earlier // than the newest stored release (for example after a clock change) is moved // up so the order stays valid. -func (s *Store) append(r ocds.Release, sync bool) error { +func (s *Store) append(release ocds.Release, sync bool) error { s.mu.Lock() defer s.mu.Unlock() - r.Date = r.Date.UTC() - if n := len(s.times); n > 0 && r.Date.Before(s.times[n-1]) { - r.Date = s.times[n-1] + release.Date = release.Date.UTC() + if count := len(s.times); count > 0 && release.Date.Before(s.times[count-1]) { + release.Date = s.times[count-1] } - b, err := json.Marshal(r) + data, err := json.Marshal(release) if err != nil { return err } - if _, err := s.f.Write(append(b, '\n')); err != nil { + if _, err := s.f.Write(append(data, '\n')); err != nil { return err } if sync { @@ -198,9 +198,9 @@ func (s *Store) append(r ocds.Release, sync bool) error { return err } } - s.releases = append(s.releases, r) - s.times = append(s.times, r.Date) - s.index(len(s.releases)-1, r) + s.releases = append(s.releases, release) + s.times = append(s.times, release.Date) + s.index(len(s.releases)-1, release) return nil } @@ -210,7 +210,9 @@ func (s *Store) Page(from, upper time.Time, start, limit int) ([]ocds.Release, i s.mu.RLock() defer s.mu.RUnlock() - i := sort.Search(len(s.times), func(i int) bool { return !s.times[i].Before(from) }) + i := sort.Search(len(s.times), func(i int) bool { + return !s.times[i].Before(from) + }) if start > i { i = start } @@ -259,11 +261,11 @@ func (s *Store) NextNoticeID(year int) string { // RandomOpen picks a procedure that has not been awarded and whose latest // release is after since. Procedures idle since before then are dropped from // the open set, as if they had been abandoned. -func (s *Store) RandomOpen(r *rand.Rand, since time.Time) (ocds.Release, bool) { +func (s *Store) RandomOpen(rng *rand.Rand, since time.Time) (ocds.Release, bool) { s.mu.Lock() defer s.mu.Unlock() for len(s.open) > 0 { - ocid := s.open[r.Intn(len(s.open))] + ocid := s.open[rng.Intn(len(s.open))] i := s.latest[ocid] if s.times[i].After(since) { return s.releases[i], true diff --git a/internal/mock/findatender/store_test.go b/internal/mock/findatender/store_test.go index ceb8f8a..ef15654 100644 --- a/internal/mock/findatender/store_test.go +++ b/internal/mock/findatender/store_test.go @@ -65,7 +65,11 @@ func TestStorePersistsAndGeneratorProgresses(t *testing.T) { } // The HTTP handler pages through the store without gaps or repeats. - srv := &server{store: store2, baseURL: "", loc: time.UTC} + srv := &server{ + store: store2, + baseURL: "", + loc: time.UTC, + } seen := map[string]bool{} next := BasePath + "?updatedFrom=2026-10-07T00:00:00&limit=7" for pages := 0; next != ""; pages++ { @@ -105,7 +109,12 @@ func writeLines(t *testing.T, path string, lines ...string) { func releaseLine(t *testing.T, ocid, id, tag string, at time.Time) string { t.Helper() - b, err := json.Marshal(ocds.Release{OCID: ocid, ID: id, Date: at, Tag: []string{tag}}) + b, err := json.Marshal(ocds.Release{ + OCID: ocid, + ID: id, + Date: at, + Tag: []string{tag}, + }) if err != nil { t.Fatal(err) } @@ -117,7 +126,9 @@ func releaseLine(t *testing.T, ocid, id, tag string, at time.Time) string { func TestOpenStoreRecoversFromMessyFile(t *testing.T) { path := filepath.Join(t.TempDir(), "r.jsonl") t0 := time.Date(2026, 10, 8, 9, 0, 0, 0, time.UTC) - writeLines(t, path, + writeLines( + t, + path, releaseLine(t, "ocds-b", "000007-2026", "tender", t0.Add(2*time.Hour)), "{not json", "", @@ -225,9 +236,19 @@ func TestAppendKeepsDateOrder(t *testing.T) { } defer store.Close() t0 := time.Date(2026, 10, 8, 9, 0, 0, 0, time.UTC) - store.Append(ocds.Release{OCID: "a", ID: "000001-2026", Date: t0, Tag: []string{"tender"}}) + store.Append(ocds.Release{ + OCID: "a", + ID: "000001-2026", + Date: t0, + Tag: []string{"tender"}, + }) store.Append( - ocds.Release{OCID: "b", ID: "000002-2026", Date: t0.Add(-time.Hour), Tag: []string{"tender"}}, + ocds.Release{ + OCID: "b", + ID: "000002-2026", + Date: t0.Add(-time.Hour), + Tag: []string{"tender"}, + }, ) if last, _ := store.Last(); !last.Equal(t0) { t.Errorf("Last = %s, want %s", last, t0) @@ -235,7 +256,11 @@ func TestAppendKeepsDateOrder(t *testing.T) { // Appending to a closed store fails and leaves the store unchanged. store.Close() - if err := store.Append(ocds.Release{OCID: "c", Date: t0, Tag: []string{"tender"}}); err == nil { + if err := store.Append(ocds.Release{ + OCID: "c", + Date: t0, + Tag: []string{"tender"}, + }); err == nil { t.Error("append after close: expected error") } if store.Len() != 2 || store.HasOCID("c") { @@ -251,9 +276,19 @@ func TestRandomOpenDropsStaleProcedures(t *testing.T) { } defer store.Close() t0 := time.Date(2026, 10, 8, 9, 0, 0, 0, time.UTC) - store.Append(ocds.Release{OCID: "old", ID: "000001-2026", Date: t0, Tag: []string{"tender"}}) + store.Append(ocds.Release{ + OCID: "old", + ID: "000001-2026", + Date: t0, + Tag: []string{"tender"}, + }) store.Append( - ocds.Release{OCID: "new", ID: "000002-2026", Date: t0.Add(time.Hour), Tag: []string{"tender"}}, + ocds.Release{ + OCID: "new", + ID: "000002-2026", + Date: t0.Add(time.Hour), + Tag: []string{"tender"}, + }, ) //nolint:gosec // G404: seeded math/rand keeps the test deterministic diff --git a/internal/store/common.go b/internal/store/common.go index 490f90b..bfafe35 100644 --- a/internal/store/common.go +++ b/internal/store/common.go @@ -24,7 +24,9 @@ func OpenDB(path string) (*sql.DB, error) { // ErrNeedsMigration. Intended for tests and throwaway databases; applications // should migrate explicitly with the migrate command. func WithAutoMigrate() Option { - return func(o *options) { o.autoMigrate = true } + return func(opts *options) { + opts.autoMigrate = true + } } // dsn builds the mattn/go-sqlite3 connection string. diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 66bf7e4..6239689 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -46,8 +46,8 @@ func NewMigrator(db *sql.DB) (*goose.Provider, error) { } // LatestVersion is the highest migration version in this build. -func LatestVersion(p *goose.Provider) int64 { - sources := p.ListSources() // sorted ascending +func LatestVersion(provider *goose.Provider) int64 { + sources := provider.ListSources() // sorted ascending if len(sources) == 0 { return 0 } @@ -56,26 +56,34 @@ func LatestVersion(p *goose.Provider) int64 { // checkVersion returns ErrNeedsMigration or ErrSchemaTooNew unless the // database is exactly at this build's schema. -func checkVersion(ctx context.Context, p *goose.Provider) error { - current, err := p.GetDBVersion(ctx) +func checkVersion(ctx context.Context, provider *goose.Provider) error { + current, err := provider.GetDBVersion(ctx) if err != nil { return err } - latest := LatestVersion(p) + latest := LatestVersion(provider) if current > latest { - return fmt.Errorf("%w (database is at version %d, this build knows up to %d)", - ErrSchemaTooNew, current, latest) + return fmt.Errorf( + "%w (database is at version %d, this build knows up to %d)", + ErrSchemaTooNew, + current, + latest, + ) } - pending, err := p.HasPending(ctx) + pending, err := provider.HasPending(ctx) if err != nil { return err } if pending { - return fmt.Errorf("%w (database is at version %d, latest is %d)", - ErrNeedsMigration, current, latest) + return fmt.Errorf( + "%w (database is at version %d, latest is %d)", + ErrNeedsMigration, + current, + latest, + ) } return nil @@ -83,53 +91,55 @@ func checkVersion(ctx context.Context, p *goose.Provider) error { // checkNotNewer refuses to work on a database migrated by a newer build: // this build has neither the newer migrations nor their down steps. -func checkNotNewer(ctx context.Context, m *goose.Provider) error { - current, err := m.GetDBVersion(ctx) +func checkNotNewer(ctx context.Context, provider *goose.Provider) error { + current, err := provider.GetDBVersion(ctx) if err != nil { return err } - if latest := LatestVersion(m); current > latest { + if latest := LatestVersion(provider); current > latest { return fmt.Errorf( "database is at version %d but this build only knows up to %d; use a newer build", - current, latest) + current, + latest, + ) } return nil } // prepareSchema migrates (with WithAutoMigrate) or verifies the schema // version. -func prepareSchema(ctx context.Context, db *sql.DB, o options) error { - p, err := NewMigrator(db) +func prepareSchema(ctx context.Context, db *sql.DB, opts options) error { + provider, err := NewMigrator(db) if err != nil { return err } - if o.autoMigrate { - if _, err := p.Up(ctx); err != nil { + if opts.autoMigrate { + if _, err := provider.Up(ctx); err != nil { return fmt.Errorf("store: migrating database: %w", err) } } - return checkVersion(ctx, p) + return checkVersion(ctx, provider) } // StepUp applies steps pending migrations, or all of them when steps is 0. func StepUp( - ctx context.Context, m *goose.Provider, steps int, + ctx context.Context, provider *goose.Provider, steps int, ) ([]*goose.MigrationResult, error) { var results []*goose.MigrationResult - if err := checkNotNewer(ctx, m); err != nil { + if err := checkNotNewer(ctx, provider); err != nil { return results, err } var err error if steps == 0 { - results, err = m.Up(ctx) + results, err = provider.Up(ctx) } else { for range steps { - var r *goose.MigrationResult - r, err = m.UpByOne(ctx) + var result *goose.MigrationResult + result, err = provider.UpByOne(ctx) if errors.Is(err, goose.ErrNoNextVersion) { err = nil break @@ -137,7 +147,7 @@ func StepUp( if err != nil { break } - results = append(results, r) + results = append(results, result) } } @@ -148,15 +158,15 @@ func StepUp( // is 0. Rolling everything back deletes all data, so confirmDrop is checked // first in that case. func StepDown( - ctx context.Context, m *goose.Provider, steps int, confirmDrop bool, + ctx context.Context, provider *goose.Provider, steps int, confirmDrop bool, ) ([]*goose.MigrationResult, error) { var results []*goose.MigrationResult - if err := checkNotNewer(ctx, m); err != nil { + if err := checkNotNewer(ctx, provider); err != nil { return results, err } - current, err := m.GetDBVersion(ctx) + current, err := provider.GetDBVersion(ctx) if err != nil { return results, err } @@ -164,13 +174,13 @@ func StepDown( return results, nil } - statuses, err := m.Status(ctx) + statuses, err := provider.Status(ctx) if err != nil { return results, err } applied := 0 - for _, s := range statuses { - if s.State == goose.StateApplied { + for _, status := range statuses { + if status.State == goose.StateApplied { applied++ } } @@ -180,11 +190,11 @@ func StepDown( } if all { - results, err = m.DownTo(ctx, 0) + results, err = provider.DownTo(ctx, 0) } else { for range steps { - var r *goose.MigrationResult - r, err = m.Down(ctx) + var result *goose.MigrationResult + result, err = provider.Down(ctx) if errors.Is(err, goose.ErrNoNextVersion) { err = nil break @@ -192,7 +202,7 @@ func StepDown( if err != nil { break } - results = append(results, r) + results = append(results, result) } } diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index f833ab6..ec5eadf 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -16,6 +16,7 @@ import ( // Run with: go test -tags sqlite_fts5 ./... func tempDBPath(t *testing.T) string { + t.Helper() return filepath.Join(t.TempDir(), "migrate.db") } @@ -25,7 +26,9 @@ func newMigrator(t *testing.T) (*goose.Provider, *sql.DB) { if err != nil { t.Fatal(err) } - t.Cleanup(func() { db.Close() }) + t.Cleanup(func() { + db.Close() + }) m, err := store.NewMigrator(db) if err != nil { t.Fatal(err) @@ -93,7 +96,8 @@ func TestOpenRejectsNewerSchema(t *testing.T) { } defer db.Close() if _, err := db.Exec( - `INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`); err != nil { + `INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`, + ); err != nil { t.Fatal(err) } @@ -268,7 +272,8 @@ func TestNewerDatabaseIsRefused(t *testing.T) { t.Fatal(err) } if _, err := db.Exec( - `INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`); err != nil { + `INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`, + ); err != nil { t.Fatal(err) } diff --git a/internal/store/queries.go b/internal/store/queries.go index 8763d51..8a3d462 100644 --- a/internal/store/queries.go +++ b/internal/store/queries.go @@ -67,15 +67,20 @@ SELECT t.* FROM opportunities t WHERE t.uuid = ?` // when the mark is set and is negated with NOT to match it unset; an // opportunity with no marks row has every mark unset. const ( + // listOpportunitiesSelect selects every opportunity, as t. listOpportunitiesSelect = ` SELECT t.* FROM opportunities t` + // listOpportunitiesOrderAndPage orders by latest release, newest first, + // and takes a page: the limit and offset arguments. listOpportunitiesOrderAndPage = ` ORDER BY t.latest_release_date DESC, t.uuid LIMIT ? OFFSET ?` + // filterOpportunityStatus matches a status. filterOpportunityStatus = `t.status = ?` + // filterOpportunityClassificationCode matches a classification code. filterOpportunityClassificationCode = `EXISTS ( SELECT 1 FROM opportunity_classifications tc @@ -83,35 +88,45 @@ LIMIT ? OFFSET ?` WHERE tc.opportunity_uuid = t.uuid AND c.code = ? )` - // The argument must come from ftsQuery, never raw user input. + // filterOpportunityTitleMatch matches a full-text search of the title. The + // argument must come from ftsQuery, never raw user input. filterOpportunityTitleMatch = `t.id IN ( SELECT rowid FROM opportunities_fts WHERE opportunities_fts MATCH ? )` + // filterOpportunityReleasedFrom matches a latest release on or after a + // time. filterOpportunityReleasedFrom = `t.latest_release_date >= ?` + // filterOpportunityReleasedBefore matches a latest release before a time. filterOpportunityReleasedBefore = `t.latest_release_date < ?` + // filterOpportunityMarkHidden matches the hidden mark set. filterOpportunityMarkHidden = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.hidden )` + // filterOpportunityMarkSkipped matches the skipped mark set. filterOpportunityMarkSkipped = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.skipped )` + // filterOpportunityMarkInterested matches the interested mark set. filterOpportunityMarkInterested = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.interested )` + // filterOpportunityMarkBidSubmitted matches the bid submitted mark set. filterOpportunityMarkBidSubmitted = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.bid_submitted )` + // filterOpportunityMarkWon matches the won mark set. filterOpportunityMarkWon = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.won )` + // filterOpportunityMarkLost matches the lost mark set. filterOpportunityMarkLost = `EXISTS ( SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.lost )` diff --git a/internal/store/queries_test.go b/internal/store/queries_test.go index aa3d988..67c711d 100644 --- a/internal/store/queries_test.go +++ b/internal/store/queries_test.go @@ -84,7 +84,9 @@ func openTestDB(t *testing.T) *sqlx.DB { if err != nil { t.Fatalf("Open: %v", err) } - t.Cleanup(func() { s.Close() }) + t.Cleanup(func() { + s.Close() + }) return s.db } diff --git a/internal/store/store.go b/internal/store/store.go index a03fff3..b0e0743 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -103,57 +103,57 @@ func (f OpportunityFilter) limit() int { return f.Limit } -// prepareOpportunity validates t and normalises it in place before saving: child -// rows get the opportunity's UUID and their release id, times are converted to -// UTC (so stored timestamps compare correctly as text), and +// prepareOpportunity validates opportunity and normalises it in place before +// saving: child rows get the opportunity's UUID and their release id, times +// are converted to UTC (so stored timestamps compare correctly as text), and // LatestReleaseDate becomes the newest release date in the save. -func prepareOpportunity(t *types.Opportunity) error { - if t == nil { +func prepareOpportunity(opportunity *types.Opportunity) error { + if opportunity == nil { return errors.New("store: nil opportunity") } - if t.UUID == "" { + if opportunity.UUID == "" { return errors.New("store: opportunity has no UUID") } - t.Marks.UUID = t.UUID - if err := t.Marks.Validate(); err != nil { - return fmt.Errorf("store: opportunity %s: %w", t.UUID, err) + opportunity.Marks.UUID = opportunity.UUID + if err := opportunity.Marks.Validate(); err != nil { + return fmt.Errorf("store: opportunity %s: %w", opportunity.UUID, err) } - latest := t.LatestReleaseDate - for i := range t.Releases { - r := &t.Releases[i] - if r.ReleaseID == "" { - return fmt.Errorf("store: opportunity %s: release %d has no release id", t.UUID, i) + latest := opportunity.LatestReleaseDate + for i := range opportunity.Releases { + release := &opportunity.Releases[i] + if release.ReleaseID == "" { + return fmt.Errorf("store: opportunity %s: release %d has no release id", opportunity.UUID, i) } - r.UUID = t.UUID - r.ReleaseDate = r.ReleaseDate.UTC() - if r.ReleaseDate.After(latest) { - latest = r.ReleaseDate + release.UUID = opportunity.UUID + release.ReleaseDate = release.ReleaseDate.UTC() + if release.ReleaseDate.After(latest) { + latest = release.ReleaseDate } - for j := range r.Tags { - r.Tags[j].UUID = t.UUID - r.Tags[j].ReleaseID = r.ReleaseID + for tagIndex := range release.Tags { + release.Tags[tagIndex].UUID = opportunity.UUID + release.Tags[tagIndex].ReleaseID = release.ReleaseID } } if latest.IsZero() { - return fmt.Errorf("store: opportunity %s has no release date", t.UUID) + return fmt.Errorf("store: opportunity %s has no release date", opportunity.UUID) } - t.LatestReleaseDate = latest.UTC() - t.StartDate = utcPtr(t.StartDate) - t.EndDate = utcPtr(t.EndDate) + opportunity.LatestReleaseDate = latest.UTC() + opportunity.StartDate = utcPtr(opportunity.StartDate) + opportunity.EndDate = utcPtr(opportunity.EndDate) return nil } -func utcPtr(t *time.Time) *time.Time { - if t == nil { +func utcPtr(when *time.Time) *time.Time { + if when == nil { return nil } - u := t.UTC() - return &u + utc := when.UTC() + return &utc } // ftsQuery turns free text into an FTS5 query that requires every word, each @@ -162,11 +162,11 @@ func utcPtr(t *time.Time) *time.Time { // FTS5 syntax (operators, column filters, unbalanced quotes). It returns "" // when the text contains no words. func ftsQuery(text string) string { - words := strings.FieldsFunc(text, func(r rune) bool { - return !unicode.IsLetter(r) && !unicode.IsNumber(r) + words := strings.FieldsFunc(text, func(char rune) bool { + return !unicode.IsLetter(char) && !unicode.IsNumber(char) }) - for i, w := range words { - words[i] = `"` + w + `"*` + for i, word := range words { + words[i] = `"` + word + `"*` } return strings.Join(words, " ") } @@ -179,9 +179,9 @@ func ftsQuery(text string) string { // // A database file that doesn't exist is created, and then needs migrating. func Open(ctx context.Context, path string, opts ...Option) (*Store, error) { - var o options - for _, opt := range opts { - opt(&o) + var settings options + for _, option := range opts { + option(&settings) } sqldb, err := OpenDB(path) @@ -191,10 +191,12 @@ func Open(ctx context.Context, path string, opts ...Option) (*Store, error) { if err := sqldb.PingContext(ctx); err != nil { return nil, errors.Join(err, sqldb.Close()) } - if err := prepareSchema(ctx, sqldb, o); err != nil { + if err := prepareSchema(ctx, sqldb, settings); err != nil { return nil, errors.Join(err, sqldb.Close()) } - return &Store{db: sqlx.NewDb(sqldb, "sqlite3")}, nil + return &Store{ + db: sqlx.NewDb(sqldb, "sqlite3"), + }, nil } // Close closes the database. @@ -214,20 +216,24 @@ type opportunityRow struct { types.Opportunity } -// SaveOpportunity stores t and its releases in one transaction. +// SaveOpportunity stores opportunity and its releases in one transaction. // // Releases (and their tags) are write-once: ones already stored are skipped. -// The opportunity row and its classifications are replaced only if t's newest -// release is at least as new as the release the stored opportunity came from, so -// a late-arriving older release never undoes a newer one. +// The opportunity row and its classifications are replaced only if the +// opportunity's newest release is at least as new as the release the stored +// opportunity came from, so a late-arriving older release never undoes a +// newer one. // -// t.Marks are stored only for a new opportunity; existing marks are kept (use -// SetMarks to change them). +// opportunity.Marks are stored only for a new opportunity; existing marks are +// kept (use SetMarks to change them). // -// t is normalised in place (see prepareOpportunity), and when the opportunity is +// opportunity is normalised in place (see prepareOpportunity), and when it is // written its classifications' IDs are filled in. -func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (SaveResult, error) { - if err := prepareOpportunity(t); err != nil { +func (s *Store) SaveOpportunity( + ctx context.Context, + opportunity *types.Opportunity, +) (SaveResult, error) { + if err := prepareOpportunity(opportunity); err != nil { return SaveResult{}, err } @@ -237,7 +243,7 @@ func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (Save } defer tx.Rollback() //nolint:errcheck // no-op after Commit - res, _, err := saveOpportunity(ctx, tx, t) + res, _, err := saveOpportunity(ctx, tx, opportunity) if err != nil { return SaveResult{}, err } @@ -247,23 +253,25 @@ func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (Save return res, nil } -// SaveOpportunities stores t and its releases in one transaction. +// SaveOpportunities stores each of opportunities and its releases, all in one +// transaction, as SaveOpportunity does for one. // // Releases (and their tags) are write-once: ones already stored are skipped. -// The opportunity row and its classifications are replaced only if t's newest -// release is at least as new as the release the stored opportunity came from, so -// a late-arriving older release never undoes a newer one. +// An opportunity row and its classifications are replaced only if the +// opportunity's newest release is at least as new as the release the stored +// opportunity came from, so a late-arriving older release never undoes a +// newer one. // -// t.Marks are stored only for a new opportunity; existing marks are kept (use +// Marks are stored only for a new opportunity; existing marks are kept (use // SetMarks to change them). // -// t is normalised in place (see prepareOpportunity), and when the opportunity is -// written its classifications' IDs are filled in. +// Each opportunity is normalised in place (see prepareOpportunity), and when +// it is written its classifications' IDs are filled in. func (s *Store) SaveOpportunities( ctx context.Context, opportunities []*types.Opportunity, ) (SaveMultipleResult, error) { - for _, t := range opportunities { - if err := prepareOpportunity(t); err != nil { + for _, opportunity := range opportunities { + if err := prepareOpportunity(opportunity); err != nil { return SaveMultipleResult{}, err } } @@ -274,9 +282,11 @@ func (s *Store) SaveOpportunities( } defer tx.Rollback() //nolint:errcheck // no-op after Commit - allRes := SaveMultipleResult{OpportunityResults: make([]SaveResult, len(opportunities))} - for i, t := range opportunities { - res, isNew, err := saveOpportunity(ctx, tx, t) + allRes := SaveMultipleResult{ + OpportunityResults: make([]SaveResult, len(opportunities)), + } + for i, opportunity := range opportunities { + res, isNew, err := saveOpportunity(ctx, tx, opportunity) if err != nil { return SaveMultipleResult{}, err } @@ -300,27 +310,35 @@ func (s *Store) SaveOpportunities( // one prepared opportunity inside tx. isNew reports that the opportunity was // not already stored. func saveOpportunity( - ctx context.Context, tx *sqlx.Tx, t *types.Opportunity, + ctx context.Context, tx *sqlx.Tx, opportunity *types.Opportunity, ) (res SaveResult, isNew bool, err error) { var stored time.Time - err = tx.GetContext(ctx, &stored, selectOpportunityLatestReleaseDate, t.UUID) + err = tx.GetContext(ctx, &stored, selectOpportunityLatestReleaseDate, opportunity.UUID) switch { case errors.Is(err, sql.ErrNoRows): isNew = true res.OpportunityUpdated = true case err != nil: - return SaveResult{}, false, fmt.Errorf("store: reading opportunity %s: %w", t.UUID, err) + return SaveResult{}, false, fmt.Errorf( + "store: reading opportunity %s: %w", + opportunity.UUID, + err, + ) default: - res.OpportunityUpdated = !t.LatestReleaseDate.Before(stored) + res.OpportunityUpdated = !opportunity.LatestReleaseDate.Before(stored) } if res.OpportunityUpdated { - if err := writeOpportunity(ctx, tx, t); err != nil { - return SaveResult{}, false, fmt.Errorf("store: writing opportunity %s: %w", t.UUID, err) + if err := writeOpportunity(ctx, tx, opportunity); err != nil { + return SaveResult{}, false, fmt.Errorf( + "store: writing opportunity %s: %w", + opportunity.UUID, + err, + ) } } - for _, r := range t.Releases { - inserted, err := insertReleaseWithTags(ctx, tx, r) + for _, release := range opportunity.Releases { + inserted, err := insertReleaseWithTags(ctx, tx, release) if err != nil { return SaveResult{}, false, err } @@ -331,69 +349,86 @@ func saveOpportunity( return res, isNew, nil } -// insertReleaseWithTags stores r and its tags unless r is already stored, and -// reports whether it was inserted. +// insertReleaseWithTags stores release and its tags unless it is already +// stored, and reports whether it was inserted. func insertReleaseWithTags( - ctx context.Context, tx *sqlx.Tx, r types.OpportunityRelease, + ctx context.Context, tx *sqlx.Tx, release types.OpportunityRelease, ) (bool, error) { - result, err := tx.NamedExecContext(ctx, insertRelease, r) + result, err := tx.NamedExecContext(ctx, insertRelease, release) if err != nil { - return false, fmt.Errorf("store: inserting release %s/%s: %w", r.UUID, r.ReleaseID, err) + return false, fmt.Errorf( + "store: inserting release %s/%s: %w", + release.UUID, + release.ReleaseID, + err, + ) } - if n, err := result.RowsAffected(); err != nil || n == 0 { + if affected, err := result.RowsAffected(); err != nil || affected == 0 { return false, err // n == 0: already stored } - for _, tag := range r.Tags { + for _, tag := range release.Tags { if _, err := tx.NamedExecContext(ctx, insertReleaseTag, tag); err != nil { return false, fmt.Errorf( - "store: inserting tag for %s/%s: %w", r.UUID, r.ReleaseID, err, + "store: inserting tag for %s/%s: %w", release.UUID, release.ReleaseID, err, ) } } return true, nil } -// writeOpportunity upserts t's row, gives it a marks row if it has none +// writeOpportunity upserts opportunity's row, gives it a marks row if it has none // (existing marks are kept), and replaces its classifications, filling in // their IDs. -func writeOpportunity(ctx context.Context, tx *sqlx.Tx, t *types.Opportunity) (err error) { - if _, err := tx.NamedExecContext(ctx, upsertOpportunity, t); err != nil { +func writeOpportunity( + ctx context.Context, + tx *sqlx.Tx, + opportunity *types.Opportunity, +) (err error) { + if _, err := tx.NamedExecContext(ctx, upsertOpportunity, opportunity); err != nil { return err } - if _, err := tx.NamedExecContext(ctx, insertOpportunityMarks, t.Marks); err != nil { + if _, err := tx.NamedExecContext(ctx, insertOpportunityMarks, opportunity.Marks); err != nil { return err } - if _, err := tx.ExecContext(ctx, deleteOpportunityClassifications, t.UUID); err != nil { + if _, err := tx.ExecContext(ctx, deleteOpportunityClassifications, opportunity.UUID); err != nil { return err } - stmt, err := tx.PrepareNamedContext(ctx, upsertClassificationCodeReturningID) + statement, err := tx.PrepareNamedContext(ctx, upsertClassificationCodeReturningID) if err != nil { return err } - defer func() { err = errors.Join(err, stmt.Close()) }() - for i := range t.Classifications { - c := &t.Classifications[i] - if err := stmt.GetContext(ctx, &c.ID, c); err != nil { + defer func() { + err = errors.Join(err, statement.Close()) + }() + for i := range opportunity.Classifications { + classification := &opportunity.Classifications[i] + if err := statement.GetContext(ctx, &classification.ID, classification); err != nil { return err } - if _, err := tx.ExecContext(ctx, insertOpportunityClassification, t.UUID, c.ID); err != nil { + if _, err := tx.ExecContext( + ctx, + insertOpportunityClassification, + opportunity.UUID, + classification.ID, + ); err != nil { return err } } return nil } -// SetMarks replaces the marks of the opportunity m.UUID, or returns ErrNotFound. -// Marks that fail m.Validate (e.g. types.ErrWonAndLost) are not stored. -func (s *Store) SetMarks(ctx context.Context, m types.OpportunityMarks) error { - if m.UUID == "" { +// SetMarks replaces the marks of the opportunity marks.UUID, or returns +// ErrNotFound. Marks that fail marks.Validate (e.g. types.ErrWonAndLost) are +// not stored. +func (s *Store) SetMarks(ctx context.Context, marks types.OpportunityMarks) error { + if marks.UUID == "" { return errors.New("store: marks have no UUID") } - if err := m.Validate(); err != nil { - return fmt.Errorf("store: opportunity %s: %w", m.UUID, err) + if err := marks.Validate(); err != nil { + return fmt.Errorf("store: opportunity %s: %w", marks.UUID, err) } - _, err := s.db.NamedExecContext(ctx, upsertOpportunityMarks, m) + _, err := s.db.NamedExecContext(ctx, upsertOpportunityMarks, marks) if isForeignKeyError(err) { return ErrNotFound } @@ -403,8 +438,8 @@ func (s *Store) SetMarks(ctx context.Context, m types.OpportunityMarks) error { // isForeignKeyError reports whether err is SQLite rejecting a write that // breaks a foreign key, e.g. one referring to a missing opportunity. func isForeignKeyError(err error) bool { - var se sqlite3.Error - return errors.As(err, &se) && se.ExtendedCode == sqlite3.ErrConstraintForeignKey + var sqliteErr sqlite3.Error + return errors.As(err, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintForeignKey } // GetOpportunity returns the opportunity with its classifications, marks and @@ -431,13 +466,14 @@ func (s *Store) GetOpportunity(ctx context.Context, uuid string) (*types.Opportu return &opportunities[0], nil } -// ListOpportunities returns opportunities matching f, with their classifications -// and marks but without releases (use ListReleases or GetOpportunity for those). +// ListOpportunities returns opportunities matching filter, with their +// classifications and marks but without releases (use ListReleases or +// GetOpportunity for those). func (s *Store) ListOpportunities( ctx context.Context, - f OpportunityFilter, + filter OpportunityFilter, ) ([]types.Opportunity, error) { - query, args := listOpportunitiesQuery(f) + query, args := listOpportunitiesQuery(filter) var rows []opportunityRow if err := s.db.SelectContext(ctx, &rows, query, args...); err != nil { return nil, err @@ -457,44 +493,62 @@ func (s *Store) ListOpportunities( // listOpportunitiesQuery assembles the ListOpportunities query from the fragments in // queries.go, with one argument per filter that is set (none for mark filters). -func listOpportunitiesQuery(f OpportunityFilter) (string, []any) { +func listOpportunitiesQuery(filter OpportunityFilter) (string, []any) { var where []string var args []any - add := func(clause string, arg any) { + add := func(clause string, argument any) { where = append(where, clause) - args = append(args, arg) + args = append(args, argument) } - if f.Status != "" { - add(filterOpportunityStatus, f.Status) + if filter.Status != "" { + add(filterOpportunityStatus, filter.Status) } - if f.ClassificationCode != "" { - add(filterOpportunityClassificationCode, f.ClassificationCode) + if filter.ClassificationCode != "" { + add(filterOpportunityClassificationCode, filter.ClassificationCode) } - if q := ftsQuery(f.Title); q != "" { - add(filterOpportunityTitleMatch, q) + if match := ftsQuery(filter.Title); match != "" { + add(filterOpportunityTitleMatch, match) } - if !f.ReleasedFrom.IsZero() { - add(filterOpportunityReleasedFrom, f.ReleasedFrom.UTC()) + if !filter.ReleasedFrom.IsZero() { + add(filterOpportunityReleasedFrom, filter.ReleasedFrom.UTC()) } - if !f.ReleasedBefore.IsZero() { - add(filterOpportunityReleasedBefore, f.ReleasedBefore.UTC()) + if !filter.ReleasedBefore.IsZero() { + add(filterOpportunityReleasedBefore, filter.ReleasedBefore.UTC()) } - for _, m := range []struct { + for _, mark := range []struct { filter MarkFilter clause string }{ - {f.Hidden, filterOpportunityMarkHidden}, - {f.Skipped, filterOpportunityMarkSkipped}, - {f.Interested, filterOpportunityMarkInterested}, - {f.BidSubmitted, filterOpportunityMarkBidSubmitted}, - {f.Won, filterOpportunityMarkWon}, - {f.Lost, filterOpportunityMarkLost}, + { + filter: filter.Hidden, + clause: filterOpportunityMarkHidden, + }, + { + filter: filter.Skipped, + clause: filterOpportunityMarkSkipped, + }, + { + filter: filter.Interested, + clause: filterOpportunityMarkInterested, + }, + { + filter: filter.BidSubmitted, + clause: filterOpportunityMarkBidSubmitted, + }, + { + filter: filter.Won, + clause: filterOpportunityMarkWon, + }, + { + filter: filter.Lost, + clause: filterOpportunityMarkLost, + }, } { - switch m.filter { + switch mark.filter { case MarkSet: - where = append(where, m.clause) + where = append(where, mark.clause) case MarkUnset: - where = append(where, "NOT "+m.clause) + where = append(where, "NOT "+mark.clause) } } @@ -503,7 +557,7 @@ func listOpportunitiesQuery(f OpportunityFilter) (string, []any) { query += "\nWHERE " + strings.Join(where, "\n\tAND ") } query += listOpportunitiesOrderAndPage - return query, append(args, f.limit(), f.Offset) + return query, append(args, filter.limit(), filter.Offset) } // loadClassifications fills in Classifications for all opportunities with one query. @@ -530,9 +584,9 @@ func (s *Store) loadClassifications(ctx context.Context, opportunities []types.O if err := s.db.SelectContext(ctx, &rows, s.db.Rebind(query), args...); err != nil { return err } - for _, r := range rows { - t := byUUID[r.OpportunityUUID] - t.Classifications = append(t.Classifications, r.ClassificationCode) + for _, releaseRow := range rows { + opportunity := byUUID[releaseRow.OpportunityUUID] + opportunity.Classifications = append(opportunity.Classifications, releaseRow.ClassificationCode) } return nil } @@ -548,7 +602,9 @@ func (s *Store) loadMarks(ctx context.Context, opportunities []types.Opportunity for i := range opportunities { uuids[i] = opportunities[i].UUID byUUID[opportunities[i].UUID] = &opportunities[i] - opportunities[i].Marks = types.OpportunityMarks{UUID: opportunities[i].UUID} + opportunities[i].Marks = types.OpportunityMarks{ + UUID: opportunities[i].UUID, + } } query, args, err := sqlx.In(selectMarksForOpportunities, uuids) @@ -559,8 +615,8 @@ func (s *Store) loadMarks(ctx context.Context, opportunities []types.Opportunity if err := s.db.SelectContext(ctx, &rows, s.db.Rebind(query), args...); err != nil { return err } - for _, m := range rows { - byUUID[m.UUID].Marks = m + for _, row := range rows { + byUUID[row.UUID].Marks = row } return nil } @@ -581,8 +637,8 @@ func (s *Store) ListReleases(ctx context.Context, uuid string) ([]types.Opportun return nil, err } for _, tag := range tags { - if r := byID[tag.ReleaseID]; r != nil { - r.Tags = append(r.Tags, tag) + if release := byID[tag.ReleaseID]; release != nil { + release.Tags = append(release.Tags, tag) } } return releases, nil diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 24d732e..c383787 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -20,7 +20,9 @@ func openStore(t *testing.T) *store.Store { if err != nil { t.Fatalf("Open: %v", err) } - t.Cleanup(func() { s.Close() }) + t.Cleanup(func() { + s.Close() + }) return s } @@ -51,7 +53,10 @@ func opportunity(uuid, title string, releaseDay int, releaseIDs ...string) *type {IsCPV: true, Code: "03000000", Classification: "Agricultural products"}, }, // No UUID: SaveOpportunity must fill it in. - Marks: types.OpportunityMarks{Interested: true, BidSubmitted: true}, + Marks: types.OpportunityMarks{ + Interested: true, + BidSubmitted: true, + }, } for i, id := range releaseIDs { t.Releases = append(t.Releases, types.OpportunityRelease{ @@ -131,7 +136,11 @@ func TestSaveAndGetRoundTrip(t *testing.T) { t.Errorf("classifications: got %+v (leading zero must survive)", got.Classifications) } - wantMarks := types.OpportunityMarks{UUID: "ocds-1", Interested: true, BidSubmitted: true} + wantMarks := types.OpportunityMarks{ + UUID: "ocds-1", + Interested: true, + BidSubmitted: true, + } if got.Marks != wantMarks { t.Errorf("marks: got %+v, want %+v", got.Marks, wantMarks) } @@ -230,7 +239,9 @@ func TestNewerReleaseUpdatesOpportunityAndSearchIndex(t *testing.T) { } for title, want := range map[string]int{"cloud": 0, "redesign": 1} { - ts, err := s.ListOpportunities(ctx, store.OpportunityFilter{Title: title}) + ts, err := s.ListOpportunities(ctx, store.OpportunityFilter{ + Title: title, + }) if err != nil { t.Fatal(err) } @@ -247,8 +258,11 @@ func TestClassificationRowsAreShared(t *testing.T) { mustSave(t, s, a) mustSave(t, s, b) if a.Classifications[0].ID == 0 || a.Classifications[0].ID != b.Classifications[0].ID { - t.Errorf("identical classifications should share an ID: %d vs %d", - a.Classifications[0].ID, b.Classifications[0].ID) + t.Errorf( + "identical classifications should share an ID: %d vs %d", + a.Classifications[0].ID, + b.Classifications[0].ID, + ) } c := opportunity("ocds-c", "C", 1, "r1") @@ -299,31 +313,59 @@ func TestListOpportunitiesFilters(t *testing.T) { want []string }{ {"all, newest first", store.OpportunityFilter{}, []string{"ocds-3", "ocds-2", "ocds-1"}}, - {"status", store.OpportunityFilter{Status: "complete"}, []string{"ocds-2"}}, + {"status", store.OpportunityFilter{ + Status: "complete", + }, []string{"ocds-2"}}, { "classification", - store.OpportunityFilter{ClassificationCode: "72000000"}, + store.OpportunityFilter{ + ClassificationCode: "72000000", + }, []string{"ocds-3", "ocds-1"}, }, - {"title word", store.OpportunityFilter{Title: "cloud"}, []string{"ocds-3", "ocds-1"}}, - {"title prefixes", store.OpportunityFilter{Title: "clo migr"}, []string{"ocds-1"}}, - {"title stemming", store.OpportunityFilter{Title: "migrating"}, []string{"ocds-1"}}, - {"title case", store.OpportunityFilter{Title: "ROAD"}, []string{"ocds-2"}}, + {"title word", store.OpportunityFilter{ + Title: "cloud", + }, []string{"ocds-3", "ocds-1"}}, + {"title prefixes", store.OpportunityFilter{ + Title: "clo migr", + }, []string{"ocds-1"}}, + {"title stemming", store.OpportunityFilter{ + Title: "migrating", + }, []string{"ocds-1"}}, + {"title case", store.OpportunityFilter{ + Title: "ROAD", + }, []string{"ocds-2"}}, { "title syntax is not FTS syntax", - store.OpportunityFilter{Title: `cloud" OR title:road*`}, + store.OpportunityFilter{ + Title: `cloud" OR title:road*`, + }, []string{}, }, { "title punctuation only", - store.OpportunityFilter{Title: `"*:()`}, + store.OpportunityFilter{ + Title: `"*:()`, + }, []string{"ocds-3", "ocds-2", "ocds-1"}, }, - {"released from", store.OpportunityFilter{ReleasedFrom: day(2)}, []string{"ocds-3", "ocds-2"}}, - {"released before", store.OpportunityFilter{ReleasedBefore: day(2)}, []string{"ocds-1"}}, - {"combined", store.OpportunityFilter{Title: "cloud", ReleasedFrom: day(2)}, []string{"ocds-3"}}, - {"limit", store.OpportunityFilter{Limit: 2}, []string{"ocds-3", "ocds-2"}}, - {"offset", store.OpportunityFilter{Limit: 2, Offset: 2}, []string{"ocds-1"}}, + {"released from", store.OpportunityFilter{ + ReleasedFrom: day(2), + }, []string{"ocds-3", "ocds-2"}}, + {"released before", store.OpportunityFilter{ + ReleasedBefore: day(2), + }, []string{"ocds-1"}}, + {"combined", store.OpportunityFilter{ + Title: "cloud", + ReleasedFrom: day(2), + }, []string{"ocds-3"}}, + {"limit", store.OpportunityFilter{ + Limit: 2, + }, []string{"ocds-3", "ocds-2"}}, + {"offset", store.OpportunityFilter{ + Limit: 2, + Offset: 2, + }, []string{"ocds-1"}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -337,7 +379,9 @@ func TestListOpportunitiesFilters(t *testing.T) { }) } - got, err := s.ListOpportunities(ctx, store.OpportunityFilter{Status: "complete"}) + got, err := s.ListOpportunities(ctx, store.OpportunityFilter{ + Status: "complete", + }) if err != nil { t.Fatal(err) } @@ -413,7 +457,9 @@ func TestReopenKeepsDataAndSkipsAppliedMigrations(t *testing.T) { t.Fatalf("reopen: %v", err) } defer s.Close() - got, err := s.ListOpportunities(ctx, store.OpportunityFilter{Title: "cloud"}) + got, err := s.ListOpportunities(ctx, store.OpportunityFilter{ + Title: "cloud", + }) if err != nil || len(got) != 1 { t.Errorf("after reopen: %v, %v", uuids(got), err) } @@ -440,8 +486,12 @@ func TestResaveLoadedOpportunity(t *testing.T) { t.Errorf("resave not applied: %+v", got) } if got.Classifications[0].ID != ids[0] || got.Classifications[1].ID != ids[1] { - t.Errorf("classification IDs changed on resave: %v -> %d, %d", - ids, got.Classifications[0].ID, got.Classifications[1].ID) + t.Errorf( + "classification IDs changed on resave: %v -> %d, %d", + ids, + got.Classifications[0].ID, + got.Classifications[1].ID, + ) } } @@ -473,14 +523,19 @@ func TestSetMarksErrors(t *testing.T) { ctx := context.Background() if err := s.SetMarks( ctx, - types.OpportunityMarks{UUID: "missing", Hidden: true}, + types.OpportunityMarks{ + UUID: "missing", + Hidden: true, + }, ); !errors.Is( err, store.ErrNotFound, ) { t.Errorf("unknown opportunity: err = %v, want ErrNotFound", err) } - if err := s.SetMarks(ctx, types.OpportunityMarks{Hidden: true}); err == nil { + if err := s.SetMarks(ctx, types.OpportunityMarks{ + Hidden: true, + }); err == nil { t.Errorf("no UUID: expected an error") } } @@ -492,7 +547,11 @@ func TestSaveKeepsExistingMarks(t *testing.T) { ctx := context.Background() mustSave(t, s, opportunity("ocds-1", "T", 1, "r1")) mustSave(t, s, opportunity("ocds-2", "T", 1, "r1")) - want := types.OpportunityMarks{UUID: "ocds-1", Hidden: true, Lost: true} + want := types.OpportunityMarks{ + UUID: "ocds-1", + Hidden: true, + Lost: true, + } mustSetMarks(t, s, want) newer := opportunity("ocds-1", "Amended", 2, "r2") @@ -518,7 +577,9 @@ func TestSaveOpportunitiesStoresMarks(t *testing.T) { s := openStore(t) a := opportunity("ocds-a", "A", 1, "r1") b := opportunity("ocds-b", "B", 1, "r1") - b.Marks = types.OpportunityMarks{Skipped: true} + b.Marks = types.OpportunityMarks{ + Skipped: true, + } if _, err := s.SaveOpportunities(context.Background(), []*types.Opportunity{a, b}); err != nil { t.Fatal(err) } @@ -539,8 +600,15 @@ func TestListOpportunitiesMarkFilters(t *testing.T) { mustSave(t, s, opportunity(uuid, "Cloud hosting", i+1, "r1")) } // ocds-1 keeps the marks it was saved with: Interested, BidSubmitted. - mustSetMarks(t, s, types.OpportunityMarks{UUID: "ocds-2", Hidden: true}) - mustSetMarks(t, s, types.OpportunityMarks{UUID: "ocds-3", Skipped: true, Lost: true}) + mustSetMarks(t, s, types.OpportunityMarks{ + UUID: "ocds-2", + Hidden: true, + }) + mustSetMarks(t, s, types.OpportunityMarks{ + UUID: "ocds-3", + Skipped: true, + Lost: true, + }) mustSetMarks(t, s, types.OpportunityMarks{ UUID: "ocds-4", Interested: true, BidSubmitted: true, Won: true, }) @@ -552,31 +620,65 @@ func TestListOpportunitiesMarkFilters(t *testing.T) { want []string }{ {"any", store.OpportunityFilter{}, []string{"ocds-4", "ocds-3", "ocds-2", "ocds-1"}}, - {"hidden", store.OpportunityFilter{Hidden: set}, []string{"ocds-2"}}, - {"not hidden", store.OpportunityFilter{Hidden: unset}, []string{"ocds-4", "ocds-3", "ocds-1"}}, - {"skipped", store.OpportunityFilter{Skipped: set}, []string{"ocds-3"}}, + {"hidden", store.OpportunityFilter{ + Hidden: set, + }, []string{"ocds-2"}}, + {"not hidden", store.OpportunityFilter{ + Hidden: unset, + }, []string{"ocds-4", "ocds-3", "ocds-1"}}, + {"skipped", store.OpportunityFilter{ + Skipped: set, + }, []string{"ocds-3"}}, { "not skipped", - store.OpportunityFilter{Skipped: unset}, + store.OpportunityFilter{ + Skipped: unset, + }, []string{"ocds-4", "ocds-2", "ocds-1"}, }, - {"interested", store.OpportunityFilter{Interested: set}, []string{"ocds-4", "ocds-1"}}, - {"not interested", store.OpportunityFilter{Interested: unset}, []string{"ocds-3", "ocds-2"}}, - {"bid submitted", store.OpportunityFilter{BidSubmitted: set}, []string{"ocds-4", "ocds-1"}}, + {"interested", store.OpportunityFilter{ + Interested: set, + }, []string{"ocds-4", "ocds-1"}}, + {"not interested", store.OpportunityFilter{ + Interested: unset, + }, []string{"ocds-3", "ocds-2"}}, + {"bid submitted", store.OpportunityFilter{ + BidSubmitted: set, + }, []string{"ocds-4", "ocds-1"}}, { "bid not submitted", - store.OpportunityFilter{BidSubmitted: unset}, + store.OpportunityFilter{ + BidSubmitted: unset, + }, []string{"ocds-3", "ocds-2"}, }, - {"won", store.OpportunityFilter{Won: set}, []string{"ocds-4"}}, - {"not won", store.OpportunityFilter{Won: unset}, []string{"ocds-3", "ocds-2", "ocds-1"}}, - {"lost", store.OpportunityFilter{Lost: set}, []string{"ocds-3"}}, - {"not lost", store.OpportunityFilter{Lost: unset}, []string{"ocds-4", "ocds-2", "ocds-1"}}, - {"two marks", store.OpportunityFilter{BidSubmitted: set, Won: unset}, []string{"ocds-1"}}, - {"no match", store.OpportunityFilter{Won: set, Lost: set}, []string{}}, + {"won", store.OpportunityFilter{ + Won: set, + }, []string{"ocds-4"}}, + {"not won", store.OpportunityFilter{ + Won: unset, + }, []string{"ocds-3", "ocds-2", "ocds-1"}}, + {"lost", store.OpportunityFilter{ + Lost: set, + }, []string{"ocds-3"}}, + {"not lost", store.OpportunityFilter{ + Lost: unset, + }, []string{"ocds-4", "ocds-2", "ocds-1"}}, + {"two marks", store.OpportunityFilter{ + BidSubmitted: set, + Won: unset, + }, []string{"ocds-1"}}, + {"no match", store.OpportunityFilter{ + Won: set, + Lost: set, + }, []string{}}, { "with other filters", - store.OpportunityFilter{Hidden: unset, Title: "cloud", ReleasedFrom: day(3)}, + store.OpportunityFilter{ + Hidden: unset, + Title: "cloud", + ReleasedFrom: day(3), + }, []string{"ocds-4", "ocds-3"}, }, } @@ -608,7 +710,11 @@ func TestWonAndLostAreExclusive(t *testing.T) { mustSave(t, s, opportunity("ocds-1", "T", 1, "r1")) before := mustGet(t, s, "ocds-1").Marks - both := types.OpportunityMarks{UUID: "ocds-1", Won: true, Lost: true} + both := types.OpportunityMarks{ + UUID: "ocds-1", + Won: true, + Lost: true, + } if err := s.SetMarks(ctx, both); !errors.Is(err, types.ErrWonAndLost) { t.Errorf("SetMarks(won and lost): err = %v, want ErrWonAndLost", err) } @@ -617,7 +723,10 @@ func TestWonAndLostAreExclusive(t *testing.T) { } in := opportunity("ocds-2", "T", 1, "r1") - in.Marks = types.OpportunityMarks{Won: true, Lost: true} + in.Marks = types.OpportunityMarks{ + Won: true, + Lost: true, + } if _, err := s.SaveOpportunity(ctx, in); !errors.Is(err, types.ErrWonAndLost) { t.Errorf("SaveOpportunity(won and lost): err = %v, want ErrWonAndLost", err) } diff --git a/internal/util/bytesize.go b/internal/util/bytesize.go index 091d3c5..c42557c 100644 --- a/internal/util/bytesize.go +++ b/internal/util/bytesize.go @@ -24,33 +24,75 @@ var byteUnits = []struct { name string size int64 }{ - {"EiB", humanize.EiByte}, {"EB", humanize.EByte}, - {"PiB", humanize.PiByte}, {"PB", humanize.PByte}, - {"TiB", humanize.TiByte}, {"TB", humanize.TByte}, - {"GiB", humanize.GiByte}, {"GB", humanize.GByte}, - {"MiB", humanize.MiByte}, {"MB", humanize.MByte}, - {"KiB", humanize.KiByte}, {"KB", humanize.KByte}, + { + name: "EiB", + size: humanize.EiByte, + }, + { + name: "EB", + size: humanize.EByte, + }, + { + name: "PiB", + size: humanize.PiByte, + }, + { + name: "PB", + size: humanize.PByte, + }, + { + name: "TiB", + size: humanize.TiByte, + }, + { + name: "TB", + size: humanize.TByte, + }, + { + name: "GiB", + size: humanize.GiByte, + }, + { + name: "GB", + size: humanize.GByte, + }, + { + name: "MiB", + size: humanize.MiByte, + }, + { + name: "MB", + size: humanize.MByte, + }, + { + name: "KiB", + size: humanize.KiByte, + }, + { + name: "KB", + size: humanize.KByte, + }, } // ParseByteSize parses a size such as "32KiB", "1 MB" or "4096". Unit names // are case-insensitive. "-1" (or "-1B") is ByteSizeNoLimit; any other // negative size is an error. -func ParseByteSize(s string) (ByteSize, error) { - trimmed := strings.TrimSpace(s) +func ParseByteSize(text string) (ByteSize, error) { + trimmed := strings.TrimSpace(text) if trimmed == "-1" || strings.EqualFold(trimmed, "-1B") { return ByteSizeNoLimit, nil } if strings.HasPrefix(trimmed, "-") { - return 0, fmt.Errorf("byte size %q is negative; only -1 (no limit) is allowed", s) + return 0, fmt.Errorf("byte size %q is negative; only -1 (no limit) is allowed", text) } - n, err := humanize.ParseBytes(s) + parsed, err := humanize.ParseBytes(text) if err != nil { return 0, err } - if n > math.MaxInt64 { - return 0, fmt.Errorf("byte size %q is too large", s) + if parsed > math.MaxInt64 { + return 0, fmt.Errorf("byte size %q is too large", text) } - return ByteSize(n), nil + return ByteSize(parsed), nil } // String formats b exactly, in the largest unit that divides it (e.g. "32KiB", @@ -61,9 +103,9 @@ func (b ByteSize) String() string { return "-1" } if b != 0 { - for _, u := range byteUnits { - if int64(b)%u.size == 0 { - return strconv.FormatInt(int64(b)/u.size, 10) + u.name + for _, unit := range byteUnits { + if int64(b)%unit.size == 0 { + return strconv.FormatInt(int64(b)/unit.size, 10) + unit.name } } } diff --git a/internal/util/worker.go b/internal/util/worker.go index 5e628d1..b7029d9 100644 --- a/internal/util/worker.go +++ b/internal/util/worker.go @@ -135,26 +135,29 @@ func (w Worker) execute( } } -// AwaitTask runs fn in its own goroutine and returns its result, or ctx.Err() +// AwaitTask runs task in its own goroutine and returns its result, or ctx.Err() // if ctx is cancelled first. This lets a Task stop waiting on a call that does // not respond promptly to cancellation. // // The result channel is buffered, so if ctx wins the race the goroutine can // still deliver its result and exit instead of blocking forever. -func AwaitTask[T any](ctx context.Context, fn func() (T, error)) (T, error) { +func AwaitTask[T any](ctx context.Context, task func() (T, error)) (T, error) { type result struct { val T err error } - ch := make(chan result, 1) + results := make(chan result, 1) go func() { - val, err := fn() - ch <- result{val, err} + value, err := task() + results <- result{ + val: value, + err: err, + } }() select { - case r := <-ch: - return r.val, r.err + case result := <-results: + return result.val, result.err case <-ctx.Done(): var zero T return zero, ctx.Err() @@ -162,15 +165,15 @@ func AwaitTask[T any](ctx context.Context, fn func() (T, error)) (T, error) { } // AwaitTaskE is AwaitTask for a function that returns only an error. -func AwaitTaskE(ctx context.Context, fn func() error) error { - ch := make(chan error, 1) +func AwaitTaskE(ctx context.Context, task func() error) error { + results := make(chan error, 1) go func() { - err := fn() - ch <- err + err := task() + results <- err }() select { - case err := <-ch: + case err := <-results: return err case <-ctx.Done(): return ctx.Err() diff --git a/internal/util/worker_test.go b/internal/util/worker_test.go index e194208..7d8b81d 100644 --- a/internal/util/worker_test.go +++ b/internal/util/worker_test.go @@ -19,14 +19,26 @@ func TestMain(m *testing.M) { } func TestWorkerRunRejectsBadConfig(t *testing.T) { - noop := func(context.Context, time.Time) error { return nil } + noop := func(context.Context, time.Time) error { + return nil + } tests := []struct { name string w Worker }{ - {"no task", Worker{Name: "a", Interval: time.Second}}, - {"zero interval", Worker{Name: "b", Task: noop}}, - {"negative interval", Worker{Name: "c", Interval: -time.Second, Task: noop}}, + {"no task", Worker{ + Name: "a", + Interval: time.Second, + }}, + {"zero interval", Worker{ + Name: "b", + Task: noop, + }}, + {"negative interval", Worker{ + Name: "c", + Interval: -time.Second, + Task: noop, + }}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -90,7 +102,9 @@ func TestWorkerRunCheckpoints(t *testing.T) { } done := make(chan error, 1) - go func() { done <- w.Run(ctx, since0) }() + go func() { + done <- w.Run(ctx, since0) + }() select { case err := <-done: if err != nil { @@ -186,7 +200,11 @@ func TestWorkerExecute(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - w := Worker{Task: func(context.Context, time.Time) error { return tc.err }} + w := Worker{ + Task: func(context.Context, time.Time) error { + return tc.err + }, + } before := time.Now() got := w.execute(context.Background(), logger, since) if tc.advance { @@ -202,11 +220,15 @@ func TestWorkerExecute(t *testing.T) { func TestAwaitTask(t *testing.T) { errFail := errors.New("boom") - v, err := AwaitTask(context.Background(), func() (int, error) { return 42, nil }) + v, err := AwaitTask(context.Background(), func() (int, error) { + return 42, nil + }) if v != 42 || err != nil { t.Fatalf("got (%d, %v), want (42, nil)", v, err) } - v, err = AwaitTask(context.Background(), func() (int, error) { return 7, errFail }) + v, err = AwaitTask(context.Background(), func() (int, error) { + return 7, errFail + }) if v != 7 || !errors.Is(err, errFail) { t.Fatalf("got (%d, %v), want (7, %v)", v, err, errFail) } @@ -214,8 +236,14 @@ func TestAwaitTask(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) release := make(chan struct{}) defer close(release) - go func() { time.Sleep(5 * time.Millisecond); cancel() }() - v, err = AwaitTask(ctx, func() (int, error) { <-release; return 1, nil }) + go func() { + time.Sleep(5 * time.Millisecond) + cancel() + }() + v, err = AwaitTask(ctx, func() (int, error) { + <-release + return 1, nil + }) if v != 0 || !errors.Is(err, context.Canceled) { t.Fatalf("got (%d, %v), want (0, context.Canceled)", v, err) } @@ -223,12 +251,16 @@ func TestAwaitTask(t *testing.T) { func TestAwaitTaskE(t *testing.T) { errFail := errors.New("boom") - if err := AwaitTaskE(context.Background(), func() error { return nil }); err != nil { + if err := AwaitTaskE(context.Background(), func() error { + return nil + }); err != nil { t.Fatal(err) } if err := AwaitTaskE( context.Background(), - func() error { return errFail }, + func() error { + return errFail + }, ); !errors.Is( err, errFail, @@ -242,7 +274,10 @@ func TestAwaitTaskE(t *testing.T) { defer close(release) if err := AwaitTaskE( ctx, - func() error { <-release; return nil }, + func() error { + <-release + return nil + }, ); !errors.Is( err, context.DeadlineExceeded, diff --git a/internal/version/version.go b/internal/version/version.go index c0180e9..82f743c 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,8 +1,11 @@ package version -// Version information (set at build time with -ldflags) +// Version information (set at build time with -ldflags). var ( - Version = "dev" + // Version is the release version, or "dev" for a local build. + Version = "dev" + // BuildTime is when the binary was built. BuildTime = "unknown" + // GitCommit is the commit the binary was built from. GitCommit = "unknown" ) diff --git a/internal/websocket/client.go b/internal/websocket/client.go index 4052dda..f9a475e 100644 --- a/internal/websocket/client.go +++ b/internal/websocket/client.go @@ -55,18 +55,18 @@ var errUnsupportedData = errors.New("binary frames are not supported") // runs with; the client calls it when it finishes or is closed. The client's // messages are limited by websocket.message_rate_limit if it is enabled. func newClient( - h *Hub, conn msgConn, ip string, cancel context.CancelFunc, + hub *Hub, conn msgConn, ip string, cancel context.CancelFunc, ) *client { limiter := rate.NewLimiter(rate.Inf, 0) - if rule := h.conf.MessageRateLimit; rule.Enabled && rule.PerSecond > 0 { + if rule := hub.conf.MessageRateLimit; rule.Enabled && rule.PerSecond > 0 { limiter = rate.NewLimiter( rate.Limit(rule.PerSecond), max(rule.Burst, 1), ) } - c := &client{ - hub: h, + client := &client{ + hub: hub, conn: conn, id: rand.Text()[:12], ip: ip, @@ -74,9 +74,9 @@ func newClient( cancel: cancel, limiter: limiter, } - c.lastSeen.Store(time.Now().UnixNano()) + client.lastSeen.Store(time.Now().UnixNano()) - return c + return client } // run starts the writer and keepalive goroutines and reads until the @@ -84,8 +84,14 @@ func newClient( func (c *client) run(ctx context.Context) { var wg sync.WaitGroup wg.Add(2) - go func() { defer wg.Done(); c.writeLoop(ctx) }() - go func() { defer wg.Done(); c.pingLoop(ctx) }() + go func() { + defer wg.Done() + c.writeLoop(ctx) + }() + go func() { + defer wg.Done() + c.pingLoop(ctx) + }() c.sendEnvelope(Envelope{ Type: TypeWelcome, @@ -134,35 +140,35 @@ func (c *client) readLoop(ctx context.Context) error { continue } // Unpack the envelope and pass to handle to process if valid - var env Envelope - if err := json.Unmarshal(data, &env); err != nil { + var envelope Envelope + if err := json.Unmarshal(data, &envelope); err != nil { c.sendError("", "invalid JSON envelope") continue } - c.handle(env) + c.handle(envelope) } } // handle answers one envelope from the client: ping and echo, or an error // for any other type. -func (c *client) handle(env Envelope) { - switch env.Type { +func (c *client) handle(envelope Envelope) { + switch envelope.Type { case TypePing: c.sendEnvelope(Envelope{ Type: TypePong, - ID: env.ID, + ID: envelope.ID, Payload: mustRaw(map[string]any{"time": time.Now().UTC()}), }) case TypeEcho: c.sendEnvelope(Envelope{ Type: TypeEcho, - ID: env.ID, - Payload: env.Payload, + ID: envelope.ID, + Payload: envelope.Payload, }) default: - c.sendError(env.ID, "unknown message type") + c.sendError(envelope.ID, "unknown message type") } } @@ -235,10 +241,10 @@ func (c *client) trySend(msg []byte) bool { } } -// sendEnvelope queues env for the client, closing the client as a slow +// sendEnvelope queues envelope for the client, closing the client as a slow // consumer if its queue is full. -func (c *client) sendEnvelope(env Envelope) { - encoded, err := json.Marshal(env) +func (c *client) sendEnvelope(envelope Envelope) { + encoded, err := json.Marshal(envelope) if err != nil { c.hub.logger().Error("marshal", "client", c.id, "error", err) return @@ -298,7 +304,8 @@ func withTimeout( ctx context.Context, timeout time.Duration, ) (context.Context, context.CancelFunc) { if timeout <= 0 { - return ctx, func() {} + return ctx, func() { + } } return context.WithTimeout(ctx, timeout) } diff --git a/internal/websocket/client_test.go b/internal/websocket/client_test.go index 6d6d6f2..0312708 100644 --- a/internal/websocket/client_test.go +++ b/internal/websocket/client_test.go @@ -23,7 +23,11 @@ type fakeConn struct { closed chan ws.StatusCode } -func newFakeConn() *fakeConn { return &fakeConn{closed: make(chan ws.StatusCode, 1)} } +func newFakeConn() *fakeConn { + return &fakeConn{ + closed: make(chan ws.StatusCode, 1), + } +} func (f *fakeConn) Read(ctx context.Context) (ws.MessageType, []byte, error) { <-ctx.Done() @@ -52,7 +56,10 @@ func newTestClient(f *fakeConn) (*client, context.Context) { // An envelope that can't be marshalled is dropped, not queued. func TestSendEnvelopeBadPayload(t *testing.T) { c, _ := newTestClient(newFakeConn()) - c.sendEnvelope(Envelope{Type: TypeEcho, Payload: json.RawMessage("{")}) + c.sendEnvelope(Envelope{ + Type: TypeEcho, + Payload: json.RawMessage("{"), + }) if len(c.send) != 0 { t.Errorf("queued %d messages; want none", len(c.send)) } @@ -63,7 +70,9 @@ func TestSendEnvelopeSlowConsumer(t *testing.T) { f := newFakeConn() c, ctx := newTestClient(f) for range clientSendBuffer { - c.sendEnvelope(Envelope{Type: TypePong}) + c.sendEnvelope(Envelope{ + Type: TypePong, + }) } select { case code := <-f.closed: @@ -71,7 +80,9 @@ func TestSendEnvelopeSlowConsumer(t *testing.T) { default: } - c.sendEnvelope(Envelope{Type: TypePong}) + c.sendEnvelope(Envelope{ + Type: TypePong, + }) select { case code := <-f.closed: if code != ws.StatusPolicyViolation { @@ -109,13 +120,31 @@ func TestClientMessageRateLimit(t *testing.T) { rule config.JSCRateLimitRule limited bool }{ - {config.JSCRateLimitRule{Enabled: false, PerSecond: 1, Burst: 1}, false}, - {config.JSCRateLimitRule{Enabled: true, PerSecond: 0, Burst: 1}, false}, - {config.JSCRateLimitRule{Enabled: true, PerSecond: 1, Burst: 1}, true}, + {config.JSCRateLimitRule{ + Enabled: false, + PerSecond: 1, + Burst: 1, + }, false}, + {config.JSCRateLimitRule{ + Enabled: true, + PerSecond: 0, + Burst: 1, + }, false}, + {config.JSCRateLimitRule{ + Enabled: true, + PerSecond: 1, + Burst: 1, + }, true}, } for _, tt := range tests { - h := NewHub(config.JSCServerWebSocket{MessageRateLimit: tt.rule}, WithLogger(discardLog)) - c := newClient(h, newFakeConn(), "", func() {}) + h := NewHub( + config.JSCServerWebSocket{ + MessageRateLimit: tt.rule, + }, + WithLogger(discardLog), + ) + c := newClient(h, newFakeConn(), "", func() { + }) c.limiter.Allow() if limited := !c.limiter.Allow(); limited != tt.limited { t.Errorf("rule %+v: second message limited = %v; want %v", tt.rule, limited, tt.limited) @@ -133,7 +162,8 @@ func TestMustRaw(t *testing.T) { {"nil", nil, ""}, {"raw message as is", raw, `{"a":1}`}, {"marshalled", map[string]int{"b": 2}, `{"b":2}`}, - {"unmarshallable", func() {}, ""}, + {"unmarshallable", func() { + }, ""}, } for _, tt := range tests { if got := string(mustRaw(tt.v)); got != tt.want { @@ -187,13 +217,20 @@ func (b *syncBuffer) entries(t *testing.T) []map[string]any { // buffer. func jsonLog(level slog.Level) (*slog.Logger, *syncBuffer) { buf := &syncBuffer{} - return slog.New(slog.NewJSONHandler(buf, &slog.HandlerOptions{Level: level})), buf + return slog.New(slog.NewJSONHandler(buf, &slog.HandlerOptions{ + Level: level, + })), buf } // At debug level, messages received from and sent to a client are logged. func TestLogMessages(t *testing.T) { log, buf := jsonLog(slog.LevelDebug) - h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(log)) + h := NewHub( + config.JSCServerWebSocket{ + WriteTimeout: time.Second, + }, + WithLogger(log), + ) peer, _ := pipeClient(t, h) ping := `{"type":"ping","id":"1"}` @@ -236,7 +273,8 @@ func TestLogMessages(t *testing.T) { func TestLogMessageLimit(t *testing.T) { log, buf := jsonLog(slog.LevelDebug) h := NewHub(config.JSCServerWebSocket{}, WithLogger(log)) - c := newClient(h, newFakeConn(), "", func() {}) + c := newClient(h, newFakeConn(), "", func() { + }) long := strings.Repeat("x", logMessageLimit+100) c.logMessage(context.Background(), "message sent", []byte(long)) diff --git a/internal/websocket/http.go b/internal/websocket/http.go index 6344fe4..2c7d008 100644 --- a/internal/websocket/http.go +++ b/internal/websocket/http.go @@ -30,16 +30,16 @@ func (h *Hub) ServeHTTP(w http.ResponseWriter, r *http.Request) { ip := middleware.ClientIP(r) ctx, cancel := context.WithCancel(r.Context()) - c := newClient(h, conn, ip, cancel) + client := newClient(h, conn, ip, cancel) - if !h.register(c) { + if !h.register(client) { cancel() conn.Close(ws.StatusGoingAway, "server shutting down") //nolint:errcheck,gosec // turned away return } - defer h.unregister(c) + defer h.unregister(client) - h.logger().Info("client connected", "client", c.id, "ip", ip) - c.run(ctx) - h.logger().Info("client disconnected", "client", c.id, "ip", ip) + h.logger().Info("client connected", "client", client.id, "ip", ip) + client.run(ctx) + h.logger().Info("client disconnected", "client", client.id, "ip", ip) } diff --git a/internal/websocket/http_test.go b/internal/websocket/http_test.go index 53bdecd..14a7f84 100644 --- a/internal/websocket/http_test.go +++ b/internal/websocket/http_test.go @@ -35,7 +35,9 @@ func dialWS(t *testing.T, ctx context.Context, url string) *ws.Conn { if err != nil { t.Fatalf("dial: %v", err) } - t.Cleanup(func() { conn.CloseNow() }) + t.Cleanup(func() { + conn.CloseNow() + }) var e Envelope if err := wsjson.Read(ctx, conn, &e); err != nil || e.Type != TypeWelcome { t.Fatalf("want welcome, got %+v, %v", e, err) @@ -46,13 +48,21 @@ func dialWS(t *testing.T, ctx context.Context, url string) *ws.Conn { // A client upgraded over HTTP gets replies. A max_message_size of 0 uses the // default limit rather than refusing every message. func TestHTTPMessages(t *testing.T) { - h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(discardLog)) + h := NewHub( + config.JSCServerWebSocket{ + WriteTimeout: time.Second, + }, + WithLogger(discardLog), + ) url := wsHub(t, h) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() conn := dialWS(t, ctx, url) - if err := wsjson.Write(ctx, conn, Envelope{Type: TypePing, ID: "1"}); err != nil { + if err := wsjson.Write(ctx, conn, Envelope{ + Type: TypePing, + ID: "1", + }); err != nil { t.Fatal(err) } var e Envelope @@ -89,7 +99,8 @@ func TestPushBadPayload(t *testing.T) { c.read() // welcome waitCount(t, h, 1) - h.Push("bad", func() {}) + h.Push("bad", func() { + }) h.Push("good", nil) if e := c.read(); e.Type != "good" || e.Payload != nil { t.Fatalf("want good with no payload, got %+v", e) @@ -98,7 +109,12 @@ func TestPushBadPayload(t *testing.T) { // A binary frame closes the connection with 1003 (unsupported data). func TestHTTPBinaryFrame(t *testing.T) { - h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(discardLog)) + h := NewHub( + config.JSCServerWebSocket{ + WriteTimeout: time.Second, + }, + WithLogger(discardLog), + ) url := wsHub(t, h) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() @@ -117,8 +133,12 @@ func TestHTTPBinaryFrame(t *testing.T) { // handled; the connection stays open. func TestHTTPMessageRateLimit(t *testing.T) { conf := config.JSCServerWebSocket{ - WriteTimeout: time.Second, - MessageRateLimit: config.JSCRateLimitRule{Enabled: true, PerSecond: 1, Burst: 1}, + WriteTimeout: time.Second, + MessageRateLimit: config.JSCRateLimitRule{ + Enabled: true, + PerSecond: 1, + Burst: 1, + }, } h := NewHub(conf, WithLogger(discardLog)) url := wsHub(t, h) @@ -127,7 +147,10 @@ func TestHTTPMessageRateLimit(t *testing.T) { conn := dialWS(t, ctx, url) for _, id := range []string{"1", "2"} { - if err := wsjson.Write(ctx, conn, Envelope{Type: TypePing, ID: id}); err != nil { + if err := wsjson.Write(ctx, conn, Envelope{ + Type: TypePing, + ID: id, + }); err != nil { t.Fatal(err) } } diff --git a/internal/websocket/hub.go b/internal/websocket/hub.go index bf74e53..12935a9 100644 --- a/internal/websocket/hub.go +++ b/internal/websocket/hub.go @@ -17,7 +17,7 @@ type Hub struct { conf config.JSCServerWebSocket log *slog.Logger - m sync.RWMutex + mu sync.RWMutex clients map[*client]struct{} closed bool wg sync.WaitGroup @@ -82,13 +82,13 @@ func (h *Hub) Push(msgType string, payload any) { } var slow []*client - h.m.RLock() + h.mu.RLock() for client := range h.clients { if !client.trySend(msg) { slow = append(slow, client) } } - h.m.RUnlock() + h.mu.RUnlock() for _, client := range slow { h.logger().Warn("dropping slow client", "client", client.id, "ip", client.ip) @@ -98,8 +98,8 @@ func (h *Hub) Push(msgType string, payload any) { // Count returns the number of connected clients. func (h *Hub) Count() int { - h.m.RLock() - defer h.m.RUnlock() + h.mu.RLock() + defer h.mu.RUnlock() return len(h.clients) } @@ -108,13 +108,13 @@ func (h *Hub) Count() int { // closed at once and ctx's error is returned. New connections are refused // afterwards. func (h *Hub) Shutdown(ctx context.Context) error { - h.m.Lock() + h.mu.Lock() h.closed = true clientShutdowns := make([]*client, 0, len(h.clients)) for client := range h.clients { clientShutdowns = append(clientShutdowns, client) } - h.m.Unlock() + h.mu.Unlock() if len(clientShutdowns) == 0 { // Nothing to close. Clients that just left are only finishing their @@ -151,7 +151,9 @@ func (h *Hub) Shutdown(ctx context.Context) error { } // maxMessageSize is the largest message read from a client: max_message_size, -// or defaultMaxMessageSize if that is 0 or less. +// or defaultMaxMessageSize if that is 0 or less. The configuration rejects -1 +// (no limit) for this setting, as a client could then exhaust the server's +// memory; a -1 set in code also gets the default, never no limit. func (h *Hub) maxMessageSize() int { if h.conf.MaxMessageSize <= 0 { return defaultMaxMessageSize @@ -161,21 +163,21 @@ func (h *Hub) maxMessageSize() int { // register adds a client to the hub. Hub clients receive all push messages and // have their lifecycles tracked by the hub. -func (h *Hub) register(c *client) bool { - h.m.Lock() - defer h.m.Unlock() +func (h *Hub) register(client *client) bool { + h.mu.Lock() + defer h.mu.Unlock() if h.closed { return false } - h.clients[c] = struct{}{} + h.clients[client] = struct{}{} h.wg.Add(1) return true } // unregister removes a client from the hub. -func (h *Hub) unregister(c *client) { - h.m.Lock() - delete(h.clients, c) - h.m.Unlock() +func (h *Hub) unregister(client *client) { + h.mu.Lock() + delete(h.clients, client) + h.mu.Unlock() h.wg.Done() } diff --git a/internal/websocket/hub_test.go b/internal/websocket/hub_test.go index 07a0851..6366853 100644 --- a/internal/websocket/hub_test.go +++ b/internal/websocket/hub_test.go @@ -17,9 +17,15 @@ import ( func pipeClient(t *testing.T, h *Hub) (net.Conn, chan struct{}) { t.Helper() server, peer := net.Pipe() - t.Cleanup(func() { server.Close(); peer.Close() }) + t.Cleanup(func() { + server.Close() + peer.Close() + }) done := make(chan struct{}) - go func() { h.ServeTCP(context.Background(), server); close(done) }() + go func() { + h.ServeTCP(context.Background(), server) + close(done) + }() peer.SetReadDeadline(time.Now().Add(time.Second)) if _, err := bufio.NewReader(peer).ReadString('\n'); err != nil { t.Fatalf("welcome: %v", err) @@ -61,10 +67,13 @@ func TestShutdownNoClients(t *testing.T) { // A ping that can't be sent within the pong timeout drops the client. func TestPingFailureDropsClient(t *testing.T) { - h := NewHub(config.JSCServerWebSocket{ - PingInterval: 10 * time.Millisecond, - PongTimeout: 20 * time.Millisecond, - }, WithLogger(discardLog)) + h := NewHub( + config.JSCServerWebSocket{ + PingInterval: 10 * time.Millisecond, + PongTimeout: 20 * time.Millisecond, + }, + WithLogger(discardLog), + ) _, done := pipeClient(t, h) select { @@ -81,8 +90,12 @@ func TestMaxMessageSize(t *testing.T) { want int }{ {config.JSCServerWebSocket{}, defaultMaxMessageSize}, - {config.JSCServerWebSocket{MaxMessageSize: -1}, defaultMaxMessageSize}, - {config.JSCServerWebSocket{MaxMessageSize: 100}, 100}, + {config.JSCServerWebSocket{ + MaxMessageSize: -1, + }, defaultMaxMessageSize}, + {config.JSCServerWebSocket{ + MaxMessageSize: 100, + }, 100}, } for _, tt := range tests { h := NewHub(tt.conf, WithLogger(discardLog)) diff --git a/internal/websocket/messages.go b/internal/websocket/messages.go index 70da3ca..d804a26 100644 --- a/internal/websocket/messages.go +++ b/internal/websocket/messages.go @@ -37,28 +37,36 @@ type Envelope struct { // Envelope types. TypeEvent, TypeRequest and TypeAck are reserved: nothing // sends or handles them yet. const ( + // TypeWelcome is sent to a client when it connects, with its client ID. TypeWelcome = "welcome" - TypePing = "ping" - TypePong = "pong" - TypeEcho = "echo" - TypeEvent = "event" + // TypePing asks the server for a TypePong. + TypePing = "ping" + // TypePong answers a TypePing, with the server's time. + TypePong = "pong" + // TypeEcho is sent back to the client with the same ID and payload. + TypeEcho = "echo" + // TypeEvent is reserved for events pushed to clients. + TypeEvent = "event" + // TypeRequest is reserved for requests from clients. TypeRequest = "request" - TypeAck = "ack" - TypeError = "error" + // TypeAck is reserved for acknowledging a request. + TypeAck = "ack" + // TypeError reports an error, with the ID of the message it answers. + TypeError = "error" ) -// mustRaw returns v as raw JSON: v itself if it is already json.RawMessage, -// nil if v is nil or can't be marshalled. -func mustRaw(v any) json.RawMessage { - if v == nil { +// mustRaw returns value as raw JSON: value itself if it is already +// json.RawMessage, nil if value is nil or can't be marshalled. +func mustRaw(value any) json.RawMessage { + if value == nil { return nil } - if raw, ok := v.(json.RawMessage); ok { + if raw, ok := value.(json.RawMessage); ok { return raw } - b, err := json.Marshal(v) + encoded, err := json.Marshal(value) if err != nil { return nil } - return b + return encoded } diff --git a/internal/websocket/tcp.go b/internal/websocket/tcp.go index ab5604d..47d9d9d 100644 --- a/internal/websocket/tcp.go +++ b/internal/websocket/tcp.go @@ -178,7 +178,10 @@ func (t *tcpConn) Ping(ctx context.Context) error { // connection. Only the first Close or CloseNow does anything; later calls // return nil. A CloseNow while Close is sending its message cuts it short. func (t *tcpConn) Close(code ws.StatusCode, reason string) error { - if !t.markClosed(ws.CloseError{Code: code, Reason: reason}) { + if !t.markClosed(ws.CloseError{ + Code: code, + Reason: reason, + }) { return nil } @@ -206,7 +209,9 @@ func (t *tcpConn) Close(code ws.StatusCode, reason string) error { // CloseNow closes the connection without a message. It doesn't wait for a // Close that is sending its message. func (t *tcpConn) CloseNow() error { - t.markClosed(ws.CloseError{Code: ws.StatusAbnormalClosure}) + t.markClosed(ws.CloseError{ + Code: ws.StatusAbnormalClosure, + }) if err := t.conn.Close(); err != nil && !errors.Is(err, net.ErrClosed) { return err } diff --git a/internal/websocket/tcp_test.go b/internal/websocket/tcp_test.go index e94790c..46ae1d5 100644 --- a/internal/websocket/tcp_test.go +++ b/internal/websocket/tcp_test.go @@ -69,8 +69,14 @@ func dialTCP(t *testing.T, addr string) *tcpClient { if err != nil { t.Fatal(err) } - t.Cleanup(func() { conn.Close() }) - return &tcpClient{t: t, conn: conn, r: bufio.NewReader(conn)} + t.Cleanup(func() { + conn.Close() + }) + return &tcpClient{ + t: t, + conn: conn, + r: bufio.NewReader(conn), + } } func (c *tcpClient) send(line string) { @@ -155,7 +161,9 @@ func TestTCPMessages(t *testing.T) { // With no write timeout, messages are still sent: a zero timeout must not // mean one that has already expired. func TestTCPNoWriteTimeout(t *testing.T) { - _, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { c.WriteTimeout = 0 }) + _, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { + c.WriteTimeout = 0 + }) c := dialTCP(t, addr) if e := c.read(); e.Type != TypeWelcome { t.Fatalf("want welcome, got %+v", e) @@ -177,7 +185,9 @@ func TestTCPClientDisconnect(t *testing.T) { // A line longer than max_message_size drops the connection. func TestTCPLineTooLong(t *testing.T) { - h, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { c.MaxMessageSize = 64 }) + h, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { + c.MaxMessageSize = 64 + }) c := dialTCP(t, addr) c.read() // welcome c.send(`{"type":"echo","payload":"` + strings.Repeat("x", 100) + `"}` + "\n") @@ -244,12 +254,21 @@ func TestTCPPing(t *testing.T) { // full. net.Pipe has no buffer, so writes to it block until the peer reads. func TestTCPSlowConsumer(t *testing.T) { h := NewHub( - config.JSCServerWebSocket{WriteTimeout: 50 * time.Millisecond}, WithLogger(discardLog), + config.JSCServerWebSocket{ + WriteTimeout: 50 * time.Millisecond, + }, + WithLogger(discardLog), ) server, peer := net.Pipe() - t.Cleanup(func() { server.Close(); peer.Close() }) + t.Cleanup(func() { + server.Close() + peer.Close() + }) done := make(chan struct{}) - go func() { h.ServeTCP(context.Background(), server); close(done) }() + go func() { + h.ServeTCP(context.Background(), server) + close(done) + }() waitCount(t, h, 1) for range clientSendBuffer + 1 { @@ -283,7 +302,9 @@ func TestTCPNoPongTimeout(t *testing.T) { // A max_message_size of 0 uses the default limit. func TestTCPDefaultMessageSize(t *testing.T) { - _, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { c.MaxMessageSize = 0 }) + _, addr := tcpHub(t, func(c *config.JSCServerWebSocket) { + c.MaxMessageSize = 0 + }) c := dialTCP(t, addr) c.read() // welcome c.send(`{"type":"echo","id":"1","payload":"` + strings.Repeat("x", 4096) + `"}` + "\n") @@ -299,7 +320,10 @@ func TestTCPDefaultMessageSize(t *testing.T) { func pipeConn(t *testing.T, closeTimeout time.Duration) (*tcpConn, net.Conn) { t.Helper() server, peer := net.Pipe() - t.Cleanup(func() { server.Close(); peer.Close() }) + t.Cleanup(func() { + server.Close() + peer.Close() + }) return newTCPConn(server, 0, closeTimeout), peer } @@ -370,13 +394,17 @@ func TestTCPConnClose(t *testing.T) { func TestTCPConnCloseNowDuringClose(t *testing.T) { tc, _ := pipeConn(t, 0) // no timeout: Close would wait for ever closed := make(chan error, 1) - go func() { closed <- tc.Close(ws.StatusGoingAway, "bye") }() + go func() { + closed <- tc.Close(ws.StatusGoingAway, "bye") + }() for !tc.isClosed() { time.Sleep(time.Millisecond) } done := make(chan error, 1) - go func() { done <- tc.CloseNow() }() + go func() { + done <- tc.CloseNow() + }() select { case err := <-done: if err != nil { @@ -396,7 +424,9 @@ func TestTCPConnCloseNowDuringClose(t *testing.T) { func TestTCPConnCloseTimeout(t *testing.T) { tc, _ := pipeConn(t, 20*time.Millisecond) done := make(chan error, 1) - go func() { done <- tc.Close(ws.StatusGoingAway, "bye") }() + go func() { + done <- tc.Close(ws.StatusGoingAway, "bye") + }() select { case err := <-done: if err != nil { diff --git a/internal/websocket/tcpserver/server_test.go b/internal/websocket/tcpserver/server_test.go index 570af84..0c9d076 100644 --- a/internal/websocket/tcpserver/server_test.go +++ b/internal/websocket/tcpserver/server_test.go @@ -23,10 +23,14 @@ var discardLog = slog.New(slog.NewTextHandler(io.Discard, nil)) // not nil. func testConf(mutate func(*config.JSCServer)) *config.JSCServer { c := &config.JSCServer{ - Host: "127.0.0.1", - Port: 0, - WebSocket: config.JSCServerWebSocket{WriteTimeout: time.Second}, - Timeouts: config.JSCServerTimeouts{ShutdownTimeout: time.Second}, + Host: "127.0.0.1", + Port: 0, + WebSocket: config.JSCServerWebSocket{ + WriteTimeout: time.Second, + }, + Timeouts: config.JSCServerTimeouts{ + ShutdownTimeout: time.Second, + }, } if mutate != nil { mutate(c) @@ -54,7 +58,9 @@ func startServer( ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) done := make(chan error, 1) - go func() { done <- s.serve(ctx, ln) }() + go func() { + done <- s.serve(ctx, ln) + }() return h, ln.Addr().String(), cancel, done } @@ -71,8 +77,14 @@ func dial(t *testing.T, addr string) *tcpClient { if err != nil { t.Fatal(err) } - t.Cleanup(func() { conn.Close() }) - return &tcpClient{t: t, conn: conn, r: bufio.NewReader(conn)} + t.Cleanup(func() { + conn.Close() + }) + return &tcpClient{ + t: t, + conn: conn, + r: bufio.NewReader(conn), + } } // read returns the next envelope, failing the test after a second. @@ -188,7 +200,9 @@ func TestServerRun(t *testing.T) { } func TestServerRunListenError(t *testing.T) { - s, _ := newServer(testConf(func(c *config.JSCServer) { c.Port = -1 })) + s, _ := newServer(testConf(func(c *config.JSCServer) { + c.Port = -1 + })) if err := s.Run(context.Background()); err == nil || !strings.Contains(err.Error(), "listen") { t.Fatalf("Run = %v; want a listen error", err) } @@ -217,7 +231,10 @@ func TestServerAcceptRetries(t *testing.T) { t.Fatal(err) } defer ln.Close() - fl := &flakyListener{Listener: ln, fails: 3} + fl := &flakyListener{ + Listener: ln, + fails: 3, + } err = s.accept(context.Background(), context.Background(), fl) if !errors.Is(err, net.ErrClosed) || fl.fails != 0 { t.Fatalf("accept = %v with %d failures left; want net.ErrClosed", err, fl.fails) @@ -233,7 +250,10 @@ func TestServerAcceptCancelledInBackOff(t *testing.T) { } defer ln.Close() ctx, cancel := context.WithCancel(context.Background()) - fl := &flakyListener{Listener: ln, fails: 1 << 30} + fl := &flakyListener{ + Listener: ln, + fails: 1 << 30, + } time.AfterFunc(20*time.Millisecond, cancel) if err := s.accept(ctx, ctx, fl); !errors.Is(err, context.Canceled) { t.Fatalf("accept = %v; want context.Canceled", err)