feat(sync): a stable identity per contact, indexed

Two PCs exchanging changes through a folder must be able to name the SAME contact in both logs. "the QSO with M0ABC at 14:32" is a guess and the two machines can disagree about which row that is, so an edit or a deletion cannot be addressed at all without an identity that travels with the record. A real indexed column, not a key inside extras_json: the identity is resolved once per incoming change, and scanning JSON for it would turn every sync into a full read of a 120 000-QSO logbook. A column costs one migration; the JSON would cost a scan every time. That was the decision to confirm, and it is confirmed. It follows the award_refs pattern — read in selectCols, absent from columnList, written only through its own methods. So an ordinary edit cannot clobber it, which matters: another machine addresses the contact by that id, and losing it makes the same QSO arrive again as a new one. Pinned by a test that saves an edit with the field deliberately blanked. Existing contacts are stamped in batches inside one transaction rather than one commit each — on a remote MySQL the round trip dominates, and 120 000 commits is the difference between a minute and an afternoon. And a dedupe-key map lets two machines that already hold the same imported log recognise each other's contacts instead of copying 120 000 of them across. MySQL nearly lost its logbook to this. It cannot index a TEXT column without a prefix length: the migration would fail with error 1170 and fail again on every startup, with no way out from the interface. The translator only emits VARCHAR for names listed by hand in varcharColumns. sync_uid is now listed — and a test reads the migrations, works out which indexed columns are TEXT, and fails if the list does not cover them, so the next one cannot reach a shared logbook.
2026-08-16 23:16:52 +02:00
parent 7d664bd1de
commit 724e68b38d
6 changed files with 464 additions and 2 deletions
@@ -0,0 +1,87 @@
+package db
+
+import (
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// A TEXT column that an index is built on must be listed in varcharColumns.
+//
+// MySQL cannot index a TEXT column without a prefix length: the migration dies
+// with error 1170 — and it dies again on every startup afterwards, leaving the
+// operator with a logbook that will not connect and no way forward from the
+// interface. The translator only emits VARCHAR for the names in varcharColumns,
+// and that list is maintained by hand.
+//
+// So: read the migrations, work out which columns are TEXT, find which are
+// indexed, and insist the list covers the overlap. Integer columns are indexed
+// perfectly well and are none of this test's business.
+func TestIndexedTextColumnsAreVarchar(t *testing.T) {
+	files, err := migrationsFS.ReadDir("migrations")
+	if err != nil {
+		t.Fatalf("read migrations: %v", err)
+	}
+
+	// name → declared type, from "ADD COLUMN name TYPE" and from the column
+	// lines inside a CREATE TABLE body.
+	declared := map[string]string{}
+	reAdd := regexp.MustCompile(`(?i)ADD\s+COLUMN\s+` + "`" + `?(\w+)` + "`" + `?\s+(\w+)`)
+	reCol := regexp.MustCompile(`(?im)^\s*` + "`" + `?(\w+)` + "`" + `?\s+(TEXT|VARCHAR|INTEGER|INT|REAL|BLOB|DATETIME|BOOLEAN)\b`)
+	reIdx := regexp.MustCompile(`(?i)CREATE\s+INDEX\s+(?:IF\s+NOT\s+EXISTS\s+)?\S+\s+ON\s+\S+\s*\(([^)]*)\)`)
+
+	var indexed []struct{ file, col string }
+	for _, f := range files {
+		if f.IsDir() || !strings.HasSuffix(f.Name(), ".sql") {
+			continue
+		}
+		body, err := migrationsFS.ReadFile("migrations/" + f.Name())
+		if err != nil {
+			t.Fatalf("read %s: %v", f.Name(), err)
+		}
+		src := string(body)
+		for _, m := range reAdd.FindAllStringSubmatch(src, -1) {
+			declared[strings.ToLower(m[1])] = strings.ToUpper(m[2])
+		}
+		for _, m := range reCol.FindAllStringSubmatch(src, -1) {
+			if _, seen := declared[strings.ToLower(m[1])]; !seen {
+				declared[strings.ToLower(m[1])] = strings.ToUpper(m[2])
+			}
+		}
+		for _, m := range reIdx.FindAllStringSubmatch(src, -1) {
+			for _, col := range strings.Split(m[1], ",") {
+				col = strings.TrimSpace(col)
+				if col == "" || strings.Contains(col, "(") {
+					continue // a prefix length or an expression is MySQL-safe already
+				}
+				col = strings.Trim(col, "`\"")
+				if i := strings.IndexAny(col, " \t"); i > 0 {
+					col = col[:i] // "col DESC"
+				}
+				indexed = append(indexed, struct{ file, col string }{f.Name(), strings.ToLower(col)})
+			}
+		}
+	}
+
+	if len(indexed) == 0 || len(declared) == 0 {
+		t.Fatal("nothing parsed out of the migrations — this test has stopped checking anything")
+	}
+
+	textIndexed := 0
+	for _, ix := range indexed {
+		typ, known := declared[ix.col]
+		if !known || typ != "TEXT" {
+			continue // an integer index, or a column this test could not type
+		}
+		textIndexed++
+		if !varcharColumns[ix.col] {
+			t.Errorf("%s indexes the TEXT column %q, which is not in varcharColumns — "+
+				"on MySQL that migration fails with error 1170 and the logbook stops connecting for good",
+				ix.file, ix.col)
+		}
+	}
+	if textIndexed == 0 {
+		t.Fatal("no indexed TEXT column found — the parsing has drifted and this test checks nothing")
+	}
+	t.Logf("%d indexed TEXT column(s) checked against varcharColumns", textIndexed)
+}
@@ -0,0 +1,20 @@
+-- A stable identity per contact, for folder-based synchronisation.
+--
+-- Two PCs exchanging changes through a shared folder need to name the SAME
+-- contact in both logs. "the QSO with M0ABC at 14:32" is a guess, and the two
+-- machines can disagree about which row that is — so an edit or a deletion
+-- cannot be addressed at all without an identity that travels with the record.
+--
+-- A REAL COLUMN, indexed, rather than a key inside extras_json. The identity is
+-- looked up once per incoming change, and on a 120 000-QSO logbook scanning
+-- JSON for it would turn every sync into a full table read. A column costs one
+-- migration; the JSON would cost a scan every time.
+--
+-- sync_uid is listed in varcharColumns (internal/db/mysql.go): MySQL cannot
+-- index a TEXT column without a prefix length, and a migration that tries dies
+-- with error 1170 on every startup thereafter, with no way out from the UI.
+--
+-- Empty on every existing row: they are given an identity the first time
+-- synchronisation is switched on, in one bulk statement, not row by row.
+ALTER TABLE qso ADD COLUMN sync_uid TEXT NOT NULL DEFAULT '';
+CREATE INDEX IF NOT EXISTS idx_qso_sync_uid ON qso (sync_uid);
@@ -67,6 +67,10 @@ var varcharColumns = map[string]bool{
 	"callsign": true, "qso_date": true, "band": true, "mode": true,
 	"grid": true, "station_callsign": true, "state": true, "contest_id": true,
 	"sat_name": true, "prop_mode": true, "sig": true, "wwff_ref": true, "skcc": true,
+	"sync_uid": true, // qso index (0029) — folder-sync identity, looked up per record
+	// A new indexed TEXT column MUST be added here. TestIndexedTextColumnsAreVarchar
+	// reads the migrations and fails if one is missing, because the alternative is
+	// a MySQL logbook that stops connecting for good on error 1170.
 	// integrations_udp index (0011)
 	"direction": true,
 	// award_references composite primary key (0017)
@@ -46,6 +46,51 @@ street, no image.
 </Callsign>
 ```

+```
+<QRZDatabase xmlns="http://xmldata.qrz.com" version="1.36">
+<div id="in-page-channel-node-id" data-channel-name="in_page_channel_ogW01U"/>
+<Callsign>
+<call>F5IRH</call>
+<dxcc>227</dxcc>
+<fname>AVRILLON</fname>
+<name>Max</name>
+<addr1>La Grand Prairie</addr1>
+<addr2>Le Palais BELLE-ILE-EN-MER</addr2>
+<zip>56360</zip>
+<country>France</country>
+<lat>47.339686</lat>
+<lon>-3.156500</lon>
+<grid>IN87ki</grid>
+<ccode>97</ccode>
+<land>France</land>
+<codes>TP</codes>
+<qslmgr>VIA BURO</qslmgr>
+<email>[email protected]</email>
+<u_views>8005</u_views>
+<bio>2463</bio>
+<biodate>2015-07-16 00:29:49</biodate>
+<image>https://cdn-xml.qrz.com/h/f5irh/qsl_F5IRH_111-3.JPG</image>
+<imageinfo>518:799:105108</imageinfo>
+<moddate>2010-08-05 01:05:37</moddate>
+<eqsl>0</eqsl>
+<mqsl>0</mqsl>
+<cqzone>14</cqzone>
+<iota>EU-048</iota>
+<lotw>0</lotw>
+<geoloc>user</geoloc>
+<name_fmt>AVRILLON Max</name_fmt>
+<serial>1570698</serial>
+</Callsign>
+<Session>
+<Key>e5ca5b3e7f88d733408ab7677e605270</Key>
+<Count>161639</Count>
+<SubExp>Sat Jul 3 21:45:38 2027</SubExp>
+<GMTime>Sun Aug 16 21:14:40 2026</GMTime>
+<Remark>cpu: 0.073s</Remark>
+</Session>
+</QRZDatabase>
+```
+
 ## Consequences

 - **A free QRZ account can never fill the locator, the coordinates, the zones
@@ -215,6 +215,12 @@ type QSO struct {
 	// that doesn't know about it can't clobber it.
 	AwardRefs string `json:"award_refs,omitempty"`

+	// SyncUID is this contact's stable identity for folder synchronisation —
+	// what lets another machine name the same QSO when it edits or deletes it.
+	// Like AwardRefs it is read here but NOT in columnList, so an ordinary edit
+	// can never clobber it; it is written only by SetSyncUID / BackfillSyncUIDs.
+	SyncUID string `json:"sync_uid,omitempty"`
+
 	CreatedAt time.Time `json:"created_at"`
 	UpdatedAt time.Time `json:"updated_at"`
 }
@@ -271,7 +277,7 @@ const columnList = `callsign, qso_date, qso_date_off, band, band_rx, mode, submo
 // award_refs is read here but is NOT part of columnList (the insert/update
 // write path) — it is a derived cache written only via SetAwardRefs, so a
 // normal QSO write can never clobber it.
-const selectCols = `id, ` + columnList + `, award_refs, created_at, updated_at`
+const selectCols = `id, ` + columnList + `, award_refs, sync_uid, created_at, updated_at`

 // columnCount is derived from columnList at init so they can never drift.
 var columnCount = countColumns(columnList)
@@ -3076,6 +3082,7 @@ func scanQSO(s scanner) (QSO, error) {
 		myARRLSect, myVUCCGrids                sql.NullString
 		extrasJSON                             sql.NullString
 		awardRefs                              sql.NullString
+		syncUID                                sql.NullString
 		createdStr, updatedStr                 string
 	)
 	if err := s.Scan(
@@ -3103,7 +3110,7 @@ func scanQSO(s scanner) (QSO, error) {
 		&skcc, &fists, &tenTen, &contactedOp, &eqCall, &pfx, &myName, &class,
 		&darcDOK, &myDarcDOK, &region, &silentKey, &swl, &qsoComplete, &qsoRandom,
 		&creditGranted, &creditSubmitted, &myARRLSect, &myVUCCGrids,
-		&extrasJSON, &awardRefs, &createdStr, &updatedStr,
+		&extrasJSON, &awardRefs, &syncUID, &createdStr, &updatedStr,
 	); err != nil {
 		return QSO{}, fmt.Errorf("scan qso: %w", err)
 	}
@@ -3303,6 +3310,7 @@ func scanQSO(s scanner) (QSO, error) {
 	q.MyVUCCGrids = myVUCCGrids.String
 	q.Extras = decodeExtras(extrasJSON.String)
 	q.AwardRefs = awardRefs.String
+	q.SyncUID = syncUID.String
 	return q, nil
 }

@@ -3413,3 +3421,131 @@ func (r *Repo) OrderedIDs(ctx context.Context) ([]int64, time.Time, error) {
 	// Parsed once, for the newest row only — the ordering came from SQL.
 	return out, parseTimeLoose(lastDate), nil
 }
+
+// --- Folder synchronisation identity -----------------------------------------
+
+// SetSyncUID stamps a contact's sync identity. Targeted UPDATE: it must never
+// go through the normal write path, or an ordinary edit would clobber it.
+func (r *Repo) SetSyncUID(ctx context.Context, id int64, uid string) error {
+	_, err := r.db.ExecContext(ctx, `UPDATE qso SET sync_uid = ? WHERE id = ?`, uid, id)
+	return err
+}
+
+// IDBySyncUID resolves an incoming change to a local row. Indexed, so this is
+// the one lookup a sync performs per record and it stays a key hit rather than
+// a scan — which is the whole reason sync_uid is a column and not a JSON key.
+func (r *Repo) IDBySyncUID(ctx context.Context, uid string) (int64, bool, error) {
+	if uid == "" {
+		return 0, false, nil
+	}
+	var id int64
+	err := r.db.QueryRowContext(ctx, `SELECT id FROM qso WHERE sync_uid = ? LIMIT 1`, uid).Scan(&id)
+	if err == sql.ErrNoRows {
+		return 0, false, nil
+	}
+	if err != nil {
+		return 0, false, err
+	}
+	return id, true, nil
+}
+
+// CountWithoutSyncUID reports how many contacts still have no identity — what
+// the settings panel shows before the first synchronisation.
+func (r *Repo) CountWithoutSyncUID(ctx context.Context) (int64, error) {
+	var n int64
+	err := r.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM qso WHERE sync_uid = '' OR sync_uid IS NULL`).Scan(&n)
+	return n, err
+}
+
+// NeedSyncUID returns the ids of contacts still without an identity, at most
+// limit of them.
+//
+// Batched rather than "all of them": a 120 000-QSO logbook is given its
+// identities a few thousand at a time, so the first synchronisation cannot hold
+// the database — or the interface — for however long a single enormous
+// transaction would take on a remote MySQL.
+func (r *Repo) NeedSyncUID(ctx context.Context, limit int) ([]int64, error) {
+	if limit <= 0 {
+		limit = 2000
+	}
+	rows, err := r.db.QueryContext(ctx,
+		`SELECT id FROM qso WHERE sync_uid = '' OR sync_uid IS NULL ORDER BY id LIMIT ?`, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var out []int64
+	for rows.Next() {
+		var id int64
+		if err := rows.Scan(&id); err != nil {
+			return nil, err
+		}
+		out = append(out, id)
+	}
+	return out, rows.Err()
+}
+
+// AssignSyncUIDs stamps a batch of identities in ONE transaction.
+//
+// One statement per row inside a single transaction, not one transaction per
+// row: on a remote MySQL the round trip dominates, and 120 000 separate commits
+// is the difference between a minute and an afternoon.
+func (r *Repo) AssignSyncUIDs(ctx context.Context, uids map[int64]string) error {
+	if len(uids) == 0 {
+		return nil
+	}
+	tx, err := r.db.BeginTx(ctx, nil)
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+	stmt, err := tx.PrepareContext(ctx, `UPDATE qso SET sync_uid = ? WHERE id = ? AND (sync_uid = '' OR sync_uid IS NULL)`)
+	if err != nil {
+		return err
+	}
+	defer stmt.Close()
+	for id, uid := range uids {
+		if _, err := stmt.ExecContext(ctx, uid, id); err != nil {
+			return err
+		}
+	}
+	return tx.Commit()
+}
+
+// SyncUIDByDedupeKey maps every contact's dedupe key to its sync identity.
+//
+// This is what stops two machines that already hold the SAME imported log from
+// copying it to each other: an incoming contact whose identity is unknown but
+// whose key matches one here is the same contact, and it adopts the incoming
+// identity instead of being inserted a second time.
+//
+// One scan of four short columns, done once per synchronisation pass — the same
+// shape as the importer's own dedupe load, which is the cheapest read in the
+// repo.
+func (r *Repo) SyncUIDByDedupeKey(ctx context.Context) (map[string]struct {
+	ID  int64
+	UID string
+}, error) {
+	rows, err := r.db.QueryContext(ctx, `
+		SELECT id, sync_uid, callsign, substr(qso_date, 1, 16), band, mode FROM qso`)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	out := make(map[string]struct {
+		ID  int64
+		UID string
+	}, 1024)
+	for rows.Next() {
+		var id int64
+		var uid, call, when, band, mode sql.NullString
+		if err := rows.Scan(&id, &uid, &call, &when, &band, &mode); err != nil {
+			return nil, err
+		}
+		out[DedupeKey(call.String, when.String, band.String, mode.String)] = struct {
+			ID  int64
+			UID string
+		}{ID: id, UID: uid.String}
+	}
+	return out, rows.Err()
+}
@@ -0,0 +1,170 @@
+package qso
+
+import (
+	"context"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"hamlog/internal/db"
+)
+
+// openRepo gives a migrated, empty logbook on disk.
+func openRepo(t *testing.T) *Repo {
+	t.Helper()
+	conn, err := db.Open(filepath.Join(t.TempDir(), "test.db"))
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	t.Cleanup(func() { conn.Close() })
+	return NewRepo(conn)
+}
+
+func addQSO(t *testing.T, r *Repo, call string, when time.Time) int64 {
+	t.Helper()
+	id, err := r.Add(context.Background(), QSO{
+		Callsign: call, QSODate: when, Band: "20m", Mode: "CW",
+	})
+	if err != nil {
+		t.Fatalf("insert %s: %v", call, err)
+	}
+	return id
+}
+
+// A QSO written and read back must come out whole. selectCols and scanQSO are
+// two hand-maintained lists that must line up column for column, and adding
+// sync_uid touched both — a drift there fails every read at runtime, which no
+// compiler catches.
+func TestSyncUIDRoundTrips(t *testing.T) {
+	r := openRepo(t)
+	ctx := context.Background()
+	id := addQSO(t, r, "M0ABC", time.Date(2026, 8, 16, 14, 32, 0, 0, time.UTC))
+
+	got, err := r.GetByID(ctx, id)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if got.Callsign != "M0ABC" {
+		t.Fatalf("read back %+v", got)
+	}
+	if got.SyncUID != "" {
+		t.Errorf("a fresh QSO has identity %q — it should have none until sync is switched on", got.SyncUID)
+	}
+
+	if err := r.SetSyncUID(ctx, id, "abc123"); err != nil {
+		t.Fatalf("SetSyncUID: %v", err)
+	}
+	got, _ = r.GetByID(ctx, id)
+	if got.SyncUID != "abc123" {
+		t.Errorf("SyncUID = %q after stamping", got.SyncUID)
+	}
+}
+
+// An ordinary edit must NEVER clobber the identity. sync_uid is deliberately
+// outside columnList for this reason: another machine that has already seen the
+// contact addresses it by that id, and losing it makes the same QSO arrive
+// again as a new one.
+func TestAnEditDoesNotClobberTheIdentity(t *testing.T) {
+	r := openRepo(t)
+	ctx := context.Background()
+	id := addQSO(t, r, "M0ABC", time.Date(2026, 8, 16, 14, 32, 0, 0, time.UTC))
+	if err := r.SetSyncUID(ctx, id, "keepme"); err != nil {
+		t.Fatal(err)
+	}
+
+	q, _ := r.GetByID(ctx, id)
+	q.Name = "Edited"
+	q.SyncUID = "" // exactly what a caller that knows nothing about sync sends
+	if err := r.Update(ctx, q); err != nil {
+		t.Fatalf("update: %v", err)
+	}
+
+	got, _ := r.GetByID(ctx, id)
+	if got.Name != "Edited" {
+		t.Errorf("the edit did not take: %+v", got)
+	}
+	if got.SyncUID != "keepme" {
+		t.Errorf("SyncUID = %q — an edit wiped the sync identity", got.SyncUID)
+	}
+}
+
+// The lookup an incoming change performs, once per record.
+func TestIDBySyncUID(t *testing.T) {
+	r := openRepo(t)
+	ctx := context.Background()
+	id := addQSO(t, r, "M0ABC", time.Date(2026, 8, 16, 14, 32, 0, 0, time.UTC))
+	_ = r.SetSyncUID(ctx, id, "u-1")
+
+	got, ok, err := r.IDBySyncUID(ctx, "u-1")
+	if err != nil || !ok || got != id {
+		t.Fatalf("IDBySyncUID = (%d,%v,%v), want (%d,true,nil)", got, ok, err, id)
+	}
+	if _, ok, _ := r.IDBySyncUID(ctx, "nope"); ok {
+		t.Error("an unknown identity was resolved")
+	}
+	// An empty id must never match the rows that have none.
+	if _, ok, _ := r.IDBySyncUID(ctx, ""); ok {
+		t.Error("the empty identity matched a row — every un-stamped QSO would be that row")
+	}
+}
+
+// Backfill: batched, and it must never overwrite an identity already there.
+func TestAssignSyncUIDsOnlyFillsTheEmptyOnes(t *testing.T) {
+	r := openRepo(t)
+	ctx := context.Background()
+	base := time.Date(2026, 8, 16, 14, 0, 0, 0, time.UTC)
+	a := addQSO(t, r, "M0AAA", base)
+	b := addQSO(t, r, "M0BBB", base.Add(time.Minute))
+	_ = r.SetSyncUID(ctx, a, "already")
+
+	n, err := r.CountWithoutSyncUID(ctx)
+	if err != nil || n != 1 {
+		t.Fatalf("CountWithoutSyncUID = %d (%v), want 1", n, err)
+	}
+	ids, err := r.NeedSyncUID(ctx, 100)
+	if err != nil || len(ids) != 1 || ids[0] != b {
+		t.Fatalf("NeedSyncUID = %v (%v), want [%d]", ids, err, b)
+	}
+
+	// Try to overwrite BOTH; only the empty one may take.
+	if err := r.AssignSyncUIDs(ctx, map[int64]string{a: "stomp", b: "fresh"}); err != nil {
+		t.Fatalf("AssignSyncUIDs: %v", err)
+	}
+	qa, _ := r.GetByID(ctx, a)
+	qb, _ := r.GetByID(ctx, b)
+	if qa.SyncUID != "already" {
+		t.Errorf("an existing identity was overwritten: %q", qa.SyncUID)
+	}
+	if qb.SyncUID != "fresh" {
+		t.Errorf("the empty one was not filled: %q", qb.SyncUID)
+	}
+	if n, _ := r.CountWithoutSyncUID(ctx); n != 0 {
+		t.Errorf("%d contacts still without an identity", n)
+	}
+}
+
+// Two machines holding the SAME imported log must not copy it to each other.
+// An incoming contact with an unknown identity but a matching dedupe key is the
+// same contact, and adopts the incoming id rather than being inserted twice.
+func TestSyncUIDByDedupeKeyFindsTheSameContact(t *testing.T) {
+	r := openRepo(t)
+	ctx := context.Background()
+	when := time.Date(2026, 8, 16, 14, 32, 0, 0, time.UTC)
+	id := addQSO(t, r, "M0ABC", when)
+
+	byKey, err := r.SyncUIDByDedupeKey(ctx)
+	if err != nil {
+		t.Fatalf("SyncUIDByDedupeKey: %v", err)
+	}
+	key := DedupeKey("M0ABC", when.UTC().Format("2006-01-02T15:04"), "20m", "CW")
+	hit, ok := byKey[key]
+	if !ok {
+		t.Fatalf("key %q not found among %d entries", key, len(byKey))
+	}
+	if hit.ID != id {
+		t.Errorf("key resolved to id %d, want %d", hit.ID, id)
+	}
+	if hit.UID != "" {
+		t.Errorf("UID = %q, want empty before any stamping", hit.UID)
+	}
+}