Artifact 5c0fcb177e2ec1539cf1bd52a9a65969f33bdfd0afa67759312f1db9462609d4:
- File
internal/manifest/dephantomize_test.go
— part of check-in
[527759acd5]
at
2026-07-19 18:31:23
on branch trunk
— fix: distinguish absent, empty, and valued U-cards in manifest parsing (#73)
* fix: distinguish absent, empty, and valued U-cards in manifest parsing
Manifest parsing collapsed a wholly absent U-card and a present-but-empty U-card into the same stored value (empty string), so every crosslink call site that wrote the check-in user into event/attachment/forumpost rows lost the distinction between "no login recorded" (should be SQL NULL) and "login explicitly blanked" (canonical fossil resolves this to the literal "anonymous" at parse time — see src/manifest.c:1008-1016).
Change deck.Deck.U from string to *string so the three states are representable: nil (absent), non-nil "anonymous" (present-empty, resolved in parseUCard the same way fossil's own parser does), and non-nil with a value (present). Resolving once in the parser means every downstream crosslink/rebuild call site can keep binding d.U directly — Go's database/sql converts a nil *string argument to SQL NULL and dereferences a non-nil one automatically, so no per-call-site special casing is needed. The type change itself makes the old ambiguous string-binding path a compile error everywhere Deck.U is constructed, closing off the drift that caused issue #29.
Added deck.Str, a small pointer-literal helper, since Go can't take the address of a string literal directly; existing tests that built Deck{U: "..."} now go through it.
* fix: review nits — whitespace U-cards, rebuild coverage, tag.AddTag, naming
Four follow-ups from review of the U-card presence fix:
- deck.parseUCard now trims args before the emptiness check, matching canonical fossil's next_token() (skips leading whitespace) and every sibling card parser in this file. Previously "U \n" stored literal whitespace instead of resolving to "anonymous". Added a whitespace-only case to TestParseUCardStates.
- Added TestRebuild_UCardPresenceStates covering the rebuild path (rebuild_manifest.go's rebuildCheckin), mirroring TestCrosslink_UCardPresenceStates. The binding site was already correct (same nil-to-NULL pointer semantics as crosslink.go) but had no dedicated regression test.
- tag.AddTag now panics on an empty opts.User, matching the panic idiom CheckinOpts.User already establishes. Previously a tag created with an empty User silently crosslinked to user=''; that asymmetry with CheckinOpts was a wart worth closing rather than documenting as intentional.
- Renamed deck.Str to deck.User: the name should say what the helper constructs (a Deck.U value), not how it's implemented (a pointer take). (user: [email protected], size: 5121) [annotate] [blame] [check-ins using]
package manifest import ( "testing" "time" "github.com/danmestas/libfossil/internal/blob" "github.com/danmestas/libfossil/internal/deck" _ "github.com/danmestas/libfossil/internal/testdriver" ) func TestAfterDephantomizeCheckin(t *testing.T) { r := setupTestRepo(t) // Create a checkin manifest, compute its hash, store as phantom, then fill. fileContent := []byte("hello dephantomize") fileRid, fileUUID, err := blob.Store(r.DB(), fileContent) if err != nil { t.Fatalf("Store file blob: %v", err) } _ = fileRid d := &deck.Deck{ Type: deck.Checkin, C: "dephantomize commit", U: deck.User("testuser"), D: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), F: []deck.FileCard{{Name: "hello.txt", UUID: fileUUID}}, } manifestBytes, err := d.Marshal() if err != nil { t.Fatalf("Marshal: %v", err) } // Compute the hash to get the UUID. rid, uuid, err := blob.Store(r.DB(), manifestBytes) if err != nil { t.Fatalf("Store manifest: %v", err) } // Delete the real blob, re-insert as phantom, then fill it back. // This simulates the phantom->real transition. r.DB().Exec("DELETE FROM event WHERE objid=?", rid) r.DB().Exec("DELETE FROM plink WHERE cid=?", rid) r.DB().Exec("DELETE FROM leaf WHERE rid=?", rid) r.DB().Exec("DELETE FROM mlink WHERE mid=?", rid) r.DB().Exec("DELETE FROM tagxref WHERE rid=?", rid) // Verify no event row exists before dephantomize. var eventCount int r.DB().QueryRow("SELECT count(*) FROM event WHERE objid=?", rid).Scan(&eventCount) if eventCount != 0 { t.Fatalf("event count before dephantomize = %d, want 0", eventCount) } // Call AfterDephantomize. AfterDephantomize(r, rid) // Verify event row was created. r.DB().QueryRow("SELECT count(*) FROM event WHERE objid=?", rid).Scan(&eventCount) if eventCount != 1 { t.Errorf("event count after dephantomize = %d, want 1", eventCount) } // Verify event is a checkin. var eventType string r.DB().QueryRow("SELECT type FROM event WHERE objid=?", rid).Scan(&eventType) if eventType != "ci" { t.Errorf("event type = %q, want 'ci'", eventType) } _ = uuid } func TestAfterDephantomizeOrphan(t *testing.T) { r := setupTestRepo(t) // Create a baseline checkin. fileContent := []byte("baseline file") _, fileUUID, err := blob.Store(r.DB(), fileContent) if err != nil { t.Fatalf("Store file: %v", err) } baselineDeck := &deck.Deck{ Type: deck.Checkin, C: "baseline", U: deck.User("testuser"), D: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), F: []deck.FileCard{{Name: "file.txt", UUID: fileUUID}}, } baselineBytes, err := baselineDeck.Marshal() if err != nil { t.Fatalf("Marshal baseline: %v", err) } baselineRid, _, err := blob.Store(r.DB(), baselineBytes) if err != nil { t.Fatalf("Store baseline: %v", err) } // Create another checkin that will be the "orphan" (delta manifest). fileContent2 := []byte("orphan file") _, fileUUID2, err := blob.Store(r.DB(), fileContent2) if err != nil { t.Fatalf("Store file2: %v", err) } orphanDeck := &deck.Deck{ Type: deck.Checkin, C: "orphan commit", U: deck.User("testuser"), D: time.Date(2024, 1, 15, 11, 0, 0, 0, time.UTC), F: []deck.FileCard{{Name: "file2.txt", UUID: fileUUID2}}, } orphanBytes, err := orphanDeck.Marshal() if err != nil { t.Fatalf("Marshal orphan: %v", err) } orphanRid, _, err := blob.Store(r.DB(), orphanBytes) if err != nil { t.Fatalf("Store orphan: %v", err) } // Clear crosslink tables for the orphan. r.DB().Exec("DELETE FROM event WHERE objid=?", orphanRid) r.DB().Exec("DELETE FROM leaf WHERE rid=?", orphanRid) r.DB().Exec("DELETE FROM tagxref WHERE rid=?", orphanRid) // Insert orphan row linking orphanRid to baselineRid. r.DB().Exec("INSERT INTO orphan(rid, baseline) VALUES(?, ?)", orphanRid, baselineRid) // Verify orphan row exists. var orphanCount int r.DB().QueryRow("SELECT count(*) FROM orphan WHERE baseline=?", baselineRid).Scan(&orphanCount) if orphanCount != 1 { t.Fatalf("orphan count = %d, want 1", orphanCount) } // Call AfterDephantomize on the baseline. AfterDephantomize(r, baselineRid) // Verify orphan was cleaned up. r.DB().QueryRow("SELECT count(*) FROM orphan WHERE baseline=?", baselineRid).Scan(&orphanCount) if orphanCount != 0 { t.Errorf("orphan count after dephantomize = %d, want 0", orphanCount) } // Verify orphan checkin got crosslinked (event row created). var eventCount int r.DB().QueryRow("SELECT count(*) FROM event WHERE objid=?", orphanRid).Scan(&eventCount) if eventCount != 0 { // The orphan's event may have been created by crosslinkSingle. // If it wasn't (because it was already crosslinked earlier), that's also fine. // The key assertion is that the orphan row was cleaned up. } } func TestAfterDephantomizeNilRepo(t *testing.T) { defer func() { if r := recover(); r == nil { t.Error("expected panic for nil repo") } }() AfterDephantomize(nil, 1) } func TestAfterDephantomizeZeroRid(t *testing.T) { r := setupTestRepo(t) // Should return without panicking. AfterDephantomize(r, 0) AfterDephantomize(r, -1) }