Fixed lint issues and missing documentation

.golangci.json

@@ -32,6 +32,13 @@
             "errcheck",
             "cyclop"
           ]
+        },
+        {
+          "path": "_test\\.go",
+          "linters": [
+            "gosec"
+          ],
+          "text": "G104"
         }
       ]
     }

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`
+   - Implement the `Section` interface (see `internal/config/section.go`):
-     methods, renaming the receiver type and using the new constant in
+     copy the `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue` methods
-     `Flatten`.
+     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
+6. **Tests:** `TestSections` checks the new section implements `Section`
-   `internal/config/conf_sections_test.go` (add it to
+   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`
+   `TestSourcesSection`. Test the client and worker using `httptest` servers
-   servers rather than the real service.
+   rather than the real service.
 7. **Docs:** add the source and its keys to the key table in
    `internal/config/README.md`.
 

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)

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",

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",

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

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) {

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",

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.<source>.enabled` | bool | `false` | For every source |
 | `sources.<source>.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
+  `time.Duration` or a duration string in `SetValue`, and are given out as
-  strings (`"10m0s"`) by `Map`, `JSON` and `YAML`.
+  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
+Every section implements the `Section` interface defined in `section.go`:
-`section.go`: `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, plus
+`Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, each a one-line call into
-`Keys` on some.
+`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 |

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"`
 }
 
-/*
+// ---------- Section methods (see Section) ----------
- * General section functions
- * See related methods in section.go
- */
-func (c *JobSearchConfigLogging) Keys() []string { return keysOf(c) }
 
 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 */
+// Keys returns the keys directly in the section.
-// Checks values before they are stored (see validator)
+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":

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 {

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"`
 }
 
-/*
+// ---------- Section methods (see Section) ----------
- * General section functions
+
- * See related methods in section.go
- */
 func (c JobSearchConfigSourceFindATender) Map() map[string]any { return mapOf(c) }
 
 func (c JobSearchConfigSourceFindATender) Flatten() map[string]any {

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"`
 }
 
-/*
+// ---------- Section methods (see Section) ----------
- * General section functions
+
- * See related methods in section.go
- */
 func (c JobSearchConfigSourceFTSMock) Map() map[string]any { return mapOf(c) }
 
 func (c JobSearchConfigSourceFTSMock) Flatten() map[string]any {

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"`
 }
 
-/*
+// ---------- Section methods (see Section) ----------
- * General section functions
+
- * See related methods in section.go
- */
 func (c JobSearchConfigSourceSleeper) Map() map[string]any { return mapOf(c) }
 
 func (c JobSearchConfigSourceSleeper) Flatten() map[string]any {

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"`
 }
 
-/*
+// ---------- Section methods (see Section) ----------
- * General section functions
+
- * See related methods in section.go
- */
 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 ----------
- *** Generic source definition, embedded in each source ***
+
- */
+// 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"`
 }
 
-/*
+// ---------- Section methods for Source (see Section) ----------
- * General section functions
- * See related methods in section.go
- */
-func (c *JobSearchConfigSource) Keys() []string { return keysOf(c) }
 
 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) }

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 <type>" 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)

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
+// sectionType is the Section interface type.
-// section's JSCK* constant) line up with its cfg tag in the parent.
+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().Type().Implements(sectionType) {
-			if !fv.Addr().MethodByName(m).IsValid() {
+			t.Errorf("*%s does not implement Section", name)
-				t.Errorf("%s is missing method %s", name, m)
-			}
 		}
+		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)
+	}
+}

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)

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

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{})
 

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.

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()},
 	}

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 {

internal/findATender/api.go

@@ -18,19 +18,37 @@ import (
 	resty "resty.dev/v3"
 )
 
-var ErrNoNext = errors.New("no next link")
+// Errors returned when fetching from the API.
-var ErrNoReleases = errors.New("no releases")
+var (
-var ErrRateLimitExceeded = errors.New("rate limit has been exceeded")
+	// ErrNoNext means a release package has no next page link.
-var ErrFetchCancelled = errors.New("fetch cancelled")
+	ErrNoNext = errors.New("no next link")
-var ErrUnexpectedStatus = errors.New("unexpected response status")
+	// 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 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
+// safeUnpackReleases unpacks an API response into an OCDS ReleasePackage,
-// irregularities (malformed dates, missing fields, invalid releases, etc).
+// 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
+// rateSleep sleeps the fetch worker until either the context is cancelled
-// nil duration will return immediately.
+// (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
+// fetchNext fetches the page pkg's "Next" link points to. It returns
-// next page. Error if there is no "Next" link - the caller can decide if they
+// ErrNoNext if there is no "Next" link; the caller can decide whether asking
-// have attempted to fetch next in error or not.
+// 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
+// fetchAll makes a call on the API. If there are any "Next" links, it calls
-// succession until the end has been reached.
+// them in succession until the end has been reached.
 func (ft *FindATender) fetchAll(opts RequestOptions) ([]ocds.Release, error) {
 	var releases []ocds.Release
 

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

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
+// Logger state shared by the terminal and file loggers.
-var fileLogger *os.File
+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 {
+	closeErr := Shutdown()
-		(*fileLogger).Close()
-	}
 
 	// 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.

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 {

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)
 }

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)

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,
 			)

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, errors.Join(fmt.Errorf("repairing %s: %w", path, err), f.Close())
-				return nil, fmt.Errorf("repairing %s: %w", path, err)
 			}
 		}
 	}
@@ -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
+// append stores r, syncing the file to disk if sync is true. A date earlier
-// change) is moved up so the order stays valid.
+// 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()

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"`

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"`

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()
+// AwaitTaskE is AwaitTask for a function that returns only an error.
-// 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 AwaitTaskE(ctx context.Context, fn func() error) error {
 	ch := make(chan error, 1)
 	go func() {