From 0dab451267525ab2e86bc654fa5a511a50c6a2af Mon Sep 17 00:00:00 2001 From: Aleksandr Matsko <90016901+AleksandrMatsko@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:33:48 +0700 Subject: [PATCH] fix: counting of trigger events (#1076) --- api/controller/events.go | 72 +++++++------- api/controller/events_test.go | 170 +++++++++++++++++++++++++++------- api/handler/trigger_test.go | 3 - interfaces.go | 2 +- 4 files changed, 176 insertions(+), 71 deletions(-) diff --git a/api/controller/events.go b/api/controller/events.go index c38fd9152..f982273c5 100644 --- 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 index 4c84f7fc2..bd1a0c18a 100644 --- a/api/controller/events_test.go +++ b/api/controller/events_test.go @@ -25,31 +25,41 @@ 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, }) @@ -57,8 +67,7 @@ func TestGetEvents(t *testing.T) { 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,19 +94,23 @@ 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) @@ -105,7 +118,7 @@ func TestGetEvents(t *testing.T) { 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 index f9d96ec3c..d5d2a8460 100644 --- 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 index b6e4bcd18..94fc776bd 100644 --- 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)