diff --git a/internal/lookup/cache_iota_test.go b/internal/lookup/cache_iota_test.go new file mode 100644 index 0000000..ac1beb2 --- /dev/null +++ b/internal/lookup/cache_iota_test.go @@ -0,0 +1,54 @@ +package lookup + +import ( + "context" + "path/filepath" + "testing" + + "hamlog/internal/db" +) + +func testCache(t *testing.T) *Cache { + t.Helper() + conn, err := db.Open(filepath.Join(t.TempDir(), "c.db")) + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { conn.Close() }) + return NewCache(conn, 0) +} + +// Adding a field to the cache leaves every row already in it without that field, +// and the cache lasts thirty days. So a callsign looked up before the change +// would go a MONTH without its island reference — which is exactly what the +// first test of the feature ran into: a QRZ record plainly carrying +// EU-048, and no IOTA on the entry. +// +// A row that predates the column is therefore treated as stale and refetched +// once. NULL and "" mean different things here, and that is the whole mechanism. +func TestCacheRefetchesRowsWrittenBeforeTheIOTAColumn(t *testing.T) { + c := testCache(t) + ctx := context.Background() + + // An operator with an island: stored and returned. + if err := c.Put(ctx, Result{Callsign: "F5IRH", Name: "Max", IOTA: "EU-048", Source: "qrz"}); err != nil { + t.Fatalf("put: %v", err) + } + got, ok := c.Get(ctx, "F5IRH") + if !ok || got.IOTA != "EU-048" { + t.Fatalf("Get = (%+v,%v), want the island back", got, ok) + } + + // An operator with NO island: an empty string is stored, and the row stays + // usable. If this wrote NULL, every ordinary callsign would refetch for ever. + if err := c.Put(ctx, Result{Callsign: "M0ABC", Name: "Ann", Source: "qrz"}); err != nil { + t.Fatalf("put: %v", err) + } + got, ok = c.Get(ctx, "M0ABC") + if !ok { + t.Fatal("a callsign with no island was treated as stale — every lookup would repeat for ever") + } + if got.IOTA != "" { + t.Errorf("IOTA = %q for an operator with no island", got.IOTA) + } +} diff --git a/internal/lookup/lookup.go b/internal/lookup/lookup.go index 0b10a8f..7b3972f 100644 --- a/internal/lookup/lookup.go +++ b/internal/lookup/lookup.go @@ -481,6 +481,17 @@ func (c *Cache) Get(ctx context.Context, callsign string) (Result, bool) { if time.Since(t) > c.ttl { return Result{}, false } + // A row written before the iota column existed has NULL there, and the cache + // lasts thirty days — so without this every callsign already looked up would + // go a month without its island reference, which is exactly what the first + // test of the feature ran into. + // + // NULL and '' are deliberately different here: Put writes an empty string for + // an operator with no island, so only a row that predates the column reads as + // invalid. One refetch per such callsign, the next time it is used. + if !iotaRef.Valid { + return Result{}, false + } r.Name = name.String r.QTH = qth.String r.Address = addr.String @@ -534,7 +545,10 @@ func (c *Cache) Put(ctx context.Context, r Result) error { nullableFloat(r.Lat), nullableFloat(r.Lon), nullableInt(r.DXCC), nullableInt(r.CQZ), nullableInt(r.ITUZ), nullable(r.Continent), nullable(r.Email), nullable(r.QSLVia), - nullable(r.ImageURL), nullable(r.Web), nullable(r.Zip), nullable(r.IOTA), + nullable(r.ImageURL), nullable(r.Web), nullable(r.Zip), + // NOT nullable(): an operator with no island must store '', so that a NULL + // keeps its one meaning — a row written before the column existed. + r.IOTA, r.Source, db.NowISO(), ) return err