From 0991aaccc074e1755f1ca345c3cf4d741d74fd18 Mon Sep 17 00:00:00 2001 From: Lewis Date: Mon, 6 Jul 2026 10:47:25 +0300 Subject: [PATCH] appview/db: now-redundant indexed col in label ops Lewis: May this revision serve well! --- appview/db/db.go | 5 ++ appview/db/label.go | 26 +++------ appview/db/label_migration_test.go | 92 ++++++++++++++++++++++++++++++ appview/labels/labels.go | 5 +- appview/models/label.go | 24 ++------ 5 files changed, 109 insertions(+), 43 deletions(-) create mode 100644 appview/db/label_migration_test.go diff --git a/appview/db/db.go b/appview/db/db.go index c7313f53..9dcdcfaf 100644 --- a/appview/db/db.go +++ b/appview/db/db.go @@ -2421,6 +2421,11 @@ func Make(ctx context.Context, dbPath string) (*DB, error) { return err }) + orm.RunMigration(conn, logger, "drop-label-ops-indexed", func(tx *sql.Tx) error { + _, err := tx.Exec(`alter table label_ops drop column indexed`) + return err + }) + return &DB{ db, logger, diff --git a/appview/db/label.go b/appview/db/label.go index 18d5780c..3cfce164 100644 --- a/appview/db/label.go +++ b/appview/db/label.go @@ -186,7 +186,6 @@ func GetLabelDefinition(e Execer, filters ...orm.Filter) (*models.LabelDefinitio } func AddLabelOp(e Execer, l *models.LabelOp) (int64, error) { - now := time.Now() result, err := e.Exec( `insert into label_ops ( did, @@ -195,15 +194,13 @@ func AddLabelOp(e Execer, l *models.LabelOp) (int64, error) { operation, operand_key, operand_value, - performed, - indexed + performed ) - values (?, ?, ?, ?, ?, ?, ?, ?) + values (?, ?, ?, ?, ?, ?, ?) on conflict(did, rkey, subject, operand_key, operand_value) do update set operation = excluded.operation, operand_value = excluded.operand_value, - performed = excluded.performed, - indexed = excluded.indexed`, + performed = excluded.performed`, l.Did, l.Rkey, l.Subject.String(), @@ -211,7 +208,6 @@ func AddLabelOp(e Execer, l *models.LabelOp) (int64, error) { l.OperandKey, l.OperandValue, l.PerformedAt.Format(time.RFC3339), - now.Format(time.RFC3339), ) if err != nil { return 0, err @@ -223,7 +219,6 @@ func AddLabelOp(e Execer, l *models.LabelOp) (int64, error) { } l.Id = id - l.IndexedAt = now return id, nil } @@ -253,11 +248,10 @@ func GetLabelOps(e Execer, filters ...orm.Filter) ([]models.LabelOp, error) { operation, operand_key, operand_value, - performed, - indexed + performed from label_ops %s - order by indexed + order by id `, whereClause, ) @@ -270,7 +264,7 @@ func GetLabelOps(e Execer, filters ...orm.Filter) ([]models.LabelOp, error) { for rows.Next() { var labelOp models.LabelOp - var performedAt, indexedAt string + var performedAt string if err := rows.Scan( &labelOp.Id, @@ -281,19 +275,13 @@ func GetLabelOps(e Execer, filters ...orm.Filter) ([]models.LabelOp, error) { &labelOp.OperandKey, &labelOp.OperandValue, &performedAt, - &indexedAt, ); err != nil { return nil, err } labelOp.PerformedAt, err = time.Parse(time.RFC3339, performedAt) if err != nil { - labelOp.PerformedAt = time.Now() - } - - labelOp.IndexedAt, err = time.Parse(time.RFC3339, indexedAt) - if err != nil { - labelOp.IndexedAt = time.Now() + labelOp.PerformedAt = time.Time{} } labelOps = append(labelOps, labelOp) diff --git a/appview/db/label_migration_test.go b/appview/db/label_migration_test.go new file mode 100644 index 00000000..aad990ea --- /dev/null +++ b/appview/db/label_migration_test.go @@ -0,0 +1,92 @@ +package db + +import ( + "database/sql" + "path/filepath" + "testing" +) + +func TestDropLabelOpsIndexedColumn(t *testing.T) { + path := filepath.Join(t.TempDir(), "legacy.db") + conn, err := sql.Open("sqlite3", path+"?_foreign_keys=1") + if err != nil { + t.Fatalf("open: %v", err) + } + defer conn.Close() + + legacy := ` + create table label_definitions ( + id integer primary key autoincrement, + did text not null, + rkey text not null, + at_uri text generated always as ('at://' || did || '/' || 'sh.tangled.label.definition' || '/' || rkey) stored, + name text not null, + unique (at_uri) + ); + create table label_ops ( + id integer primary key autoincrement, + did text not null, + rkey text not null, + at_uri text generated always as ('at://' || did || '/' || 'sh.tangled.label.op' || '/' || rkey) stored, + subject text not null, + operation text not null check (operation in ("add", "del")), + operand_key text not null, + operand_value text not null, + performed text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), + indexed text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), + foreign key (operand_key) references label_definitions (at_uri) on delete cascade, + unique (did, rkey, subject, operand_key, operand_value) + ); + ` + if _, err := conn.Exec(legacy); err != nil { + t.Fatalf("legacy schema: %v", err) + } + + defUri := "at://did:plc:boltless/sh.tangled.label.definition/prio" + if _, err := conn.Exec( + `insert into label_definitions (did, rkey, name) values (?, ?, ?)`, + "did:plc:boltless", "prio", "priority", + ); err != nil { + t.Fatalf("seed def: %v", err) + } + if _, err := conn.Exec( + `insert into label_ops (did, rkey, subject, operation, operand_key, operand_value) values (?, ?, ?, ?, ?, ?)`, + "did:plc:boltless", "op1", "at://did:plc:boltless/sh.tangled.repo.issue/issue1", "add", defUri, "high", + ); err != nil { + t.Fatalf("seed op: %v", err) + } + + indexedColumns := func() int { + var count int + if err := conn.QueryRow( + `select count(*) from pragma_table_info('label_ops') where name = 'indexed'`, + ).Scan(&count); err != nil { + t.Fatalf("pragma_table_info: %v", err) + } + return count + } + + if indexedColumns() != 1 { + t.Fatalf("legacy table must carry the indexed column before the migration") + } + if _, err := conn.Exec(`alter table label_ops drop column indexed`); err != nil { + t.Fatalf("drop column indexed must succeed against the real table shape: %v", err) + } + if indexedColumns() != 0 { + t.Fatalf("indexed column must be gone after the drop") + } + + var gotAtUri, gotVal string + if err := conn.QueryRow( + `select at_uri, operand_value from label_ops where did = ? and rkey = ?`, + "did:plc:boltless", "op1", + ).Scan(&gotAtUri, &gotVal); err != nil { + t.Fatalf("row must survive the drop: %v", err) + } + if want := "at://did:plc:boltless/sh.tangled.label.op/op1"; gotAtUri != want { + t.Fatalf("generated at_uri must survive the drop: got %q want %q", gotAtUri, want) + } + if gotVal != "high" { + t.Fatalf("operand value must survive the drop: got %q", gotVal) + } +} diff --git a/appview/labels/labels.go b/appview/labels/labels.go index f3af233d..0a131193 100644 --- a/appview/labels/labels.go +++ b/appview/labels/labels.go @@ -89,7 +89,6 @@ func (l *Labels) PerformLabelOp(w http.ResponseWriter, r *http.Request) { did := user.Did rkey := tid.TID() performedAt := time.Now() - indexedAt := time.Now() repoAt := r.Form.Get("repo") subjectUri := r.Form.Get("subject") @@ -140,7 +139,6 @@ func (l *Labels) PerformLabelOp(w http.ResponseWriter, r *http.Request) { OperandKey: key, OperandValue: val, PerformedAt: performedAt, - IndexedAt: indexedAt, }) } } @@ -160,7 +158,6 @@ func (l *Labels) PerformLabelOp(w http.ResponseWriter, r *http.Request) { OperandKey: key, OperandValue: val, PerformedAt: performedAt, - IndexedAt: indexedAt, }) } } @@ -263,7 +260,7 @@ func (l *Labels) PerformLabelOp(w http.ResponseWriter, r *http.Request) { defer rollback() for _, o := range validLabelOps { - if _, err := db.AddLabelOp(l.db, &o); err != nil { + if _, err := db.AddLabelOp(tx, &o); err != nil { fail("Failed to update labels. Try again later.", err) return } diff --git a/appview/models/label.go b/appview/models/label.go index 3448776f..506b1cd0 100644 --- a/appview/models/label.go +++ b/appview/models/label.go @@ -307,7 +307,7 @@ func (l LabelDefinition) GetColor() string { func LabelDefinitionFromRecord(did, rkey string, record tangled.LabelDefinition) (*LabelDefinition, error) { created, err := time.Parse(time.RFC3339, record.CreatedAt) if err != nil { - created = time.Now() + created = time.Time{} } multiple := false @@ -342,30 +342,14 @@ type LabelOp struct { OperandKey string OperandValue string PerformedAt time.Time - IndexedAt time.Time } func (l LabelOp) SortAt() time.Time { - createdAt := l.PerformedAt - indexedAt := l.IndexedAt - - // if we don't have an indexedat, fall back to now - if indexedAt.IsZero() { - indexedAt = time.Now() - } - // if createdat is invalid (before epoch), treat as null -> return zero time - if createdAt.Before(time.UnixMicro(0)) { + if l.PerformedAt.Before(time.UnixMicro(0)) { return time.Time{} } - - // if createdat is <= indexedat, use createdat - if createdAt.Before(indexedAt) || createdAt.Equal(indexedAt) { - return createdAt - } - - // otherwise, createdat is in the future relative to indexedat -> use indexedat - return indexedAt + return l.PerformedAt } var _ Validator = new(LabelOp) @@ -395,7 +379,7 @@ const ( func LabelOpsFromRecord(did, rkey string, record tangled.LabelOp) []LabelOp { performed, err := time.Parse(time.RFC3339, record.PerformedAt) if err != nil { - performed = time.Now() + performed = time.Time{} } mkOp := func(operand *tangled.LabelOp_Operand) LabelOp { -- 2.51.2