refactor(collections): rule evaluators take the real row; both 45-field converter twins deleted

checkRulesAgainstBook/evaluateRule and the service's EvaluateRules
existed in two parallel universes: one over database.MediaItems (what
every query actually returns) and one over ListMediaItemsRow (what two
hand-written 45-field mediaItemsToListMediaItemsRow converters
produced — verbatim twins in the handler and the dashboard service,
with a URL-resolution side effect baked into a "converter"). The rule
fields (genre/series/author/language/publisher/copyright_year/tags)
all exist on MediaItems, so the evaluators now take the real row and
the converters, their three call-site loops, and the dead
ApplyAutoAssignRules entry point (zero callers) are gone.

The rules surface is wire-covered by the collections rules scenarios;
handlers+services suites and the full pin/H1/H3 battery green.
This commit is contained in:
John O'Keefe committed 2026-10-17 16:48:39 -04:00
1 parent b47a2b86a7
commit 18727fef93
7 files changed
+54 -193

No files matched your search

+4 -59
View File
@@ -946,13 +946,8 @@ func (h *CollectionHandler) TestRules(c *echo.Context) error {
return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to load media items"})
}
listRows := make([]database.ListMediaItemsRow, len(mediaItems))
for i, item := range mediaItems {
listRows[i] = mediaItemsToListMediaItemsRow(item)
}
var matches []BookMatch
for _, item := range listRows {
for _, item := range mediaItems {
matchReason := h.checkRulesAgainstBook(item, req.Rules)
if matchReason != "" {
author := ""
@@ -976,7 +971,7 @@ func (h *CollectionHandler) TestRules(c *echo.Context) error {
})
}
func (h *CollectionHandler) checkRulesAgainstBook(item database.ListMediaItemsRow, rules []map[string]interface{}) string {
func (h *CollectionHandler) checkRulesAgainstBook(item database.MediaItems, rules []map[string]interface{}) string {
for _, rule := range rules {
field, _ := rule["field"].(string)
operator, _ := rule["operator"].(string)
@@ -989,7 +984,7 @@ func (h *CollectionHandler) checkRulesAgainstBook(item database.ListMediaItemsRo
return ""
}
func (h *CollectionHandler) evaluateRule(item database.ListMediaItemsRow, field, operator, value string) bool {
func (h *CollectionHandler) evaluateRule(item database.MediaItems, field, operator, value string) bool {
var itemValue string
switch field {
@@ -1209,8 +1204,7 @@ func (h *CollectionHandler) PreviewCollection(c *echo.Context) error {
var matchedItems []database.MediaItems
for _, item := range allItems {
listItem := mediaItemsToListMediaItemsRow(item)
evaluations := h.collectionService.EvaluateRules(listItem, req.Rules)
evaluations := h.collectionService.EvaluateRules(item, req.Rules)
for _, eval := range evaluations {
if eval.Matches {
matchedItems = append(matchedItems, item)
@@ -1255,52 +1249,3 @@ func (h *CollectionHandler) PreviewCollection(c *echo.Context) error {
return c.JSON(http.StatusOK, map[string]interface{}{"items": bookCards})
}
func mediaItemsToListMediaItemsRow(item database.MediaItems) database.ListMediaItemsRow {
return database.ListMediaItemsRow{
ID: item.ID,
LibraryID: item.LibraryID,
Title: item.Title,
Author: item.Author,
Isbn: item.Isbn,
Description: item.Description,
FilePath: utils.ResolveMediaURL(item.LibraryID, pgtype.Text{String: item.FilePath, Valid: true}),
FileSize: item.FileSize,
MimeType: item.MimeType,
CoverImagePath: pgtype.Text{String: utils.ResolveMediaURL(item.LibraryID, item.CoverImagePath), Valid: true},
Series: item.Series,
SeriesNumber: item.SeriesNumber,
Tags: item.Tags,
Asin: item.Asin,
DatePublished: item.DatePublished,
Publisher: item.Publisher,
Contributors: item.Contributors,
Language: item.Language,
Edition: item.Edition,
PageCount: item.PageCount,
Genre: item.Genre,
CopyrightYear: item.CopyrightYear,
GoodreadsID: item.GoodreadsID,
OpenlibraryID: item.OpenlibraryID,
GoogleBooksID: item.GoogleBooksID,
AddedByAdminID: item.AddedByAdminID,
CreatedAt: item.CreatedAt,
UpdatedAt: item.UpdatedAt,
FormatGroup: item.FormatGroup,
FormatMimetype: item.FormatMimetype,
IsReflowable: item.IsReflowable,
HasFixedLayout: item.HasFixedLayout,
TotalCharacters: item.TotalCharacters,
ChapterCount: item.ChapterCount,
EntitlementID: item.EntitlementID,
RevisionNumber: item.RevisionNumber,
KoboContentID: item.KoboContentID,
KoboMetadata: item.KoboMetadata,
TagsSearch: item.TagsSearch,
ContributorsSearch: item.ContributorsSearch,
FileSha256: item.FileSha256,
OpfIdentifier: item.OpfIdentifier,
OpfUuid: item.OpfUuid,
HashConfidence: item.HashConfidence,
}
}
+22 -22
View File
@@ -13,7 +13,7 @@ import (
func TestEvaluateRule_Equals_StringMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -23,7 +23,7 @@ func TestEvaluateRule_Equals_StringMatch(t *testing.T) {
func TestEvaluateRule_Equals_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Fantasy", Valid: true},
}
@@ -33,7 +33,7 @@ func TestEvaluateRule_Equals_NoMatch(t *testing.T) {
func TestEvaluateRule_NotEquals_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Fantasy", Valid: true},
}
@@ -43,7 +43,7 @@ func TestEvaluateRule_NotEquals_Matches(t *testing.T) {
func TestEvaluateRule_NotEquals_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -53,7 +53,7 @@ func TestEvaluateRule_NotEquals_NoMatch(t *testing.T) {
func TestEvaluateRule_Contains_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
}
@@ -63,7 +63,7 @@ func TestEvaluateRule_Contains_Matches(t *testing.T) {
func TestEvaluateRule_Contains_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
}
@@ -73,7 +73,7 @@ func TestEvaluateRule_Contains_NoMatch(t *testing.T) {
func TestEvaluateRule_NotContains_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
}
@@ -83,7 +83,7 @@ func TestEvaluateRule_NotContains_Matches(t *testing.T) {
func TestEvaluateRule_StartsWith_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Series: pgtype.Text{String: "Foundation", Valid: true},
}
@@ -93,7 +93,7 @@ func TestEvaluateRule_StartsWith_Matches(t *testing.T) {
func TestEvaluateRule_StartsWith_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Series: pgtype.Text{String: "Foundation", Valid: true},
}
@@ -103,7 +103,7 @@ func TestEvaluateRule_StartsWith_NoMatch(t *testing.T) {
func TestEvaluateRule_EndsWith_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Series: pgtype.Text{String: "Foundation", Valid: true},
}
@@ -113,7 +113,7 @@ func TestEvaluateRule_EndsWith_Matches(t *testing.T) {
func TestEvaluateRule_GreaterThan_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
}
@@ -123,7 +123,7 @@ func TestEvaluateRule_GreaterThan_Matches(t *testing.T) {
func TestEvaluateRule_GreaterThan_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 1999, Valid: true},
}
@@ -133,7 +133,7 @@ func TestEvaluateRule_GreaterThan_NoMatch(t *testing.T) {
func TestEvaluateRule_LessThan_Matches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 1999, Valid: true},
}
@@ -143,7 +143,7 @@ func TestEvaluateRule_LessThan_Matches(t *testing.T) {
func TestEvaluateRule_LessThan_NoMatch(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
}
@@ -153,7 +153,7 @@ func TestEvaluateRule_LessThan_NoMatch(t *testing.T) {
func TestEvaluateRule_NullField(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{Valid: false}, // NULL
}
@@ -195,7 +195,7 @@ func TestCompareValues_NumericEdgeCases(t *testing.T) {
func TestCheckRulesAgainstBook_MatchesFirstRule(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -219,7 +219,7 @@ func TestCheckRulesAgainstBook_MatchesFirstRule(t *testing.T) {
func TestCheckRulesAgainstBook_MatchesSecondRule(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Fantasy", Valid: true},
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
@@ -245,7 +245,7 @@ func TestCheckRulesAgainstBook_MatchesSecondRule(t *testing.T) {
func TestCheckRulesAgainstBook_NoMatches(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Mystery", Valid: true},
Author: pgtype.Text{String: "Agatha Christie", Valid: true},
CopyrightYear: pgtype.Int4{Int32: 1950, Valid: true},
@@ -270,7 +270,7 @@ func TestCheckRulesAgainstBook_NoMatches(t *testing.T) {
func TestCheckRulesAgainstBook_EmptyRules(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -286,7 +286,7 @@ func TestComplexRule_MultipleConditions(t *testing.T) {
handler := &CollectionHandler{}
// Book: "Foundation" by Isaac Asimov, Sci-Fi, 1951
item := database.ListMediaItemsRow{
item := database.MediaItems{
Title: "Foundation",
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
@@ -371,7 +371,7 @@ func TestComplexRule_MultipleConditions(t *testing.T) {
func TestEvaluateRule_InvalidOperator(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -381,7 +381,7 @@ func TestEvaluateRule_InvalidOperator(t *testing.T) {
func TestEvaluateRule_NonExistentField(t *testing.T) {
handler := &CollectionHandler{}
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
+3 -3
View File
@@ -51,7 +51,7 @@ func TestCompareValues_NotEquals(t *testing.T) {
}
func TestEvaluateRule_Genre(t *testing.T) {
item := database.ListMediaItemsRow{
item := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -65,7 +65,7 @@ func TestEvaluateRule_Genre(t *testing.T) {
}
func TestEvaluateRule_Author(t *testing.T) {
item := database.ListMediaItemsRow{
item := database.MediaItems{
Author: pgtype.Text{String: "Isaac Asimov", Valid: true},
}
@@ -79,7 +79,7 @@ func TestEvaluateRule_Author(t *testing.T) {
}
func TestEvaluateRule_CopyrightYear(t *testing.T) {
item := database.ListMediaItemsRow{
item := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 2021, Valid: true},
}
+1 -35
View File
@@ -509,7 +509,7 @@ func (s *CollectionService) mediaItemMatchesRule(item database.MediaItems, rule
}
// EvaluateRules evaluates auto-assign rules for a media item
func (s *CollectionService) EvaluateRules(mediaItem database.ListMediaItemsRow, rules []Rule) []RuleEvaluation {
func (s *CollectionService) EvaluateRules(mediaItem database.MediaItems, rules []Rule) []RuleEvaluation {
var evaluations []RuleEvaluation
// Sort rules by priority (higher priority first)
@@ -708,40 +708,6 @@ func (s *CollectionService) DeleteDeviceShelfMapping(ctx context.Context, mappin
return nil
}
// ApplyAutoAssignRules applies auto-assign rules for all collections to a media item
func (s *CollectionService) ApplyAutoAssignRules(ctx context.Context, mediaItem database.ListMediaItemsRow, userID uuid.UUID) ([]uuid.UUID, error) {
var matchedCollectionIDs []uuid.UUID
collections, err := s.GetUserCollections(ctx, userID)
if err != nil {
return nil, fmt.Errorf("failed to get user collections: %v", err)
}
for _, collection := range collections {
if len(collection.AutoAssignRules) == 0 {
continue
}
var rules []Rule
if err := json.Unmarshal(collection.AutoAssignRules, &rules); err != nil {
continue
}
evaluations := s.EvaluateRules(mediaItem, rules)
// Check if any rules match
for _, eval := range evaluations {
if eval.Matches && eval.Confidence > 0.7 {
matchedCollectionIDs = append(matchedCollectionIDs, uuid.UUID(collection.ID.Bytes))
break
}
}
}
return matchedCollectionIDs, nil
}
// Helper functions (Go standard library functions)
func toLower(s string) string {
if len(s) == 0 {
return s
+8 -8
View File
@@ -10,7 +10,7 @@ import (
func TestEvaluateRules_Equality(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
@@ -42,7 +42,7 @@ func TestEvaluateRules_Equality(t *testing.T) {
func TestEvaluateRules_NoMatch(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Genre: pgtype.Text{String: "Fantasy", Valid: true},
}
@@ -74,7 +74,7 @@ func TestEvaluateRules_NoMatch(t *testing.T) {
func TestEvaluateRules_Contains(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Series: pgtype.Text{String: "The Expanse", Valid: true},
}
@@ -106,7 +106,7 @@ func TestEvaluateRules_Contains(t *testing.T) {
func TestEvaluateRules_PriorityOrder(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
Series: pgtype.Text{String: "Foundation", Valid: true},
}
@@ -150,7 +150,7 @@ func TestEvaluateRules_PriorityOrder(t *testing.T) {
func TestEvaluateRules_CopyrightYear(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
CopyrightYear: pgtype.Int4{Int32: 2020, Valid: true},
}
@@ -178,7 +178,7 @@ func TestEvaluateRules_CopyrightYear(t *testing.T) {
func TestEvaluateRules_MissingField(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{}
mediaItem := database.MediaItems{}
rules := []Rule{
{
@@ -204,7 +204,7 @@ func TestEvaluateRules_MissingField(t *testing.T) {
func TestEvaluateRules_CaseInsensitive(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Genre: pgtype.Text{String: "science fiction", Valid: true},
}
@@ -232,7 +232,7 @@ func TestEvaluateRules_CaseInsensitive(t *testing.T) {
func TestEvaluateRules_PriorityBoost(t *testing.T) {
service := &CollectionService{}
mediaItem := database.ListMediaItemsRow{
mediaItem := database.MediaItems{
Genre: pgtype.Text{String: "Science Fiction", Valid: true},
}
+1 -51
View File
@@ -13,55 +13,6 @@ import (
"github.com/jackc/pgx/v5/pgtype"
)
func mediaItemsToListMediaItemsRow(item database.MediaItems) database.ListMediaItemsRow {
return database.ListMediaItemsRow{
ID: item.ID,
LibraryID: item.LibraryID,
Title: item.Title,
Author: item.Author,
Isbn: item.Isbn,
Description: item.Description,
FilePath: utils.ResolveMediaURL(item.LibraryID, pgtype.Text{String: item.FilePath, Valid: true}),
FileSize: item.FileSize,
MimeType: item.MimeType,
CoverImagePath: pgtype.Text{String: utils.ResolveMediaURL(item.LibraryID, item.CoverImagePath), Valid: true},
Series: item.Series,
SeriesNumber: item.SeriesNumber,
Tags: item.Tags,
Asin: item.Asin,
DatePublished: item.DatePublished,
Publisher: item.Publisher,
Contributors: item.Contributors,
Language: item.Language,
Edition: item.Edition,
PageCount: item.PageCount,
Genre: item.Genre,
CopyrightYear: item.CopyrightYear,
GoodreadsID: item.GoodreadsID,
OpenlibraryID: item.OpenlibraryID,
GoogleBooksID: item.GoogleBooksID,
AddedByAdminID: item.AddedByAdminID,
CreatedAt: item.CreatedAt,
UpdatedAt: item.UpdatedAt,
FormatGroup: item.FormatGroup,
FormatMimetype: item.FormatMimetype,
IsReflowable: item.IsReflowable,
HasFixedLayout: item.HasFixedLayout,
TotalCharacters: item.TotalCharacters,
ChapterCount: item.ChapterCount,
EntitlementID: item.EntitlementID,
RevisionNumber: item.RevisionNumber,
KoboContentID: item.KoboContentID,
KoboMetadata: item.KoboMetadata,
TagsSearch: item.TagsSearch,
ContributorsSearch: item.ContributorsSearch,
FileSha256: item.FileSha256,
OpfIdentifier: item.OpfIdentifier,
OpfUuid: item.OpfUuid,
HashConfidence: item.HashConfidence,
}
}
func getCollectionItemsRowToMediaItems(item database.GetCollectionItemsForDashboardRow) database.MediaItems {
return database.MediaItems{
ID: item.ID,
@@ -353,8 +304,7 @@ func (s *DashboardService) getUserCollectionItems(ctx context.Context, coll data
continue
}
listItem := mediaItemsToListMediaItemsRow(item)
evaluations := s.collectionService.EvaluateRules(listItem, rules)
evaluations := s.collectionService.EvaluateRules(item, rules)
for _, eval := range evaluations {
if eval.Matches {
autoItems = append(autoItems, item)
+15 -15
View File
@@ -561,9 +561,9 @@ type UsageItemStats struct {
Title string `json:"title"`
Author string `json:"author"`
CoverImage string `json:"cover_image_path"`
Seconds float64 `json:"seconds"` // audio duration (0 unless hours_mode)
PageCount int `json:"pages"` // text pages (0 unless !hours_mode)
SizeBytes int64 `json:"size"` // always set, both modes
Seconds float64 `json:"seconds"` // audio duration (0 unless hours_mode)
PageCount int `json:"pages"` // text pages (0 unless !hours_mode)
SizeBytes int64 `json:"size"` // always set, both modes
}
// LibraryUsageStats is the whole tab payload for one scope: "" LibraryID
@@ -573,18 +573,18 @@ type UsageItemStats struct {
// text pages. TotalSeconds/TotalPages both always carry their sums so
// the client never has to infer.
type LibraryUsageStats struct {
LibraryID string `json:"library_id"`
Items int `json:"items"`
Authors int `json:"authors"`
Narrators int `json:"narrators"`
TotalSizeBytes int64 `json:"total_size_bytes"`
TotalSeconds int64 `json:"total_seconds"`
TotalPages int `json:"total_pages"`
HoursMode bool `json:"hours_mode"`
Genres []NameCountStats `json:"genres"`
AuthorsTop []NameCountStats `json:"top_authors"`
LongestItems []UsageItemStats `json:"longest_items"`
LargestItems []UsageItemStats `json:"largest_items"`
LibraryID string `json:"library_id"`
Items int `json:"items"`
Authors int `json:"authors"`
Narrators int `json:"narrators"`
TotalSizeBytes int64 `json:"total_size_bytes"`
TotalSeconds int64 `json:"total_seconds"`
TotalPages int `json:"total_pages"`
HoursMode bool `json:"hours_mode"`
Genres []NameCountStats `json:"genres"`
AuthorsTop []NameCountStats `json:"top_authors"`
LongestItems []UsageItemStats `json:"longest_items"`
LargestItems []UsageItemStats `json:"largest_items"`
}
// LibraryUsage builds the /stats Library tab payload for one scope.