From b21bf072080a2144d99c976a35a80e34e04a8327 Mon Sep 17 00:00:00 2001 From: Vladimir Zagainov Date: Sun, 19 Jul 2026 20:01:26 +0300 Subject: [PATCH] fix: address code review findings - Store cached_at in canonical UTC layout so the cache TTL cutoff comparison is a valid time ordering (previously go-sqlite3 serialized time.Time as RFC3339, making the space-separated cutoff match only by ASCII accident; same-day expired entries were falsely served as fresh). - Remove stale no-op scanner config keys (ignore_bootlegs, include_compilations) from config.yaml.example and docs; these fields were removed from ScannerConfig but left in configs, silently doing nothing. --- README.md | 4 ---- config.yaml.example | 2 -- docs/Specification.md | 2 -- internal/database/external_releases.go | 27 ++++++++++++++++++-------- internal/musicbrainz/sync.go | 2 +- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 11f3ea8..351de0c 100644 --- a/README.md +++ b/README.md @@ -94,8 +94,6 @@ telegram: scanner: fuzzy_threshold: 0.85 - ignore_bootlegs: true - include_compilations: true ``` See [docs/Specification.md](docs/Specification.md) for the full configuration reference and architecture details. @@ -206,8 +204,6 @@ telegram: scanner: fuzzy_threshold: 0.85 - ignore_bootlegs: true - include_compilations: true ``` Полную справку по конфигурации и архитектуру см. в [docs/Specification.md](docs/Specification.md). diff --git a/config.yaml.example b/config.yaml.example index 95cc5ae..d1942c0 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -22,5 +22,3 @@ telegram: scanner: fuzzy_threshold: 0.85 - ignore_bootlegs: true - include_compilations: true diff --git a/docs/Specification.md b/docs/Specification.md index ddbed98..2922e0d 100644 --- a/docs/Specification.md +++ b/docs/Specification.md @@ -147,8 +147,6 @@ telegram: scanner: fuzzy_threshold: 0.85 - ignore_bootlegs: true - include_compilations: true ``` diff --git a/internal/database/external_releases.go b/internal/database/external_releases.go index ca4bccb..c2388b3 100644 --- a/internal/database/external_releases.go +++ b/internal/database/external_releases.go @@ -9,6 +9,21 @@ import ( _ "github.com/mattn/go-sqlite3" ) +// utcLayout is the canonical layout for the cached_at column. go-sqlite3 +// serializes a time.Time as RFC3339, which does not compare correctly against +// the space-separated cutoff used by the cache query. Storing this layout keeps +// the lexicographic comparison in GetExternalReleasesByArtistWithCache valid. +const utcLayout = "2006-01-02 15:04:05" + +// FormatCachedAt renders a timestamp in the canonical UTC layout for storage. +// A zero time yields nil so the column is left NULL. +func FormatCachedAt(t time.Time) interface{} { + if t.IsZero() { + return nil + } + return t.UTC().Format(utcLayout) +} + // joinSecondaryTypes renders a slice of secondary types as a comma-separated // string for storage in the secondary_types TEXT column (empty when none). func joinSecondaryTypes(types []string) string { @@ -54,10 +69,7 @@ func GetExternalRelease(db *DB, rgid string) (*ExternalRelease, error) { // SaveExternalRelease inserts or replaces an external_release row. func SaveExternalRelease(db *DB, release *ExternalRelease) error { - var cachedAt interface{} - if !release.CachedAt.IsZero() { - cachedAt = release.CachedAt - } + var cachedAt interface{} = FormatCachedAt(release.CachedAt) _, err := db.Conn().Exec( "INSERT OR REPLACE INTO external_releases (rgid, artist_id, title, type, release_date, is_ignored, cached_at, secondary_types) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", release.RGID, release.ArtistID, release.Title, release.Type, release.ReleaseDate, release.IsIgnored, cachedAt, joinSecondaryTypes(release.SecondaryTypes), @@ -157,10 +169,9 @@ func SetReleaseIgnored(db *DB, rgid string, ignored bool) error { // GetExternalReleasesByArtistWithCache returns cached external_release rows for a given artist_id // that are within the specified TTL. func GetExternalReleasesByArtistWithCache(db *DB, artistID string, ttl time.Duration) ([]ExternalRelease, error) { - // cached_at is a TEXT DATETIME column serialized by the driver in the - // "2006-01-02 15:04:05" UTC layout. Compare against an explicitly - // formatted cutoff string in the same layout so the lexicographic - // comparison does not depend on the driver's time serialization behavior. + // cached_at is stored in the "2006-01-02 15:04:05" UTC layout via + // FormatCachedAt. Compare against an explicitly formatted cutoff string in + // the same layout so the lexicographic comparison is a valid time ordering. const layout = "2006-01-02 15:04:05" cutoff := time.Now().UTC().Add(-ttl).Format(layout) rows, err := db.Conn().Query( diff --git a/internal/musicbrainz/sync.go b/internal/musicbrainz/sync.go index 0f7ff04..6ab7cce 100644 --- a/internal/musicbrainz/sync.go +++ b/internal/musicbrainz/sync.go @@ -137,7 +137,7 @@ func SyncArtistDiscography( if _, err := tx.Exec( "INSERT INTO external_releases (rgid, artist_id, title, type, release_date, is_ignored, cached_at) VALUES (?, ?, ?, ?, ?, ?, ?)", - ext.RGID, ext.ArtistID, ext.Title, ext.Type, ext.ReleaseDate, ext.IsIgnored, ext.CachedAt, + ext.RGID, ext.ArtistID, ext.Title, ext.Type, ext.ReleaseDate, ext.IsIgnored, database.FormatCachedAt(ext.CachedAt), ); err != nil { return nil, fmt.Errorf("sync artist discography: insert release %s: %w", rg.ID, err) }