diff --git a/internal/users/charindex.go b/internal/users/charindex.go index d54ac0478..a08cf17e3 100644 --- a/internal/users/charindex.go +++ b/internal/users/charindex.go @@ -1,6 +1,7 @@ package users import ( + "bytes" "strings" "sync" ) @@ -69,16 +70,23 @@ func (ci *CharacterIndex) Find(name string) (userId int, found bool) { return } -// Rebuild clears the index and repopulates it from every user record on disk +// Rebuild clears the index and repopulates it from every user file on disk // plus all currently online users. Only active character names are added here; // the alt-characters module is responsible for adding alt names after this // runs. func (ci *CharacterIndex) Rebuild() { + ci.RebuildFromScan(ScanUserFiles()) +} + +// RebuildFromIndex repopulates the character index straight from the user +// index records plus all currently online users, without opening a single +// user file. +func (ci *CharacterIndex) RebuildFromIndex(idx *UserIndex) { newMap := make(map[string]int) - SearchOfflineUsers(func(u *UserRecord) bool { - if u.Character != nil && u.Character.Name != "" { - newMap[strings.ToLower(u.Character.Name)] = u.UserId + idx.ForEachRecord(func(rec IndexUserRecord) bool { + if name := string(bytes.TrimRight(rec.CharacterName[:], "\x00")); name != `` { + newMap[strings.ToLower(name)] = int(rec.UserID) } return true }) @@ -93,3 +101,26 @@ func (ci *CharacterIndex) Rebuild() { ci.byName = newMap ci.mu.Unlock() } + +// RebuildFromScan is Rebuild fed by an existing user file scan, so startup +// can share one scan between the user index and the character index instead +// of fully parsing every user record a second time. +func (ci *CharacterIndex) RebuildFromScan(scan []UserFileScan) { + newMap := make(map[string]int, len(scan)) + + for _, s := range scan { + if s.CharacterName != "" { + newMap[strings.ToLower(s.CharacterName)] = s.UserId + } + } + + for _, u := range GetAllActiveUsers() { + if u.Character != nil && u.Character.Name != "" { + newMap[strings.ToLower(u.Character.Name)] = u.UserId + } + } + + ci.mu.Lock() + ci.byName = newMap + ci.mu.Unlock() +} diff --git a/internal/users/charindex_test.go b/internal/users/charindex_test.go index 8a9a74721..e5fd411b5 100644 --- a/internal/users/charindex_test.go +++ b/internal/users/charindex_test.go @@ -4,6 +4,8 @@ import ( "fmt" "sync" "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" ) func freshCharacterIndex() *CharacterIndex { @@ -116,8 +118,12 @@ func TestCharacterIndex_MultipleUsersMultipleNames(t *testing.T) { } func TestCharacterIndex_Rebuild(t *testing.T) { + // The scan warns about the missing users directory in a test env, so the + // logger must be initialized. + mudlog.SetupLogger(nil, "", "", false) + // Swap in a fresh singleton so Rebuild exercises the real code path - // without touching disk (SearchOfflineUsers finds nothing in a test env). + // without touching disk (ScanUserFiles finds nothing in a test env). orig := characterIndex defer func() { characterIndex = orig }() @@ -127,7 +133,7 @@ func TestCharacterIndex_Rebuild(t *testing.T) { // Pre-populate with stale data that Rebuild should clear. ci.Add("stale", 99) - // Rebuild will call SearchOfflineUsers (returns nothing in test env) and + // Rebuild will call ScanUserFiles (returns nothing in test env) and // GetAllActiveUsers (returns nothing since userManager is empty). The stale // entry must be gone. ci.Rebuild() diff --git a/internal/users/index.go b/internal/users/index.go index 8546b5520..04de05b19 100644 --- a/internal/users/index.go +++ b/internal/users/index.go @@ -8,6 +8,8 @@ import ( "hash/fnv" "io" "os" + "path/filepath" + "strconv" "strings" "sync" @@ -25,9 +27,10 @@ var ( ) const ( - IndexVersion = 2 + IndexVersion = 3 IndexLineTerminatorV1 = byte(10) // "\n" IndexRecordSizeV1 = 89 + IndexRecordSizeV3 = 185 // username[80] + userid(8) + charname[80] + mtime(8) + size(8) + newline FixedHeaderTotalLength = 100 // 99 bytes header content + 1 byte newline ) @@ -40,10 +43,16 @@ type IndexMetaData struct { Checksum uint64 // FNV-64 fingerprint of user directory contents; 0 when IndexChecksumEnabled is false } -// IndexUserRecord represents one fixed-width record. +// IndexUserRecord represents one fixed-width record. Alongside the lookup +// fields it stores the active character name plus the mtime and size the +// user file had when it was indexed, so startup can tell unchanged files +// apart from changed ones without opening them. type IndexUserRecord struct { - UserID int64 - Username [80]byte + UserID int64 + Username [80]byte + CharacterName [80]byte + FileModTime int64 // UnixNano mtime of the user file when last indexed + FileSize int64 // size in bytes of the user file when last indexed } // UserIndex is the central struct that holds the index filename and methods @@ -102,8 +111,17 @@ func (idx *UserIndex) loadRecords() { return } + // An index in an older record format is not readable - leave the maps + // empty so SyncWithUserFiles falls back to a full rebuild. + if idx.metaData.IndexVersion != IndexVersion || idx.metaData.RecordSize != IndexRecordSizeV3 { + mudlog.Info("UserIndex", "info", "index format is outdated, a rebuild will recreate it", "path", idx.Filename, "version", idx.metaData.IndexVersion) + idx.records = nil + return + } + f, err := os.Open(idx.Filename) if err != nil { + mudlog.Error("UserIndex", "error", "failed to open index file", "path", idx.Filename, "details", err) return } defer f.Close() @@ -111,9 +129,11 @@ func (idx *UserIndex) loadRecords() { dataSize := idx.metaData.RecordCount * idx.metaData.RecordSize buf := make([]byte, dataSize) if _, err := f.Seek(int64(idx.metaData.MetaDataSize), io.SeekStart); err != nil { + mudlog.Error("UserIndex", "error", "failed to seek past index header", "path", idx.Filename, "details", err) return } if _, err := io.ReadFull(f, buf); err != nil { + mudlog.Error("UserIndex", "error", "index file is truncated or corrupt, a rebuild will recreate it", "path", idx.Filename, "details", err) return } @@ -124,6 +144,9 @@ func (idx *UserIndex) loadRecords() { rec := &idx.records[i] copy(rec.Username[:], buf[offset:offset+80]) rec.UserID = int64(binary.LittleEndian.Uint64(buf[offset+80 : offset+88])) + copy(rec.CharacterName[:], buf[offset+88:offset+168]) + rec.FileModTime = int64(binary.LittleEndian.Uint64(buf[offset+168 : offset+176])) + rec.FileSize = int64(binary.LittleEndian.Uint64(buf[offset+176 : offset+184])) username := string(bytes.TrimRight(rec.Username[:], "\x00")) idx.byUsername[username] = rec.UserID @@ -152,7 +175,7 @@ func (idx *UserIndex) Create() error { MetaDataSize: FixedHeaderTotalLength, IndexVersion: IndexVersion, RecordCount: 0, - RecordSize: IndexRecordSizeV1, + RecordSize: IndexRecordSizeV3, } idx.highestUserId = 0 idx.records = nil @@ -209,8 +232,16 @@ func computeDirChecksum(basePath string) (uint64, error) { } // IsUpToDate returns true if the index file exists, has the current version, -// and its stored FNV-64 checksum matches the current state of the user directory. +// actually loaded every record its header claims, and its stored FNV-64 +// checksum matches the current state of the user directory. func (idx *UserIndex) IsUpToDate() bool { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return idx.isUpToDateForDir(basePath) +} + +// isUpToDateForDir is IsUpToDate parameterized by the directory to compare +// against, so tests can point it at a synthetic users directory. +func (idx *UserIndex) isUpToDateForDir(basePath string) bool { if !idx.Exists() { return false } @@ -218,7 +249,14 @@ func (idx *UserIndex) IsUpToDate() bool { return false } - basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + // A truncated or unreadable records section leaves fewer records in + // memory than the header claims. Such an index must never be trusted: + // with empty maps, GetUniqueUserId would start handing out userids that + // already belong to existing user files. + if uint64(len(idx.records)) != idx.metaData.RecordCount { + return false + } + current, err := computeDirChecksum(basePath) if err != nil { return false @@ -226,68 +264,85 @@ func (idx *UserIndex) IsUpToDate() bool { return idx.metaData.Checksum == current } -// Rebuild recreates the index from all offline user records. -// It calls Create() internally so it is self-contained. -// After building, it computes and persists a directory checksum so that -// IsUpToDate can detect stale indexes on the next startup. +// Rebuild recreates the index from the user files on disk. It runs a +// lightweight scan (userid and username only) instead of fully loading every +// user record, then writes the whole index in one atomic pass (temp file + +// rename) with a single sync, instead of appending and syncing once per +// user. The directory checksum is folded into that same write so IsUpToDate +// can detect stale indexes on the next startup. func (idx *UserIndex) Rebuild() error { - if err := idx.Create(); err != nil { - return fmt.Errorf("index create failed: %w", err) - } - - var firstErr error - SearchOfflineUsers(func(u *UserRecord) bool { - if err := idx.AddUser(u.UserId, u.Username); err != nil { - mudlog.Error("UserIndex.Rebuild", "error", err.Error(), "userId", u.UserId, "username", u.Username) - if firstErr == nil { - firstErr = err - } - } - return true - }) - - if firstErr != nil { - return firstErr - } + return idx.RebuildFromScan(ScanUserFiles()) +} +// RebuildFromScan is Rebuild fed by an existing scan, so startup can share +// one scan between the user index and the character index. +func (idx *UserIndex) RebuildFromScan(scan []UserFileScan) error { basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) checksum, err := computeDirChecksum(basePath) if err != nil { return fmt.Errorf("checksum compute failed: %w", err) } - if err := idx.writeChecksum(checksum); err != nil { - return fmt.Errorf("checksum write failed: %w", err) + return idx.applyScan(scan, checksum) +} + +// scanRecord converts one scan result into a fixed-width index record. +func scanRecord(s UserFileScan) IndexUserRecord { + rec := IndexUserRecord{ + UserID: int64(s.UserId), + FileModTime: s.FileModTime, + FileSize: s.FileSize, } + copy(rec.Username[:], strings.ToLower(s.Username)) + copy(rec.CharacterName[:], s.CharacterName) + return rec +} - return nil +// applyScan replaces the in-memory records and lookup maps with the scan +// results, then writes the complete index to disk once. +func (idx *UserIndex) applyScan(scan []UserFileScan, checksum uint64) error { + records := make([]IndexUserRecord, 0, len(scan)) + for _, s := range scan { + records = append(records, scanRecord(s)) + } + return idx.applyRecords(records, checksum) } -// writeChecksum persists a new checksum value into the index header on disk -// and updates the in-memory metadata. -func (idx *UserIndex) writeChecksum(checksum uint64) error { +// applyRecords replaces the in-memory records and lookup maps, then writes +// the complete index to disk once. +func (idx *UserIndex) applyRecords(records []IndexUserRecord, checksum uint64) error { + idx.mu.Lock() defer idx.mu.Unlock() - idx.metaData.Checksum = checksum - - headerBytes, err := idx.metaData.Format() - if err != nil { - return err + idx.metaData = IndexMetaData{ + MetaDataSize: FixedHeaderTotalLength, + IndexVersion: IndexVersion, + RecordCount: uint64(len(records)), + RecordSize: IndexRecordSizeV3, + Checksum: checksum, } - f, err := os.OpenFile(idx.Filename, os.O_RDWR, 0644) - if err != nil { - return err - } - defer f.Close() + idx.records = records + idx.byUsername = make(map[string]int64, len(records)) + idx.byUserId = make(map[int64]string, len(records)) + idx.highestUserId = 0 - if _, err := f.Seek(0, io.SeekStart); err != nil { - return err + for _, rec := range records { + username := string(bytes.TrimRight(rec.Username[:], "\x00")) + idx.byUsername[username] = rec.UserID + idx.byUserId[rec.UserID] = username + if int(rec.UserID) > idx.highestUserId { + idx.highestUserId = int(rec.UserID) + } } - if _, err := f.Write(headerBytes); err != nil { - return err + + // The in-memory state is updated even if the disk write fails - the + // running process must trust what was just scanned, not a stale file. + if err := idx.writeCompleteIndex(records); err != nil { + return fmt.Errorf("index write failed: %w", err) } - return f.Sync() + + return nil } func (idx *UserIndex) GetMetaData() IndexMetaData { @@ -360,6 +415,7 @@ func (idx *UserIndex) getMetaDataFromFile() IndexMetaData { headerContent := strings.TrimSpace(string(header[:FixedHeaderTotalLength-1])) n, _ := fmt.Sscanf(headerContent, "VERSION=%d,RECORDCOUNT=%d,RECORDSIZE=%d,CHECKSUM=%d", &meta.IndexVersion, &meta.RecordCount, &meta.RecordSize, &meta.Checksum) if n < 3 { + mudlog.Error("UserIndex", "error", "index header is unparseable, a rebuild will recreate it", "path", idx.Filename) return IndexMetaData{} } @@ -388,10 +444,11 @@ func (idx *UserIndex) AddUser(userId int, username string) error { return fmt.Errorf("error seeking to file end: %w", err) } - var recBuf [IndexRecordSizeV1]byte - copy(recBuf[:80], newRecord.Username[:]) - binary.LittleEndian.PutUint64(recBuf[80:88], uint64(newRecord.UserID)) - recBuf[88] = IndexLineTerminatorV1 + // The character name, mtime, and size are left zeroed here - the user + // file usually doesn't exist yet when a user is first registered. The + // zero mtime never matches the real file, so the next startup sync + // re-parses that one file and completes the record. + recBuf := encodeIndexRecord(newRecord) if _, err := f.Write(recBuf[:]); err != nil { return fmt.Errorf("error writing record: %w", err) } @@ -475,6 +532,126 @@ func (m IndexMetaData) Format() ([]byte, error) { return []byte(padded + string(IndexLineTerminatorV1)), nil } +// SyncWithUserFiles brings the index in line with the user files on disk, +// parsing only files that are new or changed since they were last indexed +// and dropping records whose files are gone. Unchanged files are never +// opened. Returns how many records had to be parsed or dropped; when that +// is zero the index file is not rewritten at all. +func (idx *UserIndex) SyncWithUserFiles() (int, error) { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return idx.syncWithDir(basePath) +} + +// syncWithDir is SyncWithUserFiles parameterized by the directory to sync +// against, so tests can point it at a synthetic users directory. +func (idx *UserIndex) syncWithDir(basePath string) (int, error) { + + // An index that is missing, in an older format, or that did not load + // every record its header claims cannot be trusted for incremental + // work - fall back to a full scan of every user file. + idx.mu.RLock() + trustworthy := idx.metaData.IndexVersion == IndexVersion && + idx.metaData.RecordSize == IndexRecordSizeV3 && + uint64(len(idx.records)) == idx.metaData.RecordCount + oldRecords := idx.records + idx.mu.RUnlock() + + if !idx.Exists() { + trustworthy = false + } + + if !trustworthy { + scan := scanUserFilesInDir(basePath) + checksum, err := computeDirChecksum(basePath) + if err != nil { + return 0, fmt.Errorf("checksum compute failed: %w", err) + } + return len(scan), idx.applyScan(scan, checksum) + } + + entries, err := os.ReadDir(basePath) + if err != nil { + return 0, fmt.Errorf("users directory unreadable: %w", err) + } + + onDisk := make(map[string]os.FileInfo, len(entries)) + for _, e := range entries { + if e.IsDir() { + continue + } + name := e.Name() + if !strings.HasSuffix(name, `.yaml`) || strings.HasSuffix(name, `.alts.yaml`) { + continue + } + info, err := e.Info() + if err != nil { + return 0, fmt.Errorf("stat failed for %s: %w", name, err) + } + onDisk[name] = info + } + + // Records are matched to files by the .yaml naming convention. + // A user file in the old username.yaml naming never matches a record, + // so it is re-parsed every startup - run the format migration to avoid + // that cost. + changed := 0 + kept := make([]IndexUserRecord, 0, len(oldRecords)) + reparse := []string{} + claimed := make(map[string]bool, len(oldRecords)) + + for _, rec := range oldRecords { + fileName := strconv.FormatInt(rec.UserID, 10) + `.yaml` + info, ok := onDisk[fileName] + if !ok { + changed++ + continue + } + claimed[fileName] = true + if info.ModTime().UnixNano() == rec.FileModTime && info.Size() == rec.FileSize { + kept = append(kept, rec) + continue + } + reparse = append(reparse, fileName) + changed++ + } + + for name := range onDisk { + if !claimed[name] { + reparse = append(reparse, name) + changed++ + } + } + + if changed == 0 { + return 0, nil + } + + for _, name := range reparse { + if s, ok := scanUserFile(filepath.Join(basePath, name), onDisk[name]); ok { + kept = append(kept, scanRecord(s)) + } + } + + checksum, err := computeDirChecksum(basePath) + if err != nil { + return 0, fmt.Errorf("checksum compute failed: %w", err) + } + + return changed, idx.applyRecords(kept, checksum) +} + +// encodeIndexRecord serializes one record into its fixed-width on-disk form. +func encodeIndexRecord(rec IndexUserRecord) [IndexRecordSizeV3]byte { + var recBuf [IndexRecordSizeV3]byte + copy(recBuf[:80], rec.Username[:]) + binary.LittleEndian.PutUint64(recBuf[80:88], uint64(rec.UserID)) + copy(recBuf[88:168], rec.CharacterName[:]) + binary.LittleEndian.PutUint64(recBuf[168:176], uint64(rec.FileModTime)) + binary.LittleEndian.PutUint64(recBuf[176:184], uint64(rec.FileSize)) + recBuf[184] = IndexLineTerminatorV1 + return recBuf +} + // writeCompleteIndex writes metadata and all records atomically via temp file + rename. func (idx *UserIndex) writeCompleteIndex(records []IndexUserRecord) error { tmpFile := idx.Filename + ".tmp" @@ -489,14 +666,11 @@ func (idx *UserIndex) writeCompleteIndex(records []IndexUserRecord) error { return err } - buf := make([]byte, 0, len(headerBytes)+len(records)*IndexRecordSizeV1) + buf := make([]byte, 0, len(headerBytes)+len(records)*IndexRecordSizeV3) buf = append(buf, headerBytes...) - var recBuf [IndexRecordSizeV1]byte for _, rec := range records { - copy(recBuf[:80], rec.Username[:]) - binary.LittleEndian.PutUint64(recBuf[80:88], uint64(rec.UserID)) - recBuf[88] = IndexLineTerminatorV1 + recBuf := encodeIndexRecord(rec) buf = append(buf, recBuf[:]...) } diff --git a/internal/users/indexscan.go b/internal/users/indexscan.go new file mode 100644 index 000000000..84778de24 --- /dev/null +++ b/internal/users/indexscan.go @@ -0,0 +1,130 @@ +package users + +import ( + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/GoMudEngine/GoMud/internal/configs" + "github.com/GoMudEngine/GoMud/internal/mudlog" + "github.com/GoMudEngine/GoMud/internal/util" + "gopkg.in/yaml.v2" +) + +// UserFileScan holds the fields a lightweight pass over a user file yields. +// The user index and the character index are both built from these at +// startup, without paying for a full UserRecord unmarshal per file. The +// mtime and size record what the file looked like when it was read, so a +// later sync can tell unchanged files apart without opening them. +type UserFileScan struct { + UserId int + Username string + CharacterName string + FileModTime int64 + FileSize int64 +} + +// userFileScanFields is the minimal unmarshal target for a scan. The file +// is still lexed in full, but decoding into this instead of a full +// UserRecord is measurably cheaper and builds no throwaway record - see +// BenchmarkScanVsFullUnmarshal. +type userFileScanFields struct { + UserId int `yaml:"userid"` + Username string `yaml:"username"` + Character struct { + Name string `yaml:"name"` + } `yaml:"character"` +} + +// ScanUserFiles runs a lightweight scan over the configured users directory. +func ScanUserFiles() []UserFileScan { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return scanUserFilesInDir(basePath) +} + +// scanUserFilesInDir reads the userid, username, and active character name +// of every user file under basePath. Alt files are never opened. Files that +// cannot be read or parsed are skipped with a warning instead of aborting +// the scan, and anomalies that usually mean hand-edited data (duplicate +// userids, duplicate usernames, a numeric filename that disagrees with the +// userid inside the file) are logged so they get noticed. +func scanUserFilesInDir(basePath string) []UserFileScan { + + results := []UserFileScan{} + seenIds := make(map[int]string) + seenNames := make(map[string]string) + + filepath.Walk(basePath, func(path string, info os.FileInfo, err error) error { + + if err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "walk_error", err) + return nil + } + + if info.IsDir() { + return nil + } + + if !strings.HasSuffix(path, `.yaml`) || strings.HasSuffix(path, `.alts.yaml`) { + return nil + } + + scan, ok := scanUserFile(path, info) + if !ok { + return nil + } + + if fileId, convErr := strconv.Atoi(strings.TrimSuffix(filepath.Base(path), `.yaml`)); convErr == nil && fileId != scan.UserId { + mudlog.Warn("ScanUserFiles", "info", "filename does not match userid in file", "path", path, "userid", scan.UserId) + } + + if otherPath, ok := seenIds[scan.UserId]; ok { + mudlog.Warn("ScanUserFiles", "info", "duplicate userid", "userid", scan.UserId, "path", path, "otherpath", otherPath) + } + lowerName := strings.ToLower(scan.Username) + if otherPath, ok := seenNames[lowerName]; ok { + mudlog.Warn("ScanUserFiles", "info", "duplicate username", "username", scan.Username, "path", path, "otherpath", otherPath) + } + seenIds[scan.UserId] = path + seenNames[lowerName] = path + + results = append(results, scan) + + return nil + }) + + return results +} + +// scanUserFile reads and minimally parses one user file. The provided info +// supplies the mtime and size stored with the result - stat data from +// before the read, so a write racing the scan makes the file look changed +// on the next sync rather than silently current. +func scanUserFile(path string, info os.FileInfo) (UserFileScan, bool) { + + fileBytes, err := os.ReadFile(path) + if err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "read_error", err) + return UserFileScan{}, false + } + + var scanned userFileScanFields + if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "unmarshal_error", err) + return UserFileScan{}, false + } + + if scanned.UserId < 1 || scanned.Username == `` { + mudlog.Warn("ScanUserFiles", "info", "skipping user file missing userid or username", "path", path) + return UserFileScan{}, false + } + + return UserFileScan{ + UserId: scanned.UserId, + Username: scanned.Username, + CharacterName: scanned.Character.Name, + FileModTime: info.ModTime().UnixNano(), + FileSize: info.Size(), + }, true +} diff --git a/internal/users/indexscan_test.go b/internal/users/indexscan_test.go new file mode 100644 index 000000000..de7d0862d --- /dev/null +++ b/internal/users/indexscan_test.go @@ -0,0 +1,302 @@ +package users + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" + "gopkg.in/yaml.v2" +) + +// writeScanTestUser writes a user file shaped like a real record: the index +// fields up top and an embedded character block padded out so the scan pays +// a realistic lexing cost. Production user files run 10-20KB; padKB controls +// how close a test file gets to that. +func writeScanTestUser(t testing.TB, dir string, userId int, username string, charName string, padKB int) { + t.Helper() + + var sb strings.Builder + fmt.Fprintf(&sb, "userid: %d\n", userId) + fmt.Fprintf(&sb, "username: %s\n", username) + sb.WriteString("password: 52c69134f0185daafe43fa511b6d1db16e59404a7992aa0d9bfa0bea05d592a9\n") + sb.WriteString("joined: 2026-01-07T13:52:18.427407+02:00\n") + if charName != `` { + fmt.Fprintf(&sb, "character:\n name: %s\n roomid: 1\n level: 5\n experience: 1234\n", charName) + for i := 0; sb.Len() < padKB*1024; i++ { + fmt.Fprintf(&sb, " itemfiller%d: some padding value that stands in for inventory and buffs %d\n", i, i) + } + } + + if err := os.WriteFile(filepath.Join(dir, fmt.Sprintf(`%d.yaml`, userId)), []byte(sb.String()), 0644); err != nil { + t.Fatal(err) + } +} + +func newScanTestIndex(dir string) *UserIndex { + return &UserIndex{ + Filename: filepath.Join(dir, `users.idx`), + byUsername: make(map[string]int64), + byUserId: make(map[int64]string), + } +} + +// TestScanUserFilesInDir verifies the scan picks up every valid user file, +// including old-format username.yaml files, while skipping alt files, +// malformed yaml, and files without a userid. +func TestScanUserFilesInDir(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + // A user file in the old username.yaml naming, still valid by content. + if err := os.WriteFile(filepath.Join(dir, `legacy.yaml`), []byte("userid: 77\nusername: legacy\ncharacter:\n name: Oldtimer\n"), 0644); err != nil { + t.Fatal(err) + } + + junkFiles := map[string]string{ + `3.alts.yaml`: "alts:\n- name: x\n", + `broken.yaml`: "userid: [not closed\n", + `50.yaml`: "username: idless\n", + `notes.txt`: `hello`, + } + for name, content := range junkFiles { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + + scan := scanUserFilesInDir(dir) + + if len(scan) != 6 { + t.Fatalf("expected 6 scanned users, got %d", len(scan)) + } + + byId := make(map[int]UserFileScan, len(scan)) + for _, s := range scan { + byId[s.UserId] = s + } + + if s, ok := byId[3]; !ok || s.Username != `User_3` || s.CharacterName != `Hero_3` { + t.Errorf("expected userId 3 with username 'User_3' and character 'Hero_3', got %+v", s) + } + if s, ok := byId[77]; !ok || s.Username != `legacy` || s.CharacterName != `Oldtimer` { + t.Errorf("expected old-format file to scan as userId 77 'legacy'/'Oldtimer', got %+v", s) + } +} + +// TestRebuildFromScanRoundTrip verifies applyScan builds correct lookup +// state, persists it atomically, and that a fresh UserIndex reads back the +// identical records and checksum. +func TestRebuildFromScanRoundTrip(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 10; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + checksum, err := computeDirChecksum(dir) + if err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(dir) + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + + if userId, found := idx.FindByUsername(`user_7`); !found || userId != 7 { + t.Errorf("expected to find 'user_7' with userId 7, got %d, found=%v", userId, found) + } + if highest := idx.GetHighestUserId(); highest != 10 { + t.Errorf("expected highest userId 10, got %d", highest) + } + + reloaded := newScanTestIndex(dir) + reloaded.metaData = reloaded.getMetaDataFromFile() + reloaded.loadRecords() + + if reloaded.metaData.RecordCount != 10 { + t.Fatalf("expected 10 records after reload, got %d", reloaded.metaData.RecordCount) + } + if reloaded.metaData.Checksum != checksum { + t.Errorf("expected persisted checksum %d, got %d", checksum, reloaded.metaData.Checksum) + } + for i := 1; i <= 10; i++ { + username, found := reloaded.FindByUserId(i) + if !found || username != fmt.Sprintf(`user_%d`, i) { + t.Errorf("expected 'user_%d' for userId %d, got '%s', found=%v", i, i, username, found) + } + } +} + +// TestRebuildFromScanReplacesStaleIndex verifies index entries with no +// matching user file do not survive a rebuild. +func TestRebuildFromScanReplacesStaleIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), ``, 0) + } + + idx := newScanTestIndex(dir) + if err := idx.Create(); err != nil { + t.Fatal(err) + } + if err := idx.AddUser(999, `phantom`); err != nil { + t.Fatal(err) + } + + checksum, err := computeDirChecksum(dir) + if err != nil { + t.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + + if _, found := idx.FindByUsername(`phantom`); found { + t.Error("phantom user survived rebuild") + } + if highest := idx.GetHighestUserId(); highest != 5 { + t.Errorf("expected highest userId 5 after rebuild, got %d", highest) + } +} + +// TestIsUpToDateRejectsTruncatedIndex verifies the header/records +// consistency guard: an index whose records section was cut short must +// never report up to date, even when the directory checksum still matches. +func TestIsUpToDateRejectsTruncatedIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + usersDir := t.TempDir() + + for i := 1; i <= 5; i++ { + writeScanTestUser(t, usersDir, i, fmt.Sprintf(`User_%d`, i), ``, 0) + } + + checksum, err := computeDirChecksum(usersDir) + if err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(usersDir) + if err := idx.applyScan(scanUserFilesInDir(usersDir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + if !idx.isUpToDateForDir(usersDir) { + t.Fatal("expected freshly rebuilt index to be up to date") + } + + // Cut the records section short, simulating a crash mid-write, and + // reload the way startup does. + fileBytes, err := os.ReadFile(idx.Filename) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(idx.Filename, fileBytes[:len(fileBytes)-20], 0644); err != nil { + t.Fatal(err) + } + + truncated := newScanTestIndex(usersDir) + truncated.metaData = truncated.getMetaDataFromFile() + truncated.loadRecords() + + if truncated.isUpToDateForDir(usersDir) { + t.Fatal("truncated index must not report up to date") + } +} + +// BenchmarkRebuildFromScan measures scan plus rebuild over synthetic user +// directories with realistically sized files (embedded character block). +func BenchmarkRebuildFromScan(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + for _, userCt := range []int{100, 1000, 10000} { + b.Run(fmt.Sprintf(`%d_users`, userCt), func(b *testing.B) { + dir := b.TempDir() + for i := 1; i <= userCt; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + checksum, err := computeDirChecksum(dir) + if err != nil { + b.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + b.Fatal(err) + } + } + }) + } +} + +// BenchmarkScanVsFullUnmarshal isolates the parse cost the scan avoids: +// unmarshaling a real user file (the stock admin record with an embedded +// character) into the minimal scan struct versus a full UserRecord. +func BenchmarkScanVsFullUnmarshal(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + fileBytes, err := os.ReadFile(`../../_datafiles/world/default/users/1.yaml`) + if err != nil { + b.Skip(`stock user file not available`) + } + + b.Run(`minimal_scan`, func(b *testing.B) { + for i := 0; i < b.N; i++ { + var scanned userFileScanFields + if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { + b.Fatal(err) + } + } + }) + + b.Run(`full_userrecord`, func(b *testing.B) { + for i := 0; i < b.N; i++ { + var u UserRecord + if err := yaml.Unmarshal(fileBytes, &u); err != nil { + b.Fatal(err) + } + } + }) +} + +// BenchmarkScanRealUsers scans a real users directory named by the +// BENCH_USERS_DIR environment variable. The directory is only read - the +// rebuilt index is written to the benchmark temp dir - so it is safe to +// point at a live server's users directory. Skipped when the variable is +// unset. +func BenchmarkScanRealUsers(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + usersDir := os.Getenv(`BENCH_USERS_DIR`) + if usersDir == `` { + b.Skip(`set BENCH_USERS_DIR to a users directory to run this benchmark`) + } + + idx := newScanTestIndex(b.TempDir()) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + checksum, err := computeDirChecksum(usersDir) + if err != nil { + b.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(usersDir), checksum); err != nil { + b.Fatal(err) + } + } + b.StopTimer() + + b.ReportMetric(float64(idx.GetMetaData().RecordCount), `users`) +} diff --git a/internal/users/indexsync_test.go b/internal/users/indexsync_test.go new file mode 100644 index 000000000..f7e3bff03 --- /dev/null +++ b/internal/users/indexsync_test.go @@ -0,0 +1,287 @@ +package users + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" +) + +// syncTestDir writes count realistic user files and returns a synced index +// for them. +func syncTestDir(t testing.TB, count int) (string, *UserIndex) { + t.Helper() + + dir := t.TempDir() + for i := 1; i <= count; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + idx := newScanTestIndex(dir) + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("initial sync failed: %v", err) + } + if changed != count { + t.Fatalf("expected initial sync to parse %d files, got %d", count, changed) + } + return dir, idx +} + +// TestSyncNoChangesSkipsWrite verifies a second sync over an unchanged +// directory parses nothing and leaves the index file untouched. +func TestSyncNoChangesSkipsWrite(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + before, err := os.Stat(idx.Filename) + if err != nil { + t.Fatal(err) + } + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("second sync failed: %v", err) + } + if changed != 0 { + t.Errorf("expected 0 changed on unchanged directory, got %d", changed) + } + + after, err := os.Stat(idx.Filename) + if err != nil { + t.Fatal(err) + } + if !after.ModTime().Equal(before.ModTime()) || after.Size() != before.Size() { + t.Error("index file was rewritten despite no changes") + } +} + +// TestSyncDetectsChangedFile verifies a modified user file is re-parsed and +// its index entry updated, without disturbing the others. +func TestSyncDetectsChangedFile(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + // Rename the user inside file 3. The content length changes, so the + // size comparison alone must catch it even with coarse mtimes. + writeScanTestUser(t, dir, 3, `Renamed_User_Three`, `Hero_3_Reborn`, 2) + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 1 { + t.Errorf("expected 1 changed record, got %d", changed) + } + + if userId, found := idx.FindByUsername(`renamed_user_three`); !found || userId != 3 { + t.Errorf("expected renamed user findable with userId 3, got %d, found=%v", userId, found) + } + if _, found := idx.FindByUsername(`user_3`); found { + t.Error("old username should be gone after re-parse") + } + if userId, found := idx.FindByUsername(`user_7`); !found || userId != 7 { + t.Errorf("untouched user_7 should remain, got %d, found=%v", userId, found) + } +} + +// TestSyncDetectsNewAndDeleted verifies new files are indexed and records +// for deleted files are dropped. +func TestSyncDetectsNewAndDeleted(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + writeScanTestUser(t, dir, 11, `User_11`, `Hero_11`, 1) + if err := os.Remove(filepath.Join(dir, `2.yaml`)); err != nil { + t.Fatal(err) + } + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 2 { + t.Errorf("expected 2 changed records (one new, one dropped), got %d", changed) + } + + if userId, found := idx.FindByUsername(`user_11`); !found || userId != 11 { + t.Errorf("expected new user_11 indexed, got %d, found=%v", userId, found) + } + if _, found := idx.FindByUserId(2); found { + t.Error("deleted user 2 should be gone from the index") + } + if highest := idx.GetHighestUserId(); highest != 11 { + t.Errorf("expected highest userId 11, got %d", highest) + } +} + +// TestSyncUpgradesOldFormatIndex verifies an index in the previous record +// format triggers a full rebuild into the current format. +func TestSyncUpgradesOldFormatIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + // Hand-craft a version 2 index: 89-byte records, no character name. + header := `VERSION=2,RECORDCOUNT=1,RECORDSIZE=89,CHECKSUM=12345` + header = header + strings.Repeat(` `, FixedHeaderTotalLength-1-len(header)) + "\n" + record := make([]byte, 89) + copy(record[:80], `staleuser`) + record[80] = 42 // userid 42, little-endian low byte + record[88] = IndexLineTerminatorV1 + idxPath := filepath.Join(dir, `users.idx`) + if err := os.WriteFile(idxPath, append([]byte(header), record...), 0644); err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(dir) + idx.metaData = idx.getMetaDataFromFile() + idx.loadRecords() + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 5 { + t.Errorf("expected full rebuild of 5 users after format upgrade, got %d", changed) + } + + if _, found := idx.FindByUsername(`staleuser`); found { + t.Error("record from the old-format index should not survive the upgrade") + } + if meta := idx.GetMetaData(); meta.IndexVersion != IndexVersion || meta.RecordSize != IndexRecordSizeV3 { + t.Errorf("expected index upgraded to version %d with record size %d, got %+v", IndexVersion, IndexRecordSizeV3, meta) + } +} + +// TestRebuildFromIndexReadsNoFiles verifies the character index can be +// rebuilt purely from index records: after deleting every user file, the +// names must still resolve. +func TestRebuildFromIndexReadsNoFiles(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 5) + + files, err := filepath.Glob(filepath.Join(dir, `*.yaml`)) + if err != nil { + t.Fatal(err) + } + for _, f := range files { + if err := os.Remove(f); err != nil { + t.Fatal(err) + } + } + + ci := freshCharacterIndex() + orig := characterIndex + characterIndex = ci + defer func() { characterIndex = orig }() + + ci.RebuildFromIndex(idx) + + if ci.Len() != 5 { + t.Fatalf("expected 5 characters from index records, got %d", ci.Len()) + } + if userId, found := ci.Find(`Hero_4`); !found || userId != 4 { + t.Errorf("expected Hero_4 to resolve to userId 4, got %d, found=%v", userId, found) + } +} + +// TestSyncCompletesAddUserStub verifies the runtime AddUser record (zero +// mtime and size) is re-parsed and completed by the next sync. +func TestSyncCompletesAddUserStub(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 3) + + // Simulate a runtime registration: index entry first, file second. + if err := idx.AddUser(4, `Newcomer`); err != nil { + t.Fatal(err) + } + writeScanTestUser(t, dir, 4, `Newcomer`, `Fresh_Hero`, 1) + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 1 { + t.Errorf("expected exactly the stub record to be re-parsed, got %d changed", changed) + } + + ci := freshCharacterIndex() + ci.RebuildFromIndex(idx) + if userId, found := ci.Find(`Fresh_Hero`); !found || userId != 4 { + t.Errorf("expected completed record to carry character name, got %d, found=%v", userId, found) + } +} + +// BenchmarkSyncNoChanges measures the steady-state startup cost: a sync +// over a directory where nothing changed since the last index write. +func BenchmarkSyncNoChanges(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + for _, userCt := range []int{100, 1000, 10000} { + b.Run(fmt.Sprintf(`%d_users`, userCt), func(b *testing.B) { + dir := b.TempDir() + for i := 1; i <= userCt; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + if _, err := idx.syncWithDir(dir); err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + changed, err := idx.syncWithDir(dir) + if err != nil { + b.Fatal(err) + } + if changed != 0 { + b.Fatalf("expected steady state, got %d changed", changed) + } + } + }) + } +} + +// BenchmarkSyncWithChurn measures a sync where 50 of 1000 user files +// changed since the last index write - the realistic restart shape. +func BenchmarkSyncWithChurn(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + dir := b.TempDir() + for i := 1; i <= 1000; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + if _, err := idx.syncWithDir(dir); err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + for u := 1; u <= 50; u++ { + writeScanTestUser(b, dir, u, fmt.Sprintf(`User_%d`, u), fmt.Sprintf(`Hero_%d_v%d`, u, i), 8) + } + b.StartTimer() + + changed, err := idx.syncWithDir(dir) + if err != nil { + b.Fatal(err) + } + if changed != 50 { + b.Fatalf("expected 50 changed, got %d", changed) + } + } +} diff --git a/main.go b/main.go index 7e35190af..fb7969fd3 100644 --- a/main.go +++ b/main.go @@ -261,24 +261,25 @@ func main() { // Create the user index isCopyover := flags.CopyoverFd() >= 0 - if !isCopyover { - timeStart := time.Now() - idx := users.InitUserIndex() - if !idx.Exists() { - // Since it doesn't exist yet, that's a good indication we should do a quick format migration check - users.DoUserMigrations() - } - if idx.IsUpToDate() { - mudlog.Info("UserIndex", "info", "User index up to date.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(timeStart)) - } else { - idx.Create() - idx.Rebuild() - mudlog.Info("UserIndex", "info", "User index recreated.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(timeStart)) - } + syncStart := time.Now() + idx := users.InitUserIndex() + if !isCopyover && !idx.Exists() { + // Since it doesn't exist yet, that's a good indication we should do a quick format migration check + users.DoUserMigrations() + } + + // Bring the user index in line with the user files, parsing only files + // that changed since they were last indexed. The character index is then + // rebuilt straight from the index records, so an unchanged user file is + // never opened at all. + changed, err := idx.SyncWithUserFiles() + if err != nil { + mudlog.Error("UserIndex", "error", "sync failed", "details", err) } + mudlog.Info("UserIndex", "info", "User index synced.", "users", idx.GetMetaData().RecordCount, "changed", changed, "time taken", time.Since(syncStart)) - users.GetCharacterIndex().Rebuild() - mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len()) + users.GetCharacterIndex().RebuildFromIndex(idx) + mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len(), "time taken", time.Since(syncStart)) // Load the round count from the file if !isCopyover {