usercache: cleanup, also elsewhere

removed some old FIXMEs and documented usercache nicely for once,
renaming some things too.

api-users.go

@@ -903,7 +903,7 @@ func (app *appContext) userSummary(jfUser mediabrowser.User) respUser {
 // @tags Activity
 func (app *appContext) GetUserCount(gc *gin.Context) {
 	resp := PageCountDTO{}
-	userList, err := app.userCache.Gen(app, false)
+	userList, err := app.userCache.GetUserDTOs(app, false)
 	if err != nil {
 		app.err.Printf(lm.FailedGetUsers, lm.Jellyfin, err)
 		respond(500, "Couldn't get users", gc)
@@ -925,7 +925,7 @@ func (app *appContext) GetUsers(gc *gin.Context) {
 	// We're sending all users, so this is always true
 	resp.LastPage = true
 	var err error
-	resp.UserList, err = app.userCache.Gen(app, true)
+	resp.UserList, err = app.userCache.GetUserDTOs(app, true)
 	if err != nil {
 		app.err.Printf(lm.FailedGetUsers, lm.Jellyfin, err)
 		respond(500, "Couldn't get users", gc)
@@ -950,7 +950,7 @@ func (app *appContext) SearchUsers(gc *gin.Context) {
 	}
 
 	var resp getUsersDTO
-	userList, err := app.userCache.Gen(app, req.SortByField == USER_DEFAULT_SORT_FIELD)
+	userList, err := app.userCache.GetUserDTOs(app, req.SortByField == USER_DEFAULT_SORT_FIELD)
 	if err != nil {
 		app.err.Printf(lm.FailedGetUsers, lm.Jellyfin, err)
 		respond(500, "Couldn't get users", gc)

log.go

@@ -59,7 +59,7 @@ func logOutput() (closeFunc func(), err error) {
 
 // Regex that removes ANSI color escape sequences. Used for outputting to log file and log cache.
 var stripColors = func() *regexp.Regexp {
-	r, err := regexp.Compile("\\x1b\\[[0-9;]*m")
+	r, err := regexp.Compile(`\x1b\[[0-9;]*m`)
 	if err != nil {
 		log.Fatalf("Failed to compile color escape regexp: %v", err)
 	}

logmessages/logmessages.go

@@ -118,8 +118,7 @@ const (
 	SetAdminNotify = "Set \"%s\" to %t for admin address \"%s\""
 
 	// *jellyseerr*.go
-	FailedGetUsers = "Failed to get user(s) from %s: %v"
+	FailedGetUsers                       = "Failed to get user(s) from %s: %v"
-	// FIXME: Once done, look back at uses of FailedGetUsers for places where this would make more sense.
 	FailedGetUser                        = "Failed to get user \"%s\" from %s: %v"
 	FailedGetJellyseerrNotificationPrefs = "Failed to get user \"%s\"'s notification prefs from " + Jellyseerr + ": %v"
 	FailedSyncContactMethods             = "Failed to sync contact methods with %s: %v"

main.go

@@ -528,7 +528,7 @@ func start(asDaemon, firstCall bool) {
 
 		// NOTE: The order in which these are placed in app.contactMethods matters.
 		// Add new ones to the end.
-		// FIXME: Add proxies.
+		// Proxies are added a little later through ContactMethodLinker[].SetTransport.
 		if discordEnabled {
 			app.discord, err = newDiscordDaemon(app)
 			if err != nil {

ts/modules/search.ts

@@ -54,8 +54,6 @@ export interface ServerSearchReqDTO extends PaginatedReqDTO {
     queries: QueryDTO[];
 }
 
-// FIXME: Generate ServerSearchReqDTO using Query.asDTO methods in serverSearch()!
-
 export interface QueryDTO {
     class: "bool" | "string" | "date";
     // QueryType.getter
@@ -647,7 +645,7 @@ export class Search {
 
     constructor(c: SearchConfiguration) {
         // FIXME: Remove!
-        if (c.search.id.includes("accounts")) {
+        if (c.search.id.includes("activity")) {
             (window as any).s = this;
         }
         this._c = c;

usercache.go

@@ -10,7 +10,6 @@ import (
 )
 
 const (
-	// FIXME: Follow mediabrowser, or make tuneable, or both
 	// After cache is this old, re-sync, but do it in the background and return the old cache.
 	WEB_USER_CACHE_SYNC = 30 * time.Second
 	// After cache is this old, re-sync and wait for it and return the new cache.
@@ -19,6 +18,11 @@ const (
 	USER_DEFAULT_SORT_ASCENDING  = true
 )
 
+// UserCache caches the transport representation of users,
+// complementing the built-in cache of the mediabrowser package.
+// Synchronisation runs in the background and consumers receive
+// old data for responsiveness unless an extended expiry time has passed.
+// It also provides methods for sorting, searching and filtering server-side.
 type UserCache struct {
 	Cache    []respUser
 	Ref      []*respUser
@@ -30,9 +34,11 @@ type UserCache struct {
 	Sorting  bool
 }
 
-// FIXME: If shouldSync, sync in background and return old version. If shouldWaitForSync, wait for sync and return new one.
+// MaybeSync (maybe) syncs the cache, resulting in updated UserCache.Cache/.Ref/.Sorted.
-// FIXME: If locked, just wait for unlock and return someone elses work.
+// Only syncs if WEB_USER_CACHE_SYNC duration has passed since last one.
-func (c *UserCache) gen(app *appContext) error {
+// If WEB_USER_CACHE_WAIT_FOR_SYNC duration has passed, this will block until a sync is complete, otherwise it will sync in the background
+// (expecting you to use the old cache data). Only one sync will run at a time.
+func (c *UserCache) MaybeSync(app *appContext) error {
 	shouldWaitForSync := time.Now().After(c.LastSync.Add(WEB_USER_CACHE_WAIT_FOR_SYNC)) || c.Ref == nil || len(c.Ref) == 0
 	shouldSync := time.Now().After(c.LastSync.Add(WEB_USER_CACHE_SYNC))
 
@@ -88,8 +94,8 @@ func (c *UserCache) gen(app *appContext) error {
 	return nil
 }
 
-func (c *UserCache) Gen(app *appContext, sorted bool) ([]*respUser, error) {
+func (c *UserCache) GetUserDTOs(app *appContext, sorted bool) ([]*respUser, error) {
-	if err := c.gen(app); err != nil {
+	if err := c.MaybeSync(app); err != nil {
 		return nil, err
 	}
 	if sorted && !c.Sorted {
@@ -124,10 +130,11 @@ func bool2int(b bool) int {
 	return i
 }
 
-// Returns -1 if respUser < value, 0 if equal, 1 is greater than
+// Sorter compares the given field of two respUsers, returning -1 if a < b, 0 if a == b, 1 if a > b.
 type Sorter func(a, b *respUser) int
 
 // Allow sorting by respUser's struct fields (well, it's JSON-representation's fields)
+// SortUsersBy returns a Sorter function, which compares the given field of two respUsers, returning -1 if a < b, 0 if a == b, 1 if a > b.
 func SortUsersBy(field string) Sorter {
 	switch field {
 	case "id":
@@ -204,11 +211,8 @@ func SortUsersBy(field string) Sorter {
 		}
 	}
 	panic(fmt.Errorf("got invalid field %s", field))
-	return nil
 }
 
-type Filter func(*respUser) bool
-
 type CompareResult int
 
 const (
@@ -256,10 +260,13 @@ func (d DateAttempt) Compare(subject int64) int {
 	return subjectTime.Compare(time.Date(yy, mo, dd, hh, mm, 0, 0, nil))
 }
 
-// FIXME: Consider using QueryDTO.Class rather than assuming type from name? Probably not worthwhile though.
+// Filter returns true if a specific field in the passed respUser matches some internally defined value.
-func FilterUsersBy(field string, op QueryOperator, value any) Filter {
+type Filter func(*respUser) bool
+
+// AsFilter returns a Filter function, which compares the queries value to the corresponding field's value in a passed respUser.
+func (q QueryDTO) AsFilter() Filter {
 	operator := Equal
-	switch op {
+	switch q.Operator {
 	case LesserOperator:
 		operator = Lesser
 	case EqualOperator:
@@ -268,84 +275,84 @@ func FilterUsersBy(field string, op QueryOperator, value any) Filter {
 		operator = Greater
 	}
 
-	switch field {
+	switch q.Field {
 	case "id":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.ID), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.ID), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "name":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Name), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "email":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Email), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Email), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "notify_email":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.NotifyThroughEmail), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.NotifyThroughEmail), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "last_active":
 		return func(a *respUser) bool {
-			return value.(DateAttempt).Compare(a.LastActive) == int(operator)
+			return q.Value.(DateAttempt).Compare(a.LastActive) == int(operator)
 		}
 	case "admin":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.Admin), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.Admin), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "expiry":
 		return func(a *respUser) bool {
-			return value.(DateAttempt).Compare(a.Expiry) == int(operator)
+			return q.Value.(DateAttempt).Compare(a.Expiry) == int(operator)
 		}
 	case "disabled":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.Disabled), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.Disabled), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "telegram":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Telegram), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Telegram), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "notify_telegram":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.NotifyThroughTelegram), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.NotifyThroughTelegram), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "discord":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Discord), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Discord), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "discord_id":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.DiscordID), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.DiscordID), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "notify_discord":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.NotifyThroughDiscord), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.NotifyThroughDiscord), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "matrix":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Matrix), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Matrix), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "notify_matrix":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.NotifyThroughMatrix), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.NotifyThroughMatrix), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "label":
 		return func(a *respUser) bool {
-			return cmp.Compare(strings.ToLower(a.Label), strings.ToLower(value.(string))) == int(operator)
+			return cmp.Compare(strings.ToLower(a.Label), strings.ToLower(q.Value.(string))) == int(operator)
 		}
 	case "accounts_admin":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.AccountsAdmin), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.AccountsAdmin), bool2int(q.Value.(bool))) == int(operator)
 		}
 	case "referrals_enabled":
 		return func(a *respUser) bool {
-			return cmp.Compare(bool2int(a.ReferralsEnabled), bool2int(value.(bool))) == int(operator)
+			return cmp.Compare(bool2int(a.ReferralsEnabled), bool2int(q.Value.(bool))) == int(operator)
 		}
 	}
-	panic(fmt.Errorf("got invalid field %s", field))
+	panic(fmt.Errorf("got invalid q.Field %s", q.Field))
-	return nil
 }
 
+// MatchesSearch checks (case-insensitively) if any string field in respUser includes the term string.
 func (ru *respUser) MatchesSearch(term string) bool {
 	return (strings.Contains(ru.ID, term) ||
 		strings.Contains(strings.ToLower(ru.Name), term) ||
@@ -356,6 +363,7 @@ func (ru *respUser) MatchesSearch(term string) bool {
 		strings.Contains(strings.ToLower(ru.Telegram), term))
 }
 
+// QueryClass is the class of a query (the datatype), i.e. bool, string or date.
 type QueryClass string
 
 const (
@@ -364,6 +372,7 @@ const (
 	DateQuery   QueryClass = "date"
 )
 
+// QueryOperator is the operator used for comparison in a filter, i.e. <, = or >.
 type QueryOperator string
 
 const (
@@ -372,6 +381,7 @@ const (
 	GreaterOperator QueryOperator = ">"
 )
 
+// QueryDTO is the transport representation of a Query, sent from the web app.
 type QueryDTO struct {
 	Class    QueryClass    `json:"class"`
 	Field    string        `json:"field"`
@@ -380,17 +390,20 @@ type QueryDTO struct {
 	Value any `json:"value"`
 }
 
+// ServerSearchReqDTO is a usual PaginatedReqDTO with added fields for searching and filtering.
 type ServerSearchReqDTO struct {
 	PaginatedReqDTO
 	SearchTerms []string   `json:"searchTerms"`
 	Queries     []QueryDTO `json:"queries"`
 }
 
-// Filter by AND-ing all search terms and queries.
+// Filter reduces the passed slice of *respUsers
+// by searching for each term of terms[] with respUser.MatchesSearch,
+// and by evaluating Queries with Query.AsFilter().
 func (c *UserCache) Filter(users []*respUser, terms []string, queries []QueryDTO) []*respUser {
 	filters := make([]Filter, len(queries))
 	for i, q := range queries {
-		filters[i] = FilterUsersBy(q.Field, q.Operator, q.Value)
+		filters[i] = q.AsFilter()
 	}
 	// FIXME: Properly consider pre-allocation size
 	out := make([]*respUser, 0, len(users)/4)
@@ -418,6 +431,7 @@ func (c *UserCache) Filter(users []*respUser, terms []string, queries []QueryDTO
 	return out
 }
 
+// Sort sorts the given slice of of *respUsers in-place by the field name given, in ascending or descending order.
 func (c *UserCache) Sort(users []*respUser, field string, ascending bool) {
 	slices.SortFunc(users, SortUsersBy(field))
 	if !ascending {