diff --git a/.golangci.json b/.golangci.json index 0472b6d..6fcef07 100644 --- a/.golangci.json +++ b/.golangci.json @@ -32,6 +32,13 @@ "errcheck", "cyclop" ] + }, + { + "path": "_test\\.go", + "linters": [ + "gosec" + ], + "text": "G104" } ] } diff --git a/README.md b/README.md index c36089d..9d56276 100644 --- a/README.md +++ b/README.md @@ -121,9 +121,11 @@ Using a source called `example` as an illustration: - Define `JobSearchConfigSourceExample`, embedding `JobSearchConfigSource` (for `enabled` and `interval`), plus any fields of its own with `cfg` and optional `default` tags, e.g. ``APIRoot string `cfg:"api_root"` ``. - - Copy the standard `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue` - methods, renaming the receiver type and using the new constant in - `Flatten`. + - Implement the `Section` interface (see `internal/config/section.go`): + copy the `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue` methods + from an existing source, under a + `// ---------- Section methods (see Section) ----------` banner, renaming + the receiver type and using the new constant in `Flatten`. 2. **Register the section:** add ``Example JobSearchConfigSourceExample `cfg:"example"` `` to `JobSearchConfigSources` in `internal/config/conf_sources.go`. The `cfg` tag @@ -148,11 +150,12 @@ Using a source called `example` as an illustration: start("Example", runExample) } ``` -6. **Tests:** add tests for the new config section to - `internal/config/conf_sections_test.go` (add it to +6. **Tests:** `TestSections` checks the new section implements `Section` + and that its keys match its `cfg` tag, with no changes needed. In + `internal/config/conf_sections_test.go`, add the section to `TestConcreteSourceSections` and update the key count in - `TestSourcesSection`), and for the client and worker, using `httptest` - servers rather than the real service. + `TestSourcesSection`. Test the client and worker using `httptest` servers + rather than the real service. 7. **Docs:** add the source and its keys to the key table in `internal/config/README.md`. diff --git a/cmd/mock-fts/main.go b/cmd/mock-fts/main.go index fefdd0e..2a122fa 100644 --- a/cmd/mock-fts/main.go +++ b/cmd/mock-fts/main.go @@ -191,7 +191,7 @@ 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) error { +func run(ctx context.Context, o *options) (err error) { loc, err := time.LoadLocation(o.tz) // already validated if err != nil { return fmt.Errorf("--tz: %w", err) @@ -228,7 +228,7 @@ func run(ctx context.Context, o *options) error { if err != nil { return err } - defer store.Close() + defer func() { err = errors.Join(err, store.Close()) }() // ----- generator ----- gen := mock.NewGenerator(cat, store, time.Now().UnixNano()) @@ -261,7 +261,7 @@ func run(ctx context.Context, o *options) error { <-workerDone return fmt.Errorf("opening access log: %w", err) } - defer f.Close() + defer func() { err = errors.Join(err, f.Close()) }() out = f } accessLogger := log.New(out, "", 0) @@ -279,11 +279,14 @@ func run(ctx context.Context, o *options) error { } go func() { <-ctx.Done() + // ctx is done by now; the shutdown gets its own deadline. shutdownCtx, cancelShutdown := context.WithTimeout( - context.Background(), 5*time.Second, + context.WithoutCancel(ctx), 5*time.Second, ) defer cancelShutdown() - httpSrv.Shutdown(shutdownCtx) + if err := httpSrv.Shutdown(shutdownCtx); err != nil { + log.Printf("shutdown: %v", err) + } }() log.Printf("FTS mock listening on %s%s (query times in %s)", o.addr, mock.BasePath, loc) diff --git a/internal/cli/config/command.go b/internal/cli/config/command.go index 785a205..bc2dda2 100644 --- a/internal/cli/config/command.go +++ b/internal/cli/config/command.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" ) +// NewCommand returns the "config" command, which shows and edits the configuration. func NewCommand(opts *cli.Options) *cobra.Command { cmd := &cobra.Command{ Use: "config", diff --git a/internal/cli/fetch/command.go b/internal/cli/fetch/command.go index fad712b..0cb6347 100644 --- a/internal/cli/fetch/command.go +++ b/internal/cli/fetch/command.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" ) +// NewCommand returns the "fetch" command, which fetches opportunities once or as a service. func NewCommand(opts *cli.Options) *cobra.Command { cmd := &cobra.Command{ Use: "fetch", diff --git a/internal/cli/logging.go b/internal/cli/logging.go index b66c500..eec1dde 100644 --- a/internal/cli/logging.go +++ b/internal/cli/logging.go @@ -7,6 +7,9 @@ import ( "github.com/spf13/cobra" ) +// SetupInitialLogging starts logging to the terminal, at the level set by +// cmd's --quiet and --debug flags (--debug wins), before any configuration +// is loaded. func SetupInitialLogging(cmd *cobra.Command) { // Start logging to terminal first - any commands that want to use file // logging should activate it themselves diff --git a/internal/cli/options.go b/internal/cli/options.go index c4f37a8..3d23aa5 100644 --- a/internal/cli/options.go +++ b/internal/cli/options.go @@ -9,13 +9,15 @@ import ( "github.com/spf13/cobra" ) +// Options holds the flags shared by every command. Bind registers them on the +// root command and RunSetup acts on them. type Options struct { configFile string debug bool quiet bool } -// Binds common options through the root command +// 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") @@ -25,7 +27,11 @@ func (o *Options) Bind(cmd *cobra.Command) { ) } -// Setup loads the config and ensure logging is switch to file if needed +// RunSetup prepares a command to run and returns its configuration. When +// loadConfig is true it loads the configuration (from the --config file if +// given), with --debug and --quiet overriding its log level, and then, unless +// terminalLog is true, switches logging to the configured log file (staying +// on the terminal, with a warning, if none is configured). func (o *Options) RunSetup( cmd *cobra.Command, loadConfig bool, terminalLog bool, ) (*config.JobSearchConfig, error) { diff --git a/internal/cli/version/command.go b/internal/cli/version/command.go index 2905d31..26e2e40 100644 --- a/internal/cli/version/command.go +++ b/internal/cli/version/command.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" ) +// NewCommand returns the "version" command. func NewCommand() *cobra.Command { cmd := &cobra.Command{ Use: "version", diff --git a/internal/config/README.md b/internal/config/README.md index 83cebf4..92b1925 100644 --- a/internal/config/README.md +++ b/internal/config/README.md @@ -33,10 +33,10 @@ is kept. | Key | Type | Default | Notes | |---|---|---|---| | `logging.level` | string | `warn` | One of `debug`, `info`, `warn`, `error` | -| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`) | +| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`); other commands log to the terminal | | `sources..enabled` | bool | `false` | For every source | | `sources..interval` | duration | `10m` | For every source; e.g. `30s`, `1h` | -| `sources.find_a_tender.api_root` | string | URL of the Find A Tender service API. Currently `https://www.find-tender.service.gov.uk/api/1.0` | | +| `sources.find_a_tender.api_root` | string | `https://www.find-tender.service.gov.uk/api/1.0` | URL of the Find a Tender API | | `sources.fts_mock.api_root` | string | | URL of the mock FTS server (see `internal/mock`) | | `sources.sleeper.block_shutdown` | bool | `false` | Test source: if true, its sleep ignores shutdown | @@ -73,14 +73,20 @@ per key: `section.key`. An embedded struct has its fields promoted, as Go does; every source embeds `JobSearchConfigSource` for `enabled` and `interval`. - `time.Duration` fields report `reflect.String` from `TypeOf`, accept either a - `time.Duration` or a duration string in `SetValue`, and are written as - strings (`"10m0s"`) by `Map`, `JSON` and `YAML`. + `time.Duration` or a duration string in `SetValue`, and are given out as + strings (`"10m0s"`) by `Map`, `Flatten` and `ValueOf`, and so by `JSON`, + `YAML` and `String` too. - A section can reject values by implementing `validate(key, value) error` on its pointer type. `JobSearchConfigLogging` does this for `level`. -Each section type has the same small set of methods, each a one-line call into -`section.go`: `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, plus -`Keys` on some. +Every section implements the `Section` interface defined in `section.go`: +`Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, each a one-line call into +`section.go` (some sections also have `Keys`). The methods sit under a +`// ---------- Section methods (see Section) ----------` banner in each +section's file. `TestSections` walks `JobSearchConfig` and checks every +section, including embedded ones, implements `Section` and that its +`Flatten` keys match its `cfg` tag, so a new section is checked without being +listed anywhere. ## Adding a key or section @@ -90,11 +96,14 @@ else is needed for it to be loaded, printed, and read or written by the To add a new section: 1. Create a `conf_*.go` file with the section struct, a `JSCK*` constant for - its key, and the standard methods (copy an existing section). + its key, and an implementation of `Section` (copy an existing section's + methods). 2. Add the struct as a tagged field of its parent: `JobSearchConfig` in `config.go`, or `JobSearchConfigSources` in `conf_sources.go` for a source, etc. The field's `cfg` tag must match the `JSCK*` constant. +Nothing in `section.go` needs changing. + ## API - `Init(cfgFile, &conf)`: load the configuration, as described above. @@ -119,7 +128,7 @@ To add a new section: | `config.go` | `JobSearchConfig`, defaults, Viper loading, text output | | `load.go` | `Init`, `.env` and config file loading | | `modify.go` | `StoreValue` and `PurgeValue` | -| `section.go` | Struct tag reflection shared by every section | +| `section.go` | The `Section` interface and the struct tag reflection shared by every section | | `conf_logging.go` | `logging` section and log level helpers | | `conf_sources.go` | `sources` section and the shared source fields | | `conf_source_*.go` | One file per source section | diff --git a/internal/config/conf_logging.go b/internal/config/conf_logging.go index 23d254b..7cfb3ed 100644 --- a/internal/config/conf_logging.go +++ b/internal/config/conf_logging.go @@ -6,18 +6,17 @@ import ( "reflect" ) +// JSCKLogging is the logging section's configuration key. const JSCKLogging string = "logging" +// JobSearchConfigLogging configures logging. Level is one of debug, info, +// warn or error. File, if set, is where commands that log to a file write. type JobSearchConfigLogging struct { Level string `cfg:"level" default:"warn"` File string `cfg:"file"` } -/* - * General section functions - * See related methods in section.go - */ -func (c *JobSearchConfigLogging) Keys() []string { return keysOf(c) } +// ---------- Section methods (see Section) ---------- func (c JobSearchConfigLogging) Map() map[string]any { return mapOf(c) } @@ -37,8 +36,12 @@ func (c *JobSearchConfigLogging) SetValue(key string, value any) error { return setValue(c, key, value) } -/* Section specific functions */ -// Checks values before they are stored (see validator) +// Keys returns the keys directly in the section. +func (c *JobSearchConfigLogging) Keys() []string { return keysOf(c) } + +// ---------- Section specific methods ---------- + +// validate checks values before they are stored (see validator). func (c *JobSearchConfigLogging) validate(key string, value any) error { switch key { case "level": @@ -52,7 +55,8 @@ func (c *JobSearchConfigLogging) validate(key string, value any) error { return nil } -// Utility methods for converting level strings to slog levels +// ConfigLevelToLogLevel converts a configured level name to an slog level. An +// unknown name gives 99, above every real level, so nothing is logged. func ConfigLevelToLogLevel(level string) slog.Level { switch level { case "debug": diff --git a/internal/config/conf_sections_test.go b/internal/config/conf_sections_test.go index bf70c56..8e23fc3 100644 --- a/internal/config/conf_sections_test.go +++ b/internal/config/conf_sections_test.go @@ -159,7 +159,7 @@ func TestSourceSection(t *testing.T) { if got := c.Map(); !reflect.DeepEqual(got, want) { t.Errorf("Map = %v; want %v", got, want) } - wantFlat := map[string]any{"enabled": true, "interval": time.Minute} + wantFlat := map[string]any{"enabled": true, "interval": "1m0s"} if got := c.Flatten(); !reflect.DeepEqual(got, wantFlat) { t.Errorf("Flatten = %v; want %v (no prefix)", got, wantFlat) } @@ -169,7 +169,7 @@ func TestSourceSection(t *testing.T) { if k, err := c.TypeOf("enabled"); err != nil || k != reflect.Bool { t.Errorf("TypeOf(enabled) = %v, %v; want Bool", k, err) } - if v, err := c.ValueOf("interval"); err != nil || v != time.Minute { + if v, err := c.ValueOf("interval"); err != nil || v != "1m0s" { t.Errorf("ValueOf(interval) = %v, %v", v, err) } if err := c.SetValue("interval", "5s"); err != nil || c.Interval != 5*time.Second { @@ -180,19 +180,10 @@ func TestSourceSection(t *testing.T) { } } -// sourceSection is the method set shared by every concrete source section. -type sourceSection interface { - Map() map[string]any - Flatten() map[string]any - TypeOf(string) (reflect.Kind, error) - ValueOf(string) (any, error) - SetValue(string, any) error -} - func TestConcreteSourceSections(t *testing.T) { tests := []struct { prefix string - section sourceSection + section Section apiRoot bool blockShutdown bool }{ @@ -220,7 +211,7 @@ func TestConcreteSourceSections(t *testing.T) { wantMap := map[string]any{"enabled": true, "interval": "2m0s"} wantFlat := map[string]any{ tt.prefix + ".enabled": true, - tt.prefix + ".interval": 2 * time.Minute, + tt.prefix + ".interval": "2m0s", } if tt.apiRoot { if err := s.SetValue("api_root", "http://localhost:8080"); err != nil { diff --git a/internal/config/conf_source_findATender.go b/internal/config/conf_source_findATender.go index 559ed3a..70ebd4b 100644 --- a/internal/config/conf_source_findATender.go +++ b/internal/config/conf_source_findATender.go @@ -2,17 +2,18 @@ package config import "reflect" +// JSCKSourceFindATender is the Find a Tender source's configuration key. const JSCKSourceFindATender string = "find_a_tender" +// JobSearchConfigSourceFindATender configures fetching from the Find a Tender +// OCDS API at APIRoot. type JobSearchConfigSourceFindATender struct { JobSearchConfigSource APIRoot string `cfg:"api_root" default:"https://www.find-tender.service.gov.uk/api/1.0"` } -/* - * General section functions - * See related methods in section.go - */ +// ---------- Section methods (see Section) ---------- + func (c JobSearchConfigSourceFindATender) Map() map[string]any { return mapOf(c) } func (c JobSearchConfigSourceFindATender) Flatten() map[string]any { diff --git a/internal/config/conf_source_ftsMock.go b/internal/config/conf_source_ftsMock.go index 20ff014..1113a85 100644 --- a/internal/config/conf_source_ftsMock.go +++ b/internal/config/conf_source_ftsMock.go @@ -2,17 +2,18 @@ package config import "reflect" +// JSCKSourceFTSMock is the Find a Tender mock source's configuration key. const JSCKSourceFTSMock string = "fts_mock" +// JobSearchConfigSourceFTSMock configures fetching from a Find a Tender mock +// server (cmd/mock-fts) at APIRoot. type JobSearchConfigSourceFTSMock struct { JobSearchConfigSource APIRoot string `cfg:"api_root"` } -/* - * General section functions - * See related methods in section.go - */ +// ---------- Section methods (see Section) ---------- + func (c JobSearchConfigSourceFTSMock) Map() map[string]any { return mapOf(c) } func (c JobSearchConfigSourceFTSMock) Flatten() map[string]any { diff --git a/internal/config/conf_source_sleeper.go b/internal/config/conf_source_sleeper.go index e1b970f..84f5d5d 100644 --- a/internal/config/conf_source_sleeper.go +++ b/internal/config/conf_source_sleeper.go @@ -2,17 +2,19 @@ package config import "reflect" +// JSCKSourceSleeper is the sleeper source's configuration key. const JSCKSourceSleeper string = "sleeper" +// JobSearchConfigSourceSleeper configures the sleeper, a test source that +// simulates work by sleeping. With BlockShutdown its work ignores +// cancellation, to test shutdown waiting on a non-cooperative task. type JobSearchConfigSourceSleeper struct { JobSearchConfigSource BlockShutdown bool `cfg:"block_shutdown"` } -/* - * General section functions - * See related methods in section.go - */ +// ---------- Section methods (see Section) ---------- + func (c JobSearchConfigSourceSleeper) Map() map[string]any { return mapOf(c) } func (c JobSearchConfigSourceSleeper) Flatten() map[string]any { diff --git a/internal/config/conf_sources.go b/internal/config/conf_sources.go index 56642af..7be92eb 100644 --- a/internal/config/conf_sources.go +++ b/internal/config/conf_sources.go @@ -5,19 +5,19 @@ import ( "time" ) +// JSCKSources is the sources section's configuration key. const JSCKSources string = "sources" -// Each field's cfg tag must match the section's JSCK* constant. +// JobSearchConfigSources configures each source of opportunities, one field +// per source. Each field's cfg tag must match the section's JSCK* constant. type JobSearchConfigSources struct { FindATender JobSearchConfigSourceFindATender `cfg:"find_a_tender"` FTSMock JobSearchConfigSourceFTSMock `cfg:"fts_mock"` Sleeper JobSearchConfigSourceSleeper `cfg:"sleeper"` } -/* - * General section functions - * See related methods in section.go - */ +// ---------- Section methods (see Section) ---------- + func (c JobSearchConfigSources) Map() map[string]any { return mapOf(c) } func (c JobSearchConfigSources) Flatten() map[string]any { @@ -36,19 +36,16 @@ func (c *JobSearchConfigSources) SetValue(key string, value any) error { return setValue(c, key, value) } -/* - *** Generic source definition, embedded in each source *** - */ +// ---------- Generic Source definition ---------- + +// JobSearchConfigSource holds the settings every source has, and is embedded +// in each source's section. Interval is how often an enabled source fetches. type JobSearchConfigSource struct { Enabled bool `cfg:"enabled"` Interval time.Duration `cfg:"interval" default:"10m"` } -/* - * General section functions - * See related methods in section.go - */ -func (c *JobSearchConfigSource) Keys() []string { return keysOf(c) } +// ---------- Section methods for Source (see Section) ---------- func (c JobSearchConfigSource) Map() map[string]any { return mapOf(c) } @@ -67,3 +64,6 @@ func (c JobSearchConfigSource) ValueOf(key string) (any, error) { func (c *JobSearchConfigSource) SetValue(key string, value any) error { return setValue(c, key, value) } + +// Keys returns the keys directly in the section. +func (c *JobSearchConfigSource) Keys() []string { return keysOf(c) } diff --git a/internal/config/config.go b/internal/config/config.go index 4768d5c..83dffc4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,16 +11,23 @@ import ( "go.yaml.in/yaml/v3" ) +// EnvPrefix prefixes environment variables that override configuration keys, +// e.g. JOBSEARCH_LOGGING_LEVEL for logging.level. const EnvPrefix string = "JOBSEARCH" + +// EtcConfigPath is searched for config.* when no config file is given, before +// the working directory. const EtcConfigPath string = "/etc/job-search" -// Each field's cfg tag must match the section's JSCK* constant. +// JobSearchConfig is the application's whole configuration, one field per +// section. Each field's cfg tag must match the section's JSCK* constant. type JobSearchConfig struct { Logging JobSearchConfigLogging `cfg:"logging"` Sources JobSearchConfigSources `cfg:"sources"` } -// Default configuration, taken from the default struct tags +// LoadDefaultConfig sets Viper's defaults from the configuration's default +// struct tags. func LoadDefaultConfig() { var conf JobSearchConfig if err := applyDefaults(&conf); err != nil { @@ -29,6 +36,8 @@ func LoadDefaultConfig() { LoadToViper(conf) } +// ---------- Section methods (see Section) ---------- + func (c JobSearchConfig) Map() map[string]any { return mapOf(c) } func (c JobSearchConfig) Flatten() map[string]any { return flattenOf("", c) } @@ -45,7 +54,8 @@ func (c *JobSearchConfig) SetValue(key string, value any) error { return setValue(c, key, value) } -// Utility method for loading a configuration from Viper +// LoadFromViper copies every key Viper knows that conf also has into conf. +// Unknown keys are skipped; invalid values are logged and ignored. func LoadFromViper(conf *JobSearchConfig) { keys := viper.AllKeys() for idx := range keys { @@ -71,7 +81,7 @@ func LoadFromViper(conf *JobSearchConfig) { } } -// Utility method for loading a configuration into Viper as defaults +// LoadToViper sets every key of conf as a Viper default. func LoadToViper(conf JobSearchConfig) { flatMap := conf.Flatten() for key := range flatMap { @@ -79,7 +89,8 @@ func LoadToViper(conf JobSearchConfig) { } } -// Converts configuration to a string in flat dotted format +// String formats the configuration one "key = value " line per key, with +// dotted keys. func (conf JobSearchConfig) String() string { var sb strings.Builder for k, v := range conf.Flatten() { @@ -89,14 +100,14 @@ func (conf JobSearchConfig) String() string { return sb.String() } -// Converts configuration to JSON +// JSON formats the configuration as indented JSON. func (conf JobSearchConfig) JSON() string { confMap := conf.Map() jbytes, _ := json.MarshalIndent(confMap, "", " ") return string(jbytes) } -// Converts configuration to YAML +// YAML formats the configuration as YAML. func (conf JobSearchConfig) YAML() string { confMap := conf.Map() ybytes, _ := yaml.Marshal(confMap) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3b6dce2..40b99d3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -34,10 +34,18 @@ func TestKeysRoundTrip(t *testing.T) { } } -// Every section has the full method set, and its Flatten keys (which use the -// section's JSCK* constant) line up with its cfg tag in the parent. +// sectionType is the Section interface type. +var sectionType = reflect.TypeOf((*Section)(nil)).Elem() + +// Every section, including embedded ones, implements Section, and its +// Flatten keys (which use the section's JSCK* constant) line up with its cfg +// tag in the parent. Sections are found by walking JobSearchConfig, so a new +// section is checked without being listed anywhere. func TestSections(t *testing.T) { var conf JobSearchConfig + if !reflect.PointerTo(reflect.TypeOf(conf)).Implements(sectionType) { + t.Error("*JobSearchConfig does not implement Section") + } all := conf.Flatten() checkSections(t, all, "", reflect.ValueOf(&conf).Elem()) } @@ -49,11 +57,10 @@ func checkSections(t *testing.T, all map[string]any, prefix string, section refl } fv := section.FieldByIndex(f.index) name := fv.Type().Name() - for _, m := range []string{"Map", "Flatten", "TypeOf", "ValueOf", "SetValue"} { - if !fv.Addr().MethodByName(m).IsValid() { - t.Errorf("%s is missing method %s", name, m) - } + if !fv.Addr().Type().Implements(sectionType) { + t.Errorf("*%s does not implement Section", name) } + checkEmbedded(t, fv.Type()) if flatten := fv.MethodByName("Flatten"); flatten.IsValid() { keys := flatten.Call(nil)[0].Interface().(map[string]any) for k := range keys { @@ -66,3 +73,19 @@ func checkSections(t *testing.T, all map[string]any, prefix string, section refl checkSections(t, all, joinKey(prefix, f.key), fv) } } + +// checkEmbedded checks that structs embedded in a section, such as +// JobSearchConfigSource, implement Section too. +func checkEmbedded(t *testing.T, section reflect.Type) { + 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()) + } + checkEmbedded(t, sf.Type) + } +} diff --git a/internal/config/debug.go b/internal/config/debug.go index 8cbc4cb..8fd269d 100644 --- a/internal/config/debug.go +++ b/internal/config/debug.go @@ -11,7 +11,7 @@ import ( "github.com/spf13/viper" ) -// Debug: Prints detected environment variables +// printEnv logs the JOBSEARCH* environment variables at debug level. func printEnv() { slog.Debug("Environment:") env := os.Environ() @@ -24,7 +24,8 @@ func printEnv() { } } -// Debug: Prints loaded configuration +// printLoadedConfig logs every key Viper has loaded, with its value and type, +// at debug level. func printLoadedConfig() { slog.Debug("Loaded config:") keys := viper.AllKeys() @@ -37,7 +38,7 @@ func printLoadedConfig() { } } -// Debug: Prints parsed configuration +// printConfig logs conf at debug level. func printConfig(conf JobSearchConfig) { slog.Debug("Config:") conf.PrintLog(slog.LevelDebug) diff --git a/internal/config/load.go b/internal/config/load.go index ccda30a..4d3005f 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -9,7 +9,8 @@ import ( "github.com/spf13/viper" ) -// Reads in dotenv file for configuration +// loadEnv loads environment variables from a .env file in the working +// directory. func loadEnv() error { if err := godotenv.Load(); err != nil { return fmt.Errorf("invalid .env file: %w", err) @@ -17,7 +18,9 @@ func loadEnv() error { return nil } -// Reads in config file and ENV variables if set to form a configuration. +// loadConfig sets up Viper with the defaults, cfgFile (or config.* found in +// EtcConfigPath or the working directory) and JOBSEARCH_* environment +// variables. A missing config file is only a warning. func loadConfig(cfgFile string) error { if cfgFile != "" { // Use config file from the flag. @@ -54,7 +57,9 @@ func loadConfig(cfgFile string) error { return nil } -// Initilises environment file configuration and general app configuration +// Init loads the .env file and the configuration (see loadConfig) into conf. +// A debug or error log level already set in conf, from --debug or --quiet, +// is kept over the loaded one. func Init(cfgFile string, conf *JobSearchConfig) error { if err := loadEnv(); err != nil { return err diff --git a/internal/config/print.go b/internal/config/print.go index ca18409..698b008 100644 --- a/internal/config/print.go +++ b/internal/config/print.go @@ -5,22 +5,23 @@ import ( "log/slog" ) -// Prints configuration to stdout in flat dotted format +// PrintTerminal prints the configuration to stdout as String formats it. func (conf JobSearchConfig) PrintTerminal() { fmt.Print(conf.String() + "\n") } -// Prints configuration to stdout as JSON +// PrintJson prints the configuration to stdout as JSON. func (conf JobSearchConfig) PrintJson() { fmt.Print(conf.JSON() + "\n") } -// Prints configuration to stdout as YAML +// PrintYaml prints the configuration to stdout as YAML. func (conf JobSearchConfig) PrintYaml() { fmt.Print(conf.YAML() + "\n") } -// Prints configuration to logrus log at debug, info (default) or error level +// PrintLog logs each configuration key and value at atLevel: debug, warn, +// error, or info for any other level. func (conf JobSearchConfig) PrintLog(atLevel slog.Level) { var logFn func(args ...interface{}) diff --git a/internal/config/section.go b/internal/config/section.go index 55d7333..747c7d8 100644 --- a/internal/config/section.go +++ b/internal/config/section.go @@ -36,6 +36,32 @@ var ( timeType = reflect.TypeOf(time.Time{}) ) +// Section is the method set of every configuration section, including +// JobSearchConfig itself, implemented on the section's pointer type. Keys are +// dotted paths relative to the section, e.g. "sleeper.interval" in +// JobSearchConfigSources. Durations are given out as strings such as "10m0s", +// matching TypeOf's reflect.String for them. TestSections checks every +// section, found by walking JobSearchConfig, implements it. +type Section interface { + // Map returns the section as a map of keys to values, with nested + // sections as nested maps. + Map() map[string]any + // Flatten returns the section as a flat map of dotted keys, each + // prefixed with the section's own key (its JSCK* constant), so a + // section's keys match how its parent flattens it. JobSearchConfig and + // the embedded JobSearchConfigSource have no prefix. + Flatten() map[string]any + // TypeOf returns the kind of the value at key, which is how it is read + // from Viper, or an error for an unknown key. + TypeOf(key string) (reflect.Kind, error) + // ValueOf returns the value at key, or an error for an unknown key. + ValueOf(key string) (any, error) + // SetValue stores value at key. value may be of the field's type or a + // string to parse (e.g. "10m", "true", "42"). The section's validation, + // if any, runs first; on error the field is unchanged. + SetValue(key string, value any) error +} + // validator is implemented by sections that need to reject some values. // key is the field's own key within the section; value has already been // converted to the field's type. @@ -43,6 +69,8 @@ type validator interface { validate(key string, value any) error } +// field is a section struct field that is part of the configuration (see +// fieldsOf). type field struct { key string index []int // path for reflect.Value.FieldByIndex @@ -136,16 +164,15 @@ func mapOf(v any) map[string]any { return mapOfValue(structValue(v)) } +// 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) - } else if f.typ == durationType { - m[f.key] = fv.Interface().(time.Duration).String() } else { - m[f.key] = fv.Interface() + m[f.key] = exportValue(fv) } } return m @@ -159,6 +186,8 @@ func flattenOf(prefix string, v any) map[string]any { return m } +// 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) @@ -166,11 +195,21 @@ func flattenInto(m map[string]any, prefix string, section reflect.Value) { if f.section { flattenInto(m, key, fv) } else { - m[key] = fv.Interface() + m[key] = exportValue(fv) } } } +// exportValue returns a field's value as Map, Flatten and ValueOf give it +// out: durations become strings such as "1m30s", so values agree with kindOf +// and can be passed back to SetValue; everything else is unchanged. +func exportValue(fv reflect.Value) any { + if fv.Type() == durationType { + return time.Duration(fv.Int()).String() + } + return fv.Interface() +} + // kindOf is the kind reported by TypeOf, i.e. how the value is read from // Viper. Durations are read as strings and parsed by SetValue. func kindOf(t reflect.Type) reflect.Kind { @@ -180,6 +219,8 @@ func kindOf(t reflect.Type) reflect.Kind { return t.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) if err != nil { @@ -188,12 +229,13 @@ func typeOf(v any, key string) (reflect.Kind, error) { return kindOf(f.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) if err != nil { return nil, err } - return section.FieldByIndex(f.index).Interface(), nil + return exportValue(section.FieldByIndex(f.index)), nil } // setValue sets a dotted key on the section that ptr points to. diff --git a/internal/config/section_test.go b/internal/config/section_test.go index 2a0b6e2..ab5c6fb 100644 --- a/internal/config/section_test.go +++ b/internal/config/section_test.go @@ -33,7 +33,7 @@ type testSection struct { NoTag string Skipped string `cfg:"-"` Empty string `cfg:""` - unexport string `cfg:"unexported"` + unexport string `cfg:"unexported"` //nolint:unused // checks unexported fields are ignored } type testValidated struct { @@ -85,6 +85,9 @@ func TestMapOf(t *testing.T) { if got := m["on"]; got != true { t.Errorf("m[on] (promoted) = %#v", got) } + if got := m["dur"]; got != "1m30s" { + t.Errorf("m[dur] = %#v; want the duration as a string", got) + } inner, ok := m["inner"].(map[string]any) if !ok { t.Fatalf("m[inner] = %#v; want nested map", m["inner"]) @@ -102,12 +105,15 @@ func TestMapOf(t *testing.T) { } func TestFlattenOfPrefix(t *testing.T) { - m := flattenOf("p", testSection{}) + 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) } } + if got := m["p.dur"]; got != "1m30s" { + t.Errorf("m[p.dur] = %#v; want the duration as a string", got) + } } func TestStructValuePanics(t *testing.T) { @@ -220,12 +226,12 @@ func TestSetValue(t *testing.T) { {"uint16", "65535", uint16(65535)}, {"float", "2.25", 2.25}, {"float32", "0.5", float32(0.5)}, - {"dur", "1h30m", 90 * time.Minute}, + {"dur", "1h30m", "1h30m0s"}, // durations are given out as strings {"inner.name", "nested", "nested"}, // values of the field's type are used as-is {"int", 5, 5}, {"on", true, true}, - {"dur", 3 * time.Second, 3 * time.Second}, + {"dur", 3 * time.Second, "3s"}, {"slice", []string{"a", "b"}, []string{"a", "b"}}, {"when", time.Unix(0, 0).UTC(), time.Unix(0, 0).UTC()}, } diff --git a/internal/fetcher/worker_sleeper.go b/internal/fetcher/worker_sleeper.go index ce00409..8404b91 100644 --- a/internal/fetcher/worker_sleeper.go +++ b/internal/fetcher/worker_sleeper.go @@ -11,7 +11,8 @@ import ( // sleepDuration is how long the sleeper's simulated work takes. const sleepDuration = 60 * time.Second -// Test source that just sleeps for 60 seconds to simulate doing work +// 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, conf *config.JobSearchConfig, since time.Time, ) error { diff --git a/internal/findATender/api.go b/internal/findATender/api.go index c3d5ada..e63292b 100644 --- a/internal/findATender/api.go +++ b/internal/findATender/api.go @@ -18,19 +18,37 @@ import ( resty "resty.dev/v3" ) -var ErrNoNext = errors.New("no next link") -var ErrNoReleases = errors.New("no releases") -var ErrRateLimitExceeded = errors.New("rate limit has been exceeded") -var ErrFetchCancelled = errors.New("fetch cancelled") -var ErrUnexpectedStatus = errors.New("unexpected response status") +// Errors returned when fetching from the API. +var ( + // ErrNoNext means a release package has no next page link. + ErrNoNext = errors.New("no next link") + // ErrNoReleases means the API returned no releases. + ErrNoReleases = errors.New("no releases") + // ErrRateLimitExceeded means the API refused a request with 429 Too Many + // Requests, and waiting for it was not allowed or did not help. + ErrRateLimitExceeded = errors.New("rate limit has been exceeded") + // ErrFetchCancelled means the fetch's context was cancelled while waiting + // out a rate limit. + ErrFetchCancelled = errors.New("fetch cancelled") + // ErrUnexpectedStatus wraps an API response status other than 200 OK. + ErrUnexpectedStatus = errors.New("unexpected response status") +) +// RequestOptions selects the releases a fetch asks for. type RequestOptions struct { - Since time.Time - Limit int + // Since asks for releases updated from this time. Zero means 24 hours ago. + Since time.Time + // Limit is the number of releases per page. Zero means 100. + Limit int + // AllowRateLimit waits out a 429 response and retries once, instead of + // failing with ErrRateLimitExceeded. AllowRateLimit bool } // ---------- Utility methods for handling API irregularities ----------- + +// safeReleasePackage is ocds.ReleasePackage with the releases left undecoded, +// so safeUnpackReleases can repair or skip each one. type safeReleasePackage struct { URI string `json:"uri,omitempty"` Version string `json:"version,omitempty"` @@ -127,7 +145,7 @@ func isDateField(name string) bool { strings.HasSuffix(name, "Deadline") } -// ReleaseIDs reads just the ocid and the release id from a release, without +// releaseIDs reads just the ocid and the release id from a release, without // decoding the rest of it. It reads the top-level fields in order and stops // as soon as it has both, so an error later in the release (invalid JSON or a // value that wouldn't fit the release struct) doesn't matter. Fields before @@ -202,15 +220,16 @@ func missingIDsError(haveOCID, haveID bool) error { } } -// Unpacks an API response into an OCDS ReleasePackage. Handles API -// irregularities (malformed dates, missing fields, invalid releases, etc). +// safeUnpackReleases unpacks an API response into an OCDS ReleasePackage, +// handling API irregularities (malformed dates, missing fields, invalid +// releases, etc). // // This method takes ownership of the response body and closes it when done. func safeUnpackReleases(response *resty.Response) (*ocds.ReleasePackage, error) { var pkg safeReleasePackage // Take ownership of the response body. - defer response.Body.Close() + defer response.Body.Close() //nolint:errcheck // read only decoder := json.NewDecoder(response.Body) if err := decoder.Decode(&pkg); err != nil { @@ -255,9 +274,10 @@ func safeUnpackReleases(response *resty.Response) (*ocds.ReleasePackage, error) } // ---------- API interaction methods ----------- -// Sleep the fetch worker until either the context is cancelled (shutdown) or -// the duration has expired. The duration is usually provided by the API - a -// nil duration will return immediately. + +// rateSleep sleeps the fetch worker until either the context is cancelled +// (shutdown), returning ctx.Err(), or the duration has expired. The duration +// is usually provided by the API; a zero duration returns immediately. func rateSleep(ctx context.Context, duration time.Duration) error { timer := time.NewTimer(duration) defer timer.Stop() @@ -270,14 +290,16 @@ func rateSleep(ctx context.Context, duration time.Duration) error { } } -// Check and close the response body - avoids double close issues +// closeBody closes res's body if it has one, avoiding a double close. func closeBody(res *resty.Response) { if res.Body != nil { - res.Body.Close() + res.Body.Close() //nolint:errcheck,gosec // read only } } -// Actually perform a single fetch against the API +// fetchURI performs a single GET of uri, using ft'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 @@ -310,7 +332,7 @@ func fetchURI(ft *FindATender, uri string) (*resty.Response, error) { return res, err } -// Perform a fetch against the API. If the API returns with a rate limit +// fetchURI performs a fetch against the API. If the API returns with a rate limit // reached response and allowRateLimit is true, then wait and retry like a good // client. If allowRateLimit is false, return to the caller informing them of // the rate limit exceeded error. If, after waiting, the API still thinks we @@ -367,7 +389,8 @@ func (ft *FindATender) fetchURI( return res, nil } -// Perform a fetch against the API with some sanitised default options. +// fetchWithOpts performs a fetch against the API with some sanitised default +// options (see RequestOptions). func (ft *FindATender) fetchWithOpts( opts RequestOptions, ) (*resty.Response, error) { @@ -388,9 +411,9 @@ func (ft *FindATender) fetchWithOpts( return ft.fetchURI(uri, opts.AllowRateLimit) } -// If the response provided a "Next" link, perform another API call to get the -// next page. Error if there is no "Next" link - the caller can decide if they -// have attempted to fetch next in error or not. +// fetchNext fetches the page pkg's "Next" link points to. It returns +// 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, allowRateLimit bool, @@ -401,7 +424,7 @@ func (ft *FindATender) fetchNext( return ft.fetchURI(pkg.Links.Next, allowRateLimit) } -// Make one call on the API. Do not follow any "Next" links. +// fetchOnce makes one call on the API. It does not follow any "Next" links. func (ft *FindATender) fetchOnce(opts RequestOptions) ([]ocds.Release, error) { res, err := ft.fetchWithOpts(opts) if err != nil { @@ -420,8 +443,8 @@ func (ft *FindATender) fetchOnce(opts RequestOptions) ([]ocds.Release, error) { return pkg.Releases, nil } -// Make a call on the API. If there are any "Next" links, call them in -// succession until the end has been reached. +// fetchAll makes a call on the API. If there are any "Next" links, it calls +// them in succession until the end has been reached. func (ft *FindATender) fetchAll(opts RequestOptions) ([]ocds.Release, error) { var releases []ocds.Release diff --git a/internal/findATender/findATender.go b/internal/findATender/findATender.go index eb66897..77eb328 100644 --- a/internal/findATender/findATender.go +++ b/internal/findATender/findATender.go @@ -2,6 +2,7 @@ package findATender import ( "context" + "log/slog" "time" "git.avsdev.uk/Go/job-search/internal/config" @@ -9,6 +10,8 @@ import ( resty "resty.dev/v3" ) +// FindATender is a client for the Find a Tender OCDS API (or the mock +// server). Create it with New and call Shutdown when finished with it. type FindATender struct { httpClient *resty.Client config *config.JobSearchConfig @@ -47,9 +50,12 @@ func (ft *FindATender) With(ctx context.Context) *FindATender { return &copied } -// Shutdown closes the client's HTTP connections. +// Shutdown closes the client's HTTP connections. A failure to close is +// logged, as there is nothing a caller could do about it. func (ft *FindATender) Shutdown() { - ft.httpClient.Close() + if err := ft.httpClient.Close(); err != nil { + slog.Warn("findATender: closing HTTP client", "error", err) + } } // FetchOnce fetches the first page of releases updated since since, and diff --git a/internal/logging/logging.go b/internal/logging/logging.go index ad198fb..9323cd2 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -1,23 +1,33 @@ package logging import ( + "errors" "log/slog" "os" + "path/filepath" "github.com/lmittmann/tint" ) // ---------- Standard log handling (terminal or file based) ---------- -var logLevel slog.LevelVar -var fileLogger *os.File +// Logger state shared by the terminal and file loggers. +var ( + // logLevel is the minimum level logged, read by whichever handler is + // installed, so changing it takes effect immediately. + logLevel slog.LevelVar + // fileLogger is the log file opened by LogToFile, or nil when logging + // to the terminal. + fileLogger *os.File +) -// Set the logging to a particular level, quietly +// SetLevel sets the logging level without logging the change. func SetLevel(level slog.Level) { logLevel.Set(level) } -// Enable debug level logging +// EnableDebug lowers the logging level to debug, if it isn't already, and +// logs that it did. func EnableDebug() { if logLevel.Level() > slog.LevelDebug { logLevel.Set(slog.LevelDebug) @@ -29,15 +39,15 @@ func EnableDebug() { // logger. The file is created if needed and appended to; any log file opened // by an earlier call is closed. func LogToFile(logfile string) error { - if fileLogger != nil { - (*fileLogger).Close() - } + closeErr := Shutdown() // Open the log file var err error - fileLogger, err = os.OpenFile(logfile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) + fileLogger, err = os.OpenFile( + filepath.Clean(logfile), os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600, + ) if err != nil { - return err + return errors.Join(err, closeErr) } opts := &slog.HandlerOptions{ @@ -47,7 +57,7 @@ func LogToFile(logfile string) error { logger := slog.New(handler) slog.SetDefault(logger) - return nil + return closeErr } // Shutdown closes the log file opened by LogToFile, if any. diff --git a/internal/mock/findATender/catalog.go b/internal/mock/findATender/catalog.go index 31b5db3..6759398 100644 --- a/internal/mock/findATender/catalog.go +++ b/internal/mock/findATender/catalog.go @@ -168,6 +168,8 @@ func (c *Catalog) resolveOne(key string) *ocds.Classification { return nil } +// resolve returns copies of the classifications with keys, skipping keys +// that have none. func (c *Catalog) resolve(keys []string) []ocds.Classification { var out []ocds.Classification for _, k := range keys { diff --git a/internal/mock/findATender/generator.go b/internal/mock/findATender/generator.go index be90ed7..ed1cb10 100644 --- a/internal/mock/findATender/generator.go +++ b/internal/mock/findATender/generator.go @@ -29,7 +29,7 @@ func NewGenerator(cat *Catalog, store *Store, seed int64) *Generator { return &Generator{ cat: cat, store: store, - rnd: rand.New(rand.NewSource(seed)), + rnd: rand.New(rand.NewSource(seed)), //nolint:gosec // seeded for repeatable mock data } } @@ -46,6 +46,10 @@ func (g *Generator) Make(at time.Time) ocds.Release { return g.newProcedure(at) } +// newProcedure returns the first release of a new procedure: a tender (or, +// one time in five, a planning notice) from a random catalog buyer and +// subject, using a random procurement method. Values are scaled by a random +// factor, and a tender without lots gets a single lot covering it all. func (g *Generator) newProcedure(at time.Time) ocds.Release { buyer := g.cat.buyer(g.rnd) ensurePartyID(&buyer) @@ -189,6 +193,7 @@ func (g *Generator) advance(prev ocds.Release, at time.Time) ocds.Release { return r } +// newOCID returns a random ocid not already in the store. func (g *Generator) newOCID() string { for { id := fmt.Sprintf("%s%06x", ocidPrefix, g.rnd.Intn(1<<24)) @@ -263,6 +268,7 @@ func setStatus(t *ocds.Tender, status string) { } } +// scaleValue multiplies v's amounts by f, rounded to the nearest 100. func scaleValue(v *ocds.Value, f float64) { if v == nil { return @@ -274,11 +280,13 @@ func scaleValue(v *ocds.Value, f float64) { } } +// ensurePartyID gives p 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 != "" { return } h := fnv.New32a() - h.Write([]byte(p.Name)) + _, _ = h.Write([]byte(p.Name)) // never fails p.ID = fmt.Sprintf("GB-FTS-%d", h.Sum32()%1000000) } diff --git a/internal/mock/findATender/handler.go b/internal/mock/findATender/handler.go index 8a27bf2..78c65a2 100644 --- a/internal/mock/findATender/handler.go +++ b/internal/mock/findATender/handler.go @@ -22,6 +22,7 @@ const ( BasePath = "/api/1.0/ocdsReleasePackages" ) +// server serves release packages from a Store; see NewHandler. type server struct { store *Store baseURL string @@ -39,6 +40,8 @@ func NewHandler( return (&server{store: store, baseURL: baseURL, loc: loc}).handle } +// encodeCursor makes the opaque cursor for a next link that resumes at store +// position i. func encodeCursor(i int) string { return base64.RawURLEncoding.EncodeToString([]byte("idx:" + strconv.Itoa(i))) } @@ -62,7 +65,7 @@ func decodeCursor(c string) (int, error) { func badRequest(w http.ResponseWriter, msg string) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) - json.NewEncoder(w).Encode(map[string]string{"error": msg}) + _ = json.NewEncoder(w).Encode(map[string]string{"error": msg}) // the client has gone } // parseTime reads a query timestamp. Values without an offset (the documented @@ -93,6 +96,8 @@ type pageQuery struct { limit int } +// handle answers a GET for a page of releases as an OCDS release package, +// with a links.next when more releases match. func (s *server) handle(w http.ResponseWriter, req *http.Request) { if req.Method != http.MethodGet { w.WriteHeader(http.StatusMethodNotAllowed) diff --git a/internal/mock/findATender/ratelimit.go b/internal/mock/findATender/ratelimit.go index 6cacda6..21ee41d 100644 --- a/internal/mock/findATender/ratelimit.go +++ b/internal/mock/findATender/ratelimit.go @@ -11,6 +11,7 @@ import ( // ---------- rate limiting ---------- +// clientWindow is one client's rate limiting state. type clientWindow struct { hour time.Time // clock hour the anchor belongs to anchor time.Time // first request of that hour @@ -18,6 +19,10 @@ type clientWindow struct { count int // accepted requests in the current window } +// RateLimiter limits each client (by remote host) to a number of requests per +// fixed window, answering the rest with 429 Too Many Requests and a +// Retry-After header, as the Find a Tender API does. A client's windows are +// counted from its first request in each clock hour. type RateLimiter struct { mu sync.Mutex limit int @@ -26,6 +31,7 @@ type RateLimiter struct { now func() time.Time } +// NewRateLimiter returns a RateLimiter allowing limit requests per window. func NewRateLimiter(limit int, window time.Duration) *RateLimiter { return &RateLimiter{ limit: limit, @@ -108,7 +114,7 @@ func (rl *RateLimiter) Middleware(next http.HandlerFunc) http.HandlerFunc { w.Header().Set("Retry-After", strconv.Itoa(retry)) w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusTooManyRequests) - fmt.Fprintf( + _, _ = fmt.Fprintf( w, "Rate limit of %d exceeded. Please retry after %d seconds.", rl.limit, retry, ) diff --git a/internal/mock/findATender/store.go b/internal/mock/findATender/store.go index 1b0737d..d4342e5 100644 --- a/internal/mock/findATender/store.go +++ b/internal/mock/findATender/store.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "encoding/json" + "errors" "fmt" "log" "math/rand" @@ -69,14 +70,14 @@ func OpenStore(path string) (*Store, error) { // readReleases reads the releases stored at path, skipping lines that are // blank or can't be decoded. A missing file holds no releases. func readReleases(path string) ([]ocds.Release, error) { - in, err := os.Open(path) + in, err := os.Open(filepath.Clean(path)) if os.IsNotExist(err) { return nil, nil } if err != nil { return nil, err } - defer in.Close() + defer in.Close() //nolint:errcheck // read only var releases []ocds.Release sc := bufio.NewScanner(in) @@ -103,7 +104,7 @@ func readReleases(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(path, os.O_CREATE|os.O_RDWR|os.O_APPEND, 0o600) + f, err := os.OpenFile(filepath.Clean(path), os.O_CREATE|os.O_RDWR|os.O_APPEND, 0o600) if err != nil { return nil, err } @@ -111,8 +112,7 @@ func openForAppend(path string) (*os.File, error) { 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 { - f.Close() - return nil, fmt.Errorf("repairing %s: %w", path, err) + return nil, errors.Join(fmt.Errorf("repairing %s: %w", path, err), f.Close()) } } } @@ -166,8 +166,9 @@ func (s *Store) Sync() error { return s.f.Sync() } -// A date earlier than the newest stored release (for example after a clock -// change) is moved up so the order stays valid. +// append stores r, 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 { s.mu.Lock() defer s.mu.Unlock() @@ -224,12 +225,14 @@ func (s *Store) Last() (time.Time, bool) { return s.times[len(s.times)-1], true } +// Len returns the number of stored releases. func (s *Store) Len() int { s.mu.RLock() defer s.mu.RUnlock() return len(s.releases) } +// HasOCID reports whether any release of the procedure ocid is stored. func (s *Store) HasOCID(ocid string) bool { s.mu.RLock() defer s.mu.RUnlock() @@ -261,6 +264,7 @@ func (s *Store) RandomOpen(r *rand.Rand, since time.Time) (ocds.Release, bool) { return ocds.Release{}, false } +// Close closes the store's file. The store must not be used afterwards. func (s *Store) Close() error { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/types/classifications.go b/internal/types/classifications.go index 23531a7..11e4238 100644 --- a/internal/types/classifications.go +++ b/internal/types/classifications.go @@ -1,5 +1,9 @@ package types +// ClassificationCode is a code classifying what an opportunity buys. IsCPV +// says the code is from the Common Procurement Vocabulary; Classification is +// its description. ID is the database row, shared by every opportunity with +// the same code. type ClassificationCode struct { ID int64 `db:"id" json:"id"` IsCPV bool `db:"is_cpv" json:"is_cpv"` diff --git a/internal/types/opportunities.go b/internal/types/opportunities.go index 39ac0eb..59c1739 100644 --- a/internal/types/opportunities.go +++ b/internal/types/opportunities.go @@ -26,6 +26,8 @@ type Opportunity struct { Releases []OpportunityRelease `db:"-" json:"releases"` } +// OpportunityRelease is one OCDS release of an opportunity. JSON holds the +// whole release as received; releases are never changed once stored. type OpportunityRelease struct { UUID string `db:"uuid" json:"uuid"` ReleaseID string `db:"release_id" json:"release_id"` @@ -34,6 +36,8 @@ type OpportunityRelease struct { JSON string `db:"data" json:"data"` } +// OpportunityReleaseTag is one of a release's OCDS tags, e.g. "tender" or +// "award". type OpportunityReleaseTag struct { UUID string `db:"uuid" json:"uuid"` ReleaseID string `db:"release_id" json:"release_id"` diff --git a/internal/util/worker.go b/internal/util/worker.go index be50d24..5b11c36 100644 --- a/internal/util/worker.go +++ b/internal/util/worker.go @@ -154,12 +154,7 @@ func AwaitTask[T any](ctx context.Context, fn func() (T, error)) (T, error) { } } -// AwaitTask runs fn 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. +// 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) go func() {