Rewrite individual entry pagination SQL queries

10 files changed, 211 insertions, 73 deletions

diff --git a/fever/fever.go b/fever/fever.go
index 7aa4885..6986353 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 0000000..72fae8d
--- /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 d596526..0577192 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 0a91ef5..c598227 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 c0dca82..f1c4fc2 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 0635128..19c13b6 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 9ab415e..0000000
--- 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 355dc27..9b98ab2 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 8ccdab5..a599bcc 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 6283a13..5dd3f7b 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"))