Artifact [cf5cce4d6b]

Artifact cf5cce4d6b5eb3042dcafd4a79e67f29170fbc48f2a81d056b15df3d16e76f19:

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

package manifest

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"
)

// 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 (see marshal.go). userLine must be "" (card
// omitted entirely), "U \n" (card present, empty), or "U <name>\n" (card
// present with a value); it is inserted in the same ASCII card position
// deck.Marshal would use.
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))
}

// TestCrosslink_UCardPresenceStates verifies the three-way U-card
// resolution required by issue #50: a wholly absent U-card must crosslink
// to SQL NULL, a present-but-empty U-card must crosslink to the literal
// "anonymous" (matching fossil's own parse-time substitution, see
// src/manifest.c:1008-1016), and a present U-card with a value must
// crosslink to that value unchanged. Before this fix, both the absent and
// present-empty cases collapsed to the same stored value: empty string.
func TestCrosslink_UCardPresenceStates(t *testing.T) {
	r := setupTestRepo(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)
	}

	linked, err := Crosslink(r)
	if err != nil {
		t.Fatalf("Crosslink: %v", err)
	}
	if linked != 3 {
		t.Fatalf("Crosslink: linked = %d, want 3", linked)
	}

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

// assertEventUser 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 assertEventUser(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)
	}
}