From c5373ff2bffb3a3e3d37b3eb6bd1cdc733d9c590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Sat, 9 Jun 2018 13:40:20 -0700 Subject: [PATCH] Rewrite individual entry pagination SQL queries --- fever/fever.go | 6 +- storage/entry_pagination_builder.go | 158 ++++++++++++++++++++++++++++ storage/entry_query_builder.go | 42 +++++--- ui/entry_bookmark.go | 8 +- ui/entry_category.go | 8 +- ui/entry_feed.go | 8 +- ui/entry_prev_next.go | 36 ------- ui/entry_read.go | 8 +- ui/entry_unread.go | 8 +- ui/unread_entries.go | 2 +- 10 files changed, 211 insertions(+), 73 deletions(-) create mode 100644 storage/entry_pagination_builder.go delete mode 100644 ui/entry_prev_next.go diff --git a/fever/fever.go b/fever/fever.go index 7aa48856..69863537 100644 --- a/fever/fever.go +++ b/fever/fever.go @@ -353,7 +353,7 @@ func (c *Controller) handleItems(w http.ResponseWriter, r *http.Request) { sinceID := request.QueryIntParam(r, "since_id", 0) if sinceID > 0 { - builder.WithGreaterThanEntryID(int64(sinceID)) + builder.AfterEntryID(int64(sinceID)) } maxID := request.QueryIntParam(r, "max_id", 0) @@ -560,7 +560,7 @@ func (c *Controller) handleWriteFeeds(w http.ResponseWriter, r *http.Request) { before := request.FormIntValue(r, "before") if before > 0 { t := time.Unix(before, 0) - builder.Before(&t) + builder.BeforeDate(t) } entryIDs, err := builder.GetEntryIDs() @@ -601,7 +601,7 @@ func (c *Controller) handleWriteGroups(w http.ResponseWriter, r *http.Request) { before := request.FormIntValue(r, "before") if before > 0 { t := time.Unix(before, 0) - builder.Before(&t) + builder.BeforeDate(t) } entryIDs, err := builder.GetEntryIDs() diff --git a/storage/entry_pagination_builder.go b/storage/entry_pagination_builder.go new file mode 100644 index 00000000..72fae8d0 --- /dev/null +++ b/storage/entry_pagination_builder.go @@ -0,0 +1,158 @@ +// Copyright 2018 Frédéric Guillot. All rights reserved. +// Use of this source code is governed by the Apache 2.0 +// license that can be found in the LICENSE file. + +package storage + +import ( + "database/sql" + "fmt" + "strings" + "time" + + "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/timer" +) + +// EntryPaginationBuilder is a builder for entry prev/next queries. +type EntryPaginationBuilder struct { + store *Storage + conditions []string + args []interface{} + entryID int64 + direction string +} + +// WithStarred adds starred to the condition. +func (e *EntryPaginationBuilder) WithStarred() { + e.conditions = append(e.conditions, "e.starred is true") +} + +// WithFeedID adds feed_id to the condition. +func (e *EntryPaginationBuilder) WithFeedID(feedID int64) { + if feedID != 0 { + e.conditions = append(e.conditions, fmt.Sprintf("e.feed_id = $%d", len(e.args)+1)) + e.args = append(e.args, feedID) + } +} + +// WithCategoryID adds category_id to the condition. +func (e *EntryPaginationBuilder) WithCategoryID(categoryID int64) { + if categoryID != 0 { + e.conditions = append(e.conditions, fmt.Sprintf("f.category_id = $%d", len(e.args)+1)) + e.args = append(e.args, categoryID) + } +} + +// WithStatus adds status to the condition. +func (e *EntryPaginationBuilder) WithStatus(status string) { + if status != "" { + e.conditions = append(e.conditions, fmt.Sprintf("e.status = $%d", len(e.args)+1)) + e.args = append(e.args, status) + } +} + +// Entries returns previous and next entries. +func (e *EntryPaginationBuilder) Entries() (*model.Entry, *model.Entry, error) { + tx, err := e.store.db.Begin() + if err != nil { + return nil, nil, fmt.Errorf("begin transaction for entry pagination: %v", err) + } + + prevID, nextID, err := e.getPrevNextID(tx) + if err != nil { + tx.Rollback() + return nil, nil, err + } + + prevEntry, err := e.getEntry(tx, prevID) + if err != nil { + tx.Rollback() + return nil, nil, err + } + + nextEntry, err := e.getEntry(tx, nextID) + if err != nil { + tx.Rollback() + return nil, nil, err + } + + tx.Commit() + + if e.direction == "desc" { + return nextEntry, prevEntry, nil + } + + return prevEntry, nextEntry, nil +} + +func (e *EntryPaginationBuilder) getPrevNextID(tx *sql.Tx) (prevID int64, nextID int64, err error) { + defer timer.ExecutionTime(time.Now(), fmt.Sprintf("[EntryPaginationBuilder] %v, %v", e.conditions, e.args)) + + cte := ` + WITH entry_pagination AS ( + SELECT + e.id, + lag(e.id) over (order by e.published_at asc, e.id desc) as prev_id, + lead(e.id) over (order by e.published_at asc, e.id desc) as next_id + FROM entries AS e + LEFT JOIN feeds AS f ON f.id=e.feed_id + WHERE %s + ORDER BY e.published_at asc, e.id desc + ) + SELECT prev_id, next_id FROM entry_pagination AS ep WHERE %s; + ` + + subCondition := strings.Join(e.conditions, " AND ") + finalCondition := fmt.Sprintf("ep.id = $%d", len(e.args)+1) + query := fmt.Sprintf(cte, subCondition, finalCondition) + e.args = append(e.args, e.entryID) + + var pID, nID sql.NullInt64 + err = tx.QueryRow(query, e.args...).Scan(&pID, &nID) + switch { + case err == sql.ErrNoRows: + return 0, 0, nil + case err != nil: + return 0, 0, fmt.Errorf("entry pagination: %v", err) + } + + if pID.Valid { + prevID = pID.Int64 + } + + if nID.Valid { + nextID = nID.Int64 + } + + return prevID, nextID, nil +} + +func (e *EntryPaginationBuilder) getEntry(tx *sql.Tx, entryID int64) (*model.Entry, error) { + var entry model.Entry + + err := tx.QueryRow(`SELECT id, title FROM entries WHERE id = $1`, entryID).Scan( + &entry.ID, + &entry.Title, + ) + + switch { + case err == sql.ErrNoRows: + return nil, nil + case err != nil: + return nil, fmt.Errorf("fetching sibling entry: %v", err) + } + + return &entry, nil +} + +// NewEntryPaginationBuilder returns a new EntryPaginationBuilder. +func NewEntryPaginationBuilder(store *Storage, userID, entryID int64, direction string) *EntryPaginationBuilder { + return &EntryPaginationBuilder{ + store: store, + args: []interface{}{userID, "removed"}, + conditions: []string{"e.user_id = $1", "e.status <> $2"}, + entryID: entryID, + direction: direction, + } +} diff --git a/storage/entry_query_builder.go b/storage/entry_query_builder.go index d5965264..05771921 100644 --- a/storage/entry_query_builder.go +++ b/storage/entry_query_builder.go @@ -33,15 +33,31 @@ func (e *EntryQueryBuilder) WithStarred() *EntryQueryBuilder { return e } -// Before add condition based on the entry date. -func (e *EntryQueryBuilder) Before(date *time.Time) *EntryQueryBuilder { +// BeforeDate adds a condition < published_at +func (e *EntryQueryBuilder) BeforeDate(date time.Time) *EntryQueryBuilder { e.conditions = append(e.conditions, fmt.Sprintf("e.published_at < $%d", len(e.args)+1)) e.args = append(e.args, date) return e } -// WithGreaterThanEntryID adds a condition > entryID. -func (e *EntryQueryBuilder) WithGreaterThanEntryID(entryID int64) *EntryQueryBuilder { +// AfterDate adds a condition > published_at +func (e *EntryQueryBuilder) AfterDate(date time.Time) *EntryQueryBuilder { + e.conditions = append(e.conditions, fmt.Sprintf("e.published_at > $%d", len(e.args)+1)) + e.args = append(e.args, date) + return e +} + +// BeforeEntryID adds a condition < entryID. +func (e *EntryQueryBuilder) BeforeEntryID(entryID int64) *EntryQueryBuilder { + if entryID != 0 { + e.conditions = append(e.conditions, fmt.Sprintf("e.id < $%d", len(e.args)+1)) + e.args = append(e.args, entryID) + } + return e +} + +// AfterEntryID adds a condition > entryID. +func (e *EntryQueryBuilder) AfterEntryID(entryID int64) *EntryQueryBuilder { if entryID != 0 { e.conditions = append(e.conditions, fmt.Sprintf("e.id > $%d", len(e.args)+1)) e.args = append(e.args, entryID) @@ -179,10 +195,10 @@ func (e *EntryQueryBuilder) GetEntries() (model.Entries, error) { ` condition := e.buildCondition() - query = fmt.Sprintf(query, condition, e.buildSorting()) - // log.Println(query) + sorting := e.buildSorting() + query = fmt.Sprintf(query, condition, sorting) - defer timer.ExecutionTime(time.Now(), fmt.Sprintf("[EntryQueryBuilder:GetEntries] condition=%s, args=%v", condition, e.args)) + defer timer.ExecutionTime(time.Now(), fmt.Sprintf("[EntryQueryBuilder:GetEntries] %s, args=%v, sorting=%s", condition, e.args, sorting)) rows, err := e.store.db.Query(query, e.args...) if err != nil { @@ -286,25 +302,25 @@ func (e *EntryQueryBuilder) buildCondition() string { } func (e *EntryQueryBuilder) buildSorting() string { - var queries []string + var parts []string if e.order != "" { - queries = append(queries, fmt.Sprintf(`ORDER BY "%s"`, e.order)) + parts = append(parts, fmt.Sprintf(`ORDER BY "%s"`, e.order)) } if e.direction != "" { - queries = append(queries, fmt.Sprintf(`%s`, e.direction)) + parts = append(parts, fmt.Sprintf(`%s`, e.direction)) } if e.limit != 0 { - queries = append(queries, fmt.Sprintf(`LIMIT %d`, e.limit)) + parts = append(parts, fmt.Sprintf(`LIMIT %d`, e.limit)) } if e.offset != 0 { - queries = append(queries, fmt.Sprintf(`OFFSET %d`, e.offset)) + parts = append(parts, fmt.Sprintf(`OFFSET %d`, e.offset)) } - return strings.Join(queries, " ") + return strings.Join(parts, " ") } // NewEntryQueryBuilder returns a new EntryQueryBuilder. diff --git a/ui/entry_bookmark.go b/ui/entry_bookmark.go index 0a91ef54..c5982273 100644 --- a/ui/entry_bookmark.go +++ b/ui/entry_bookmark.go @@ -13,6 +13,7 @@ import ( "github.com/miniflux/miniflux/http/route" "github.com/miniflux/miniflux/logger" "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/storage" "github.com/miniflux/miniflux/ui/session" "github.com/miniflux/miniflux/ui/view" ) @@ -57,10 +58,9 @@ func (c *Controller) ShowStarredEntry(w http.ResponseWriter, r *http.Request) { } } - builder = c.store.NewEntryQueryBuilder(user.ID) - builder.WithStarred() - - prevEntry, nextEntry, err := c.getEntryPrevNext(user, builder, entry.ID) + entryPaginationBuilder := storage.NewEntryPaginationBuilder(c.store, user.ID, entry.ID, user.EntryDirection) + entryPaginationBuilder.WithStarred() + prevEntry, nextEntry, err := entryPaginationBuilder.Entries() if err != nil { html.ServerError(w, err) return diff --git a/ui/entry_category.go b/ui/entry_category.go index c0dca826..f1c4fc2a 100644 --- a/ui/entry_category.go +++ b/ui/entry_category.go @@ -13,6 +13,7 @@ import ( "github.com/miniflux/miniflux/http/route" "github.com/miniflux/miniflux/logger" "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/storage" "github.com/miniflux/miniflux/ui/session" "github.com/miniflux/miniflux/ui/view" ) @@ -64,10 +65,9 @@ func (c *Controller) ShowCategoryEntry(w http.ResponseWriter, r *http.Request) { } } - builder = c.store.NewEntryQueryBuilder(user.ID) - builder.WithCategoryID(categoryID) - - prevEntry, nextEntry, err := c.getEntryPrevNext(user, builder, entry.ID) + entryPaginationBuilder := storage.NewEntryPaginationBuilder(c.store, user.ID, entry.ID, user.EntryDirection) + entryPaginationBuilder.WithCategoryID(categoryID) + prevEntry, nextEntry, err := entryPaginationBuilder.Entries() if err != nil { html.ServerError(w, err) return diff --git a/ui/entry_feed.go b/ui/entry_feed.go index 06351280..19c13b63 100644 --- a/ui/entry_feed.go +++ b/ui/entry_feed.go @@ -13,6 +13,7 @@ import ( "github.com/miniflux/miniflux/http/route" "github.com/miniflux/miniflux/logger" "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/storage" "github.com/miniflux/miniflux/ui/session" "github.com/miniflux/miniflux/ui/view" ) @@ -64,10 +65,9 @@ func (c *Controller) ShowFeedEntry(w http.ResponseWriter, r *http.Request) { } } - builder = c.store.NewEntryQueryBuilder(user.ID) - builder.WithFeedID(feedID) - - prevEntry, nextEntry, err := c.getEntryPrevNext(user, builder, entry.ID) + entryPaginationBuilder := storage.NewEntryPaginationBuilder(c.store, user.ID, entry.ID, user.EntryDirection) + entryPaginationBuilder.WithFeedID(feedID) + prevEntry, nextEntry, err := entryPaginationBuilder.Entries() if err != nil { html.ServerError(w, err) return diff --git a/ui/entry_prev_next.go b/ui/entry_prev_next.go deleted file mode 100644 index 9ab415e9..00000000 --- a/ui/entry_prev_next.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2017 Frédéric Guillot. All rights reserved. -// Use of this source code is governed by the Apache 2.0 -// license that can be found in the LICENSE file. - -package ui - -import ( - "github.com/miniflux/miniflux/model" - "github.com/miniflux/miniflux/storage" -) - -func (c *Controller) getEntryPrevNext(user *model.User, builder *storage.EntryQueryBuilder, entryID int64) (prev *model.Entry, next *model.Entry, err error) { - builder.WithoutStatus(model.EntryStatusRemoved) - builder.WithOrder(model.DefaultSortingOrder) - builder.WithDirection(user.EntryDirection) - - entries, err := builder.GetEntries() - if err != nil { - return nil, nil, err - } - - n := len(entries) - for i := 0; i < n; i++ { - if entries[i].ID == entryID { - if i-1 >= 0 { - prev = entries[i-1] - } - - if i+1 < n { - next = entries[i+1] - } - } - } - - return prev, next, nil -} diff --git a/ui/entry_read.go b/ui/entry_read.go index 355dc27e..9b98ab22 100644 --- a/ui/entry_read.go +++ b/ui/entry_read.go @@ -12,6 +12,7 @@ import ( "github.com/miniflux/miniflux/http/response/html" "github.com/miniflux/miniflux/http/route" "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/storage" "github.com/miniflux/miniflux/ui/session" "github.com/miniflux/miniflux/ui/view" ) @@ -47,10 +48,9 @@ func (c *Controller) ShowReadEntry(w http.ResponseWriter, r *http.Request) { return } - builder = c.store.NewEntryQueryBuilder(user.ID) - builder.WithStatus(model.EntryStatusRead) - - prevEntry, nextEntry, err := c.getEntryPrevNext(user, builder, entry.ID) + entryPaginationBuilder := storage.NewEntryPaginationBuilder(c.store, user.ID, entry.ID, user.EntryDirection) + entryPaginationBuilder.WithStatus(model.EntryStatusRead) + prevEntry, nextEntry, err := entryPaginationBuilder.Entries() if err != nil { html.ServerError(w, err) return diff --git a/ui/entry_unread.go b/ui/entry_unread.go index 8ccdab5b..a599bcc4 100644 --- a/ui/entry_unread.go +++ b/ui/entry_unread.go @@ -13,6 +13,7 @@ import ( "github.com/miniflux/miniflux/http/route" "github.com/miniflux/miniflux/logger" "github.com/miniflux/miniflux/model" + "github.com/miniflux/miniflux/storage" "github.com/miniflux/miniflux/ui/session" "github.com/miniflux/miniflux/ui/view" ) @@ -48,10 +49,9 @@ func (c *Controller) ShowUnreadEntry(w http.ResponseWriter, r *http.Request) { return } - builder = c.store.NewEntryQueryBuilder(user.ID) - builder.WithStatus(model.EntryStatusUnread) - - prevEntry, nextEntry, err := c.getEntryPrevNext(user, builder, entry.ID) + entryPaginationBuilder := storage.NewEntryPaginationBuilder(c.store, user.ID, entry.ID, user.EntryDirection) + entryPaginationBuilder.WithStatus(model.EntryStatusUnread) + prevEntry, nextEntry, err := entryPaginationBuilder.Entries() if err != nil { html.ServerError(w, err) return diff --git a/ui/unread_entries.go b/ui/unread_entries.go index 6283a130..5dd3f7be 100644 --- a/ui/unread_entries.go +++ b/ui/unread_entries.go @@ -57,7 +57,7 @@ func (c *Controller) ShowUnreadPage(w http.ResponseWriter, r *http.Request) { view.Set("pagination", c.getPagination(route.Path(c.router, "unread"), countUnread, offset)) view.Set("menu", "unread") view.Set("user", user) - view.Set("countUnread", c.store.CountUnreadEntries(user.ID)) + view.Set("countUnread", countUnread) view.Set("hasSaveEntry", c.store.HasSaveEntry(user.ID)) html.OK(w, view.Render("unread_entries"))