Naming, layout and comments: descriptive full-word names, one item per line in wrapped calls and literals, and doc comments on exported names

Short and single-letter names in application code become full words (tests
keep theirs). Wrapped calls, signatures and composite literals put one item
per line, and one-line function bodies are expanded. Exported names and
groups of constants get doc comments, and comments refer to the renamed
identifiers.
This commit is contained in:
bwilliams committed 2026-10-15 16:12:56 +01:00
1 parent 043e307663
commit 465358609a
78 files changed
+2374 -1402

No files matched your search

+3 -1
View File
@@ -24,7 +24,9 @@ func OpenDB(path string) (*sql.DB, error) {
// ErrNeedsMigration. Intended for tests and throwaway databases; applications
// should migrate explicitly with the migrate command.
func WithAutoMigrate() Option {
return func(o *options) { o.autoMigrate = true }
return func(opts *options) {
opts.autoMigrate = true
}
}
// dsn builds the mattn/go-sqlite3 connection string.
+45 -35
View File
@@ -46,8 +46,8 @@ func NewMigrator(db *sql.DB) (*goose.Provider, error) {
}
// LatestVersion is the highest migration version in this build.
func LatestVersion(p *goose.Provider) int64 {
sources := p.ListSources() // sorted ascending
func LatestVersion(provider *goose.Provider) int64 {
sources := provider.ListSources() // sorted ascending
if len(sources) == 0 {
return 0
}
@@ -56,26 +56,34 @@ func LatestVersion(p *goose.Provider) int64 {
// checkVersion returns ErrNeedsMigration or ErrSchemaTooNew unless the
// database is exactly at this build's schema.
func checkVersion(ctx context.Context, p *goose.Provider) error {
current, err := p.GetDBVersion(ctx)
func checkVersion(ctx context.Context, provider *goose.Provider) error {
current, err := provider.GetDBVersion(ctx)
if err != nil {
return err
}
latest := LatestVersion(p)
latest := LatestVersion(provider)
if current > latest {
return fmt.Errorf("%w (database is at version %d, this build knows up to %d)",
ErrSchemaTooNew, current, latest)
return fmt.Errorf(
"%w (database is at version %d, this build knows up to %d)",
ErrSchemaTooNew,
current,
latest,
)
}
pending, err := p.HasPending(ctx)
pending, err := provider.HasPending(ctx)
if err != nil {
return err
}
if pending {
return fmt.Errorf("%w (database is at version %d, latest is %d)",
ErrNeedsMigration, current, latest)
return fmt.Errorf(
"%w (database is at version %d, latest is %d)",
ErrNeedsMigration,
current,
latest,
)
}
return nil
@@ -83,53 +91,55 @@ func checkVersion(ctx context.Context, p *goose.Provider) error {
// checkNotNewer refuses to work on a database migrated by a newer build:
// this build has neither the newer migrations nor their down steps.
func checkNotNewer(ctx context.Context, m *goose.Provider) error {
current, err := m.GetDBVersion(ctx)
func checkNotNewer(ctx context.Context, provider *goose.Provider) error {
current, err := provider.GetDBVersion(ctx)
if err != nil {
return err
}
if latest := LatestVersion(m); current > latest {
if latest := LatestVersion(provider); current > latest {
return fmt.Errorf(
"database is at version %d but this build only knows up to %d; use a newer build",
current, latest)
current,
latest,
)
}
return nil
}
// prepareSchema migrates (with WithAutoMigrate) or verifies the schema
// version.
func prepareSchema(ctx context.Context, db *sql.DB, o options) error {
p, err := NewMigrator(db)
func prepareSchema(ctx context.Context, db *sql.DB, opts options) error {
provider, err := NewMigrator(db)
if err != nil {
return err
}
if o.autoMigrate {
if _, err := p.Up(ctx); err != nil {
if opts.autoMigrate {
if _, err := provider.Up(ctx); err != nil {
return fmt.Errorf("store: migrating database: %w", err)
}
}
return checkVersion(ctx, p)
return checkVersion(ctx, provider)
}
// StepUp applies steps pending migrations, or all of them when steps is 0.
func StepUp(
ctx context.Context, m *goose.Provider, steps int,
ctx context.Context, provider *goose.Provider, steps int,
) ([]*goose.MigrationResult, error) {
var results []*goose.MigrationResult
if err := checkNotNewer(ctx, m); err != nil {
if err := checkNotNewer(ctx, provider); err != nil {
return results, err
}
var err error
if steps == 0 {
results, err = m.Up(ctx)
results, err = provider.Up(ctx)
} else {
for range steps {
var r *goose.MigrationResult
r, err = m.UpByOne(ctx)
var result *goose.MigrationResult
result, err = provider.UpByOne(ctx)
if errors.Is(err, goose.ErrNoNextVersion) {
err = nil
break
@@ -137,7 +147,7 @@ func StepUp(
if err != nil {
break
}
results = append(results, r)
results = append(results, result)
}
}
@@ -148,15 +158,15 @@ func StepUp(
// is 0. Rolling everything back deletes all data, so confirmDrop is checked
// first in that case.
func StepDown(
ctx context.Context, m *goose.Provider, steps int, confirmDrop bool,
ctx context.Context, provider *goose.Provider, steps int, confirmDrop bool,
) ([]*goose.MigrationResult, error) {
var results []*goose.MigrationResult
if err := checkNotNewer(ctx, m); err != nil {
if err := checkNotNewer(ctx, provider); err != nil {
return results, err
}
current, err := m.GetDBVersion(ctx)
current, err := provider.GetDBVersion(ctx)
if err != nil {
return results, err
}
@@ -164,13 +174,13 @@ func StepDown(
return results, nil
}
statuses, err := m.Status(ctx)
statuses, err := provider.Status(ctx)
if err != nil {
return results, err
}
applied := 0
for _, s := range statuses {
if s.State == goose.StateApplied {
for _, status := range statuses {
if status.State == goose.StateApplied {
applied++
}
}
@@ -180,11 +190,11 @@ func StepDown(
}
if all {
results, err = m.DownTo(ctx, 0)
results, err = provider.DownTo(ctx, 0)
} else {
for range steps {
var r *goose.MigrationResult
r, err = m.Down(ctx)
var result *goose.MigrationResult
result, err = provider.Down(ctx)
if errors.Is(err, goose.ErrNoNextVersion) {
err = nil
break
@@ -192,7 +202,7 @@ func StepDown(
if err != nil {
break
}
results = append(results, r)
results = append(results, result)
}
}
+8 -3
View File
@@ -16,6 +16,7 @@ import (
// Run with: go test -tags sqlite_fts5 ./...
func tempDBPath(t *testing.T) string {
t.Helper()
return filepath.Join(t.TempDir(), "migrate.db")
}
@@ -25,7 +26,9 @@ func newMigrator(t *testing.T) (*goose.Provider, *sql.DB) {
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { db.Close() })
t.Cleanup(func() {
db.Close()
})
m, err := store.NewMigrator(db)
if err != nil {
t.Fatal(err)
@@ -93,7 +96,8 @@ func TestOpenRejectsNewerSchema(t *testing.T) {
}
defer db.Close()
if _, err := db.Exec(
`INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`); err != nil {
`INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`,
); err != nil {
t.Fatal(err)
}
@@ -268,7 +272,8 @@ func TestNewerDatabaseIsRefused(t *testing.T) {
t.Fatal(err)
}
if _, err := db.Exec(
`INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`); err != nil {
`INSERT INTO goose_db_version (version_id, is_applied) VALUES (9999, 1)`,
); err != nil {
t.Fatal(err)
}
+16 -1
View File
@@ -67,15 +67,20 @@ SELECT t.* FROM opportunities t WHERE t.uuid = ?`
// when the mark is set and is negated with NOT to match it unset; an
// opportunity with no marks row has every mark unset.
const (
// listOpportunitiesSelect selects every opportunity, as t.
listOpportunitiesSelect = `
SELECT t.* FROM opportunities t`
// listOpportunitiesOrderAndPage orders by latest release, newest first,
// and takes a page: the limit and offset arguments.
listOpportunitiesOrderAndPage = `
ORDER BY t.latest_release_date DESC, t.uuid
LIMIT ? OFFSET ?`
// filterOpportunityStatus matches a status.
filterOpportunityStatus = `t.status = ?`
// filterOpportunityClassificationCode matches a classification code.
filterOpportunityClassificationCode = `EXISTS (
SELECT 1
FROM opportunity_classifications tc
@@ -83,35 +88,45 @@ LIMIT ? OFFSET ?`
WHERE tc.opportunity_uuid = t.uuid AND c.code = ?
)`
// The argument must come from ftsQuery, never raw user input.
// filterOpportunityTitleMatch matches a full-text search of the title. The
// argument must come from ftsQuery, never raw user input.
filterOpportunityTitleMatch = `t.id IN (
SELECT rowid FROM opportunities_fts WHERE opportunities_fts MATCH ?
)`
// filterOpportunityReleasedFrom matches a latest release on or after a
// time.
filterOpportunityReleasedFrom = `t.latest_release_date >= ?`
// filterOpportunityReleasedBefore matches a latest release before a time.
filterOpportunityReleasedBefore = `t.latest_release_date < ?`
// filterOpportunityMarkHidden matches the hidden mark set.
filterOpportunityMarkHidden = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.hidden
)`
// filterOpportunityMarkSkipped matches the skipped mark set.
filterOpportunityMarkSkipped = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.skipped
)`
// filterOpportunityMarkInterested matches the interested mark set.
filterOpportunityMarkInterested = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.interested
)`
// filterOpportunityMarkBidSubmitted matches the bid submitted mark set.
filterOpportunityMarkBidSubmitted = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.bid_submitted
)`
// filterOpportunityMarkWon matches the won mark set.
filterOpportunityMarkWon = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.won
)`
// filterOpportunityMarkLost matches the lost mark set.
filterOpportunityMarkLost = `EXISTS (
SELECT 1 FROM opportunity_marks m WHERE m.uuid = t.uuid AND m.lost
)`
+3 -1
View File
@@ -84,7 +84,9 @@ func openTestDB(t *testing.T) *sqlx.DB {
if err != nil {
t.Fatalf("Open: %v", err)
}
t.Cleanup(func() { s.Close() })
t.Cleanup(func() {
s.Close()
})
return s.db
}
+188 -132
View File
@@ -103,57 +103,57 @@ func (f OpportunityFilter) limit() int {
return f.Limit
}
// prepareOpportunity validates t and normalises it in place before saving: child
// rows get the opportunity's UUID and their release id, times are converted to
// UTC (so stored timestamps compare correctly as text), and
// prepareOpportunity validates opportunity and normalises it in place before
// saving: child rows get the opportunity's UUID and their release id, times
// are converted to UTC (so stored timestamps compare correctly as text), and
// LatestReleaseDate becomes the newest release date in the save.
func prepareOpportunity(t *types.Opportunity) error {
if t == nil {
func prepareOpportunity(opportunity *types.Opportunity) error {
if opportunity == nil {
return errors.New("store: nil opportunity")
}
if t.UUID == "" {
if opportunity.UUID == "" {
return errors.New("store: opportunity has no UUID")
}
t.Marks.UUID = t.UUID
if err := t.Marks.Validate(); err != nil {
return fmt.Errorf("store: opportunity %s: %w", t.UUID, err)
opportunity.Marks.UUID = opportunity.UUID
if err := opportunity.Marks.Validate(); err != nil {
return fmt.Errorf("store: opportunity %s: %w", opportunity.UUID, err)
}
latest := t.LatestReleaseDate
for i := range t.Releases {
r := &t.Releases[i]
if r.ReleaseID == "" {
return fmt.Errorf("store: opportunity %s: release %d has no release id", t.UUID, i)
latest := opportunity.LatestReleaseDate
for i := range opportunity.Releases {
release := &opportunity.Releases[i]
if release.ReleaseID == "" {
return fmt.Errorf("store: opportunity %s: release %d has no release id", opportunity.UUID, i)
}
r.UUID = t.UUID
r.ReleaseDate = r.ReleaseDate.UTC()
if r.ReleaseDate.After(latest) {
latest = r.ReleaseDate
release.UUID = opportunity.UUID
release.ReleaseDate = release.ReleaseDate.UTC()
if release.ReleaseDate.After(latest) {
latest = release.ReleaseDate
}
for j := range r.Tags {
r.Tags[j].UUID = t.UUID
r.Tags[j].ReleaseID = r.ReleaseID
for tagIndex := range release.Tags {
release.Tags[tagIndex].UUID = opportunity.UUID
release.Tags[tagIndex].ReleaseID = release.ReleaseID
}
}
if latest.IsZero() {
return fmt.Errorf("store: opportunity %s has no release date", t.UUID)
return fmt.Errorf("store: opportunity %s has no release date", opportunity.UUID)
}
t.LatestReleaseDate = latest.UTC()
t.StartDate = utcPtr(t.StartDate)
t.EndDate = utcPtr(t.EndDate)
opportunity.LatestReleaseDate = latest.UTC()
opportunity.StartDate = utcPtr(opportunity.StartDate)
opportunity.EndDate = utcPtr(opportunity.EndDate)
return nil
}
func utcPtr(t *time.Time) *time.Time {
if t == nil {
func utcPtr(when *time.Time) *time.Time {
if when == nil {
return nil
}
u := t.UTC()
return &u
utc := when.UTC()
return &utc
}
// ftsQuery turns free text into an FTS5 query that requires every word, each
@@ -162,11 +162,11 @@ func utcPtr(t *time.Time) *time.Time {
// FTS5 syntax (operators, column filters, unbalanced quotes). It returns ""
// when the text contains no words.
func ftsQuery(text string) string {
words := strings.FieldsFunc(text, func(r rune) bool {
return !unicode.IsLetter(r) && !unicode.IsNumber(r)
words := strings.FieldsFunc(text, func(char rune) bool {
return !unicode.IsLetter(char) && !unicode.IsNumber(char)
})
for i, w := range words {
words[i] = `"` + w + `"*`
for i, word := range words {
words[i] = `"` + word + `"*`
}
return strings.Join(words, " ")
}
@@ -179,9 +179,9 @@ func ftsQuery(text string) string {
//
// A database file that doesn't exist is created, and then needs migrating.
func Open(ctx context.Context, path string, opts ...Option) (*Store, error) {
var o options
for _, opt := range opts {
opt(&o)
var settings options
for _, option := range opts {
option(&settings)
}
sqldb, err := OpenDB(path)
@@ -191,10 +191,12 @@ func Open(ctx context.Context, path string, opts ...Option) (*Store, error) {
if err := sqldb.PingContext(ctx); err != nil {
return nil, errors.Join(err, sqldb.Close())
}
if err := prepareSchema(ctx, sqldb, o); err != nil {
if err := prepareSchema(ctx, sqldb, settings); err != nil {
return nil, errors.Join(err, sqldb.Close())
}
return &Store{db: sqlx.NewDb(sqldb, "sqlite3")}, nil
return &Store{
db: sqlx.NewDb(sqldb, "sqlite3"),
}, nil
}
// Close closes the database.
@@ -214,20 +216,24 @@ type opportunityRow struct {
types.Opportunity
}
// SaveOpportunity stores t and its releases in one transaction.
// SaveOpportunity stores opportunity and its releases in one transaction.
//
// Releases (and their tags) are write-once: ones already stored are skipped.
// The opportunity row and its classifications are replaced only if t's newest
// release is at least as new as the release the stored opportunity came from, so
// a late-arriving older release never undoes a newer one.
// The opportunity row and its classifications are replaced only if the
// opportunity's newest release is at least as new as the release the stored
// opportunity came from, so a late-arriving older release never undoes a
// newer one.
//
// t.Marks are stored only for a new opportunity; existing marks are kept (use
// SetMarks to change them).
// opportunity.Marks are stored only for a new opportunity; existing marks are
// kept (use SetMarks to change them).
//
// t is normalised in place (see prepareOpportunity), and when the opportunity is
// opportunity is normalised in place (see prepareOpportunity), and when it is
// written its classifications' IDs are filled in.
func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (SaveResult, error) {
if err := prepareOpportunity(t); err != nil {
func (s *Store) SaveOpportunity(
ctx context.Context,
opportunity *types.Opportunity,
) (SaveResult, error) {
if err := prepareOpportunity(opportunity); err != nil {
return SaveResult{}, err
}
@@ -237,7 +243,7 @@ func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (Save
}
defer tx.Rollback() //nolint:errcheck // no-op after Commit
res, _, err := saveOpportunity(ctx, tx, t)
res, _, err := saveOpportunity(ctx, tx, opportunity)
if err != nil {
return SaveResult{}, err
}
@@ -247,23 +253,25 @@ func (s *Store) SaveOpportunity(ctx context.Context, t *types.Opportunity) (Save
return res, nil
}
// SaveOpportunities stores t and its releases in one transaction.
// SaveOpportunities stores each of opportunities and its releases, all in one
// transaction, as SaveOpportunity does for one.
//
// Releases (and their tags) are write-once: ones already stored are skipped.
// The opportunity row and its classifications are replaced only if t's newest
// release is at least as new as the release the stored opportunity came from, so
// a late-arriving older release never undoes a newer one.
// An opportunity row and its classifications are replaced only if the
// opportunity's newest release is at least as new as the release the stored
// opportunity came from, so a late-arriving older release never undoes a
// newer one.
//
// t.Marks are stored only for a new opportunity; existing marks are kept (use
// Marks are stored only for a new opportunity; existing marks are kept (use
// SetMarks to change them).
//
// t is normalised in place (see prepareOpportunity), and when the opportunity is
// written its classifications' IDs are filled in.
// Each opportunity is normalised in place (see prepareOpportunity), and when
// it is written its classifications' IDs are filled in.
func (s *Store) SaveOpportunities(
ctx context.Context, opportunities []*types.Opportunity,
) (SaveMultipleResult, error) {
for _, t := range opportunities {
if err := prepareOpportunity(t); err != nil {
for _, opportunity := range opportunities {
if err := prepareOpportunity(opportunity); err != nil {
return SaveMultipleResult{}, err
}
}
@@ -274,9 +282,11 @@ func (s *Store) SaveOpportunities(
}
defer tx.Rollback() //nolint:errcheck // no-op after Commit
allRes := SaveMultipleResult{OpportunityResults: make([]SaveResult, len(opportunities))}
for i, t := range opportunities {
res, isNew, err := saveOpportunity(ctx, tx, t)
allRes := SaveMultipleResult{
OpportunityResults: make([]SaveResult, len(opportunities)),
}
for i, opportunity := range opportunities {
res, isNew, err := saveOpportunity(ctx, tx, opportunity)
if err != nil {
return SaveMultipleResult{}, err
}
@@ -300,27 +310,35 @@ func (s *Store) SaveOpportunities(
// one prepared opportunity inside tx. isNew reports that the opportunity was
// not already stored.
func saveOpportunity(
ctx context.Context, tx *sqlx.Tx, t *types.Opportunity,
ctx context.Context, tx *sqlx.Tx, opportunity *types.Opportunity,
) (res SaveResult, isNew bool, err error) {
var stored time.Time
err = tx.GetContext(ctx, &stored, selectOpportunityLatestReleaseDate, t.UUID)
err = tx.GetContext(ctx, &stored, selectOpportunityLatestReleaseDate, opportunity.UUID)
switch {
case errors.Is(err, sql.ErrNoRows):
isNew = true
res.OpportunityUpdated = true
case err != nil:
return SaveResult{}, false, fmt.Errorf("store: reading opportunity %s: %w", t.UUID, err)
return SaveResult{}, false, fmt.Errorf(
"store: reading opportunity %s: %w",
opportunity.UUID,
err,
)
default:
res.OpportunityUpdated = !t.LatestReleaseDate.Before(stored)
res.OpportunityUpdated = !opportunity.LatestReleaseDate.Before(stored)
}
if res.OpportunityUpdated {
if err := writeOpportunity(ctx, tx, t); err != nil {
return SaveResult{}, false, fmt.Errorf("store: writing opportunity %s: %w", t.UUID, err)
if err := writeOpportunity(ctx, tx, opportunity); err != nil {
return SaveResult{}, false, fmt.Errorf(
"store: writing opportunity %s: %w",
opportunity.UUID,
err,
)
}
}
for _, r := range t.Releases {
inserted, err := insertReleaseWithTags(ctx, tx, r)
for _, release := range opportunity.Releases {
inserted, err := insertReleaseWithTags(ctx, tx, release)
if err != nil {
return SaveResult{}, false, err
}
@@ -331,69 +349,86 @@ func saveOpportunity(
return res, isNew, nil
}
// insertReleaseWithTags stores r and its tags unless r is already stored, and
// reports whether it was inserted.
// insertReleaseWithTags stores release and its tags unless it is already
// stored, and reports whether it was inserted.
func insertReleaseWithTags(
ctx context.Context, tx *sqlx.Tx, r types.OpportunityRelease,
ctx context.Context, tx *sqlx.Tx, release types.OpportunityRelease,
) (bool, error) {
result, err := tx.NamedExecContext(ctx, insertRelease, r)
result, err := tx.NamedExecContext(ctx, insertRelease, release)
if err != nil {
return false, fmt.Errorf("store: inserting release %s/%s: %w", r.UUID, r.ReleaseID, err)
return false, fmt.Errorf(
"store: inserting release %s/%s: %w",
release.UUID,
release.ReleaseID,
err,
)
}
if n, err := result.RowsAffected(); err != nil || n == 0 {
if affected, err := result.RowsAffected(); err != nil || affected == 0 {
return false, err // n == 0: already stored
}
for _, tag := range r.Tags {
for _, tag := range release.Tags {
if _, err := tx.NamedExecContext(ctx, insertReleaseTag, tag); err != nil {
return false, fmt.Errorf(
"store: inserting tag for %s/%s: %w", r.UUID, r.ReleaseID, err,
"store: inserting tag for %s/%s: %w", release.UUID, release.ReleaseID, err,
)
}
}
return true, nil
}
// writeOpportunity upserts t's row, gives it a marks row if it has none
// writeOpportunity upserts opportunity's row, gives it a marks row if it has none
// (existing marks are kept), and replaces its classifications, filling in
// their IDs.
func writeOpportunity(ctx context.Context, tx *sqlx.Tx, t *types.Opportunity) (err error) {
if _, err := tx.NamedExecContext(ctx, upsertOpportunity, t); err != nil {
func writeOpportunity(
ctx context.Context,
tx *sqlx.Tx,
opportunity *types.Opportunity,
) (err error) {
if _, err := tx.NamedExecContext(ctx, upsertOpportunity, opportunity); err != nil {
return err
}
if _, err := tx.NamedExecContext(ctx, insertOpportunityMarks, t.Marks); err != nil {
if _, err := tx.NamedExecContext(ctx, insertOpportunityMarks, opportunity.Marks); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, deleteOpportunityClassifications, t.UUID); err != nil {
if _, err := tx.ExecContext(ctx, deleteOpportunityClassifications, opportunity.UUID); err != nil {
return err
}
stmt, err := tx.PrepareNamedContext(ctx, upsertClassificationCodeReturningID)
statement, err := tx.PrepareNamedContext(ctx, upsertClassificationCodeReturningID)
if err != nil {
return err
}
defer func() { err = errors.Join(err, stmt.Close()) }()
for i := range t.Classifications {
c := &t.Classifications[i]
if err := stmt.GetContext(ctx, &c.ID, c); err != nil {
defer func() {
err = errors.Join(err, statement.Close())
}()
for i := range opportunity.Classifications {
classification := &opportunity.Classifications[i]
if err := statement.GetContext(ctx, &classification.ID, classification); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, insertOpportunityClassification, t.UUID, c.ID); err != nil {
if _, err := tx.ExecContext(
ctx,
insertOpportunityClassification,
opportunity.UUID,
classification.ID,
); err != nil {
return err
}
}
return nil
}
// SetMarks replaces the marks of the opportunity m.UUID, or returns ErrNotFound.
// Marks that fail m.Validate (e.g. types.ErrWonAndLost) are not stored.
func (s *Store) SetMarks(ctx context.Context, m types.OpportunityMarks) error {
if m.UUID == "" {
// SetMarks replaces the marks of the opportunity marks.UUID, or returns
// ErrNotFound. Marks that fail marks.Validate (e.g. types.ErrWonAndLost) are
// not stored.
func (s *Store) SetMarks(ctx context.Context, marks types.OpportunityMarks) error {
if marks.UUID == "" {
return errors.New("store: marks have no UUID")
}
if err := m.Validate(); err != nil {
return fmt.Errorf("store: opportunity %s: %w", m.UUID, err)
if err := marks.Validate(); err != nil {
return fmt.Errorf("store: opportunity %s: %w", marks.UUID, err)
}
_, err := s.db.NamedExecContext(ctx, upsertOpportunityMarks, m)
_, err := s.db.NamedExecContext(ctx, upsertOpportunityMarks, marks)
if isForeignKeyError(err) {
return ErrNotFound
}
@@ -403,8 +438,8 @@ func (s *Store) SetMarks(ctx context.Context, m types.OpportunityMarks) error {
// isForeignKeyError reports whether err is SQLite rejecting a write that
// breaks a foreign key, e.g. one referring to a missing opportunity.
func isForeignKeyError(err error) bool {
var se sqlite3.Error
return errors.As(err, &se) && se.ExtendedCode == sqlite3.ErrConstraintForeignKey
var sqliteErr sqlite3.Error
return errors.As(err, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintForeignKey
}
// GetOpportunity returns the opportunity with its classifications, marks and
@@ -431,13 +466,14 @@ func (s *Store) GetOpportunity(ctx context.Context, uuid string) (*types.Opportu
return &opportunities[0], nil
}
// ListOpportunities returns opportunities matching f, with their classifications
// and marks but without releases (use ListReleases or GetOpportunity for those).
// ListOpportunities returns opportunities matching filter, with their
// classifications and marks but without releases (use ListReleases or
// GetOpportunity for those).
func (s *Store) ListOpportunities(
ctx context.Context,
f OpportunityFilter,
filter OpportunityFilter,
) ([]types.Opportunity, error) {
query, args := listOpportunitiesQuery(f)
query, args := listOpportunitiesQuery(filter)
var rows []opportunityRow
if err := s.db.SelectContext(ctx, &rows, query, args...); err != nil {
return nil, err
@@ -457,44 +493,62 @@ func (s *Store) ListOpportunities(
// listOpportunitiesQuery assembles the ListOpportunities query from the fragments in
// queries.go, with one argument per filter that is set (none for mark filters).
func listOpportunitiesQuery(f OpportunityFilter) (string, []any) {
func listOpportunitiesQuery(filter OpportunityFilter) (string, []any) {
var where []string
var args []any
add := func(clause string, arg any) {
add := func(clause string, argument any) {
where = append(where, clause)
args = append(args, arg)
args = append(args, argument)
}
if f.Status != "" {
add(filterOpportunityStatus, f.Status)
if filter.Status != "" {
add(filterOpportunityStatus, filter.Status)
}
if f.ClassificationCode != "" {
add(filterOpportunityClassificationCode, f.ClassificationCode)
if filter.ClassificationCode != "" {
add(filterOpportunityClassificationCode, filter.ClassificationCode)
}
if q := ftsQuery(f.Title); q != "" {
add(filterOpportunityTitleMatch, q)
if match := ftsQuery(filter.Title); match != "" {
add(filterOpportunityTitleMatch, match)
}
if !f.ReleasedFrom.IsZero() {
add(filterOpportunityReleasedFrom, f.ReleasedFrom.UTC())
if !filter.ReleasedFrom.IsZero() {
add(filterOpportunityReleasedFrom, filter.ReleasedFrom.UTC())
}
if !f.ReleasedBefore.IsZero() {
add(filterOpportunityReleasedBefore, f.ReleasedBefore.UTC())
if !filter.ReleasedBefore.IsZero() {
add(filterOpportunityReleasedBefore, filter.ReleasedBefore.UTC())
}
for _, m := range []struct {
for _, mark := range []struct {
filter MarkFilter
clause string
}{
{f.Hidden, filterOpportunityMarkHidden},
{f.Skipped, filterOpportunityMarkSkipped},
{f.Interested, filterOpportunityMarkInterested},
{f.BidSubmitted, filterOpportunityMarkBidSubmitted},
{f.Won, filterOpportunityMarkWon},
{f.Lost, filterOpportunityMarkLost},
{
filter: filter.Hidden,
clause: filterOpportunityMarkHidden,
},
{
filter: filter.Skipped,
clause: filterOpportunityMarkSkipped,
},
{
filter: filter.Interested,
clause: filterOpportunityMarkInterested,
},
{
filter: filter.BidSubmitted,
clause: filterOpportunityMarkBidSubmitted,
},
{
filter: filter.Won,
clause: filterOpportunityMarkWon,
},
{
filter: filter.Lost,
clause: filterOpportunityMarkLost,
},
} {
switch m.filter {
switch mark.filter {
case MarkSet:
where = append(where, m.clause)
where = append(where, mark.clause)
case MarkUnset:
where = append(where, "NOT "+m.clause)
where = append(where, "NOT "+mark.clause)
}
}
@@ -503,7 +557,7 @@ func listOpportunitiesQuery(f OpportunityFilter) (string, []any) {
query += "\nWHERE " + strings.Join(where, "\n\tAND ")
}
query += listOpportunitiesOrderAndPage
return query, append(args, f.limit(), f.Offset)
return query, append(args, filter.limit(), filter.Offset)
}
// loadClassifications fills in Classifications for all opportunities with one query.
@@ -530,9 +584,9 @@ func (s *Store) loadClassifications(ctx context.Context, opportunities []types.O
if err := s.db.SelectContext(ctx, &rows, s.db.Rebind(query), args...); err != nil {
return err
}
for _, r := range rows {
t := byUUID[r.OpportunityUUID]
t.Classifications = append(t.Classifications, r.ClassificationCode)
for _, releaseRow := range rows {
opportunity := byUUID[releaseRow.OpportunityUUID]
opportunity.Classifications = append(opportunity.Classifications, releaseRow.ClassificationCode)
}
return nil
}
@@ -548,7 +602,9 @@ func (s *Store) loadMarks(ctx context.Context, opportunities []types.Opportunity
for i := range opportunities {
uuids[i] = opportunities[i].UUID
byUUID[opportunities[i].UUID] = &opportunities[i]
opportunities[i].Marks = types.OpportunityMarks{UUID: opportunities[i].UUID}
opportunities[i].Marks = types.OpportunityMarks{
UUID: opportunities[i].UUID,
}
}
query, args, err := sqlx.In(selectMarksForOpportunities, uuids)
@@ -559,8 +615,8 @@ func (s *Store) loadMarks(ctx context.Context, opportunities []types.Opportunity
if err := s.db.SelectContext(ctx, &rows, s.db.Rebind(query), args...); err != nil {
return err
}
for _, m := range rows {
byUUID[m.UUID].Marks = m
for _, row := range rows {
byUUID[row.UUID].Marks = row
}
return nil
}
@@ -581,8 +637,8 @@ func (s *Store) ListReleases(ctx context.Context, uuid string) ([]types.Opportun
return nil, err
}
for _, tag := range tags {
if r := byID[tag.ReleaseID]; r != nil {
r.Tags = append(r.Tags, tag)
if release := byID[tag.ReleaseID]; release != nil {
release.Tags = append(release.Tags, tag)
}
}
return releases, nil
+155 -46
View File
@@ -20,7 +20,9 @@ func openStore(t *testing.T) *store.Store {
if err != nil {
t.Fatalf("Open: %v", err)
}
t.Cleanup(func() { s.Close() })
t.Cleanup(func() {
s.Close()
})
return s
}
@@ -51,7 +53,10 @@ func opportunity(uuid, title string, releaseDay int, releaseIDs ...string) *type
{IsCPV: true, Code: "03000000", Classification: "Agricultural products"},
},
// No UUID: SaveOpportunity must fill it in.
Marks: types.OpportunityMarks{Interested: true, BidSubmitted: true},
Marks: types.OpportunityMarks{
Interested: true,
BidSubmitted: true,
},
}
for i, id := range releaseIDs {
t.Releases = append(t.Releases, types.OpportunityRelease{
@@ -131,7 +136,11 @@ func TestSaveAndGetRoundTrip(t *testing.T) {
t.Errorf("classifications: got %+v (leading zero must survive)", got.Classifications)
}
wantMarks := types.OpportunityMarks{UUID: "ocds-1", Interested: true, BidSubmitted: true}
wantMarks := types.OpportunityMarks{
UUID: "ocds-1",
Interested: true,
BidSubmitted: true,
}
if got.Marks != wantMarks {
t.Errorf("marks: got %+v, want %+v", got.Marks, wantMarks)
}
@@ -230,7 +239,9 @@ func TestNewerReleaseUpdatesOpportunityAndSearchIndex(t *testing.T) {
}
for title, want := range map[string]int{"cloud": 0, "redesign": 1} {
ts, err := s.ListOpportunities(ctx, store.OpportunityFilter{Title: title})
ts, err := s.ListOpportunities(ctx, store.OpportunityFilter{
Title: title,
})
if err != nil {
t.Fatal(err)
}
@@ -247,8 +258,11 @@ func TestClassificationRowsAreShared(t *testing.T) {
mustSave(t, s, a)
mustSave(t, s, b)
if a.Classifications[0].ID == 0 || a.Classifications[0].ID != b.Classifications[0].ID {
t.Errorf("identical classifications should share an ID: %d vs %d",
a.Classifications[0].ID, b.Classifications[0].ID)
t.Errorf(
"identical classifications should share an ID: %d vs %d",
a.Classifications[0].ID,
b.Classifications[0].ID,
)
}
c := opportunity("ocds-c", "C", 1, "r1")
@@ -299,31 +313,59 @@ func TestListOpportunitiesFilters(t *testing.T) {
want []string
}{
{"all, newest first", store.OpportunityFilter{}, []string{"ocds-3", "ocds-2", "ocds-1"}},
{"status", store.OpportunityFilter{Status: "complete"}, []string{"ocds-2"}},
{"status", store.OpportunityFilter{
Status: "complete",
}, []string{"ocds-2"}},
{
"classification",
store.OpportunityFilter{ClassificationCode: "72000000"},
store.OpportunityFilter{
ClassificationCode: "72000000",
},
[]string{"ocds-3", "ocds-1"},
},
{"title word", store.OpportunityFilter{Title: "cloud"}, []string{"ocds-3", "ocds-1"}},
{"title prefixes", store.OpportunityFilter{Title: "clo migr"}, []string{"ocds-1"}},
{"title stemming", store.OpportunityFilter{Title: "migrating"}, []string{"ocds-1"}},
{"title case", store.OpportunityFilter{Title: "ROAD"}, []string{"ocds-2"}},
{"title word", store.OpportunityFilter{
Title: "cloud",
}, []string{"ocds-3", "ocds-1"}},
{"title prefixes", store.OpportunityFilter{
Title: "clo migr",
}, []string{"ocds-1"}},
{"title stemming", store.OpportunityFilter{
Title: "migrating",
}, []string{"ocds-1"}},
{"title case", store.OpportunityFilter{
Title: "ROAD",
}, []string{"ocds-2"}},
{
"title syntax is not FTS syntax",
store.OpportunityFilter{Title: `cloud" OR title:road*`},
store.OpportunityFilter{
Title: `cloud" OR title:road*`,
},
[]string{},
},
{
"title punctuation only",
store.OpportunityFilter{Title: `"*:()`},
store.OpportunityFilter{
Title: `"*:()`,
},
[]string{"ocds-3", "ocds-2", "ocds-1"},
},
{"released from", store.OpportunityFilter{ReleasedFrom: day(2)}, []string{"ocds-3", "ocds-2"}},
{"released before", store.OpportunityFilter{ReleasedBefore: day(2)}, []string{"ocds-1"}},
{"combined", store.OpportunityFilter{Title: "cloud", ReleasedFrom: day(2)}, []string{"ocds-3"}},
{"limit", store.OpportunityFilter{Limit: 2}, []string{"ocds-3", "ocds-2"}},
{"offset", store.OpportunityFilter{Limit: 2, Offset: 2}, []string{"ocds-1"}},
{"released from", store.OpportunityFilter{
ReleasedFrom: day(2),
}, []string{"ocds-3", "ocds-2"}},
{"released before", store.OpportunityFilter{
ReleasedBefore: day(2),
}, []string{"ocds-1"}},
{"combined", store.OpportunityFilter{
Title: "cloud",
ReleasedFrom: day(2),
}, []string{"ocds-3"}},
{"limit", store.OpportunityFilter{
Limit: 2,
}, []string{"ocds-3", "ocds-2"}},
{"offset", store.OpportunityFilter{
Limit: 2,
Offset: 2,
}, []string{"ocds-1"}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
@@ -337,7 +379,9 @@ func TestListOpportunitiesFilters(t *testing.T) {
})
}
got, err := s.ListOpportunities(ctx, store.OpportunityFilter{Status: "complete"})
got, err := s.ListOpportunities(ctx, store.OpportunityFilter{
Status: "complete",
})
if err != nil {
t.Fatal(err)
}
@@ -413,7 +457,9 @@ func TestReopenKeepsDataAndSkipsAppliedMigrations(t *testing.T) {
t.Fatalf("reopen: %v", err)
}
defer s.Close()
got, err := s.ListOpportunities(ctx, store.OpportunityFilter{Title: "cloud"})
got, err := s.ListOpportunities(ctx, store.OpportunityFilter{
Title: "cloud",
})
if err != nil || len(got) != 1 {
t.Errorf("after reopen: %v, %v", uuids(got), err)
}
@@ -440,8 +486,12 @@ func TestResaveLoadedOpportunity(t *testing.T) {
t.Errorf("resave not applied: %+v", got)
}
if got.Classifications[0].ID != ids[0] || got.Classifications[1].ID != ids[1] {
t.Errorf("classification IDs changed on resave: %v -> %d, %d",
ids, got.Classifications[0].ID, got.Classifications[1].ID)
t.Errorf(
"classification IDs changed on resave: %v -> %d, %d",
ids,
got.Classifications[0].ID,
got.Classifications[1].ID,
)
}
}
@@ -473,14 +523,19 @@ func TestSetMarksErrors(t *testing.T) {
ctx := context.Background()
if err := s.SetMarks(
ctx,
types.OpportunityMarks{UUID: "missing", Hidden: true},
types.OpportunityMarks{
UUID: "missing",
Hidden: true,
},
); !errors.Is(
err,
store.ErrNotFound,
) {
t.Errorf("unknown opportunity: err = %v, want ErrNotFound", err)
}
if err := s.SetMarks(ctx, types.OpportunityMarks{Hidden: true}); err == nil {
if err := s.SetMarks(ctx, types.OpportunityMarks{
Hidden: true,
}); err == nil {
t.Errorf("no UUID: expected an error")
}
}
@@ -492,7 +547,11 @@ func TestSaveKeepsExistingMarks(t *testing.T) {
ctx := context.Background()
mustSave(t, s, opportunity("ocds-1", "T", 1, "r1"))
mustSave(t, s, opportunity("ocds-2", "T", 1, "r1"))
want := types.OpportunityMarks{UUID: "ocds-1", Hidden: true, Lost: true}
want := types.OpportunityMarks{
UUID: "ocds-1",
Hidden: true,
Lost: true,
}
mustSetMarks(t, s, want)
newer := opportunity("ocds-1", "Amended", 2, "r2")
@@ -518,7 +577,9 @@ func TestSaveOpportunitiesStoresMarks(t *testing.T) {
s := openStore(t)
a := opportunity("ocds-a", "A", 1, "r1")
b := opportunity("ocds-b", "B", 1, "r1")
b.Marks = types.OpportunityMarks{Skipped: true}
b.Marks = types.OpportunityMarks{
Skipped: true,
}
if _, err := s.SaveOpportunities(context.Background(), []*types.Opportunity{a, b}); err != nil {
t.Fatal(err)
}
@@ -539,8 +600,15 @@ func TestListOpportunitiesMarkFilters(t *testing.T) {
mustSave(t, s, opportunity(uuid, "Cloud hosting", i+1, "r1"))
}
// ocds-1 keeps the marks it was saved with: Interested, BidSubmitted.
mustSetMarks(t, s, types.OpportunityMarks{UUID: "ocds-2", Hidden: true})
mustSetMarks(t, s, types.OpportunityMarks{UUID: "ocds-3", Skipped: true, Lost: true})
mustSetMarks(t, s, types.OpportunityMarks{
UUID: "ocds-2",
Hidden: true,
})
mustSetMarks(t, s, types.OpportunityMarks{
UUID: "ocds-3",
Skipped: true,
Lost: true,
})
mustSetMarks(t, s, types.OpportunityMarks{
UUID: "ocds-4", Interested: true, BidSubmitted: true, Won: true,
})
@@ -552,31 +620,65 @@ func TestListOpportunitiesMarkFilters(t *testing.T) {
want []string
}{
{"any", store.OpportunityFilter{}, []string{"ocds-4", "ocds-3", "ocds-2", "ocds-1"}},
{"hidden", store.OpportunityFilter{Hidden: set}, []string{"ocds-2"}},
{"not hidden", store.OpportunityFilter{Hidden: unset}, []string{"ocds-4", "ocds-3", "ocds-1"}},
{"skipped", store.OpportunityFilter{Skipped: set}, []string{"ocds-3"}},
{"hidden", store.OpportunityFilter{
Hidden: set,
}, []string{"ocds-2"}},
{"not hidden", store.OpportunityFilter{
Hidden: unset,
}, []string{"ocds-4", "ocds-3", "ocds-1"}},
{"skipped", store.OpportunityFilter{
Skipped: set,
}, []string{"ocds-3"}},
{
"not skipped",
store.OpportunityFilter{Skipped: unset},
store.OpportunityFilter{
Skipped: unset,
},
[]string{"ocds-4", "ocds-2", "ocds-1"},
},
{"interested", store.OpportunityFilter{Interested: set}, []string{"ocds-4", "ocds-1"}},
{"not interested", store.OpportunityFilter{Interested: unset}, []string{"ocds-3", "ocds-2"}},
{"bid submitted", store.OpportunityFilter{BidSubmitted: set}, []string{"ocds-4", "ocds-1"}},
{"interested", store.OpportunityFilter{
Interested: set,
}, []string{"ocds-4", "ocds-1"}},
{"not interested", store.OpportunityFilter{
Interested: unset,
}, []string{"ocds-3", "ocds-2"}},
{"bid submitted", store.OpportunityFilter{
BidSubmitted: set,
}, []string{"ocds-4", "ocds-1"}},
{
"bid not submitted",
store.OpportunityFilter{BidSubmitted: unset},
store.OpportunityFilter{
BidSubmitted: unset,
},
[]string{"ocds-3", "ocds-2"},
},
{"won", store.OpportunityFilter{Won: set}, []string{"ocds-4"}},
{"not won", store.OpportunityFilter{Won: unset}, []string{"ocds-3", "ocds-2", "ocds-1"}},
{"lost", store.OpportunityFilter{Lost: set}, []string{"ocds-3"}},
{"not lost", store.OpportunityFilter{Lost: unset}, []string{"ocds-4", "ocds-2", "ocds-1"}},
{"two marks", store.OpportunityFilter{BidSubmitted: set, Won: unset}, []string{"ocds-1"}},
{"no match", store.OpportunityFilter{Won: set, Lost: set}, []string{}},
{"won", store.OpportunityFilter{
Won: set,
}, []string{"ocds-4"}},
{"not won", store.OpportunityFilter{
Won: unset,
}, []string{"ocds-3", "ocds-2", "ocds-1"}},
{"lost", store.OpportunityFilter{
Lost: set,
}, []string{"ocds-3"}},
{"not lost", store.OpportunityFilter{
Lost: unset,
}, []string{"ocds-4", "ocds-2", "ocds-1"}},
{"two marks", store.OpportunityFilter{
BidSubmitted: set,
Won: unset,
}, []string{"ocds-1"}},
{"no match", store.OpportunityFilter{
Won: set,
Lost: set,
}, []string{}},
{
"with other filters",
store.OpportunityFilter{Hidden: unset, Title: "cloud", ReleasedFrom: day(3)},
store.OpportunityFilter{
Hidden: unset,
Title: "cloud",
ReleasedFrom: day(3),
},
[]string{"ocds-4", "ocds-3"},
},
}
@@ -608,7 +710,11 @@ func TestWonAndLostAreExclusive(t *testing.T) {
mustSave(t, s, opportunity("ocds-1", "T", 1, "r1"))
before := mustGet(t, s, "ocds-1").Marks
both := types.OpportunityMarks{UUID: "ocds-1", Won: true, Lost: true}
both := types.OpportunityMarks{
UUID: "ocds-1",
Won: true,
Lost: true,
}
if err := s.SetMarks(ctx, both); !errors.Is(err, types.ErrWonAndLost) {
t.Errorf("SetMarks(won and lost): err = %v, want ErrWonAndLost", err)
}
@@ -617,7 +723,10 @@ func TestWonAndLostAreExclusive(t *testing.T) {
}
in := opportunity("ocds-2", "T", 1, "r1")
in.Marks = types.OpportunityMarks{Won: true, Lost: true}
in.Marks = types.OpportunityMarks{
Won: true,
Lost: true,
}
if _, err := s.SaveOpportunity(ctx, in); !errors.Is(err, types.ErrWonAndLost) {
t.Errorf("SaveOpportunity(won and lost): err = %v, want ErrWonAndLost", err)
}