feat(api): Implement stable pagination

Add ability to have stable pagination on GET /api/triggers/search. Relates #457
moira-alert · Mar 3, 2020 · 5bd9320 · 5bd9320
1 parent a560784
commit 5bd9320
Show file tree

Hide file tree

Showing 13 changed files with 707 additions and 21 deletions.
diff --git a/api/controller/triggers.go b/api/controller/triggers.go
@@ -2,6 +2,7 @@ package controller
 
 import (
 	"fmt"
+	"math"
 
 	"github.com/gofrs/uuid"
 
@@ -11,6 +12,8 @@ import (
 	"github.com/moira-alert/moira/database"
 )
 
+const pageSizeUnlimited int64 = -1
+
 // CreateTrigger creates new trigger
 func CreateTrigger(dataBase moira.Database, trigger *dto.TriggerModel, timeSeriesNames map[string]bool) (*dto.SaveTriggerResponse, *api.ErrorResponse) {
 	if trigger.ID == "" {
@@ -57,10 +60,51 @@ func GetAllTriggers(database moira.Database) (*dto.TriggersList, *api.ErrorRespo
 }
 
 // SearchTriggers gets trigger page and filter trigger by tags and search request terms
-func SearchTriggers(database moira.Database, searcher moira.Searcher, page int64, size int64, onlyErrors bool, filterTags []string, searchString string) (*dto.TriggersList, *api.ErrorResponse) {
-	searchResults, total, err := searcher.SearchTriggers(filterTags, searchString, onlyErrors, page, size)
-	if err != nil {
-		return nil, api.ErrorInternalServer(err)
+func SearchTriggers(database moira.Database, searcher moira.Searcher, page int64, size int64, onlyErrors bool, filterTags []string, searchString string, createPager bool, pagerID string) (*dto.TriggersList, *api.ErrorResponse) {
+	var searchResults []*moira.SearchResult
+	var total int64
+	pagerShouldExist := pagerID != ""
+
+	if pagerShouldExist && searchString != "" {
+		return nil, api.ErrorInvalidRequest(fmt.Errorf("cannot handle request with both search string and pager ID set"))
+	}
+	if pagerShouldExist {
+		var err error
+		searchResults, total, err = database.GetTriggersSearchResults(pagerID, page, size)
+		if err != nil {
+			return nil, api.ErrorInternalServer(err)
+		}
+		if searchResults == nil {
+			return nil, api.ErrorNotFound("Pager not found")
+		}
+	} else {
+		var err error
+		var passSize = size
+		if createPager {
+			passSize = pageSizeUnlimited
+		}
+		searchResults, total, err = searcher.SearchTriggers(filterTags, searchString, onlyErrors, page, passSize)
+		if err != nil {
+			return nil, api.ErrorInternalServer(err)
+		}
+	}
+
+	if createPager && !pagerShouldExist {
+		uuid4, err := uuid.NewV4()
+		if err != nil {
+			return nil, api.ErrorInternalServer(err)
+		}
+		pagerID = uuid4.String()
+		database.SaveTriggersSearchResults(pagerID, searchResults)
+	}
+
+	if createPager {
+		var from, to int64 = 0, int64(len(searchResults))
+		if size >= 0 {
+			from = int64(math.Min(float64(page*size), float64(len(searchResults))))
+			to = int64(math.Min(float64(from+size), float64(len(searchResults))))
+		}
+		searchResults = searchResults[from:to]
 	}
 
 	var triggerIDs []string
@@ -72,11 +116,18 @@ func SearchTriggers(database moira.Database, searcher moira.Searcher, page int64
 	if err != nil {
 		return nil, api.ErrorInternalServer(err)
 	}
+
+	var pagerIDPtr *string
+	if pagerID != "" {
+		pagerIDPtr = &pagerID
+	}
+
 	triggersList := dto.TriggersList{
 		List:  make([]moira.TriggerCheck, 0),
 		Total: &total,
 		Page:  &page,
 		Size:  &size,
+		Pager: pagerIDPtr,
 	}
 
 	for triggerCheckInd := range triggerChecks {

diff --git a/api/controller/triggers_test.go b/api/controller/triggers_test.go
@@ -159,7 +159,7 @@ func TestSearchTriggers(t *testing.T) {
 		Convey("Page is bigger than triggers number", func() {
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(triggerSearchResults, exp, nil)
 			mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(triggersPointers, nil)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 			So(err, ShouldBeNil)
 			So(list, ShouldResemble, &dto.TriggersList{
 				List:  triggerChecks,
@@ -173,7 +173,7 @@ func TestSearchTriggers(t *testing.T) {
 			size = -1
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(triggerSearchResults, exp, nil)
 			mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(triggersPointers, nil)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 			So(err, ShouldBeNil)
 			So(list, ShouldResemble, &dto.TriggersList{
 				List:  triggerChecks,
@@ -187,7 +187,7 @@ func TestSearchTriggers(t *testing.T) {
 			size = 10
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(triggerSearchResults[:10], exp, nil)
 			mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[:10]).Return(triggersPointers[:10], nil)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 			So(err, ShouldBeNil)
 			So(list, ShouldResemble, &dto.TriggersList{
 				List:  triggerChecks[:10],
@@ -200,7 +200,7 @@ func TestSearchTriggers(t *testing.T) {
 				page = 1
 				mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(triggerSearchResults[10:20], exp, nil)
 				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[10:20]).Return(triggersPointers[10:20], nil)
-				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 				So(err, ShouldBeNil)
 				So(list, ShouldResemble, &dto.TriggersList{
 					List:  triggerChecks[10:20],
@@ -220,7 +220,7 @@ func TestSearchTriggers(t *testing.T) {
 			// superTrigger31 is the only trigger without errors
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, true, page, size).Return(triggerSearchResults[:10], exp, nil)
 			mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[:10]).Return(triggersPointers[:10], nil)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString, false, "")
 			So(err, ShouldBeNil)
 			So(list, ShouldResemble, &dto.TriggersList{
 				List:  triggerChecks[0:10],
@@ -234,7 +234,7 @@ func TestSearchTriggers(t *testing.T) {
 				exp = 2
 				mockIndex.EXPECT().SearchTriggers(tags, searchString, true, page, size).Return(triggerSearchResults[1:3], exp, nil)
 				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[1:3]).Return(triggersPointers[1:3], nil)
-				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString)
+				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString, false, "")
 				So(err, ShouldBeNil)
 				So(list, ShouldResemble, &dto.TriggersList{
 					List:  triggerChecks[1:3],
@@ -249,7 +249,7 @@ func TestSearchTriggers(t *testing.T) {
 				exp = 1
 				mockIndex.EXPECT().SearchTriggers(tags, searchString, true, page, size).Return(triggerSearchResults[2:3], exp, nil)
 				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[2:3]).Return(triggersPointers[2:3], nil)
-				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString)
+				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString, false, "")
 				So(err, ShouldBeNil)
 				So(list, ShouldResemble, &dto.TriggersList{
 					List:  triggerChecks[2:3],
@@ -290,7 +290,7 @@ func TestSearchTriggers(t *testing.T) {
 
 				mockIndex.EXPECT().SearchTriggers(tags, searchString, true, page, size).Return(deadlyTrapsSearchResults, exp, nil)
 				mockDatabase.EXPECT().GetTriggerChecks(deadlyTrapsTriggerIDs).Return(deadlyTrapsPointers, nil)
-				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString)
+				list, err := SearchTriggers(mockDatabase, mockIndex, page, size, true, tags, searchString, false, "")
 				So(err, ShouldBeNil)
 				So(list, ShouldResemble, &dto.TriggersList{
 					List:  deadlyTraps,
@@ -309,7 +309,7 @@ func TestSearchTriggers(t *testing.T) {
 		Convey("Error from searcher", func() {
 			searcherError := fmt.Errorf("very bad request")
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(make([]*moira.SearchResult, 0), int64(0), searcherError)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 			So(err, ShouldNotBeNil)
 			So(list, ShouldBeNil)
 		})
@@ -319,11 +319,73 @@ func TestSearchTriggers(t *testing.T) {
 			searcherError := fmt.Errorf("very bad request")
 			mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, size).Return(triggerSearchResults, exp, nil)
 			mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(nil, searcherError)
-			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, "")
 			So(err, ShouldNotBeNil)
 			So(list, ShouldBeNil)
 		})
 	})
+
+	Convey("Search with pager", t, func() {
+		Convey("Create pager", func() {
+			pagerID := ""
+			var page int64 = 0
+			var size int64 = -1
+			var exp int64 = 31
+			gomock.InOrder(
+				mockIndex.EXPECT().SearchTriggers(tags, searchString, false, page, int64(-1)).Return(triggerSearchResults, exp, nil),
+				mockDatabase.EXPECT().SaveTriggersSearchResults(gomock.Any(), triggerSearchResults).Return(nil).Do(func(pID string, _ interface{}) {
+					pagerID = pID
+				}),
+				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(triggersPointers, nil),
+			)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, true, "")
+			So(err, ShouldBeNil)
+			So(list, ShouldResemble, &dto.TriggersList{
+				List:  triggerChecks,
+				Total: &exp,
+				Page:  &page,
+				Size:  &size,
+				Pager: &pagerID,
+			})
+		})
+		Convey("Use pager", func() {
+			pagerID := "TestPagerID"
+			var page int64 = 0
+			var size int64 = -1
+			var exp int64 = 31
+			gomock.InOrder(
+				mockDatabase.EXPECT().GetTriggersSearchResults(pagerID, page, size).Return(triggerSearchResults, exp, nil),
+				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs).Return(triggersPointers, nil),
+			)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, pagerID)
+			So(err, ShouldBeNil)
+			So(list, ShouldResemble, &dto.TriggersList{
+				List:  triggerChecks,
+				Total: &exp,
+				Page:  &page,
+				Size:  &size,
+				Pager: &pagerID,
+			})
+		})
+		Convey("Use pager and page size higher than amount of search results", func() {
+			pagerID := "TestPagerID"
+			var exp int64 = 2
+			var size int64 = 10
+			gomock.InOrder(
+				mockDatabase.EXPECT().GetTriggersSearchResults(pagerID, page, size).Return(triggerSearchResults[:2], exp, nil),
+				mockDatabase.EXPECT().GetTriggerChecks(triggerIDs[:2]).Return(triggersPointers[:2], nil),
+			)
+			list, err := SearchTriggers(mockDatabase, mockIndex, page, size, false, tags, searchString, false, pagerID)
+			So(err, ShouldBeNil)
+			So(list, ShouldResemble, &dto.TriggersList{
+				List:  triggerChecks[:2],
+				Total: &exp,
+				Page:  &page,
+				Size:  &size,
+				Pager: &pagerID,
+			})
+		})
+	})
 }
 
 var triggerChecks = []moira.TriggerCheck{

diff --git a/api/dto/triggers.go b/api/dto/triggers.go
@@ -17,6 +17,7 @@ type TriggersList struct {
 	Page  *int64               `json:"page,omitempty"`
 	Size  *int64               `json:"size,omitempty"`
 	Total *int64               `json:"total,omitempty"`
+	Pager *string              `json:"pager,omitempty"`
 	List  []moira.TriggerCheck `json:"list"`
 }
 

diff --git a/api/handler/triggers.go b/api/handler/triggers.go
@@ -27,7 +27,7 @@ func triggers(metricSourceProvider *metricSource.SourceProvider, searcher moira.
 		router.Get("/", getAllTriggers)
 		router.Put("/", createTrigger)
 		router.Route("/{triggerId}", trigger)
-		router.With(middleware.Paginate(0, 10)).Get("/search", searchTriggers)
+		router.With(middleware.Paginate(0, 10)).With(middleware.Pager(false, "")).Get("/search", searchTriggers)
 		// ToDo: DEPRECATED method. Remove in Moira 2.6
 		router.With(middleware.Paginate(0, 10)).Get("/page", searchTriggers)
 	}
@@ -85,7 +85,10 @@ func searchTriggers(writer http.ResponseWriter, request *http.Request) {
 	page := middleware.GetPage(request)
 	size := middleware.GetSize(request)
 
-	triggersList, errorResponse := controller.SearchTriggers(database, searchIndex, page, size, onlyErrors, filterTags, searchString)
+	createPager := middleware.GetCreatePager(request)
+	pagerID := middleware.GetPagerID(request)
+
+	triggersList, errorResponse := controller.SearchTriggers(database, searchIndex, page, size, onlyErrors, filterTags, searchString, createPager, pagerID)
 	if errorResponse != nil {
 		render.Render(writer, request, errorResponse)
 		return

diff --git a/api/middleware/context.go b/api/middleware/context.go
@@ -124,6 +124,27 @@ func Paginate(defaultPage, defaultSize int64) func(next http.Handler) http.Handl
 	}
 }
 
+// Pager is a function that takes pager id from query
+func Pager(defaultCreatePager bool, defaultPagerID string) func(next http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+			pagerID := request.URL.Query().Get("pagerID")
+			if pagerID == "" {
+				pagerID = defaultPagerID
+			}
+
+			createPager, err := strconv.ParseBool(request.URL.Query().Get("createPager"))
+			if err != nil {
+				createPager = defaultCreatePager
+			}
+
+			ctxPager := context.WithValue(request.Context(), pagerIDKey, pagerID)
+			ctxSize := context.WithValue(ctxPager, createPagerKey, createPager)
+			next.ServeHTTP(writer, request.WithContext(ctxSize))
+		})
+	}
+}
+
 // DateRange gets from and to values from URI query and set it to request context. If query has not values sets given values
 func DateRange(defaultFrom, defaultTo string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {

diff --git a/api/middleware/middleware.go b/api/middleware/middleware.go
@@ -24,6 +24,8 @@ var (
 	subscriptionIDKey    ContextKey = "subscriptionID"
 	pageKey              ContextKey = "page"
 	sizeKey              ContextKey = "size"
+	pagerIDKey           ContextKey = "pagerID"
+	createPagerKey       ContextKey = "createPager"
 	fromKey              ContextKey = "from"
 	toKey                ContextKey = "to"
 	loginKey             ContextKey = "login"
@@ -71,6 +73,16 @@ func GetSize(request *http.Request) int64 {
 	return request.Context().Value(sizeKey).(int64)
 }
 
+// GetPagerID is a function that gets pagerID value from request context, which was sets in Pager middleware
+func GetPagerID(request *http.Request) string {
+	return request.Context().Value(pagerIDKey).(string)
+}
+
+// GetCreatePager is a function that gets createPager value from request context, which was sets in Pager middleware
+func GetCreatePager(request *http.Request) bool {
+	return request.Context().Value(createPagerKey).(bool)
+}
+
 // GetFromStr gets 'from' value from request context, which was sets in DateRange middleware
 func GetFromStr(request *http.Request) string {
 	return request.Context().Value(fromKey).(string)