diff --git a/README.md b/README.md index ed0e915..3704626 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,8 @@ Global flags: `--config`/`-c` (config file), `--debug`/`-d` and | `job-search migrate up` | Apply pending migrations (`--steps N` for only N; `--create` to create a missing database file without asking) | | `job-search migrate down --steps N` | Roll back the last N migrations (`--steps 0` rolls back all of them, deleting all data, and needs `--yes`) | | `job-search migrate status` | Show the database version and which migrations are applied or pending | +| `job-search skip-rules check [file]` | Check a skip rules file (default: `sources.skip_rules_file`) | +| `job-search skip-rules apply [file]` | Mark the stored opportunities that match the skip rules Skipped, listing them (`--dry-run` only lists them) | | `job-search version` | Print version information | `job-search serve` describes its REST API in OpenAPI 3.1 at @@ -93,6 +95,14 @@ The `migrate` commands work on the database in `database.file`. The store refuses to open a database that isn't at this build's schema version, so run `job-search migrate` after installing a new version. +### Skip rules +`sources.skip_rules_file` names a JSON file of rules that mark new +opportunities Skipped as `fetcher service` and `job-search serve` fetch them, +by buyer, title words or CPV codes. The file is read again when it changes; +an invalid edit is logged and the last good rules kept. Opportunities already +stored are only changed by `job-search skip-rules apply`. See +[internal/skiprules](internal/skiprules/README.md) for the format. + ## Fetch tool `fetcher` reads the same configuration as `job-search`: @@ -164,6 +174,7 @@ second one fails with "address already in use". | `internal/cli` | Cobra commands and shared CLI setup | | `internal/config` | Configuration loading, validation and editing | | `internal/fetcher` | Runs a worker per enabled source | +| `internal/skiprules` | The rules that mark new opportunities Skipped (see [internal/skiprules](internal/skiprules/README.md)) | | `internal/store` | SQLite storage of opportunities, and its migrations (see [internal/store](internal/store/README.md)) | | `internal/findatender` | FTS API client and conversion from OCDS to JobSearch types | | `internal/types` | JobSearch's own opportunity types | diff --git a/cmd/jobsearch/main.go b/cmd/jobsearch/main.go index 0e7a498..12bbb7b 100644 --- a/cmd/jobsearch/main.go +++ b/cmd/jobsearch/main.go @@ -29,6 +29,7 @@ import ( "git.avsdev.uk/Go/job-search/internal/cli/config" "git.avsdev.uk/Go/job-search/internal/cli/migrate" "git.avsdev.uk/Go/job-search/internal/cli/serve" + "git.avsdev.uk/Go/job-search/internal/cli/skiprules" "git.avsdev.uk/Go/job-search/internal/cli/version" ) @@ -75,6 +76,7 @@ opportunities. config.NewCommand(opts), migrate.NewCommand(opts), serve.NewCommand(opts), + skiprules.NewCommand(opts), ) return root diff --git a/cmd/jobsearch/main_test.go b/cmd/jobsearch/main_test.go index 865f1b6..ca3c27d 100644 --- a/cmd/jobsearch/main_test.go +++ b/cmd/jobsearch/main_test.go @@ -18,7 +18,7 @@ func TestCommandTree(t *testing.T) { names = append(names, c.Name()) } - commands := []string{"config", "migrate", "serve", "version"} + commands := []string{"config", "migrate", "serve", "skip-rules", "version"} for _, want := range commands { if !slices.Contains(names, want) { t.Errorf("job-search has no %q command; has %v", want, names) diff --git a/internal/cli/fetch/service.go b/internal/cli/fetch/service.go index 58054c4..d80c52a 100644 --- a/internal/cli/fetch/service.go +++ b/internal/cli/fetch/service.go @@ -54,7 +54,11 @@ func commandService(opts *cli.Options) *cobra.Command { stop() }() - var fetcherOptions []fetcher.Option + fetcherOptions, err := cli.SkipRulesOptions(conf) + if err != nil { + return err + } + opportunityStore, openErr := cli.OpenConfiguredStore(ctx, conf) switch { case errors.Is(openErr, cli.ErrNoDatabase): diff --git a/internal/cli/serve/command.go b/internal/cli/serve/command.go index 0fe8521..8f93f48 100644 --- a/internal/cli/serve/command.go +++ b/internal/cli/serve/command.go @@ -110,7 +110,11 @@ set, the API serves the stored opportunities under /api/opportunities.`, // The store is shared by the sources, which save to it, and the API, // which serves it. - var fetcherOptions []fetcher.Option + fetcherOptions, err := cli.SkipRulesOptions(conf) + if err != nil { + return err + } + opportunityStore, openErr := cli.OpenConfiguredStore(ctx, conf) switch { case errors.Is(openErr, cli.ErrNoDatabase): diff --git a/internal/cli/skiprules.go b/internal/cli/skiprules.go new file mode 100644 index 0000000..3a816f4 --- /dev/null +++ b/internal/cli/skiprules.go @@ -0,0 +1,27 @@ +package cli + +import ( + "fmt" + + "git.avsdev.uk/Go/job-search/internal/config" + "git.avsdev.uk/Go/job-search/internal/fetcher" + "git.avsdev.uk/Go/job-search/internal/skiprules" +) + +// SkipRulesOptions opens the rules file at sources.skip_rules_file and +// returns the fetcher option that applies it, or none if no file is +// configured. A missing or invalid file is an error, so a command doesn't +// start without the rules it was given. +func SkipRulesOptions(conf *config.JobSearchConfig) ([]fetcher.Option, error) { + path := conf.Sources.SkipRulesFile + if path == "" { + return nil, nil + } + + rulesFile, err := skiprules.Open(path) + if err != nil { + return nil, fmt.Errorf("sources.skip_rules_file: %w", err) + } + + return []fetcher.Option{fetcher.WithSkipRules(rulesFile)}, nil +} diff --git a/internal/cli/skiprules/apply.go b/internal/cli/skiprules/apply.go new file mode 100644 index 0000000..4ddfa71 --- /dev/null +++ b/internal/cli/skiprules/apply.go @@ -0,0 +1,174 @@ +package skiprules + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "text/tabwriter" + + "github.com/spf13/cobra" + + "git.avsdev.uk/Go/job-search/internal/cli" + "git.avsdev.uk/Go/job-search/internal/skiprules" + "git.avsdev.uk/Go/job-search/internal/store" + "git.avsdev.uk/Go/job-search/internal/types" +) + +// applyPageSize is how many opportunities apply reads at a time. +const applyPageSize = 500 + +// match is a stored opportunity the rules skip, and the filter that does. +type match struct { + opportunity types.Opportunity + filter string +} + +// commandApply returns the "apply" command, which marks the stored +// opportunities that match the rules Skipped. +func commandApply(opts *cli.Options) *cobra.Command { + cmd := &cobra.Command{ + Use: "apply [file]", + Short: "Mark stored opportunities that match skip rules Skipped", + Long: `Mark the stored opportunities that match the skip rules Skipped, +listing each one. The rules are read from the file given, or else from +sources.skip_rules_file. Only the Skipped mark is ever set, and never +cleared: opportunities already skipped are left alone. With --dry-run, the +matches are listed but nothing is changed. + +Each change is recorded for clients to catch up from, but isn't pushed to +open browsers as a running "serve" pushes its own changes.`, + Args: cobra.MaximumNArgs(1), + PersistentPreRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent pre-run", "command", "skip-rules.apply") + }, + PersistentPostRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent post-run", "command", "skip-rules.apply") + }, + RunE: func(cmd *cobra.Command, args []string) (err error) { + slog.Debug("command run", "command", "skip-rules.apply") + + dryRun, _ := cmd.Flags().GetBool("dry-run") + + conf, err := opts.RunSetup(cmd, true, true) + if err != nil { + return fmt.Errorf("load configuration: %w", err) + } + + rules, _, err := loadRules(conf, args) + if err != nil { + return err + } + + opportunityStore, err := cli.OpenConfiguredStore(cmd.Context(), conf) + if err != nil { + return err + } + defer func() { + err = errors.Join(err, opportunityStore.Close()) + }() + + matches, err := findMatches(cmd.Context(), opportunityStore, rules) + if err != nil { + return err + } + err = printMatches(cmd.OutOrStdout(), matches) + if err != nil { + return err + } + + if dryRun { + _, err = fmt.Fprintf( + cmd.OutOrStdout(), + "%d opportunities would be skipped (dry run)\n", + len(matches), + ) + return err + } + + err = skipAll(cmd.Context(), opportunityStore, matches) + if err != nil { + return err + } + + _, err = fmt.Fprintf( + cmd.OutOrStdout(), "Skipped %d opportunities\n", len(matches), + ) + return err + }, + } + cmd.Flags().Bool("dry-run", false, "list the matches without changing them") + + return cmd +} + +// findMatches returns the stored opportunities not yet skipped that rules +// match, newest first. They are all read before any is changed, so changing +// them doesn't move the pages being read. +func findMatches( + ctx context.Context, opportunityStore *store.Store, rules *skiprules.Rules, +) ([]match, error) { + var matches []match + for offset := 0; ; offset += applyPageSize { + page, err := opportunityStore.ListOpportunities( + ctx, + store.OpportunityFilter{ + Skipped: store.MarkUnset, + Limit: applyPageSize, + Offset: offset, + }, + ) + if err != nil { + return nil, fmt.Errorf("listing opportunities: %w", err) + } + + for i := range page { + filter, matched := rules.Match(&page[i]) + if matched { + matches = append(matches, match{ + opportunity: page[i], + filter: filter, + }) + } + } + + if len(page) < applyPageSize { + return matches, nil + } + } +} + +// printMatches lists matches, one per line: UUID, filter and title. +func printMatches(out io.Writer, matches []match) error { + table := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) + for _, found := range matches { + _, err := fmt.Fprintf( + table, + "%s\t%s\t%s\n", + found.opportunity.UUID, + found.filter, + found.opportunity.Title, + ) + if err != nil { + return err + } + } + return table.Flush() +} + +// skipAll sets the Skipped mark of every match, keeping its other marks. +func skipAll( + ctx context.Context, opportunityStore *store.Store, matches []match, +) error { + for _, found := range matches { + marks := found.opportunity.Marks + marks.Skipped = true + if err := opportunityStore.SetMarks(ctx, marks); err != nil { + return fmt.Errorf( + "skipping %s: %w", found.opportunity.UUID, err, + ) + } + } + return nil +} diff --git a/internal/cli/skiprules/check.go b/internal/cli/skiprules/check.go new file mode 100644 index 0000000..425ac80 --- /dev/null +++ b/internal/cli/skiprules/check.go @@ -0,0 +1,44 @@ +package skiprules + +import ( + "fmt" + "log/slog" + + "github.com/spf13/cobra" + + "git.avsdev.uk/Go/job-search/internal/cli" +) + +// commandCheck returns the "check" command, which reports whether a rules +// file is valid. +func commandCheck(opts *cli.Options) *cobra.Command { + return &cobra.Command{ + Use: "check [file]", + Short: "Check a skip rules file (default: sources.skip_rules_file)", + Args: cobra.MaximumNArgs(1), + PersistentPreRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent pre-run", "command", "skip-rules.check") + }, + PersistentPostRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent post-run", "command", "skip-rules.check") + }, + RunE: func(cmd *cobra.Command, args []string) error { + slog.Debug("command run", "command", "skip-rules.check") + + conf, err := opts.RunSetup(cmd, true, true) + if err != nil { + return fmt.Errorf("load configuration: %w", err) + } + + rules, path, err := loadRules(conf, args) + if err != nil { + return err + } + + _, err = fmt.Fprintf( + cmd.OutOrStdout(), "%s is valid: %s\n", path, describeRules(rules), + ) + return err + }, + } +} diff --git a/internal/cli/skiprules/command.go b/internal/cli/skiprules/command.go new file mode 100644 index 0000000..5f11de2 --- /dev/null +++ b/internal/cli/skiprules/command.go @@ -0,0 +1,72 @@ +// Package skiprules provides the "skip-rules" command and its subcommands, +// which check a skip rules file and apply it to the opportunities already +// stored (see internal/skiprules). +package skiprules + +import ( + "errors" + "fmt" + "log/slog" + + "github.com/spf13/cobra" + + "git.avsdev.uk/Go/job-search/internal/cli" + "git.avsdev.uk/Go/job-search/internal/config" + "git.avsdev.uk/Go/job-search/internal/skiprules" +) + +// NewCommand returns the "skip-rules" command, with "check" and "apply". +func NewCommand(opts *cli.Options) *cobra.Command { + cmd := &cobra.Command{ + Use: "skip-rules", + Short: "Check skip rules, or apply them to stored opportunities", + Long: `Skip rules mark new opportunities Skipped as the sources fetch +them. They are read from sources.skip_rules_file, and read again when it +changes. + +"check" reports whether a rules file is valid, and "apply" marks the stored +opportunities that match Skipped. Both read sources.skip_rules_file unless +given another file.`, + Args: cobra.NoArgs, + PersistentPreRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent pre-run", "command", "skip-rules") + }, + PersistentPostRun: func(cmd *cobra.Command, args []string) { + slog.Debug("command persistent post-run", "command", "skip-rules") + }, + } + cmd.AddCommand(commandCheck(opts)) + cmd.AddCommand(commandApply(opts)) + + return cmd +} + +// loadRules returns the rules in the file named by args, if given, or else +// in sources.skip_rules_file, and the file's path. +func loadRules( + conf *config.JobSearchConfig, args []string, +) (*skiprules.Rules, string, error) { + path := conf.Sources.SkipRulesFile + if len(args) > 0 { + path = args[0] + } + if path == "" { + return nil, "", errors.New( + "no rules file given, and sources.skip_rules_file is not set", + ) + } + + rules, err := skiprules.Load(path) + if err != nil { + return nil, "", err + } + + return rules, path, nil +} + +// describeRules returns a summary of rules for output, e.g. "3 filters (2 +// active)". +func describeRules(rules *skiprules.Rules) string { + filters, active := rules.Filters() + return fmt.Sprintf("%d filters (%d active)", filters, active) +} diff --git a/internal/cli/skiprules/skiprules_test.go b/internal/cli/skiprules/skiprules_test.go new file mode 100644 index 0000000..43824bd --- /dev/null +++ b/internal/cli/skiprules/skiprules_test.go @@ -0,0 +1,175 @@ +package skiprules + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + + "git.avsdev.uk/Go/job-search/internal/cli" + "git.avsdev.uk/Go/job-search/internal/cli/cmdtest" + "git.avsdev.uk/Go/job-search/internal/store" + "git.avsdev.uk/Go/job-search/internal/types" +) + +// Run with: go test -tags sqlite_fts5 ./... +// +// These tests run the skip-rules commands as a user would, through a root +// command like job-search's (see cmdtest). + +// conf is a config.yaml with a database and rules file in the test's +// directory. +const conf = "logging:\n level: info\n" + + "database:\n file: ./test.db\n" + + "sources:\n skip_rules_file: ./rules.json\n" + +// rules skips buyer X's opportunities. +const rules = `[{"comment": "buyer X", "active": true, + "rules": [{"rule_type": "buyer", "values": ["X"]}]}]` + +func newRoot() *cobra.Command { + opts := &cli.Options{} + return cmdtest.NewRoot(opts, NewCommand(opts)) +} + +// storeOpportunities creates test.db holding ocds-1 (buyer X), ocds-2 (buyer +// Y) and ocds-3 (buyer X, already skipped and interested). +func storeOpportunities(t *testing.T) { + t.Helper() + ctx := context.Background() + s, err := store.Open(ctx, "test.db", store.WithAutoMigrate()) + if err != nil { + t.Fatal(err) + } + defer s.Close() + + buyers := []string{"X", "Y", "X"} + for i, buyer := range buyers { + uuid := "ocds-" + string(rune('1'+i)) + _, err := s.SaveOpportunity(ctx, &types.Opportunity{ + UUID: uuid, + UUIDType: "ocid", + Title: "Opportunity " + uuid, + BuyerID: buyer, + Releases: []types.OpportunityRelease{ + { + ReleaseID: "r1", + ReleaseDate: time.Date(2026, 10, 1+i, 12, 0, 0, 0, time.UTC), + JSON: "{}", + }, + }, + }) + if err != nil { + t.Fatal(err) + } + } + err = s.SetMarks(ctx, types.OpportunityMarks{ + UUID: "ocds-3", + Skipped: true, + Interested: true, + }) + if err != nil { + t.Fatal(err) + } +} + +// marks returns the stored marks of uuid. +func marks(t *testing.T, uuid string) types.OpportunityMarks { + t.Helper() + s, err := store.Open(context.Background(), "test.db") + if err != nil { + t.Fatal(err) + } + defer s.Close() + opportunity, err := s.GetOpportunity(context.Background(), uuid) + if err != nil { + t.Fatal(err) + } + return opportunity.Marks +} + +func TestCheck(t *testing.T) { + cmdtest.Setup(t, conf) + cmdtest.WriteFile(t, "rules.json", rules) + cmdtest.WriteFile(t, "other.json", `[]`) + cmdtest.WriteFile(t, "bad.json", `[{"comment": "c"}]`) + + out := cmdtest.MustRun(t, newRoot, "skip-rules", "check") + cmdtest.Contains(t, out, "./rules.json is valid: 1 filters (1 active)") + + out = cmdtest.MustRun(t, newRoot, "skip-rules", "check", "other.json") + cmdtest.Contains(t, out, "other.json is valid: 0 filters (0 active)") + + _, err := cmdtest.Run(t, newRoot, "", "skip-rules", "check", "bad.json") + cmdtest.ErrContains(t, err, `bad.json: filter 1 ("c"): skiprules: active`) + + _, err = cmdtest.Run(t, newRoot, "", "skip-rules", "check", "none.json") + cmdtest.ErrContains(t, err, "none.json") +} + +func TestCheckNoFile(t *testing.T) { + cmdtest.Setup(t, "logging:\n level: info\n") + _, err := cmdtest.Run(t, newRoot, "", "skip-rules", "check") + cmdtest.ErrContains(t, err, "sources.skip_rules_file is not set") +} + +func TestApplyDryRun(t *testing.T) { + cmdtest.Setup(t, conf) + cmdtest.WriteFile(t, "rules.json", rules) + storeOpportunities(t) + + out := cmdtest.MustRun(t, newRoot, "skip-rules", "apply", "--dry-run") + cmdtest.Contains( + t, + out, + "ocds-1 buyer X Opportunity ocds-1", + "1 opportunities would be skipped (dry run)", + ) + if strings.Contains(out, "ocds-2") || strings.Contains(out, "ocds-3") { + t.Errorf("listed a non-match or an already skipped one:\n%s", out) + } + if marks(t, "ocds-1").Skipped { + t.Error("a dry run skipped ocds-1") + } +} + +// apply sets only Skipped, on the matches not yet skipped, and leaves the +// other marks alone. +func TestApply(t *testing.T) { + cmdtest.Setup(t, conf) + cmdtest.WriteFile(t, "rules.json", rules) + storeOpportunities(t) + + out := cmdtest.MustRun(t, newRoot, "skip-rules", "apply") + cmdtest.Contains(t, out, "ocds-1 buyer X", "Skipped 1 opportunities") + + if !marks(t, "ocds-1").Skipped { + t.Error("ocds-1 wasn't skipped") + } + if marks(t, "ocds-2").Skipped { + t.Error("ocds-2 (buyer Y) was skipped") + } + if got := marks(t, "ocds-3"); !got.Skipped || !got.Interested { + t.Errorf("ocds-3's marks changed: %+v", got) + } + + out = cmdtest.MustRun(t, newRoot, "skip-rules", "apply") + cmdtest.Contains(t, out, "Skipped 0 opportunities") +} + +// A rules file given on the command line is used instead of the +// configured one. +func TestApplyGivenFile(t *testing.T) { + cmdtest.Setup(t, conf) + cmdtest.WriteFile(t, "rules.json", rules) + cmdtest.WriteFile(t, "y.json", strings.ReplaceAll(rules, `"X"`, `"Y"`)) + storeOpportunities(t) + + out := cmdtest.MustRun(t, newRoot, "skip-rules", "apply", "y.json") + cmdtest.Contains(t, out, "ocds-2", "Skipped 1 opportunities") + if !marks(t, "ocds-2").Skipped || marks(t, "ocds-1").Skipped { + t.Error("y.json's rules weren't the ones applied") + } +} diff --git a/internal/cli/skiprules_test.go b/internal/cli/skiprules_test.go new file mode 100644 index 0000000..e4925d7 --- /dev/null +++ b/internal/cli/skiprules_test.go @@ -0,0 +1,43 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "git.avsdev.uk/Go/job-search/internal/config" +) + +// SkipRulesOptions gives no option without a file, one with a valid file, +// and refuses a missing or invalid file, naming the setting. +func TestSkipRulesOptions(t *testing.T) { + dir := t.TempDir() + valid := filepath.Join(dir, "valid.json") + invalid := filepath.Join(dir, "invalid.json") + os.WriteFile(valid, []byte(`[]`), 0o600) + os.WriteFile(invalid, []byte(`[{}]`), 0o600) + + tests := []struct { + file string + options int + err string + }{ + {"", 0, ""}, + {valid, 1, ""}, + {filepath.Join(dir, "missing.json"), 0, "sources.skip_rules_file: "}, + {invalid, 0, "sources.skip_rules_file: skip rules "}, + } + for _, tt := range tests { + conf := &config.JobSearchConfig{} + conf.Sources.SkipRulesFile = tt.file + options, err := SkipRulesOptions(conf) + if len(options) != tt.options { + t.Errorf("%q: %d options; want %d", tt.file, len(options), tt.options) + } + if (err == nil) != (tt.err == "") || + err != nil && !strings.Contains(err.Error(), tt.err) { + t.Errorf("%q: err %v; want %q", tt.file, err, tt.err) + } + } +} diff --git a/internal/config/README.md b/internal/config/README.md index be66e05..5c95c57 100644 --- a/internal/config/README.md +++ b/internal/config/README.md @@ -68,6 +68,7 @@ supported. | Key | Type | Default | Notes | |---|---|---|---| +| `sources.skip_rules_file` | string | | JSON file of rules that mark new opportunities Skipped (see `internal/skiprules`); read again when it changes. Empty: none. Missing or invalid: `fetcher service` and `serve` refuse to start | | `sources..enabled` | bool | `false` | For every source | | `sources..interval` | duration | `10m` | For every source; e.g. `30s`, `1h`. Must be greater than 0; turn a source off with `enabled` | | `sources.find_a_tender.api_root` | string | `https://www.find-tender.service.gov.uk/api/1.0` | URL of the Find a Tender API. Empty: the source fails to start if enabled, and `fetcher fts-once` fails | diff --git a/internal/config/conf_sections_test.go b/internal/config/conf_sections_test.go index 3ef8e35..37060e1 100644 --- a/internal/config/conf_sections_test.go +++ b/internal/config/conf_sections_test.go @@ -295,8 +295,13 @@ func TestSourcesSection(t *testing.T) { if flat["sources.fts_mock.api_root"] != "http://mock" { t.Errorf("Flatten missing sources.fts_mock.api_root: %v", flat) } - if len(flat) != 9 { - t.Errorf("Flatten has %d keys; want 9: %v", len(flat), flat) + if len(flat) != 10 { + t.Errorf("Flatten has %d keys; want 10: %v", len(flat), flat) + } + + if err := c.SetValue("skip_rules_file", "rules.json"); err != nil || + c.SkipRulesFile != "rules.json" { + t.Errorf("SetValue(skip_rules_file): %v, %q", err, c.SkipRulesFile) } } diff --git a/internal/config/conf_sources.go b/internal/config/conf_sources.go index da542d4..ba6a80b 100644 --- a/internal/config/conf_sources.go +++ b/internal/config/conf_sources.go @@ -13,10 +13,14 @@ const JSCKSources string = "sources" // JSCSources configures each source of opportunities, one field // per source. Each field's cfg tag must match the section's JSCK* constant. +// SkipRulesFile is the JSON file of rules that mark new opportunities +// Skipped (see internal/skiprules), read again when it changes; empty means +// none. type JSCSources struct { - FindATender JSCSourceFindATender `cfg:"find_a_tender"` - FTSMock JSCSourceFTSMock `cfg:"fts_mock"` - Sleeper JSCSourceSleeper `cfg:"sleeper"` + SkipRulesFile string `cfg:"skip_rules_file"` + FindATender JSCSourceFindATender `cfg:"find_a_tender"` + FTSMock JSCSourceFTSMock `cfg:"fts_mock"` + Sleeper JSCSourceSleeper `cfg:"sleeper"` } // ---------- Section methods (see Section) ---------- diff --git a/internal/fetcher/fetcher.go b/internal/fetcher/fetcher.go index f60f16b..27b0394 100644 --- a/internal/fetcher/fetcher.go +++ b/internal/fetcher/fetcher.go @@ -1,6 +1,7 @@ package fetcher import ( + "cmp" "context" "errors" "fmt" @@ -10,6 +11,7 @@ import ( "git.avsdev.uk/Go/job-search/internal/config" "git.avsdev.uk/Go/job-search/internal/events" + "git.avsdev.uk/Go/job-search/internal/skiprules" "git.avsdev.uk/Go/job-search/internal/store" "git.avsdev.uk/Go/job-search/internal/types" "git.avsdev.uk/Go/job-search/internal/util" @@ -49,6 +51,15 @@ func WithEvents(bus *events.Bus) Option { } } +// WithSkipRules makes the workers mark the new opportunities that match the +// rules in rulesFile Skipped as they save them (see Fetcher.save). The file +// is checked for changes before each save. +func WithSkipRules(rulesFile *skiprules.File) Option { + return func(f *Fetcher) { + f.skipRules = rulesFile + } +} + // saveFunc saves the opportunities a worker fetched; see Fetcher.save. type saveFunc func( ctx context.Context, @@ -81,6 +92,9 @@ type Fetcher struct { events *events.Bus // nil: nothing is published started time.Time + // skipRules mark new opportunities Skipped; nil: none are. + skipRules *skiprules.File + // 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. @@ -216,6 +230,10 @@ func (f *Fetcher) startFrom( // and publishes it (see WithEvents). Without a store it does nothing. A // failure fails the worker's run, so its checkpoint isn't moved on and the // next run fetches them again. +// +// Opportunities that match the skip rules (see WithSkipRules) are marked +// Skipped first. The store keeps the marks of an opportunity it already +// has, so only new ones are skipped. func (f *Fetcher) save( ctx context.Context, logger *slog.Logger, @@ -225,14 +243,30 @@ func (f *Fetcher) save( return nil } + filters := f.markSkipped(opportunities) + result, err := f.store.SaveOpportunities(ctx, opportunities) if err != nil { return fmt.Errorf("saving opportunities: %w", err) } + skipped := 0 + for i, saved := range result.OpportunityResults { + if !saved.NewOpportunity || filters[i] == "" { + continue + } + skipped++ + tagged(logger).Debug( + "skipped new opportunity", + "uuid", opportunities[i].UUID, + "filter", filters[i], + ) + } + tagged(logger).Info( "saved opportunities", "new_opportunities", result.NewOpportunities, + "skipped_opportunities", skipped, "updated_opportunities", result.UpdatedOpportunities, "new_releases", result.NewReleases, ) @@ -243,6 +277,30 @@ func (f *Fetcher) save( return nil } +// markSkipped marks the opportunities that match the skip rules Skipped, +// and returns the comment of the filter each matched, indexed like +// opportunities: "" for one that matched none (every one, with no skip +// rules). +func (f *Fetcher) markSkipped(opportunities []*types.Opportunity) []string { + filters := make([]string, len(opportunities)) + if f.skipRules == nil { + return filters + } + + rules := f.skipRules.Rules() + for i, opportunity := range opportunities { + comment, matched := rules.Match(opportunity) + if !matched { + continue + } + opportunity.Marks.Skipped = true + // A filter's comment may be empty; it still names one here. + filters[i] = cmp.Or(comment, "(no comment)") + } + + return filters +} + // publishClassificationChanges publishes an events.ClassificationsUpdated // event for the classification codes a save, with result, brought into or // out of use, if it changed any and a bus was given WithEvents. It's diff --git a/internal/fetcher/fetcher_test.go b/internal/fetcher/fetcher_test.go index 4cd8e2b..f8916b4 100644 --- a/internal/fetcher/fetcher_test.go +++ b/internal/fetcher/fetcher_test.go @@ -18,6 +18,7 @@ import ( "git.avsdev.uk/Go/job-search/internal/config" "git.avsdev.uk/Go/job-search/internal/events" + "git.avsdev.uk/Go/job-search/internal/skiprules" "git.avsdev.uk/Go/job-search/internal/store" "git.avsdev.uk/Go/job-search/internal/types" "git.avsdev.uk/Go/job-search/internal/util" @@ -876,3 +877,80 @@ func TestSavePublishesClassificationChanges(t *testing.T) { ) } } + +// skipRulesFile writes rules to a file and opens it. +func skipRulesFile(t *testing.T, rules string) *skiprules.File { + t.Helper() + path := t.TempDir() + "/skip-rules.json" + if err := os.WriteFile(path, []byte(rules), 0o600); err != nil { + t.Fatal(err) + } + file, err := skiprules.Open(path) + if err != nil { + t.Fatal(err) + } + return file +} + +// A new opportunity that matches the skip rules is saved Skipped; one +// already stored keeps its marks. +func TestSaveSkipsNewOpportunities(t *testing.T) { + rules := skipRulesFile(t, `[{"comment": "IT", "active": true, + "rules": [{"rule_type": "cpv", "values": ["72000000"]}]}]`) + ctx := context.Background() + skipped := func(s *store.Store) bool { + t.Helper() + opportunity, err := s.GetOpportunity(ctx, "ocds-1") + if err != nil { + t.Fatal(err) + } + return opportunity.Marks.Skipped + } + matching := withClassification("r1", "2026-10-01T12:00:00Z", "72000000") + + // New and matching: skipped, and counted in the log. + var logged bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logged, &slog.HandlerOptions{ + Level: slog.LevelDebug, + })) + s := openTestStore(t) + f := New(&config.JSCSources{}, WithStore(s), WithSkipRules(rules)) + if err := f.save(ctx, logger, fetched(t, matching)); err != nil { + t.Fatal(err) + } + if !skipped(s) { + t.Error("a new matching opportunity wasn't skipped") + } + for _, want := range []string{ + "skipped_opportunities=1", + "msg=\"skipped new opportunity\" component=fetcher uuid=ocds-1 filter=IT", + } { + if !strings.Contains(logged.String(), want) { + t.Errorf("log has no %q:\n%s", want, logged.String()) + } + } + + // New and not matching: not skipped. + s = openTestStore(t) + f = New(&config.JSCSources{}, WithStore(s), WithSkipRules(rules)) + other := withClassification("r1", "2026-10-01T12:00:00Z", "03000000") + if err := f.save(ctx, slog.Default(), fetched(t, other)); err != nil { + t.Fatal(err) + } + if skipped(s) { + t.Error("a new opportunity that doesn't match was skipped") + } + + // Stored already: a new release that matches doesn't skip it. + newRelease := withClassification("r2", "2026-10-02T12:00:00Z", "72000000") + logged.Reset() + if err := f.save(ctx, logger, fetched(t, newRelease)); err != nil { + t.Fatal(err) + } + if skipped(s) { + t.Error("a stored opportunity was skipped by a new release") + } + if !strings.Contains(logged.String(), "skipped_opportunities=0") { + t.Errorf("log doesn't count 0 skipped:\n%s", logged.String()) + } +} diff --git a/internal/skiprules/README.md b/internal/skiprules/README.md new file mode 100644 index 0000000..888a4aa --- /dev/null +++ b/internal/skiprules/README.md @@ -0,0 +1,93 @@ +# Skip rules + +Skip rules mark new opportunities **Skipped** as the sources fetch them, so +opportunities that are never of interest don't need skipping by hand. The +rules file is named by `sources.skip_rules_file`, and used by +`fetcher service` and `job-search serve`. + +- Only **new** opportunities are skipped: the store keeps the marks of an + opportunity it already has, so a new release never changes them. To apply + rules to the opportunities already stored, run + `job-search skip-rules apply` (`--dry-run` lists the matches only). +- The file is checked before each save and read again when it has changed. + An invalid edit is logged as an error and the last good rules are kept; a + deleted file is logged as a warning and nothing is skipped until it is + back. At startup, a missing or invalid file stops the command. +- `job-search skip-rules check [file]` reports whether a file is valid. + +## Format + +The file is a JSON list of filters. An opportunity is skipped when **any** +active filter matches it. + +```json +[ + { + "comment": "Buyer X's IT work", + "active": true, + "logic_mode": "and", + "rules": [ + { "rule_type": "buyer", "values": ["GB-FTS-12345"] }, + { "rule_type": "cpv", "values": ["72000000", "48000000"], "exact": false } + ] + }, + { + "comment": "Road repairs", + "active": true, + "rules": [ + { "rule_type": "title", "values": ["road", "repairs"] } + ] + }, + { + "comment": "Agriculture only (turned off)", + "active": false, + "rules": [ + { "rule_type": "cpv", "values": ["03000000"] }, + { "rule_type": "cpv", "values": ["03100000", "03200000"] } + ] + }, + { + "comment": "Anything with a medical equipment code", + "active": true, + "rules": [ + { "rule_type": "cpv", "values": ["33100000"], "exact": false, "exclusive": false } + ] + } +] +``` + +### Filters + +| Field | Required | Meaning | +|---|---|---| +| `comment` | yes | What the filter is for; logged with each opportunity it skips. May be `""` | +| `active` | yes | `false` turns the filter off | +| `rules` | yes | One or more rules | +| `logic_mode` | no | `"or"` (default): any rule matching is enough. `"and"`: every rule must match | + +### Rules + +| Field | Required | Meaning | +|---|---|---| +| `rule_type` | yes | `"buyer"`, `"title"` or `"cpv"` | +| `values` | yes | One or more strings; see below | +| `exact` | no | CPV rules only; default `true`. See below | +| `exclusive` | no | CPV rules only; default `true`. See below | + +| `rule_type` | Matches when | +|---|---| +| `buyer` | The buyer's ID is any one of `values` | +| `title` | Every word in `values` is a word of the title. Matching ignores case and is by whole words (`road` doesn't match "Railroad"); anything but letters and digits separates words, and a value with a space in it is several words | +| `cpv` | The opportunity has CPV codes, and **every** one of them is in `values`: they are a subset. `["C", "D", "E"]` matches an opportunity with only C and D, but not one with C and F. For "A or B or C+D+E", use three rules (or three filters). With `"exclusive": false`, **any** one of them being in `values` is enough: `["C"]` matches an opportunity with C and F | + +CPV codes are 8 digits; check digits (`72000000-5`) are ignored on both +sides. With `"exact": false`, a value's trailing zeros are dropped and the +rest matched as a prefix, keeping at least the first two digits (the +division): `72000000` covers every `72…` code, `72200000` every `722…`, and +`70000000` every `70…` but not `71…`. + +Anything else is refused when the file is read, with the filter and rule +named by their place in the file: an unknown field, a missing required +field, an empty `rules` or `values`, an unknown `rule_type` or `logic_mode`, +`exact` or `exclusive` on a rule that isn't a CPV rule, a title value with +no words, or a CPV value that isn't 8 digits. diff --git a/internal/skiprules/file.go b/internal/skiprules/file.go new file mode 100644 index 0000000..c6e0707 --- /dev/null +++ b/internal/skiprules/file.go @@ -0,0 +1,149 @@ +package skiprules + +import ( + "errors" + "log/slog" + "os" + "sync" + "time" +) + +// Option configures a File. +type Option func(*File) + +// WithLogger makes the file log its reloads and problems to logger instead +// of slog.Default(). +func WithLogger(logger *slog.Logger) Option { + return func(f *File) { + f.log = logger + } +} + +// File is a rules file that is read again when it changes (see Rules). +// Create it with Open. It is safe for concurrent use. +type File struct { + path string + log *slog.Logger + + mu sync.Mutex + // rules are the last good rules, or nil while the file is missing. + rules *Rules + // modTime and size are the file's when it was last read, good or not, so + // a bad file isn't read again until it changes. + modTime time.Time + size int64 + // problem is the missing or unreadable file last logged, so it is + // logged once rather than on every call; "" once the file is found. + problem string +} + +// Open reads the rules file at path. Unlike a reload, a missing or invalid +// file is an error. +func Open(path string, opts ...Option) (*File, error) { + file := &File{ + path: path, + } + for _, option := range opts { + option(file) + } + + info, err := os.Stat(path) + if err != nil { + return nil, err + } + rules, err := Load(path) + if err != nil { + return nil, err + } + + file.rules = rules + file.modTime = info.ModTime() + file.size = info.Size() + + return file, nil +} + +// logger returns the logger the file logs to, tagged with its component. +func (f *File) logger() *slog.Logger { + log := f.log + if log == nil { + log = slog.Default() + } + return log.With("component", "skiprules") +} + +// Path returns the path of the rules file. +func (f *File) Path() string { return f.path } + +// Rules returns the current rules, first reading the file again if its +// modification time or size has changed since it was last read. +// +// If the file can't be read or parsed, the error is logged and the last good +// rules are kept. If it has been deleted, a warning is logged and the result +// is nil (no rules) until it is back. A file that is missing or can't be +// checked is logged once, not on every call; an invalid one once per change. +func (f *File) Rules() *Rules { + f.mu.Lock() + defer f.mu.Unlock() + + info, err := os.Stat(f.path) + if errors.Is(err, os.ErrNotExist) { + f.rules = nil + f.modTime = time.Time{} + f.size = 0 + if f.newProblem("missing") { + f.logger().Warn( + "skip rules file missing, skipping nothing", "file", f.path, + ) + } + return nil + } + if err != nil { + if f.newProblem(err.Error()) { + f.logger().Error( + "skip rules file unreadable, keeping the last good rules", + "file", f.path, + "error", err, + ) + } + return f.rules + } + + f.problem = "" + if info.ModTime().Equal(f.modTime) && info.Size() == f.size { + return f.rules + } + + f.modTime = info.ModTime() + f.size = info.Size() + rules, err := Load(f.path) + if err != nil { + f.logger().Error( + "skip rules file invalid, keeping the last good rules", + "file", f.path, + "error", err, + ) + return f.rules + } + + f.rules = rules + filters, active := rules.Filters() + f.logger().Info( + "skip rules reloaded", + "file", f.path, + "filters", filters, + "active_filters", active, + ) + + return f.rules +} + +// newProblem reports whether problem isn't the one last logged, and +// remembers it. Call it with f.mu held. +func (f *File) newProblem(problem string) bool { + if problem == f.problem { + return false + } + f.problem = problem + return true +} diff --git a/internal/skiprules/file_test.go b/internal/skiprules/file_test.go new file mode 100644 index 0000000..643e200 --- /dev/null +++ b/internal/skiprules/file_test.go @@ -0,0 +1,119 @@ +package skiprules + +import ( + "bytes" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +const ( + buyerA = `[{"comment":"a","active":true,"rules":[{"rule_type":"buyer","values":["A"]}]}]` + buyerB = `[{"comment":"b","active":true,"rules":[{"rule_type":"buyer","values":["B"]}]}]` +) + +// write writes contents to path with a modification time of when, so each +// write is seen as a change however fast the test runs. +func write(t *testing.T, path, contents string, when time.Time) { + t.Helper() + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(path, when, when); err != nil { + t.Fatal(err) + } +} + +// comment returns the comment of the filter rules match o with, or "". +func comment(rules *Rules, buyer string) string { + matched, _ := rules.Match(opportunity(buyer, "x")) + return matched +} + +func TestOpenRefuses(t *testing.T) { + dir := t.TempDir() + if _, err := Open(filepath.Join(dir, "missing.json")); err == nil { + t.Error("a missing file was accepted") + } + + path := filepath.Join(dir, "bad.json") + write(t, path, `[{}]`, time.Now()) + _, err := Open(path) + if err == nil || !strings.Contains(err.Error(), "bad.json: filter 1") { + t.Errorf("err = %v; want one naming the file and filter", err) + } +} + +// Rules reads the file again when it changes, keeps the last good rules when +// it is invalid, and has none while it is missing, logging each problem once. +func TestFileReloads(t *testing.T) { + path := filepath.Join(t.TempDir(), "rules.json") + start := time.Now().Add(-time.Hour) + write(t, path, buyerA, start) + + var logged bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logged, nil)) + file, err := Open(path, WithLogger(logger)) + if err != nil { + t.Fatal(err) + } + if got := comment(file.Rules(), "A"); got != "a" { + t.Fatalf("first rules: %q; want a", got) + } + + write(t, path, buyerB, start.Add(time.Minute)) + if got := comment(file.Rules(), "B"); got != "b" { + t.Errorf("after an edit: %q; want b", got) + } + if !strings.Contains(logged.String(), "skip rules reloaded") { + t.Errorf("reload not logged:\n%s", logged.String()) + } + + // Invalid: the last good rules are kept, and the error logged once. + logged.Reset() + write(t, path, `[{]`, start.Add(2*time.Minute)) + file.Rules() + if got := comment(file.Rules(), "B"); got != "b" { + t.Errorf("after a bad edit: %q; want b kept", got) + } + if strings.Count(logged.String(), "skip rules file invalid") != 1 { + t.Errorf("want the bad file logged once:\n%s", logged.String()) + } + + // Missing: no rules, and a warning logged once. + logged.Reset() + os.Remove(path) + file.Rules() + if rules := file.Rules(); rules != nil { + t.Errorf("missing file: rules %v; want nil", rules) + } + if strings.Count(logged.String(), "skip rules file missing") != 1 { + t.Errorf("want the missing file logged once:\n%s", logged.String()) + } + + // Back again, even with an old modification time. + write(t, path, buyerA, start) + if got := comment(file.Rules(), "A"); got != "a" { + t.Errorf("file back: %q; want a", got) + } +} + +// A change that keeps the modification time but changes the size is still +// seen. +func TestFileReloadsOnSize(t *testing.T) { + path := filepath.Join(t.TempDir(), "rules.json") + when := time.Now().Add(-time.Hour) + write(t, path, buyerA, when) + file, err := Open(path, WithLogger(slog.New(slog.DiscardHandler))) + if err != nil { + t.Fatal(err) + } + + write(t, path, buyerB+"\n", when) + if got := comment(file.Rules(), "B"); got != "b" { + t.Errorf("after a same-time edit: %q; want b", got) + } +} diff --git a/internal/skiprules/rules.go b/internal/skiprules/rules.go new file mode 100644 index 0000000..a1b0d8f --- /dev/null +++ b/internal/skiprules/rules.go @@ -0,0 +1,395 @@ +// Package skiprules reads the rules that mark newly fetched opportunities +// Skipped, and matches opportunities against them. See README.md for the +// file's format. +package skiprules + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "slices" + "strings" + "unicode" + + "git.avsdev.uk/Go/job-search/internal/types" +) + +// Rule types, the values of a rule's rule_type. +const ( + // RuleTypeBuyer matches the buyer's ID. + RuleTypeBuyer = "buyer" + // RuleTypeTitle matches words in the title. + RuleTypeTitle = "title" + // RuleTypeCPV matches the opportunity's CPV codes. + RuleTypeCPV = "cpv" +) + +// Logic modes, the values of a filter's logic_mode. +const ( + // LogicModeOr makes a filter match when any of its rules matches. It is + // the default. + LogicModeOr = "or" + // LogicModeAnd makes a filter match only when all of its rules match. + LogicModeAnd = "and" +) + +// Rules are the filters of a rules file, in file order. Create them with +// Parse or Load. A nil *Rules has no filters and matches nothing. +type Rules struct { + filters []filter +} + +// filter is one parsed filter. all is set by logic_mode "and". +type filter struct { + comment string + active bool + all bool + rules []rule +} + +// rule is one parsed rule. Only the fields for its ruleType are set: values +// for a buyer rule, words (lowercase) for a title rule, codes (without check +// digits), exact and exclusive for a CPV rule. When exact is false, codes +// are prefixes (see cpvPrefix). +type rule struct { + ruleType string + values []string + words []string + codes []string + exact bool + exclusive bool +} + +// fileFilter is a filter as it is read from the file. Pointers and nil +// slices tell a missing field from a zero one. +type fileFilter struct { + Comment *string `json:"comment"` + Active *bool `json:"active"` + LogicMode *string `json:"logic_mode"` + Rules []fileRule `json:"rules"` +} + +// fileRule is a rule as it is read from the file. +type fileRule struct { + RuleType *string `json:"rule_type"` + Values []string `json:"values"` + Exact *bool `json:"exact"` + Exclusive *bool `json:"exclusive"` +} + +// Load reads and parses the rules file at path (see Parse). +func Load(path string) (*Rules, error) { + file, err := os.Open(path) //nolint:gosec // G304: the configured rules file + if err != nil { + return nil, err + } + defer file.Close() //nolint:errcheck // read only + + rules, err := Parse(file) + if err != nil { + return nil, fmt.Errorf("skip rules %s: %w", path, err) + } + + return rules, nil +} + +// Parse reads a rules file: a JSON list of filters. Unknown fields, missing +// fields (any but logic_mode, exact and exclusive), empty lists of rules or +// values, unknown rule types or logic modes, exact or exclusive on a rule +// that isn't a CPV rule, +// and CPV codes that aren't 8 digits are all refused, naming the filter. +func Parse(reader io.Reader) (*Rules, error) { + decoder := json.NewDecoder(reader) + decoder.DisallowUnknownFields() + + var fileFilters []fileFilter + if err := decoder.Decode(&fileFilters); err != nil { + return nil, fmt.Errorf("reading JSON: %w", err) + } + if fileFilters == nil { + return nil, errors.New("skiprules: the file must be a list of filters") + } + if !errors.Is(decoder.Decode(&struct{}{}), io.EOF) { + return nil, errors.New( + "skiprules: unexpected content after the list of filters", + ) + } + + rules := &Rules{ + filters: make([]filter, 0, len(fileFilters)), + } + for i, fileFilter := range fileFilters { + parsed, err := fileFilter.parse() + if err != nil { + return nil, fmt.Errorf( + "filter %d%s: %w", i+1, describe(fileFilter.Comment), err, + ) + } + rules.filters = append(rules.filters, parsed) + } + + return rules, nil +} + +// describe returns comment quoted after a space, to name a filter in an +// error, or "" if it has none. +func describe(comment *string) string { + if comment == nil || *comment == "" { + return "" + } + return fmt.Sprintf(" (%q)", *comment) +} + +// parse checks a filter as read from the file and returns it parsed. +func (f fileFilter) parse() (filter, error) { + switch { + case f.Comment == nil: + return filter{}, errors.New("skiprules: comment is required") + case f.Active == nil: + return filter{}, errors.New("skiprules: active is required") + case len(f.Rules) == 0: + return filter{}, errors.New( + "skiprules: rules is required and must not be empty", + ) + } + + logicMode := LogicModeOr + if f.LogicMode != nil { + logicMode = *f.LogicMode + } + if logicMode != LogicModeOr && logicMode != LogicModeAnd { + return filter{}, fmt.Errorf( + "skiprules: logic_mode %q must be %q or %q", + logicMode, LogicModeOr, LogicModeAnd, + ) + } + + parsed := filter{ + comment: *f.Comment, + active: *f.Active, + all: logicMode == LogicModeAnd, + rules: make([]rule, 0, len(f.Rules)), + } + for i, fileRule := range f.Rules { + parsedRule, err := fileRule.parse() + if err != nil { + return filter{}, fmt.Errorf("rule %d: %w", i+1, err) + } + parsed.rules = append(parsed.rules, parsedRule) + } + + return parsed, nil +} + +// parse checks a rule as read from the file and returns it parsed. +func (r fileRule) parse() (rule, error) { + switch { + case r.RuleType == nil: + return rule{}, errors.New("skiprules: rule_type is required") + case len(r.Values) == 0: + return rule{}, errors.New( + "skiprules: values is required and must not be empty", + ) + case r.Exact != nil && *r.RuleType != RuleTypeCPV: + return rule{}, fmt.Errorf( + "skiprules: exact is only for %q rules", RuleTypeCPV, + ) + case r.Exclusive != nil && *r.RuleType != RuleTypeCPV: + return rule{}, fmt.Errorf( + "skiprules: exclusive is only for %q rules", RuleTypeCPV, + ) + } + + parsed := rule{ + ruleType: *r.RuleType, + } + switch parsed.ruleType { + case RuleTypeBuyer: + parsed.values = r.Values + case RuleTypeTitle: + for _, value := range r.Values { + parsed.words = append(parsed.words, words(value)...) + } + if len(parsed.words) == 0 { + return rule{}, errors.New("skiprules: values has no words") + } + case RuleTypeCPV: + return r.parseCPV() + default: + return rule{}, fmt.Errorf( + "skiprules: rule_type %q must be %q, %q or %q", + parsed.ruleType, RuleTypeBuyer, RuleTypeTitle, RuleTypeCPV, + ) + } + + return parsed, nil +} + +// parseCPV returns a CPV rule parsed: exact and exclusive default to true, +// and each value must be a CPV code. +func (r fileRule) parseCPV() (rule, error) { + parsed := rule{ + ruleType: RuleTypeCPV, + exact: r.Exact == nil || *r.Exact, + exclusive: r.Exclusive == nil || *r.Exclusive, + } + for _, value := range r.Values { + code := cpvCode(value) + if !isCPVCode(code) { + return rule{}, fmt.Errorf( + "skiprules: %q is not a CPV code (8 digits, optionally with a check digit)", + value, + ) + } + if !parsed.exact { + code = cpvPrefix(code) + } + parsed.codes = append(parsed.codes, code) + } + + return parsed, nil +} + +// Filters returns the number of filters, and how many of them are active. +func (r *Rules) Filters() (count, active int) { + if r == nil { + return 0, 0 + } + for _, entry := range r.filters { + if entry.active { + active++ + } + } + return len(r.filters), active +} + +// Match reports whether opportunity should be skipped: whether any active +// filter matches it. If one does, it also returns the first such filter's +// comment. +func (r *Rules) Match(opportunity *types.Opportunity) (string, bool) { + if r == nil { + return "", false + } + for _, entry := range r.filters { + if entry.matches(opportunity) { + return entry.comment, true + } + } + return "", false +} + +// matches reports whether the filter is active and its rules match +// opportunity: all of them with logic_mode "and", otherwise any. +func (f filter) matches(opportunity *types.Opportunity) bool { + if !f.active { + return false + } + // The first rule that doesn't match settles "and" (false), and the + // first that does settles "or" (true); otherwise every rule agreed. + for _, entry := range f.rules { + matched := entry.matches(opportunity) + if matched != f.all { + return matched + } + } + return f.all +} + +// matches reports whether the rule matches opportunity: for a buyer rule, +// its buyer ID is one of the values; for a title rule, every word is a word +// of its title; for a CPV rule, see matchesCPV. +func (r rule) matches(opportunity *types.Opportunity) bool { + switch r.ruleType { + case RuleTypeBuyer: + return slices.Contains(r.values, opportunity.BuyerID) + case RuleTypeTitle: + title := words(opportunity.Title) + for _, word := range r.words { + if !slices.Contains(title, word) { + return false + } + } + return true + case RuleTypeCPV: + return r.matchesCPV(opportunity) + } + return false +} + +// matchesCPV reports whether a CPV rule matches opportunity: it has CPV +// codes, and the rule's codes cover every one of them if the rule is +// exclusive, or any one of them if it isn't. +func (r rule) matchesCPV(opportunity *types.Opportunity) bool { + found := false + for _, classification := range opportunity.Classifications { + if !classification.IsCPV { + continue + } + found = true + + // The first code not covered settles an exclusive rule (false), and + // the first code covered settles one that isn't (true). + covered := r.coversCPV(cpvCode(classification.Code)) + if covered != r.exclusive { + return covered + } + } + return found && r.exclusive +} + +// coversCPV reports whether code (without its check digit) is one of the +// rule's codes or, unless the rule is exact, starts with one of them. +func (r rule) coversCPV(code string) bool { + if r.exact { + return slices.Contains(r.codes, code) + } + for _, prefix := range r.codes { + if strings.HasPrefix(code, prefix) { + return true + } + } + return false +} + +// words returns the lowercase words of text. Anything but a letter or a +// digit separates words, so "Road-repairs, phase 2" is road, repairs, phase +// and 2. +func words(text string) []string { + return strings.FieldsFunc(strings.ToLower(text), func(char rune) bool { + return !unicode.IsLetter(char) && !unicode.IsDigit(char) + }) +} + +// cpvCode returns a CPV code without its check digit: "72000000-5" is +// "72000000". +func cpvCode(code string) string { + code, _, _ = strings.Cut(strings.TrimSpace(code), "-") + return code +} + +// isCPVCode reports whether code (without its check digit) is 8 digits. +func isCPVCode(code string) bool { + if len(code) != 8 { + return false + } + for _, char := range code { + if char < '0' || char > '9' { + return false + } + } + return true +} + +// cpvPrefix returns the prefix a CPV code stands for in a rule that isn't +// exact: the code without its trailing zeros, but at least its first two +// digits (its division), so "72000000" is "72", "72200000" is "722" and +// "70000000" is "70", not "7". +func cpvPrefix(code string) string { + prefix := strings.TrimRight(code, "0") + if len(prefix) < 2 { + return code[:2] + } + return prefix +} diff --git a/internal/skiprules/rules_test.go b/internal/skiprules/rules_test.go new file mode 100644 index 0000000..aadc7be --- /dev/null +++ b/internal/skiprules/rules_test.go @@ -0,0 +1,300 @@ +package skiprules + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "git.avsdev.uk/Go/job-search/internal/types" +) + +// mustParse parses text as a rules file, failing the test on error. +func mustParse(t *testing.T, text string) *Rules { + t.Helper() + rules, err := Parse(strings.NewReader(text)) + if err != nil { + t.Fatal(err) + } + return rules +} + +// withFilter returns a rules file of one filter with fields (and a comment +// "c"); withRule one of an active filter holding rules. +func withFilter(fields string) string { + return `[{"comment":"c",` + fields + `}]` +} + +func withRule(rules ...string) string { + return withFilter(`"active":true,"rules":[` + strings.Join(rules, ",") + `]`) +} + +// ruleJSON returns a rule of ruleType with values as JSON, and extra fields. +func ruleJSON(ruleType, extra string, values ...string) string { + encoded, _ := json.Marshal(values) + return `{"rule_type":"` + ruleType + `","values":` + string(encoded) + + extra + `}` +} + +func buyer(values ...string) string { return ruleJSON("buyer", "", values...) } +func title(values ...string) string { return ruleJSON("title", "", values...) } +func cpv(values ...string) string { return ruleJSON("cpv", "", values...) } + +// cpvPrefixes is a CPV rule with exact false, cpvOnly one with exclusive +// true (the default), cpvAny one with exclusive false, and cpvAnyPrefixes +// one with exact and exclusive false. +func cpvPrefixes(values ...string) string { + return ruleJSON("cpv", `,"exact":false`, values...) +} + +func cpvOnly(values ...string) string { + return ruleJSON("cpv", `,"exclusive":true`, values...) +} + +func cpvAny(values ...string) string { + return ruleJSON("cpv", `,"exclusive":false`, values...) +} + +func cpvAnyPrefixes(values ...string) string { + return ruleJSON("cpv", `,"exact":false,"exclusive":false`, values...) +} + +// opportunity returns an opportunity with buyer, title and CPV codes. +func opportunity(buyer, title string, cpvs ...string) *types.Opportunity { + o := &types.Opportunity{ + UUID: "ocds-1", + BuyerID: buyer, + Title: title, + } + for _, code := range cpvs { + o.Classifications = append(o.Classifications, types.ClassificationCode{ + IsCPV: true, + Code: code, + }) + } + return o +} + +// withCPVs and titled are opportunities with only CPV codes or a title. +func withCPVs(codes ...string) *types.Opportunity { + return opportunity("", "x", codes...) +} + +func titled(text string) *types.Opportunity { return opportunity("", text) } + +func TestParseRefuses(t *testing.T) { + active := `"active":true,` + tests := []struct { + name, text, want string + }{ + {"not JSON", `{`, "reading JSON"}, + {"not a list", `{}`, "reading JSON"}, + {"null", `null`, "must be a list of filters"}, + {"trailing", `[] []`, "unexpected content"}, + {"unknown filter field", withFilter(`"filter_type":"x"`), `unknown field "filter_type"`}, + {"unknown rule field", withRule(`{"value":"a"}`), `unknown field "value"`}, + {"no comment", `[{"active":true,"rules":[` + buyer("a") + `]}]`, "filter 1: skiprules: comm"}, + {"no active", withFilter(`"rules":[` + buyer("a") + `]`), `filter 1 ("c"): skiprules: active`}, + {"no rules", withFilter(`"active":true`), "rules is required"}, + {"empty rules", withRule(), "rules is required"}, + {"bad logic", withFilter(active + `"logic_mode":"xor","rules":[` + buyer("a") + `]`), `"xor"`}, + {"empty logic", withFilter(active + `"logic_mode":"","rules":[` + buyer("a") + `]`), `""`}, + {"no rule type", withRule(`{"values":["a"]}`), "rule 1: skiprules: rule_type is required"}, + {"bad rule type", withRule(ruleJSON("status", "", "a")), `rule_type "status"`}, + {"no values", withRule(`{"rule_type":"buyer"}`), "values is required"}, + {"empty values", withRule(buyer()), "values is required"}, + {"exact on title", withRule(ruleJSON("title", `,"exact":true`, "a")), `exact is only for`}, + {"exclusive on buyer", withRule(ruleJSON("buyer", `,"exclusive":false`, "a")), "exclusive is"}, + {"no words", withRule(title("--")), "values has no words"}, + {"short CPV", withRule(cpv("7200000")), `"7200000" is not a CPV code`}, + {"letters CPV", withRule(cpv("7200000x")), "is not a CPV code"}, + } + for _, tt := range tests { + _, err := Parse(strings.NewReader(tt.text)) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("%s: err = %v; want one containing %q", tt.name, err, tt.want) + } + } +} + +// An error names the filter and rule by their place in the file. +func TestParseNamesFilterAndRule(t *testing.T) { + _, err := Parse(strings.NewReader(`[ + {"comment": "a", "active": true, "rules": [ + {"rule_type": "buyer", "values": ["a"]} + ]}, + {"comment": "b", "active": true, "rules": [ + {"rule_type": "buyer", "values": ["a"]}, + {"rule_type": "cpv", "values": ["x"]} + ]} + ]`)) + want := `filter 2 ("b"): rule 2: skiprules: "x" is not a CPV code` + if err == nil || !strings.Contains(err.Error(), want) { + t.Errorf("err = %v; want one containing %q", err, want) + } +} + +func TestParseAccepts(t *testing.T) { + rules := mustParse(t, `[ + {"comment": "a", "active": true, "rules": [ + {"rule_type": "buyer", "values": ["b"]} + ]}, + {"comment": "", "active": false, "logic_mode": "and", "rules": [ + {"rule_type": "cpv", "values": ["72000000-5"], "exact": false}, + {"rule_type": "title", "values": ["road"]} + ]} + ]`) + if filters, active := rules.Filters(); filters != 2 || active != 1 { + t.Errorf("Filters() = %d, %d; want 2, 1", filters, active) + } + + if filters, _ := mustParse(t, `[]`).Filters(); filters != 0 { + t.Errorf("empty list: %d filters", filters) + } + var none *Rules + if _, matched := none.Match(opportunity("b", "x")); matched { + t.Error("nil rules matched") + } +} + +func TestMatch(t *testing.T) { + tests := []struct { + name string + rule string + o *types.Opportunity + match bool + }{ + // buyer: any of the values + {"buyer", buyer("GB-1", "GB-2"), opportunity("GB-2", "x"), true}, + {"other buyer", buyer("GB-1"), opportunity("GB-10", "x"), false}, + + // title: every word, whole words, any case, punctuation splits + {"title", title("road", "REPAIRS"), titled("Road-repairs, phase 2"), true}, + {"title phrase", title("road repairs"), titled("Repairs to the road"), true}, + {"title word missing", title("road", "bridge"), titled("Road repairs"), false}, + {"title part word", title("road"), titled("Railroad works"), false}, + {"title digits", title("phase 2"), titled("Road-repairs, phase 2"), true}, + + // cpv: all the opportunity's CPVs in the values (a subset) + {"cpv subset", cpv("03000000", "72000000", "48000000"), withCPVs("72000000", "48000000"), true}, + {"cpv equal", cpv("72000000"), withCPVs("72000000"), true}, + {"cpv one outside", cpv("72000000"), withCPVs("72000000", "48000000"), false}, + {"cpv none", cpv("72000000"), withCPVs(), false}, + {"cpv check digits", cpv("72000000-5"), withCPVs("72000000-1"), true}, + {"cpv exact by default", cpv("72000000"), withCPVs("72200000"), false}, + {"cpv prefix", cpvPrefixes("72000000"), withCPVs("72212000-4", "72000000"), true}, + {"cpv prefix group", cpvPrefixes("72200000"), withCPVs("72300000"), false}, + {"cpv prefix division", cpvPrefixes("70000000"), withCPVs("71000000"), false}, + {"cpv prefix in division", cpvPrefixes("70000000"), withCPVs("70110000"), true}, + {"cpv exclusive", cpvOnly("72000000"), withCPVs("72000000", "48000000"), false}, + + // cpv, exclusive false: any of the opportunity's CPVs in the values + {"cpv any: one of two", cpvAny("72000000"), withCPVs("48000000", "72000000"), true}, + {"cpv any: all", cpvAny("72000000", "48000000"), withCPVs("72000000", "48000000"), true}, + {"cpv any: none in values", cpvAny("72000000"), withCPVs("48000000", "03000000"), false}, + {"cpv any: no CPVs", cpvAny("72000000"), withCPVs(), false}, + {"cpv any: exact by default", cpvAny("72000000"), withCPVs("72200000", "48000000"), false}, + {"cpv any: prefix", cpvAnyPrefixes("72000000"), withCPVs("48000000", "72212000-4"), true}, + {"cpv any: check digits", cpvAny("72000000-5"), withCPVs("48000000", "72000000-1"), true}, + } + for _, tt := range tests { + rules := mustParse(t, withRule(tt.rule)) + comment, matched := rules.Match(tt.o) + if matched != tt.match { + t.Errorf("%s: matched %v; want %v", tt.name, matched, tt.match) + } + if matched && comment != "c" { + t.Errorf("%s: comment %q; want c", tt.name, comment) + } + } +} + +// Non-CPV classifications are ignored: only CPVs must be covered. +func TestMatchIgnoresOtherSchemes(t *testing.T) { + rules := mustParse(t, withRule(cpv("72000000"))) + o := withCPVs("72000000") + o.Classifications = append(o.Classifications, types.ClassificationCode{ + Code: "1234", + }) + if _, matched := rules.Match(o); !matched { + t.Error("a non-CPV code stopped the match") + } +} + +// Buyer X, or CPVs within 03000000 and 04000000; "and" needs both. +func TestMatchLogicModes(t *testing.T) { + rules := buyer("X") + "," + cpv("03000000", "04000000") + or, and := `"logic_mode":"or",`, `"logic_mode":"and",` + tests := []struct { + name, mode string + o *types.Opportunity + match bool + }{ + {"default: buyer only", ``, opportunity("X", "t", "72000000"), true}, + {"or: cpv only", or, opportunity("Y", "t", "03000000"), true}, + {"default: neither", ``, opportunity("Y", "t", "72000000"), false}, + {"and: both", and, opportunity("X", "t", "03000000", "04000000"), true}, + {"and: buyer only", and, opportunity("X", "t", "72000000"), false}, + {"and: cpv only", and, opportunity("Y", "t", "03000000"), false}, + } + for _, tt := range tests { + text := withFilter(`"active":true,` + tt.mode + `"rules":[` + rules + `]`) + if _, matched := mustParse(t, text).Match(tt.o); matched != tt.match { + t.Errorf("%s: matched %v; want %v", tt.name, matched, tt.match) + } + } +} + +// Inactive filters are ignored; any active filter that matches is enough, +// and the first one's comment is returned. +func TestMatchFilters(t *testing.T) { + rules := mustParse(t, `[ + {"comment": "off", "active": false, "rules": [ + {"rule_type": "buyer", "values": ["X"]} + ]}, + {"comment": "title", "active": true, "rules": [ + {"rule_type": "title", "values": ["road"]} + ]}, + {"comment": "buyer", "active": true, "rules": [ + {"rule_type": "buyer", "values": ["X"]} + ]} + ]`) + + tests := []struct { + o *types.Opportunity + comment string + match bool + }{ + {opportunity("X", "Road works"), "title", true}, + {opportunity("X", "Bridge works"), "buyer", true}, + {opportunity("Y", "Road works"), "title", true}, + {opportunity("Y", "Bridge works"), "", false}, + } + for _, tt := range tests { + comment, matched := rules.Match(tt.o) + if comment != tt.comment || matched != tt.match { + t.Errorf( + "%s/%s: %q, %v; want %q, %v", + tt.o.BuyerID, tt.o.Title, comment, matched, tt.comment, tt.match, + ) + } + } +} + +// The example in README.md is a valid rules file. +func TestREADMEExample(t *testing.T) { + readme, err := os.ReadFile("README.md") + if err != nil { + t.Fatal(err) + } + _, example, found := strings.Cut(string(readme), "```json\n") + example, _, _ = strings.Cut(example, "```") + if !found { + t.Fatal("README.md has no JSON example") + } + + filters, active := mustParse(t, example).Filters() + if filters != 4 || active != 3 { + t.Errorf("example: %d filters, %d active; want 4, 3", filters, active) + } +}