fix: counting of trigger events (#1076)

moira-alert · Sep 17, 2024 · 0dab451 · 0dab451
1 parent a8c5b00
commit 0dab451
Show file tree

Hide file tree

Showing 4 changed files with 176 additions and 71 deletions.
diff --git a/api/controller/events.go b/api/controller/events.go
@@ -8,7 +8,12 @@ import (
 	"github.com/moira-alert/moira/api/dto"
 )
 
-// GetTriggerEvents gets trigger event from current page and all trigger event count. Events list is filtered by time range
+const (
+	zeroPage      int64 = 0
+	allEventsSize int64 = -1
+)
+
+// GetTriggerEvents gets trigger events from current page and total count of filtered trigger events. Events list is filtered by time range
 // with `from` and `to` params (`from` and `to` should be "+inf", "-inf" or int64 converted to string),
 // by metric (regular expression) and by states. If `states` map is empty or nil then all states are accepted.
 func GetTriggerEvents(
@@ -19,11 +24,36 @@ func GetTriggerEvents(
 	metricRegexp *regexp.Regexp,
 	states map[string]struct{},
 ) (*dto.EventsList, *api.ErrorResponse) {
-	events, err := getFilteredNotificationEvents(database, triggerID, page, size, from, to, metricRegexp, states)
+	events, err := getFilteredNotificationEvents(database, triggerID, from, to, metricRegexp, states)
 	if err != nil {
 		return nil, api.ErrorInternalServer(err)
 	}
-	eventCount := database.GetNotificationEventCount(triggerID, -1)
+
+	eventCount := int64(len(events))
+
+	if page < 0 || (page > 0 && size < 0) {
+		return &dto.EventsList{
+			Size:  size,
+			Page:  page,
+			Total: eventCount,
+			List:  []moira.NotificationEvent{},
+		}, nil
+	}
+
+	if page >= 0 && size >= 0 {
+		start := page * size
+		end := start + size
+
+		if start >= eventCount {
+			events = []*moira.NotificationEvent{}
+		} else {
+			if end > eventCount {
+				end = eventCount
+			}
+
+			events = events[start:end]
+		}
+	}
 
 	eventsList := &dto.EventsList{
 		Size:  size,
@@ -42,44 +72,16 @@ func GetTriggerEvents(
 func getFilteredNotificationEvents(
 	database moira.Database,
 	triggerID string,
-	page, size int64,
 	from, to string,
 	metricRegexp *regexp.Regexp,
 	states map[string]struct{},
 ) ([]*moira.NotificationEvent, error) {
-	// fetch all events
-	if size < 0 {
-		events, err := database.GetNotificationEvents(triggerID, page, size, from, to)
-		if err != nil {
-			return nil, err
-		}
-
-		return filterNotificationEvents(events, metricRegexp, states), nil
-	}
-
-	// fetch at most `size` events
-	filtered := make([]*moira.NotificationEvent, 0, size)
-	var count int64
-
-	for int64(len(filtered)) < size {
-		eventsData, err := database.GetNotificationEvents(triggerID, page+count, size, from, to)
-		if err != nil {
-			return nil, err
-		}
-
-		if len(eventsData) == 0 {
-			break
-		}
-
-		filtered = append(filtered, filterNotificationEvents(eventsData, metricRegexp, states)...)
-		count += 1
-
-		if int64(len(eventsData)) < size {
-			break
-		}
+	events, err := database.GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to)
+	if err != nil {
+		return nil, err
 	}
 
-	return filtered, nil
+	return filterNotificationEvents(events, metricRegexp, states), nil
 }
 
 func filterNotificationEvents(

diff --git a/api/controller/events_test.go b/api/controller/events_test.go
@@ -25,40 +25,49 @@ func TestGetEvents(t *testing.T) {
 	defer mockCtrl.Finish()
 	triggerID := uuid.Must(uuid.NewV4()).String()
 
-	var page int64 = 10
-	var size int64 = 100
+	var page int64 = 1
+	var size int64 = 2
 	from := "-inf"
 	to := "+inf"
 
 	Convey("Test has events", t, func() {
-		var total int64 = 6000000
-		dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).
-			Return([]*moira.NotificationEvent{
-				{
-					State:    moira.StateNODATA,
-					OldState: moira.StateOK,
-				},
-				{
-					State:    moira.StateOK,
-					OldState: moira.StateNODATA,
-				},
-			}, nil)
-		dataBase.EXPECT().GetNotificationEventCount(triggerID, int64(-1)).Return(total)
+		events := []*moira.NotificationEvent{
+			{
+				State:    moira.StateNODATA,
+				OldState: moira.StateOK,
+			},
+			{
+				State:    moira.StateOK,
+				OldState: moira.StateNODATA,
+			},
+			{
+				State:    moira.StateWARN,
+				OldState: moira.StateOK,
+			},
+			{
+				State:    moira.StateERROR,
+				OldState: moira.StateWARN,
+			},
+		}
+		dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).
+			Return(events, nil)
 
 		list, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, allMetrics, allStates)
 		So(err, ShouldBeNil)
 		So(list, ShouldResemble, &dto.EventsList{
-			List:  []moira.NotificationEvent{{State: moira.StateNODATA, OldState: moira.StateOK}, {State: moira.StateOK, OldState: moira.StateNODATA}},
-			Total: total,
+			List: []moira.NotificationEvent{
+				*events[2],
+				*events[3],
+			},
+			Total: int64(len(events)),
 			Size:  size,
 			Page:  page,
 		})
 	})
 
 	Convey("Test no events", t, func() {
 		var total int64
-		dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).Return(make([]*moira.NotificationEvent, 0), nil)
-		dataBase.EXPECT().GetNotificationEventCount(triggerID, int64(-1)).Return(total)
+		dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(make([]*moira.NotificationEvent, 0), nil)
 		list, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, allMetrics, allStates)
 		So(err, ShouldBeNil)
 		So(list, ShouldResemble, &dto.EventsList{
@@ -71,7 +80,7 @@ func TestGetEvents(t *testing.T) {
 
 	Convey("Test error", t, func() {
 		expected := fmt.Errorf("oooops! Can not get all contacts")
-		dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).Return(nil, expected)
+		dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(nil, expected)
 		list, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, allMetrics, allStates)
 		So(err, ShouldResemble, api.ErrorInternalServer(expected))
 		So(list, ShouldBeNil)
@@ -85,27 +94,31 @@ func TestGetEvents(t *testing.T) {
 				filtered := []*moira.NotificationEvent{
 					{Metric: "metric.test.event1"},
 					{Metric: "a.metric.test.event2"},
+					{Metric: "metric.test.event.other"},
 				}
 				notFiltered := []*moira.NotificationEvent{
 					{Metric: "another.mEtric.test.event"},
 					{Metric: "metric.test"},
 				}
-				firstPortion := append(make([]*moira.NotificationEvent, 0), notFiltered[0], filtered[0])
-				dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).Return(firstPortion, nil)
 
-				secondPortion := append(make([]*moira.NotificationEvent, 0), filtered[1], notFiltered[1])
-				dataBase.EXPECT().GetNotificationEvents(triggerID, page+1, size, from, to).Return(secondPortion, nil)
+				events := []*moira.NotificationEvent{
+					notFiltered[0],
+					filtered[0],
+					notFiltered[1],
+					filtered[1],
+					filtered[2],
+				}
+				dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(events, nil)
 
-				total := int64(len(firstPortion) + len(secondPortion))
-				dataBase.EXPECT().GetNotificationEventCount(triggerID, int64(-1)).Return(total)
+				total := int64(len(filtered))
 
 				actual, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, regexp.MustCompile(`metric\.test\.event`), allStates)
 				So(err, ShouldBeNil)
 				So(actual, ShouldResemble, &dto.EventsList{
 					Page:  page,
 					Size:  size,
 					Total: total,
-					List:  toDTOList(filtered),
+					List:  toDTOList(filtered[:size]),
 				})
 			})
 		})
@@ -125,8 +138,7 @@ func TestGetEvents(t *testing.T) {
 			}
 			Convey("with empty map all allowed", func() {
 				total := int64(len(filtered) + len(notFiltered))
-				dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).Return(append(filtered, notFiltered...), nil)
-				dataBase.EXPECT().GetNotificationEventCount(triggerID, int64(-1)).Return(total)
+				dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(append(filtered, notFiltered...), nil)
 
 				actual, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, allMetrics, allStates)
 				So(err, ShouldBeNil)
@@ -139,9 +151,8 @@ func TestGetEvents(t *testing.T) {
 			})
 
 			Convey("with given states", func() {
-				total := int64(len(filtered) + len(notFiltered))
-				dataBase.EXPECT().GetNotificationEvents(triggerID, page, size, from, to).Return(append(filtered, notFiltered...), nil)
-				dataBase.EXPECT().GetNotificationEventCount(triggerID, int64(-1)).Return(total)
+				total := int64(len(filtered))
+				dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(append(filtered, notFiltered...), nil)
 
 				actual, err := GetTriggerEvents(dataBase, triggerID, page, size, from, to, allMetrics, map[string]struct{}{
 					string(moira.StateOK):        {},
@@ -158,6 +169,101 @@ func TestGetEvents(t *testing.T) {
 			})
 		})
 	})
+
+	Convey("test paginating", t, func() {
+		events := []*moira.NotificationEvent{
+			{
+				State:    moira.StateNODATA,
+				OldState: moira.StateOK,
+			},
+			{
+				State:    moira.StateOK,
+				OldState: moira.StateNODATA,
+			},
+			{
+				State:    moira.StateWARN,
+				OldState: moira.StateOK,
+			},
+			{
+				State:    moira.StateERROR,
+				OldState: moira.StateWARN,
+			},
+		}
+		total := int64(len(events))
+
+		type testcase struct {
+			description    string
+			expectedEvents []moira.NotificationEvent
+			givenPage      int64
+			givenSize      int64
+		}
+
+		testcases := []testcase{
+			{
+				description: "with page > 0 and size > 0",
+				givenPage:   1,
+				givenSize:   1,
+				expectedEvents: []moira.NotificationEvent{
+					*events[1],
+				},
+			},
+			{
+				description: "with page == 0 and size > 0",
+				givenPage:   0,
+				givenSize:   1,
+				expectedEvents: []moira.NotificationEvent{
+					*events[0],
+				},
+			},
+			{
+				description: "with page > 0, size > 0, page * size + size > events count",
+				givenPage:   1,
+				givenSize:   3,
+				expectedEvents: []moira.NotificationEvent{
+					*events[3],
+				},
+			},
+			{
+				description:    "with page = 0, size < 0 fetch all events",
+				givenPage:      0,
+				givenSize:      -10,
+				expectedEvents: toDTOList(events),
+			},
+			{
+				description:    "with page > 0, size < 0 return no events",
+				givenPage:      1,
+				givenSize:      -1,
+				expectedEvents: []moira.NotificationEvent{},
+			},
+			{
+				description:    "with page < 0 return no events",
+				givenPage:      -1,
+				givenSize:      1,
+				expectedEvents: []moira.NotificationEvent{},
+			},
+			{
+				description:    "with page * size >= len(events)",
+				givenPage:      1,
+				givenSize:      int64(len(events)),
+				expectedEvents: []moira.NotificationEvent{},
+			},
+		}
+
+		for i := range testcases {
+			Convey(fmt.Sprintf("test case %d: %s", i+1, testcases[i].description), func() {
+				dataBase.EXPECT().GetNotificationEvents(triggerID, zeroPage, allEventsSize, from, to).Return(events, nil)
+
+				actual, err := GetTriggerEvents(dataBase, triggerID, testcases[i].givenPage, testcases[i].givenSize, from, to, allMetrics, allStates)
+				So(err, ShouldBeNil)
+				So(actual, ShouldResemble, &dto.EventsList{
+					Page:  testcases[i].givenPage,
+					Size:  testcases[i].givenSize,
+					Total: total,
+					List:  testcases[i].expectedEvents,
+				})
+			})
+		}
+	})
 }
 
 func toDTOList(eventPtrs []*moira.NotificationEvent) []moira.NotificationEvent {

diff --git a/api/handler/trigger_test.go b/api/handler/trigger_test.go
@@ -421,7 +421,6 @@ func TestGetTriggerWithTriggerSource(t *testing.T) {
 		db.EXPECT().GetTrigger(triggerId).Return(trigger, nil)
 		db.EXPECT().GetTriggerThrottling(triggerId)
 		db.EXPECT().GetNotificationEvents(triggerId, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(make([]*moira.NotificationEvent, 0), nil)
-		db.EXPECT().GetNotificationEventCount(triggerId, gomock.Any()).Return(int64(0))
 
 		responseWriter := httptest.NewRecorder()
 		getTrigger(responseWriter, request)
@@ -465,7 +464,6 @@ func TestGetTriggerWithTriggerSource(t *testing.T) {
 		db.EXPECT().GetTrigger(triggerId).Return(trigger, nil)
 		db.EXPECT().GetTriggerThrottling(triggerId)
 		db.EXPECT().GetNotificationEvents(triggerId, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(make([]*moira.NotificationEvent, 0), nil)
-		db.EXPECT().GetNotificationEventCount(triggerId, gomock.Any()).Return(int64(0))
 
 		responseWriter := httptest.NewRecorder()
 		getTrigger(responseWriter, request)
@@ -509,7 +507,6 @@ func TestGetTriggerWithTriggerSource(t *testing.T) {
 		db.EXPECT().GetTrigger(triggerId).Return(trigger, nil)
 		db.EXPECT().GetTriggerThrottling(triggerId)
 		db.EXPECT().GetNotificationEvents(triggerId, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(make([]*moira.NotificationEvent, 0), nil)
-		db.EXPECT().GetNotificationEventCount(triggerId, gomock.Any()).Return(int64(0))
 
 		responseWriter := httptest.NewRecorder()
 		getTrigger(responseWriter, request)

diff --git a/interfaces.go b/interfaces.go
@@ -60,7 +60,7 @@ type Database interface {
 	DeleteTriggerThrottling(triggerID string) error
 
 	// NotificationEvent storing
-	GetNotificationEvents(triggerID string, start, size int64, from, to string) ([]*NotificationEvent, error)
+	GetNotificationEvents(triggerID string, page, size int64, from, to string) ([]*NotificationEvent, error)
 	PushNotificationEvent(event *NotificationEvent, ui bool) error
 	GetNotificationEventCount(triggerID string, from int64) int64
 	FetchNotificationEvent() (NotificationEvent, error)