From 5b9886e73df453a84983bcf5fb962958015319b9 Mon Sep 17 00:00:00 2001 From: litleleprikon Date: Tue, 13 Apr 2021 15:22:08 +0300 Subject: [PATCH] feat(api): Add pager deletion During stable sorting we create a pager with TTL. Sometimes this TTL is huge and storing this search results in database is not necessary. In this cases we add a new API endpoint to delete pager manually. Relates #457 --- api/controller/triggers.go | 15 +++++++ api/controller/triggers_test.go | 42 +++++++++++++++++++ api/dto/triggers.go | 8 ++++ api/handler/triggers.go | 16 +++++++ database/redis/triggers_search_results.go | 33 ++++++++++++++- .../redis/triggers_search_results_test.go | 15 +++++++ interfaces.go | 4 +- mock/moira-alert/database.go | 29 +++++++++++++ 8 files changed, 159 insertions(+), 3 deletions(-) diff --git a/api/controller/triggers.go b/api/controller/triggers.go index d9f39bd16..c47fe796a 100644 --- a/api/controller/triggers.go +++ b/api/controller/triggers.go @@ -145,6 +145,21 @@ func SearchTriggers(database moira.Database, searcher moira.Searcher, page int64 return &triggersList, nil } +func DeleteTriggersPager(dataBase moira.Database, pagerID string) (dto.TriggersSearchResultDeleteResponse, *api.ErrorResponse) { + exists, err := dataBase.IsTriggersSearchResultsExist(pagerID) + if err != nil { + return dto.TriggersSearchResultDeleteResponse{}, api.ErrorInternalServer(err) + } + if !exists { + return dto.TriggersSearchResultDeleteResponse{}, api.ErrorNotFound(fmt.Sprintf("pager with id %s not found", pagerID)) + } + err = dataBase.DeleteTriggersSearchResults(pagerID) + if err != nil { + return dto.TriggersSearchResultDeleteResponse{}, api.ErrorInternalServer(err) + } + return dto.TriggersSearchResultDeleteResponse{PagerID: pagerID}, nil +} + func triggerExists(dataBase moira.Database, triggerID string) (bool, error) { _, err := dataBase.GetTrigger(triggerID) if err == database.ErrNil { diff --git a/api/controller/triggers_test.go b/api/controller/triggers_test.go index c9e17e409..290e84126 100644 --- a/api/controller/triggers_test.go +++ b/api/controller/triggers_test.go @@ -1,6 +1,7 @@ package controller import ( + "errors" "fmt" "testing" @@ -741,3 +742,44 @@ var triggerChecks = []moira.TriggerCheck{ } var testHighlightsMap = map[string]string{"testField": "testHighlight"} + +func TestDeleteTriggersPager(t *testing.T) { + Convey("DeleteTriggersPager", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + dataBase := mock_moira_alert.NewMockDatabase(mockCtrl) + const pagerID = "pagerID" + + Convey("Pager exists", func() { + dataBase.EXPECT().IsTriggersSearchResultsExist(pagerID).Return(true, nil) + dataBase.EXPECT().DeleteTriggersSearchResults(pagerID).Return(nil) + response, err := DeleteTriggersPager(dataBase, pagerID) + So(err, ShouldBeNil) + So(response, ShouldResemble, dto.TriggersSearchResultDeleteResponse{PagerID: pagerID}) + }) + + Convey("Pager is not exist", func() { + dataBase.EXPECT().IsTriggersSearchResultsExist(pagerID).Return(false, nil) + response, err := DeleteTriggersPager(dataBase, pagerID) + So(err, ShouldResemble, api.ErrorNotFound("pager with id pagerID not found")) + So(response, ShouldResemble, dto.TriggersSearchResultDeleteResponse{}) + }) + + Convey("Error while checking pager existence", func() { + errReturning := errors.New("example error") + dataBase.EXPECT().IsTriggersSearchResultsExist(pagerID).Return(false, errReturning) + response, err := DeleteTriggersPager(dataBase, pagerID) + So(err, ShouldResemble, api.ErrorInternalServer(errReturning)) + So(response, ShouldResemble, dto.TriggersSearchResultDeleteResponse{}) + }) + + Convey("Error while deleting pager", func() { + errReturning := errors.New("example error") + dataBase.EXPECT().IsTriggersSearchResultsExist(pagerID).Return(true, nil) + dataBase.EXPECT().DeleteTriggersSearchResults(pagerID).Return(errReturning) + response, err := DeleteTriggersPager(dataBase, pagerID) + So(err, ShouldResemble, api.ErrorInternalServer(errReturning)) + So(response, ShouldResemble, dto.TriggersSearchResultDeleteResponse{}) + }) + }) +} diff --git a/api/dto/triggers.go b/api/dto/triggers.go index cd3d3cf31..27143d5f9 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -397,3 +397,11 @@ type TriggerDump struct { Trigger moira.Trigger `json:"trigger"` Metrics []PatternMetrics `json:"metrics"` } + +type TriggersSearchResultDeleteResponse struct { + PagerID string `json:"pager_id"` +} + +func (TriggersSearchResultDeleteResponse) Render(w http.ResponseWriter, r *http.Request) error { + return nil +} diff --git a/api/handler/triggers.go b/api/handler/triggers.go index 789384f81..6f17f11f4 100644 --- a/api/handler/triggers.go +++ b/api/handler/triggers.go @@ -30,6 +30,7 @@ func triggers(metricSourceProvider *metricSource.SourceProvider, searcher moira. router.Put("/check", triggerCheck) router.Route("/{triggerId}", trigger) router.With(middleware.Paginate(0, 10)).With(middleware.Pager(false, "")).Get("/search", searchTriggers) + router.With(middleware.Pager(false, "")).Delete("/search/pager", deletePager) // ToDo: DEPRECATED method. Remove in Moira 2.6 router.With(middleware.Paginate(0, 10)).With(middleware.Pager(false, "")).Get("/page", searchTriggers) } @@ -144,6 +145,21 @@ func searchTriggers(writer http.ResponseWriter, request *http.Request) { } } +func deletePager(writer http.ResponseWriter, request *http.Request) { + pagerID := middleware.GetPagerID(request) + + response, errorResponse := controller.DeleteTriggersPager(database, pagerID) + if errorResponse != nil { + render.Render(writer, request, errorResponse) //nolint + return + } + + if err := render.Render(writer, request, response); err != nil { + render.Render(writer, request, api.ErrorRender(err)) //nolint + return + } +} + func getRequestTags(request *http.Request) []string { var filterTags []string i := 0 diff --git a/database/redis/triggers_search_results.go b/database/redis/triggers_search_results.go index 8d5b1407e..8bf19aae8 100644 --- a/database/redis/triggers_search_results.go +++ b/database/redis/triggers_search_results.go @@ -53,9 +53,9 @@ func (connector *DbConnector) GetTriggersSearchResults(searchResultsID string, p resultsID := triggersSearchResultsKey(searchResultsID) - c.Send("MULTI") //nolint + c.Send("MULTI") //nolint c.Send("LRANGE", resultsID, from, to) //nolint - c.Send("LLEN", resultsID) //nolint + c.Send("LLEN", resultsID) //nolint response, err := redis.Values(c.Do("EXEC")) if err != nil { return nil, 0, fmt.Errorf("failed to EXEC: %w", err) @@ -66,6 +66,35 @@ func (connector *DbConnector) GetTriggersSearchResults(searchResultsID string, p return reply.SearchResults(response[0], response[1], nil) } +// IsTriggersSearchResultsExist is a function that checks if there exists pager for triggers search by it's ID +func (connector *DbConnector) IsTriggersSearchResultsExist(pagerID string) (bool, error) { + c := connector.pool.Get() + defer c.Close() + + pagerIDKey := triggersSearchResultsKey(pagerID) + reply, err := c.Do("EXISTS", pagerIDKey) + + result, err := redis.Bool(reply, err) + if err != nil { + return false, fmt.Errorf("failed to check if pager exists: %w", err) + } + return result, nil +} + +// DeleteTriggersSearchResults is a function that checks if there exists pager for triggers search by it's ID +func (connector *DbConnector) DeleteTriggersSearchResults(pagerID string) error { + c := connector.pool.Get() + defer c.Close() + + pagerIDKey := triggersSearchResultsKey(pagerID) + _, err := c.Do("DEL", pagerIDKey) + + if err != nil { + return fmt.Errorf("failed to check if pager exists: %w", err) + } + return nil +} + func triggersSearchResultsKey(searchResultsID string) string { return fmt.Sprintf("moira-triggersSearchResults:%s", searchResultsID) } diff --git a/database/redis/triggers_search_results_test.go b/database/redis/triggers_search_results_test.go index 111a5c9e7..ac1529112 100644 --- a/database/redis/triggers_search_results_test.go +++ b/database/redis/triggers_search_results_test.go @@ -127,6 +127,21 @@ func TestTriggersSearchResultsStoring(t *testing.T) { So(actual, ShouldHaveLength, 3) So(actual, ShouldResemble, searchResults[3:6]) So(total, ShouldResemble, int64(8)) + + actualExists, err := dataBase.IsTriggersSearchResultsExist(searchResultsID) + So(err, ShouldBeNil) + So(actualExists, ShouldBeTrue) + + actualExists, err = dataBase.IsTriggersSearchResultsExist("nonexistentPagerID") + So(err, ShouldBeNil) + So(actualExists, ShouldBeFalse) + + err = dataBase.DeleteTriggersSearchResults(searchResultsID) + So(err, ShouldBeNil) + + actualExists, err = dataBase.IsTriggersSearchResultsExist(searchResultsID) + So(err, ShouldBeNil) + So(actualExists, ShouldBeFalse) }) } diff --git a/interfaces.go b/interfaces.go index 944d5bd9f..3447c88a0 100644 --- a/interfaces.go +++ b/interfaces.go @@ -43,9 +43,11 @@ type Database interface { GetPatternTriggerIDs(pattern string) ([]string, error) RemovePatternTriggerIDs(pattern string) error - // SearchResult storing + // SearchResult AKA pager storing GetTriggersSearchResults(searchResultsID string, page, size int64) ([]*SearchResult, int64, error) SaveTriggersSearchResults(searchResultsID string, searchResults []*SearchResult) error + IsTriggersSearchResultsExist(pagerID string) (bool, error) + DeleteTriggersSearchResults(pagerID string) error // Throttling GetTriggerThrottling(triggerID string) (time.Time, time.Time) diff --git a/mock/moira-alert/database.go b/mock/moira-alert/database.go index fb9044092..44b1918af 100644 --- a/mock/moira-alert/database.go +++ b/mock/moira-alert/database.go @@ -176,6 +176,20 @@ func (mr *MockDatabaseMockRecorder) DeleteTriggerThrottling(arg0 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteTriggerThrottling", reflect.TypeOf((*MockDatabase)(nil).DeleteTriggerThrottling), arg0) } +// DeleteTriggersSearchResults mocks base method. +func (m *MockDatabase) DeleteTriggersSearchResults(arg0 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteTriggersSearchResults", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteTriggersSearchResults indicates an expected call of DeleteTriggersSearchResults. +func (mr *MockDatabaseMockRecorder) DeleteTriggersSearchResults(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteTriggersSearchResults", reflect.TypeOf((*MockDatabase)(nil).DeleteTriggersSearchResults), arg0) +} + // FetchNotificationEvent mocks base method. func (m *MockDatabase) FetchNotificationEvent() (moira.NotificationEvent, error) { m.ctrl.T.Helper() @@ -881,6 +895,21 @@ func (mr *MockDatabaseMockRecorder) IsTeamContainUser(arg0, arg1 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsTeamContainUser", reflect.TypeOf((*MockDatabase)(nil).IsTeamContainUser), arg0, arg1) } +// IsTriggersSearchResultsExist mocks base method. +func (m *MockDatabase) IsTriggersSearchResultsExist(arg0 string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsTriggersSearchResultsExist", arg0) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsTriggersSearchResultsExist indicates an expected call of IsTriggersSearchResultsExist. +func (mr *MockDatabaseMockRecorder) IsTriggersSearchResultsExist(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsTriggersSearchResultsExist", reflect.TypeOf((*MockDatabase)(nil).IsTriggersSearchResultsExist), arg0) +} + // MarkTriggersAsUnused mocks base method. func (m *MockDatabase) MarkTriggersAsUnused(arg0 ...string) error { m.ctrl.T.Helper()