diff --git a/internal/cli/fetch/fts_once.go b/internal/cli/fetch/fts_once.go index 885e3dd..47e24ba 100644 --- a/internal/cli/fetch/fts_once.go +++ b/internal/cli/fetch/fts_once.go @@ -79,7 +79,7 @@ func commandFTSOnce(opts *cli.Options) *cobra.Command { if conf.Sources.FindATender.APIRoot == "" { return errors.New("sources.find_a_tender.api_root is not set") } - findATender := findatender.New(conf, conf.Sources.FindATender.APIRoot) + findATender := findatender.New(conf.Sources.FindATender.APIRoot) defer findATender.Shutdown() ignoreRateLimit, _ := cmd.Flags().GetBool("ignore-rate-limit") diff --git a/internal/cli/fetch/service.go b/internal/cli/fetch/service.go index b3c0339..a339086 100644 --- a/internal/cli/fetch/service.go +++ b/internal/cli/fetch/service.go @@ -53,7 +53,7 @@ func commandService(opts *cli.Options) *cobra.Command { stop() }() - return fetcher.Run(ctx, conf, since) + return fetcher.New(&conf.Sources).Run(ctx, since) }, } cmd.Flags().Time( diff --git a/internal/cli/serve/command.go b/internal/cli/serve/command.go index 762603d..bd06158 100644 --- a/internal/cli/serve/command.go +++ b/internal/cli/serve/command.go @@ -11,22 +11,27 @@ import ( "strconv" "strings" "syscall" + "time" "github.com/spf13/cobra" "git.avsdev.uk/Go/job-search/internal/cli" + "git.avsdev.uk/Go/job-search/internal/fetcher" "git.avsdev.uk/Go/job-search/internal/logging" "git.avsdev.uk/Go/job-search/internal/server" ) -// NewCommand returns the serve command, which serves the frontend, the REST -// API and the WebSocket endpoint. +// NewCommand returns the serve command, which serves the REST API and the +// WebSocket endpoint, and runs a worker for every enabled source. func NewCommand(opts *cli.Options) *cobra.Command { cmd := &cobra.Command{ Use: "serve", - Short: "Serve the frontend (/), REST API (/api) and WebSocket (/ws)", - Long: `Serve the embedded Vite frontend, the REST API under /api and the -WebSocket endpoint under /ws.`, + Short: "Serve the REST API (/api) and WebSocket (/ws), and fetch from sources", + Long: `Serve the REST API under /api and the WebSocket endpoint under /ws. + +It also runs a worker for every enabled source, as fetcher service does. A +source that stops with an error stops the server too, and the server +stopping stops the sources.`, PersistentPreRun: func(cmd *cobra.Command, args []string) { slog.Debug("command persistent pre-run", "command", "serve") }, @@ -108,7 +113,27 @@ WebSocket endpoint under /ws.`, return err } - return appServer.Run(ctx) + // runCtx stops whichever of the server and the sources is still + // running once the other has stopped. + runCtx, cancelRun := context.WithCancel(ctx) + defer cancelRun() + + sourceFetcher := fetcher.New(&conf.Sources) + fetcherDone := make(chan error, 1) + go func() { + fetchErr := sourceFetcher.Run(runCtx, time.Time{}) + // Only a failure stops the server: with no sources enabled, Run + // returns nil straight away. + if fetchErr != nil { + cancelRun() + } + fetcherDone <- fetchErr + }() + + serverErr := appServer.Run(runCtx) + cancelRun() + + return errors.Join(serverErr, <-fetcherDone) }, } diff --git a/internal/fetcher/fetcher.go b/internal/fetcher/fetcher.go index 9aa3084..6217ae0 100644 --- a/internal/fetcher/fetcher.go +++ b/internal/fetcher/fetcher.go @@ -16,30 +16,25 @@ import ( // to fetch; it wraps util.ErrWorkerNoWork. var ErrNoNewReleases = fmt.Errorf("%w: no new releases", util.ErrWorkerNoWork) -// Option configures Run. -type Option func(*options) - -// options holds the settings Options change. -type options struct { - log *slog.Logger -} +// Option configures a Fetcher. +type Option func(*Fetcher) // WithLogger makes Run, its workers and their clients log to logger instead // of slog.Default(). func WithLogger(logger *slog.Logger) Option { - return func(opts *options) { - opts.log = logger + return func(f *Fetcher) { + f.log = logger } } // logger returns the logger Run logs to: the given one, or slog.Default(). // It isn't tagged here, as it is also given to the clients the workers // create, which add their own component; see tagged. -func (opts options) logger() *slog.Logger { - if opts.log == nil { +func (f *Fetcher) logger() *slog.Logger { + if f.log == nil { return slog.Default() } - return opts.log + return f.log } // tagged returns logger tagged with the fetcher's component, for the lines @@ -48,35 +43,52 @@ func tagged(logger *slog.Logger) *slog.Logger { return logger.With("component", "fetcher") } -// runFunc runs one source's worker until ctx is cancelled. logger is -// untagged: the worker tags its own lines with tagged, and passes logger as -// it is to any client it creates. -type runFunc func( - ctx context.Context, - logger *slog.Logger, - conf *config.JobSearchConfig, - since time.Time, -) error +// Fetcher runs a worker for each enabled source. Create it with New and +// start it with Run. +type Fetcher struct { + conf *config.JSCSources + log *slog.Logger + started time.Time + + // backgroundCtx is the context the workers run under. backgroundCancel + // stops them: Run calls it when its ctx is cancelled, when a worker + // fails, and as it returns. + backgroundCtx context.Context + backgroundCancel context.CancelFunc +} + +// New returns a Fetcher for the sources conf configures. It logs to +// slog.Default() unless given WithLogger. +func New(conf *config.JSCSources, opts ...Option) *Fetcher { + fetcher := &Fetcher{ + conf: conf, + started: time.Now(), + } + for _, option := range opts { + option(fetcher) + } + + fetcher.backgroundCtx, fetcher.backgroundCancel = context.WithCancel( + context.Background(), + ) + + return fetcher +} // Run starts a worker for each enabled source and blocks until every worker // has stopped. Workers stop when ctx is cancelled, or when any worker stops // with an error, which starts a shutdown of all of them. The errors are -// logged as they happen and returned joined; nil means a clean shutdown. It -// logs to slog.Default() unless given WithLogger. -func Run( - ctx context.Context, - conf *config.JobSearchConfig, - since time.Time, - opts ...Option, -) error { - ctx, cancel := context.WithCancel(ctx) - defer cancel() +// logged as they happen and returned joined; nil means a clean shutdown. Run +// cancels the Fetcher's background context as it returns, so it can only be +// called once. +func (f *Fetcher) Run(ctx context.Context, since time.Time) error { + defer f.backgroundCancel() - var settings options - for _, opt := range opts { - opt(&settings) - } - logger := settings.logger() + // Cancelling ctx stops the workers, through the background context. + stopAfter := context.AfterFunc(ctx, f.backgroundCancel) + defer stopAfter() + + logger := f.logger() var ( wg sync.WaitGroup @@ -84,11 +96,14 @@ func Run( errs []error ) - start := func(name string, run runFunc) { + // start runs one source's worker until f.backgroundCtx is cancelled. Each + // worker is given logger untagged: it tags its own lines with tagged, and + // passes logger as it is to any client it creates. + start := func(name string, run func(ctx context.Context) error) { wg.Add(1) go func() { defer wg.Done() - if err := run(ctx, logger, conf, since); err != nil { + if err := run(f.backgroundCtx); err != nil { tagged(logger).Error( "source stopped, shutting down", "source", name, @@ -97,21 +112,29 @@ func Run( mu.Lock() errs = append(errs, fmt.Errorf("%s: %w", name, err)) mu.Unlock() - cancel() + f.backgroundCancel() } }() } - if conf.Sources.FindATender.Enabled { - start("FindATender", runFindAOpportunity) + if f.conf.FindATender.Enabled { + start("FindATender", func(ctx context.Context) error { + return runFindATender(ctx, logger, &f.conf.FindATender, since) + }) } - if conf.Sources.FTSMock.Enabled { - start("FTSMock", runFTSMock) + if f.conf.FTSMock.Enabled { + start("FTSMock", func(ctx context.Context) error { + return runFTSMock(ctx, logger, &f.conf.FTSMock, since) + }) } - if conf.Sources.Sleeper.Enabled { - start("Sleeper", runSleeper) + if f.conf.Sleeper.Enabled { + start("Sleeper", func(ctx context.Context) error { + return runSleeper(ctx, logger, &f.conf.Sleeper, since) + }) } + // Every worker has stopped once Wait returns, so errs is no longer + // written. wg.Wait() return errors.Join(errs...) diff --git a/internal/fetcher/fetcher_test.go b/internal/fetcher/fetcher_test.go index e8a51b7..12d4ace 100644 --- a/internal/fetcher/fetcher_test.go +++ b/internal/fetcher/fetcher_test.go @@ -58,18 +58,22 @@ func ftsServer( return srv, &hits } -func confFor(apiRoot string) *config.JobSearchConfig { - conf := &config.JobSearchConfig{} - conf.Sources.FindATender.APIRoot = apiRoot - conf.Sources.FTSMock.APIRoot = apiRoot - return conf +// fetchFunc fetches from one source at apiRoot. +type fetchFunc func(ctx context.Context, apiRoot string, since time.Time) error + +// fetchers are the sources' fetch functions, each given only its API root. +var fetchers = map[string]fetchFunc{ + "FindATender": func(ctx context.Context, apiRoot string, since time.Time) error { + conf := &config.JSCSourceFindATender{APIRoot: apiRoot} + return fetchFindATender(ctx, slog.Default(), conf, since) + }, + "FTSMock": func(ctx context.Context, apiRoot string, since time.Time) error { + conf := &config.JSCSourceFTSMock{APIRoot: apiRoot} + return fetchFTSMock(ctx, slog.Default(), conf, since) + }, } func TestFetchFunctions(t *testing.T) { - fetchers := map[string]runFunc{ - "FindAOpportunity": fetchFindAOpportunity, - "FTSMock": fetchFTSMock, - } tests := []struct { name string status int @@ -99,10 +103,7 @@ func TestFetchFunctions(t *testing.T) { t.Run(fname+"/"+tc.name, func(t *testing.T) { srv, hits := ftsServer(t, tc.status, tc.body) err := fetch( - context.Background(), - slog.Default(), - confFor(srv.URL), - time.Now().Add(-time.Hour), + context.Background(), srv.URL, time.Now().Add(-time.Hour), ) if !tc.check(err) { t.Fatalf("err = %v, want %s", err, tc.wantErr) @@ -115,23 +116,44 @@ func TestFetchFunctions(t *testing.T) { } } -func TestFetchUsesConfiguredAPIRoot(t *testing.T) { - fat, fatHits := ftsServer(t, http.StatusOK, emptyPackage) - mock, mockHits := ftsServer(t, http.StatusOK, emptyPackage) - conf := &config.JobSearchConfig{} - conf.Sources.FindATender.APIRoot = fat.URL - conf.Sources.FTSMock.APIRoot = mock.URL +// Run gives each worker its own source's configuration: with only one source +// enabled, only its API root is called. +func TestRunUsesEachSourcesAPIRoot(t *testing.T) { + for _, enabled := range []string{"FindATender", "FTSMock"} { + fat, fatHits := ftsServer(t, http.StatusOK, emptyPackage) + mock, mockHits := ftsServer(t, http.StatusOK, emptyPackage) + sources := &config.JSCSources{} + sources.FindATender.APIRoot = fat.URL + sources.FindATender.Interval = time.Hour + sources.FTSMock.APIRoot = mock.URL + sources.FTSMock.Interval = time.Hour + wanted, other := fatHits, mockHits + if enabled == "FindATender" { + sources.FindATender.Enabled = true + } else { + sources.FTSMock.Enabled = true + wanted, other = mockHits, fatHits + } - fetchFindAOpportunity(context.Background(), slog.Default(), conf, time.Time{}) - if fatHits.Load() != 1 || mockHits.Load() != 0 { - t.Fatalf( - "FindAOpportunity hits: fat=%d mock=%d", fatHits.Load(), mockHits.Load(), - ) - } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + done <- New(sources).Run(ctx, time.Time{}) + }() - fetchFTSMock(context.Background(), slog.Default(), conf, time.Time{}) - if fatHits.Load() != 1 || mockHits.Load() != 1 { - t.Fatalf("FTSMock hits: fat=%d mock=%d", fatHits.Load(), mockHits.Load()) + deadline := time.Now().Add(5 * time.Second) + for wanted.Load() == 0 && time.Now().Before(deadline) { + time.Sleep(5 * time.Millisecond) + } + cancel() + <-done + + if wanted.Load() == 0 || other.Load() != 0 { + t.Errorf( + "%s enabled: its root hit %d times, the other's %d; want >0 and 0", + enabled, wanted.Load(), other.Load(), + ) + } } } @@ -139,9 +161,8 @@ func TestFetchCancelled(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // Unroutable root: the request should never be waited on. - conf := confFor("http://192.0.2.1:9") start := time.Now() - err := fetchFTSMock(ctx, slog.Default(), conf, time.Time{}) + err := fetchers["FTSMock"](ctx, "http://192.0.2.1:9", time.Time{}) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) } @@ -175,9 +196,8 @@ func TestSleepCompletes(t *testing.T) { // one holds shutdown open until the worker's shutdown timeout. func TestRunSleeperShutdown(t *testing.T) { for _, block := range []bool{false, true} { - conf := &config.JobSearchConfig{} - conf.Sources.Sleeper.Interval = time.Hour - conf.Sources.Sleeper.BlockShutdown = block + conf := &config.JSCSourceSleeper{BlockShutdown: block} + conf.Interval = time.Hour ctx, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) @@ -202,17 +222,24 @@ func TestRunSleeperShutdown(t *testing.T) { } func TestRunWorkersRejectZeroInterval(t *testing.T) { - conf := &config.JobSearchConfig{} - runs := map[string]runFunc{ - "FindAOpportunity": runFindAOpportunity, - "FTSMock": runFTSMock, - "Sleeper": runSleeper, + ctx, logger := context.Background(), slog.Default() + runs := map[string]func() error{ + "FindATender": func() error { + conf := &config.JSCSourceFindATender{APIRoot: "http://127.0.0.1:1"} + return runFindATender(ctx, logger, conf, time.Time{}) + }, + "FTSMock": func() error { + conf := &config.JSCSourceFTSMock{APIRoot: "http://127.0.0.1:1"} + return runFTSMock(ctx, logger, conf, time.Time{}) + }, + "Sleeper": func() error { + return runSleeper(ctx, logger, &config.JSCSourceSleeper{}, time.Time{}) + }, } for name, run := range runs { - if err := run( - context.Background(), slog.Default(), conf, time.Time{}, - ); err == nil { - t.Errorf("%s: expected an error for a zero interval", name) + err := run() + if err == nil || !strings.Contains(err.Error(), "interval") { + t.Errorf("%s: err = %v; want an error for a zero interval", name, err) } } } @@ -227,13 +254,13 @@ func TestRunLogsSourceErrors(t *testing.T) { }) // A zero interval makes each worker fail straight away. - conf := &config.JobSearchConfig{} - conf.Sources.FindATender.Enabled = true - conf.Sources.FindATender.APIRoot = "http://127.0.0.1:1" - conf.Sources.FTSMock.Enabled = true - conf.Sources.FTSMock.APIRoot = "http://127.0.0.1:1" - conf.Sources.Sleeper.Enabled = true - err := Run(context.Background(), conf, time.Time{}) + sources := &config.JSCSources{} + sources.FindATender.Enabled = true + sources.FindATender.APIRoot = "http://127.0.0.1:1" + sources.FTSMock.Enabled = true + sources.FTSMock.APIRoot = "http://127.0.0.1:1" + sources.Sleeper.Enabled = true + err := New(sources).Run(context.Background(), time.Time{}) for _, name := range []string{"FindATender", "FTSMock", "Sleeper"} { if !strings.Contains(buf.String(), "source="+name) { @@ -256,24 +283,26 @@ func TestRunMissingAPIRoot(t *testing.T) { quiet := slog.New(slog.DiscardHandler) tests := []struct { name string - enable func(*config.JobSearchConfig) + enable func(*config.JSCSources) key string }{ - {"find_a_tender", func(conf *config.JobSearchConfig) { - conf.Sources.FindATender.Enabled = true - conf.Sources.FindATender.Interval = time.Hour + {"find_a_tender", func(sources *config.JSCSources) { + sources.FindATender.Enabled = true + sources.FindATender.Interval = time.Hour }, "sources.find_a_tender.api_root"}, - {"fts_mock", func(conf *config.JobSearchConfig) { - conf.Sources.FTSMock.Enabled = true - conf.Sources.FTSMock.Interval = time.Hour + {"fts_mock", func(sources *config.JSCSources) { + sources.FTSMock.Enabled = true + sources.FTSMock.Interval = time.Hour }, "sources.fts_mock.api_root"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - conf := &config.JobSearchConfig{} - tt.enable(conf) + sources := &config.JSCSources{} + tt.enable(sources) - err := Run(context.Background(), conf, time.Time{}, WithLogger(quiet)) + err := New(sources, WithLogger(quiet)).Run( + context.Background(), time.Time{}, + ) if err == nil || !strings.Contains(err.Error(), tt.key) { t.Errorf("Run = %v; want an error naming %s", err, tt.key) } @@ -284,9 +313,8 @@ func TestRunMissingAPIRoot(t *testing.T) { func TestRunNoSourcesEnabled(t *testing.T) { done := make(chan struct{}) go func() { - if err := Run( - context.Background(), &config.JobSearchConfig{}, time.Time{}, - ); err != nil { + err := New(&config.JSCSources{}).Run(context.Background(), time.Time{}) + if err != nil { t.Errorf("Run = %v; want nil", err) } close(done) @@ -303,16 +331,16 @@ func TestRunNoSourcesEnabled(t *testing.T) { // cancelled. func TestRunShutsDownOnWorkerError(t *testing.T) { mock, mockHits := ftsServer(t, http.StatusOK, emptyPackage) - conf := &config.JobSearchConfig{} - conf.Sources.FTSMock.Enabled = true - conf.Sources.FTSMock.Interval = time.Hour - conf.Sources.FTSMock.APIRoot = mock.URL + sources := &config.JSCSources{} + sources.FTSMock.Enabled = true + sources.FTSMock.Interval = time.Hour + sources.FTSMock.APIRoot = mock.URL // The sleeper fails straight away: a zero interval is invalid. - conf.Sources.Sleeper.Enabled = true + sources.Sleeper.Enabled = true done := make(chan error, 1) go func() { - done <- Run(context.Background(), conf, time.Now().Add(-time.Hour)) + done <- New(sources).Run(context.Background(), time.Now().Add(-time.Hour)) }() select { @@ -337,20 +365,21 @@ func TestRunShutsDownOnWorkerError(t *testing.T) { func TestRunEnabledSources(t *testing.T) { fat, fatHits := ftsServer(t, http.StatusOK, onePackage) mock, mockHits := ftsServer(t, http.StatusOK, emptyPackage) - conf := &config.JobSearchConfig{} - conf.Sources.FindATender.Enabled = true - conf.Sources.FindATender.Interval = time.Hour - conf.Sources.FindATender.APIRoot = fat.URL - conf.Sources.FTSMock.Enabled = true - conf.Sources.FTSMock.Interval = time.Hour - conf.Sources.FTSMock.APIRoot = mock.URL - conf.Sources.Sleeper.Enabled = true - conf.Sources.Sleeper.Interval = time.Hour + sources := &config.JSCSources{} + sources.FindATender.Enabled = true + sources.FindATender.Interval = time.Hour + sources.FindATender.APIRoot = fat.URL + sources.FTSMock.Enabled = true + sources.FTSMock.Interval = time.Hour + sources.FTSMock.APIRoot = mock.URL + sources.Sleeper.Enabled = true + sources.Sleeper.Interval = time.Hour ctx, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) go func() { - if err := Run(ctx, conf, time.Now().Add(-time.Hour)); err != nil { + err := New(sources).Run(ctx, time.Now().Add(-time.Hour)) + if err != nil { t.Errorf("Run = %v; want nil after cancellation", err) } close(done) diff --git a/internal/fetcher/worker_findatender.go b/internal/fetcher/worker_findatender.go index 32811d5..2c89d86 100644 --- a/internal/fetcher/worker_findatender.go +++ b/internal/fetcher/worker_findatender.go @@ -12,43 +12,43 @@ import ( "git.avsdev.uk/Go/job-search/internal/util" ) -// runFindAOpportunity polls Find a Tender every configured interval until ctx +// runFindATender polls Find a Tender every configured interval until ctx // is cancelled. -func runFindAOpportunity( +func runFindATender( ctx context.Context, logger *slog.Logger, - conf *config.JobSearchConfig, + conf *config.JSCSourceFindATender, since time.Time, ) error { - if conf.Sources.FindATender.APIRoot == "" { + if conf.APIRoot == "" { return errors.New("fetcher: sources.find_a_tender.api_root is not set") } worker := util.Worker{ Logger: tagged(logger), - Name: "FindAOpportunity", - Interval: conf.Sources.FindATender.Interval, + Name: "FindATender", + Interval: conf.Interval, Task: func(ctx context.Context, since time.Time) error { - return fetchFindAOpportunity(ctx, logger, conf, since) + return fetchFindATender(ctx, logger, conf, since) }, } return worker.Run(ctx, since) } -// fetchFindAOpportunity fetches everything updated since since from Find a +// fetchFindATender fetches everything updated since since from Find a // Tender. Finding nothing returns ErrNoNewReleases, so the worker still moves // its checkpoint on. -func fetchFindAOpportunity( +func fetchFindATender( ctx context.Context, logger *slog.Logger, - conf *config.JobSearchConfig, + conf *config.JSCSourceFindATender, since time.Time, ) error { opportunities, err := util.AwaitTask( ctx, func() ([]*types.Opportunity, error) { findATender := findatender.New( - conf, conf.Sources.FindATender.APIRoot, findatender.WithLogger(logger), + conf.APIRoot, findatender.WithLogger(logger), ) defer findATender.Shutdown() return findATender.With(ctx).FetchAll(since, true) diff --git a/internal/fetcher/worker_ftsmock.go b/internal/fetcher/worker_ftsmock.go index a46839b..a35ec09 100644 --- a/internal/fetcher/worker_ftsmock.go +++ b/internal/fetcher/worker_ftsmock.go @@ -17,17 +17,17 @@ import ( func runFTSMock( ctx context.Context, logger *slog.Logger, - conf *config.JobSearchConfig, + conf *config.JSCSourceFTSMock, since time.Time, ) error { - if conf.Sources.FTSMock.APIRoot == "" { + if conf.APIRoot == "" { return errors.New("fetcher: sources.fts_mock.api_root is not set") } worker := util.Worker{ Logger: tagged(logger), Name: "FTSMock", - Interval: conf.Sources.FTSMock.Interval, + Interval: conf.Interval, Task: func(ctx context.Context, since time.Time) error { return fetchFTSMock(ctx, logger, conf, since) }, @@ -41,14 +41,14 @@ func runFTSMock( func fetchFTSMock( ctx context.Context, logger *slog.Logger, - conf *config.JobSearchConfig, + conf *config.JSCSourceFTSMock, since time.Time, ) error { opportunities, err := util.AwaitTask( ctx, func() ([]*types.Opportunity, error) { findATender := findatender.New( - conf, conf.Sources.FTSMock.APIRoot, findatender.WithLogger(logger), + conf.APIRoot, findatender.WithLogger(logger), ) defer findATender.Shutdown() return findATender.With(ctx).FetchAll(since, true) diff --git a/internal/fetcher/worker_sleeper.go b/internal/fetcher/worker_sleeper.go index 10e40f2..bce43ba 100644 --- a/internal/fetcher/worker_sleeper.go +++ b/internal/fetcher/worker_sleeper.go @@ -17,14 +17,14 @@ const sleepDuration = 60 * time.Second func runSleeper( ctx context.Context, logger *slog.Logger, - conf *config.JobSearchConfig, + conf *config.JSCSourceSleeper, since time.Time, ) error { - blockShutdown := conf.Sources.Sleeper.BlockShutdown + blockShutdown := conf.BlockShutdown worker := util.Worker{ Logger: tagged(logger), Name: "Sleeper", - Interval: conf.Sources.Sleeper.Interval, + Interval: conf.Interval, Task: func(taskCtx context.Context, since time.Time) error { if blockShutdown { return sleep(taskCtx, true, sleepDuration) diff --git a/internal/findatender/api.go b/internal/findatender/api.go index 40a194b..e341033 100644 --- a/internal/findatender/api.go +++ b/internal/findatender/api.go @@ -307,10 +307,10 @@ func closeBody(res *resty.Response) { } // fetchURI performs a single GET of uri, using findATender's context, and -// traces it at debug level. Unlike the method of the same name it does not -// check the response status. +// traces it when debug logging is on (logging.IsDebugEnabled). Unlike the +// method of the same name it does not check the response status. func fetchURI(findATender *FindATender, uri string) (*resty.Response, error) { - withTrace := findATender.config.Logging.LogLevel() <= slog.LevelDebug + withTrace := logging.IsDebugEnabled() logger := findATender.logger() logger.Info("fetching", "uri", uri) diff --git a/internal/findatender/api_test.go b/internal/findatender/api_test.go index 86fd3a6..f69980c 100644 --- a/internal/findatender/api_test.go +++ b/internal/findatender/api_test.go @@ -17,18 +17,10 @@ import ( resty "resty.dev/v3" - "git.avsdev.uk/Go/job-search/internal/config" "git.avsdev.uk/Go/job-search/internal/findatender/ocds" + "git.avsdev.uk/Go/job-search/internal/logging" ) -func testConfig(level string) *config.JobSearchConfig { - return &config.JobSearchConfig{ - Logging: config.JSCLogging{ - Level: level, - }, - } -} - // quietLogs discards slog output for the duration of the test, or sends it // through h when given. func quietLogs(t *testing.T, h slog.Handler) { @@ -44,16 +36,24 @@ func quietLogs(t *testing.T, h slog.Handler) { } // newTestFT starts h on a test server and returns a FindATender pointed at it. +// With debug, the logging level is debug for the test, so requests are +// traced. func newTestFT( - t *testing.T, level string, h http.HandlerFunc, + t *testing.T, debug bool, h http.HandlerFunc, ) (*FindATender, *httptest.Server) { t.Helper() quietLogs(t, nil) + if debug { + logging.SetLevel(slog.LevelDebug) + t.Cleanup(func() { + logging.SetLevel(slog.LevelInfo) + }) + } srv := httptest.NewServer(h) t.Cleanup(srv.Close) - ft := New(testConfig(level), srv.URL) + ft := New(srv.URL) t.Cleanup(ft.Shutdown) return ft, srv @@ -97,7 +97,7 @@ func TestRateSleep(t *testing.T) { func TestFetchWithOptsBuildsURI(t *testing.T) { var gotPath, gotQuery string - ft, _ := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { gotPath, gotQuery = r.URL.Path, r.URL.RawQuery io.WriteString(w, packageJSON("")) }) @@ -133,7 +133,7 @@ func TestFetchWithOptsBuildsURI(t *testing.T) { // A zero Since defaults to 24 hours ago. func TestFetchWithOptsDefaultSince(t *testing.T) { var gotQuery string - ft, _ := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { gotQuery = r.URL.Query().Get("updatedFrom") io.WriteString(w, packageJSON("")) }) @@ -188,7 +188,7 @@ func TestFetchURIRateLimit(t *testing.T) { var hits atomic.Int32 ft, srv := newTestFT( t, - "warn", + false, func(w http.ResponseWriter, r *http.Request) { if hits.Add(1) == 1 { if tt.retryAfter != "" { @@ -228,7 +228,7 @@ func TestFetchURIRateLimit(t *testing.T) { } func TestFetchURIRateLimitCancelled(t *testing.T) { - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "3600") w.WriteHeader(http.StatusTooManyRequests) }) @@ -252,7 +252,7 @@ func TestFetchURIRateLimitCancelled(t *testing.T) { func TestFetchURIRateLimitTwice(t *testing.T) { // A second 429 after waiting is reported as ErrRateLimitExceeded. var hits atomic.Int32 - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { hits.Add(1) w.Header().Set("Retry-After", "0") w.WriteHeader(http.StatusTooManyRequests) @@ -274,7 +274,7 @@ func TestFetchURIUnexpectedStatus(t *testing.T) { t.Run(http.StatusText(status), func(t *testing.T) { ft, srv := newTestFT( t, - "warn", + false, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(status) io.WriteString(w, packageJSON("", releaseJSON("ocds-1", "1"))) @@ -294,7 +294,7 @@ func TestFetchURIUnexpectedStatus(t *testing.T) { // A non-2xx after waiting out a 429 is reported as ErrUnexpectedStatus. func TestFetchURIRateLimitThenError(t *testing.T) { var hits atomic.Int32 - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { if hits.Add(1) == 1 { w.Header().Set("Retry-After", "0") w.WriteHeader(http.StatusTooManyRequests) @@ -309,7 +309,7 @@ func TestFetchURIRateLimitThenError(t *testing.T) { } func TestFetchURIConnectionError(t *testing.T) { - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { }) srv.Close() ft.httpClient.SetRetryCount(0) @@ -319,7 +319,7 @@ func TestFetchURIConnectionError(t *testing.T) { } func TestFetchURICancelledContext(t *testing.T) { - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, packageJSON("")) }) ctx, cancel := context.WithCancel(context.Background()) @@ -346,7 +346,7 @@ func TestFetchURIDebugTrace(t *testing.T) { t.Run(name, func(t *testing.T) { ft, _ := newTestFT( t, - "debug", + true, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, packageJSON("", releaseJSON("ocds-1", "1"))) }, @@ -379,7 +379,7 @@ func TestFetchURIDebugTrace(t *testing.T) { func TestFetchURIDebugTraceConnectionError(t *testing.T) { ft, srv := newTestFT( t, - "debug", + true, func(w http.ResponseWriter, r *http.Request) { }, ) @@ -392,7 +392,7 @@ func TestFetchURIDebugTraceConnectionError(t *testing.T) { func TestFetchNext(t *testing.T) { var gotPath string - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path io.WriteString(w, packageJSON("")) }) @@ -428,7 +428,7 @@ func TestFetchNext(t *testing.T) { // responseFor fetches body from a test server so it can be unpacked. func responseFor(t *testing.T, body string) *resty.Response { t.Helper() - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, body) }) res, err := ft.fetchURI(srv.URL, false) @@ -551,7 +551,7 @@ func pagedServer( t.Helper() var hits atomic.Int32 var srvURL string - ft, srv := newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, srv := newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { hits.Add(1) i := 0 @@ -598,7 +598,7 @@ func TestFetchOnceErrors(t *testing.T) { t.Errorf("empty page: err = %v, want ErrNoReleases", err) } - ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ = newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }) if _, err := ft.fetchOnce(RequestOptions{ @@ -607,7 +607,7 @@ func TestFetchOnceErrors(t *testing.T) { t.Error("bad body: expected an error") } - ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ = newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTooManyRequests) }) if _, err := ft.fetchOnce( @@ -655,7 +655,7 @@ func TestFetchAllErrors(t *testing.T) { } // First page fails. - ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ = newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTooManyRequests) }) if _, err := ft.fetchAll( @@ -666,7 +666,7 @@ func TestFetchAllErrors(t *testing.T) { t.Errorf("first page rate limited: err = %v", err) } - ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ = newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }) if _, err := ft.fetchAll(RequestOptions{ @@ -688,7 +688,7 @@ func TestFetchAllErrors(t *testing.T) { var srvURL string ft, srv := newTestFT( t, - "warn", + false, func(w http.ResponseWriter, r *http.Request) { if r.URL.Query().Get("page") == "" { io.WriteString( @@ -730,7 +730,7 @@ func TestFetchAllRepeatedNextLink(t *testing.T) { var srvURL string ft, srv := newTestFT( t, - "error", + false, func(w http.ResponseWriter, r *http.Request) { if hits.Add(1) > 10 { t.Error("fetchAll did not stop following next links") diff --git a/internal/findatender/findatender.go b/internal/findatender/findatender.go index fa82fa5..60cfc26 100644 --- a/internal/findatender/findatender.go +++ b/internal/findatender/findatender.go @@ -7,7 +7,6 @@ import ( resty "resty.dev/v3" - "git.avsdev.uk/Go/job-search/internal/config" "git.avsdev.uk/Go/job-search/internal/types" ) @@ -15,7 +14,6 @@ import ( // server). Create it with New and call Shutdown when finished with it. type FindATender struct { httpClient *resty.Client - config *config.JobSearchConfig apiRoot string ctx context.Context log *slog.Logger @@ -34,11 +32,8 @@ func WithLogger(logger *slog.Logger) Option { // New returns a client for the OCDS API at apiRoot. Requests time out after // 60 seconds and are retried once. Call Shutdown when finished with it. It // logs to slog.Default() unless given WithLogger. -func New( - conf *config.JobSearchConfig, apiRoot string, opts ...Option, -) *FindATender { +func New(apiRoot string, opts ...Option) *FindATender { findATender := &FindATender{ - config: conf, apiRoot: apiRoot, } for _, opt := range opts { @@ -61,12 +56,6 @@ func (ft *FindATender) logger() *slog.Logger { return log.With("component", "findatender") } -// ReloadConfig replaces the configuration the client reads, such as the log -// level. The HTTP client and API root set by New are not changed. -func (ft *FindATender) ReloadConfig(conf *config.JobSearchConfig) { - ft.config = conf -} - // With returns a copy of the client whose requests use ctx, so they stop // when it is cancelled. A nil ctx returns the client unchanged. func (ft *FindATender) With(ctx context.Context) *FindATender { diff --git a/internal/findatender/findatender_test.go b/internal/findatender/findatender_test.go index 2284df6..eb08b6c 100644 --- a/internal/findatender/findatender_test.go +++ b/internal/findatender/findatender_test.go @@ -14,12 +14,11 @@ import ( ) func TestNew(t *testing.T) { - conf := testConfig("warn") - ft := New(conf, "https://example/api") + ft := New("https://example/api") defer ft.Shutdown() - if ft.config != conf || ft.apiRoot != "https://example/api" { - t.Errorf("New did not keep its arguments: %+v", ft) + if ft.apiRoot != "https://example/api" { + t.Errorf("New did not keep its API root: %+v", ft) } if ft.httpClient == nil { t.Fatal("no http client") @@ -37,18 +36,8 @@ func TestNew(t *testing.T) { } } -func TestReloadConfig(t *testing.T) { - ft := New(testConfig("warn"), "") - defer ft.Shutdown() - conf := testConfig("debug") - ft.ReloadConfig(conf) - if ft.config != conf { - t.Error("config not replaced") - } -} - func TestWith(t *testing.T) { - ft := New(testConfig("warn"), "") + ft := New("") defer ft.Shutdown() // Passing nil on purpose to test that case. @@ -69,8 +58,8 @@ func TestWith(t *testing.T) { if ft.ctx != nil { t.Error("With changed the original") } - if got.httpClient != ft.httpClient || got.config != ft.config { - t.Error("copy should share the client and config") + if got.httpClient != ft.httpClient { + t.Error("copy should share the client") } } @@ -128,7 +117,7 @@ func TestFetchAllPublic(t *testing.T) { } } - ft, _ = newTestFT(t, "warn", func(w http.ResponseWriter, r *http.Request) { + ft, _ = newTestFT(t, false, func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "not json") }) if _, err := ft.FetchAll(time.Now(), false); err == nil {