Artifact [137f15897d]

Artifact 137f15897dceaf9c11148eb42ed7cf6089dd56302ca65977e3712bd6d7e98ea9:

• File internal/verify/ucard_presence_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: 3280) [annotate] [blame] [check-ins using]

package verify_test

import (
	"crypto/md5"
	"database/sql"
	"fmt"
	"testing"

	"github.com/danmestas/libfossil/internal/blob"
	libfossil "github.com/danmestas/libfossil/internal/fsltype"
	"github.com/danmestas/libfossil/internal/repo"
	"github.com/danmestas/libfossil/internal/verify"
)

// rawCheckinManifest hand-builds a minimal, valid Checkin manifest so the
// test controls the U-card line byte-for-byte — something deck.Deck can't
// express for the present-but-empty case, since Marshal deliberately never
// emits an empty U-card. userLine must be "" (card omitted entirely),
// "U \n" (card present, empty), or "U <name>\n" (card present with a
// value). Mirrors manifest.rawCheckinManifest in
// internal/manifest/ucard_presence_test.go — duplicated here rather than
// exported since it's test-only fixture, not production surface.
func rawCheckinManifest(userLine string) []byte {
	body := "D 2024-01-15T10:30:00.000\nR d41d8cd98f00b204e9800998ecf8427e\n" + userLine
	h := md5.Sum([]byte(body))
	return []byte(fmt.Sprintf("%sZ %x\n", body, h))
}

// TestRebuild_UCardPresenceStates covers the rebuild path
// (internal/verify/rebuild_manifest.go's rebuildCheckin) with the same
// three-state U-card coverage as manifest.TestCrosslink_UCardPresenceStates.
// rebuildCheckin binds d.U into event.user the same way crosslink.go does
// (rebuild_manifest.go:91 is byte-identical to crosslink.go:242), but had
// no dedicated regression test — a future edit adding a stray
// deck.User("") default or a deref here would silently collapse SQL NULL
// back into "" on the rebuild path with nothing to catch it.
func TestRebuild_UCardPresenceStates(t *testing.T) {
	r := newTestRepo(t)

	absentRID, _, err := blob.Store(r.DB(), rawCheckinManifest(""))
	if err != nil {
		t.Fatalf("blob.Store(absent): %v", err)
	}
	emptyRID, _, err := blob.Store(r.DB(), rawCheckinManifest("U \n"))
	if err != nil {
		t.Fatalf("blob.Store(empty): %v", err)
	}
	valuedRID, _, err := blob.Store(r.DB(), rawCheckinManifest("U alice\n"))
	if err != nil {
		t.Fatalf("blob.Store(valued): %v", err)
	}

	// Rebuild drops and reconstructs event (and friends) from raw blob
	// content, exercising rebuildManifests -> rebuildCheckin directly.
	if _, err := verify.Rebuild(r); err != nil {
		t.Fatalf("verify.Rebuild: %v", err)
	}

	assertRebuiltEventUser(t, r, absentRID, sql.NullString{Valid: false}, "absent U-card")
	assertRebuiltEventUser(t, r, emptyRID, sql.NullString{String: "anonymous", Valid: true}, "present-empty U-card")
	assertRebuiltEventUser(t, r, valuedRID, sql.NullString{String: "alice", Valid: true}, "present-valued U-card")
}

// assertRebuiltEventUser reads the event.user column for objid=rid and
// compares it against want, distinguishing SQL NULL (Valid=false) from any
// string value (including the empty string) so a regression that collapses
// NULL back into "" is caught.
func assertRebuiltEventUser(t *testing.T, r *repo.Repo, rid libfossil.FslID, want sql.NullString, label string) {
	t.Helper()
	var got sql.NullString
	if err := r.DB().QueryRow("SELECT user FROM event WHERE objid=?", rid).Scan(&got); err != nil {
		t.Fatalf("%s: query event.user: %v", label, err)
	}
	if got != want {
		t.Fatalf("%s: event.user = %+v, want %+v", label, got, want)
	}
}