From 28775f5e10b1b00e717abd06f369c45eae2bdea1 Mon Sep 17 00:00:00 2001 From: fred Date: Mon, 19 Jun 2023 14:00:10 -0700 Subject: [PATCH] Refactor entry/feed query builder sorting to match SQL semantic --- .gitignore | 3 ++- api/entry.go | 3 +-- fever/handler.go | 7 +++--- googlereader/handler.go | 15 +++++-------- storage/entry_query_builder.go | 39 +++++++++++++--------------------- storage/feed.go | 9 +++----- storage/feed_query_builder.go | 23 ++++++-------------- ui/bookmark_entries.go | 3 +-- ui/category_entries.go | 3 +-- ui/category_entries_all.go | 3 +-- ui/feed_entries.go | 3 +-- ui/feed_entries_all.go | 3 +-- ui/history_entries.go | 3 ++- ui/shared_entries.go | 3 +-- ui/unread_entries.go | 3 +-- 15 files changed, 44 insertions(+), 79 deletions(-) diff --git a/.gitignore b/.gitignore index 5e30ffea..f53dc253 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ miniflux-* miniflux *.rpm *.deb -.idea \ No newline at end of file +.idea +.vscode \ No newline at end of file diff --git a/api/entry.go b/api/entry.go index e64e867f..5545b200 100644 --- a/api/entry.go +++ b/api/entry.go @@ -143,8 +143,7 @@ func (h *handler) findEntries(w http.ResponseWriter, r *http.Request, feedID int builder.WithFeedID(feedID) builder.WithCategoryID(categoryID) builder.WithStatuses(statuses) - builder.WithOrder(order) - builder.WithDirection(direction) + builder.WithSorting(order, direction) builder.WithOffset(offset) builder.WithLimit(limit) builder.WithTags(tags) diff --git a/fever/handler.go b/fever/handler.go index ce5919c0..3be63916 100644 --- a/fever/handler.go +++ b/fever/handler.go @@ -242,8 +242,7 @@ func (h *handler) handleItems(w http.ResponseWriter, r *http.Request) { builder := h.store.NewEntryQueryBuilder(userID) builder.WithoutStatus(model.EntryStatusRemoved) builder.WithLimit(50) - builder.WithOrder("id") - builder.WithDirection(model.DefaultSortingDirection) + builder.WithSorting("id", model.DefaultSortingDirection) switch { case request.HasQueryParam(r, "since_id"): @@ -256,11 +255,11 @@ func (h *handler) handleItems(w http.ResponseWriter, r *http.Request) { maxID := request.QueryInt64Param(r, "max_id", 0) if maxID == 0 { logger.Debug("[Fever] Fetching most recent items for user #%d", userID) - builder.WithDirection("desc") + builder.WithSorting("id", "DESC") } else if maxID > 0 { logger.Debug("[Fever] Fetching items before #%d for user #%d", maxID, userID) builder.BeforeEntryID(maxID) - builder.WithDirection("desc") + builder.WithSorting("id", "DESC") } case request.HasQueryParam(r, "with_ids"): csvItemIDs := request.QueryStringParam(r, "with_ids", "") diff --git a/googlereader/handler.go b/googlereader/handler.go index 4e5272ac..f52b38bf 100644 --- a/googlereader/handler.go +++ b/googlereader/handler.go @@ -790,8 +790,7 @@ func (h *handler) streamItemContents(w http.ResponseWriter, r *http.Request) { builder := h.store.NewEntryQueryBuilder(userID) builder.WithoutStatus(model.EntryStatusRemoved) builder.WithEntryIDs(itemIDs) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(requestModifiers.SortDirection) + builder.WithSorting(model.DefaultSortingOrder, requestModifiers.SortDirection) entries, err := builder.GetEntries() if err != nil { @@ -1144,8 +1143,7 @@ func (h *handler) handleReadingListStream(w http.ResponseWriter, r *http.Request builder.WithoutStatus(model.EntryStatusRemoved) builder.WithLimit(rm.Count) builder.WithOffset(rm.Offset) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(rm.SortDirection) + builder.WithSorting(model.DefaultSortingOrder, rm.SortDirection) if rm.StartTime > 0 { builder.AfterDate(time.Unix(rm.StartTime, 0)) } @@ -1187,8 +1185,7 @@ func (h *handler) handleStarredStream(w http.ResponseWriter, r *http.Request, rm builder.WithStarred(true) builder.WithLimit(rm.Count) builder.WithOffset(rm.Offset) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(rm.SortDirection) + builder.WithSorting(model.DefaultSortingOrder, rm.SortDirection) if rm.StartTime > 0 { builder.AfterDate(time.Unix(rm.StartTime, 0)) } @@ -1230,8 +1227,7 @@ func (h *handler) handleReadStream(w http.ResponseWriter, r *http.Request, rm Re builder.WithStatus(model.EntryStatusRead) builder.WithLimit(rm.Count) builder.WithOffset(rm.Offset) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(rm.SortDirection) + builder.WithSorting(model.DefaultSortingOrder, rm.SortDirection) if rm.StartTime > 0 { builder.AfterDate(time.Unix(rm.StartTime, 0)) } @@ -1279,8 +1275,7 @@ func (h *handler) handleFeedStream(w http.ResponseWriter, r *http.Request, rm Re builder.WithFeedID(feedID) builder.WithLimit(rm.Count) builder.WithOffset(rm.Offset) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(rm.SortDirection) + builder.WithSorting(model.DefaultSortingOrder, rm.SortDirection) if rm.StartTime > 0 { builder.AfterDate(time.Unix(rm.StartTime, 0)) } diff --git a/storage/entry_query_builder.go b/storage/entry_query_builder.go index d47ca3af..ec54334a 100644 --- a/storage/entry_query_builder.go +++ b/storage/entry_query_builder.go @@ -18,13 +18,12 @@ import ( // EntryQueryBuilder builds a SQL query to fetch entries. type EntryQueryBuilder struct { - store *Storage - args []interface{} - conditions []string - order string - direction string - limit int - offset int + store *Storage + args []interface{} + conditions []string + sortExpressions []string + limit int + offset int } // WithSearchQuery adds full-text search query to the condition. @@ -35,8 +34,10 @@ func (e *EntryQueryBuilder) WithSearchQuery(query string) *EntryQueryBuilder { e.args = append(e.args, query) // 0.0000001 = 0.1 / (seconds_in_a_day) - e.WithOrder(fmt.Sprintf("ts_rank(document_vectors, plainto_tsquery($%d)) - extract (epoch from now() - published_at)::float * 0.0000001", nArgs)) - e.WithDirection("DESC") + e.WithSorting( + fmt.Sprintf("ts_rank(document_vectors, plainto_tsquery($%d)) - extract (epoch from now() - published_at)::float * 0.0000001", nArgs), + "DESC", + ) } return e } @@ -168,15 +169,9 @@ func (e *EntryQueryBuilder) WithShareCodeNotEmpty() *EntryQueryBuilder { return e } -// WithOrder set the sorting order. -func (e *EntryQueryBuilder) WithOrder(order string) *EntryQueryBuilder { - e.order = order - return e -} - -// WithDirection set the sorting direction. -func (e *EntryQueryBuilder) WithDirection(direction string) *EntryQueryBuilder { - e.direction = direction +// WithSorting add a sort expression. +func (e *EntryQueryBuilder) WithSorting(column, direction string) *EntryQueryBuilder { + e.sortExpressions = append(e.sortExpressions, fmt.Sprintf("%s %s", column, direction)) return e } @@ -403,12 +398,8 @@ func (e *EntryQueryBuilder) buildCondition() string { func (e *EntryQueryBuilder) buildSorting() string { var parts []string - if e.order != "" { - parts = append(parts, fmt.Sprintf(`ORDER BY %s`, e.order)) - } - - if e.direction != "" { - parts = append(parts, e.direction) + if len(e.sortExpressions) > 0 { + parts = append(parts, fmt.Sprintf(`ORDER BY %s`, strings.Join(e.sortExpressions, ", "))) } if e.limit > 0 { diff --git a/storage/feed.go b/storage/feed.go index efa1b516..b10b2196 100644 --- a/storage/feed.go +++ b/storage/feed.go @@ -135,8 +135,7 @@ func (s *Storage) CountAllFeedsWithErrors() int { // Feeds returns all feeds that belongs to the given user. func (s *Storage) Feeds(userID int64) (model.Feeds, error) { builder := NewFeedQueryBuilder(s, userID) - builder.WithOrder(model.DefaultFeedSorting) - builder.WithDirection(model.DefaultFeedSortingDirection) + builder.WithSorting(model.DefaultFeedSorting, model.DefaultFeedSortingDirection) return builder.GetFeeds() } @@ -153,8 +152,7 @@ func getFeedsSorted(builder *FeedQueryBuilder) (model.Feeds, error) { func (s *Storage) FeedsWithCounters(userID int64) (model.Feeds, error) { builder := NewFeedQueryBuilder(s, userID) builder.WithCounters() - builder.WithOrder(model.DefaultFeedSorting) - builder.WithDirection(model.DefaultFeedSortingDirection) + builder.WithSorting(model.DefaultFeedSorting, model.DefaultFeedSortingDirection) return getFeedsSorted(builder) } @@ -171,8 +169,7 @@ func (s *Storage) FeedsByCategoryWithCounters(userID, categoryID int64) (model.F builder := NewFeedQueryBuilder(s, userID) builder.WithCategoryID(categoryID) builder.WithCounters() - builder.WithOrder(model.DefaultFeedSorting) - builder.WithDirection(model.DefaultFeedSortingDirection) + builder.WithSorting(model.DefaultFeedSorting, model.DefaultFeedSortingDirection) return getFeedsSorted(builder) } diff --git a/storage/feed_query_builder.go b/storage/feed_query_builder.go index 1c310eb7..8ef2d9a7 100644 --- a/storage/feed_query_builder.go +++ b/storage/feed_query_builder.go @@ -18,8 +18,7 @@ type FeedQueryBuilder struct { store *Storage args []interface{} conditions []string - order string - direction string + sortExpressions []string limit int offset int withCounters bool @@ -66,15 +65,9 @@ func (f *FeedQueryBuilder) WithCounters() *FeedQueryBuilder { return f } -// WithOrder set the sorting order. -func (f *FeedQueryBuilder) WithOrder(order string) *FeedQueryBuilder { - f.order = order - return f -} - -// WithDirection set the sorting direction. -func (f *FeedQueryBuilder) WithDirection(direction string) *FeedQueryBuilder { - f.direction = direction +// WithSorting add a sort expression. +func (f *FeedQueryBuilder) WithSorting(column, direction string) *FeedQueryBuilder { + f.sortExpressions = append(f.sortExpressions, fmt.Sprintf("%s %s", column, direction)) return f } @@ -101,12 +94,8 @@ func (f *FeedQueryBuilder) buildCounterCondition() string { func (f *FeedQueryBuilder) buildSorting() string { var parts []string - if f.order != "" { - parts = append(parts, fmt.Sprintf(`ORDER BY %s`, f.order)) - } - - if f.direction != "" { - parts = append(parts, f.direction) + if len(f.sortExpressions) > 0 { + parts = append(parts, fmt.Sprintf(`ORDER BY %s`, strings.Join(f.sortExpressions, ", "))) } if len(parts) > 0 { diff --git a/ui/bookmark_entries.go b/ui/bookmark_entries.go index 225131d0..fcbd9876 100644 --- a/ui/bookmark_entries.go +++ b/ui/bookmark_entries.go @@ -26,8 +26,7 @@ func (h *handler) showStarredPage(w http.ResponseWriter, r *http.Request) { builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithoutStatus(model.EntryStatusRemoved) builder.WithStarred(true) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/category_entries.go b/ui/category_entries.go index c0889452..b2ef2ff6 100644 --- a/ui/category_entries.go +++ b/ui/category_entries.go @@ -37,8 +37,7 @@ func (h *handler) showCategoryEntriesPage(w http.ResponseWriter, r *http.Request offset := request.QueryIntParam(r, "offset", 0) builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithCategoryID(category.ID) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithStatus(model.EntryStatusUnread) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/category_entries_all.go b/ui/category_entries_all.go index 84478251..25fd24c9 100644 --- a/ui/category_entries_all.go +++ b/ui/category_entries_all.go @@ -37,8 +37,7 @@ func (h *handler) showCategoryEntriesAllPage(w http.ResponseWriter, r *http.Requ offset := request.QueryIntParam(r, "offset", 0) builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithCategoryID(category.ID) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithoutStatus(model.EntryStatusRemoved) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/feed_entries.go b/ui/feed_entries.go index fa975f78..674cbdcf 100644 --- a/ui/feed_entries.go +++ b/ui/feed_entries.go @@ -38,8 +38,7 @@ func (h *handler) showFeedEntriesPage(w http.ResponseWriter, r *http.Request) { builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithFeedID(feed.ID) builder.WithStatus(model.EntryStatusUnread) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/feed_entries_all.go b/ui/feed_entries_all.go index 0c42f8a0..4501706a 100644 --- a/ui/feed_entries_all.go +++ b/ui/feed_entries_all.go @@ -38,8 +38,7 @@ func (h *handler) showFeedEntriesAllPage(w http.ResponseWriter, r *http.Request) builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithFeedID(feed.ID) builder.WithoutStatus(model.EntryStatusRemoved) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/history_entries.go b/ui/history_entries.go index 5576ecd9..4107fb69 100644 --- a/ui/history_entries.go +++ b/ui/history_entries.go @@ -25,7 +25,8 @@ func (h *handler) showHistoryPage(w http.ResponseWriter, r *http.Request) { offset := request.QueryIntParam(r, "offset", 0) builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithStatus(model.EntryStatusRead) - builder.WithOrder("changed_at DESC, published_at DESC") + builder.WithSorting("changed_at", "DESC") + builder.WithSorting("published_at", "DESC") builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) diff --git a/ui/shared_entries.go b/ui/shared_entries.go index da84eeb2..16dc7561 100644 --- a/ui/shared_entries.go +++ b/ui/shared_entries.go @@ -22,8 +22,7 @@ func (h *handler) sharedEntries(w http.ResponseWriter, r *http.Request) { builder := h.store.NewEntryQueryBuilder(user.ID) builder.WithShareCodeNotEmpty() - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) entries, err := builder.GetEntries() if err != nil { diff --git a/ui/unread_entries.go b/ui/unread_entries.go index ed22eae9..31d626a0 100644 --- a/ui/unread_entries.go +++ b/ui/unread_entries.go @@ -49,8 +49,7 @@ func (h *handler) showUnreadPage(w http.ResponseWriter, r *http.Request) { beginSqlFetchUnreadEntries := time.Now() builder = h.store.NewEntryQueryBuilder(user.ID) builder.WithStatus(model.EntryStatusUnread) - builder.WithOrder(user.EntryOrder) - builder.WithDirection(user.EntryDirection) + builder.WithSorting(user.EntryOrder, user.EntryDirection) builder.WithOffset(offset) builder.WithLimit(user.EntriesPerPage) builder.WithGloballyVisible()