From 770a0053e08f185203aea8ae63b1d870be14b425 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Mon, 16 Nov 2020 19:01:09 +0100 Subject: [PATCH] Bespoke paging (non-cursor) for actionlog --- pkg/actionlog/types.go | 18 ++- store/actionlog.yaml | 2 + store/rdbms/actionlog.gen.go | 192 +------------------------------ store/rdbms/actionlog.go | 21 +++- store/rdbms/rdbms.go | 2 + store/tests/actionlog_test.go | 91 ++++----------- system/rest.yaml | 7 +- system/rest/actionlog.go | 15 +-- system/rest/request/actionlog.go | 46 ++++---- 9 files changed, 88 insertions(+), 306 deletions(-) diff --git a/pkg/actionlog/types.go b/pkg/actionlog/types.go index a41f2193e..f345518ba 100644 --- a/pkg/actionlog/types.go +++ b/pkg/actionlog/types.go @@ -1,7 +1,6 @@ package actionlog import ( - "github.com/cortezaproject/corteza-server/pkg/filter" "time" ) @@ -53,18 +52,15 @@ type ( } Filter struct { - From *time.Time `json:"from"` - To *time.Time `json:"to"` - ActorID []uint64 `json:"actorID"` - Resource string `json:"resource"` - Action string `json:"action"` + FromTimestamp *time.Time `json:"from"` + ToTimestamp *time.Time `json:"to"` - // @todo pending implementation - // Query string `json:"query"` + BeforeActionID uint64 `json:"beforeActionID"` - // Standard helpers for paging and sorting - filter.Sorting - filter.Paging + ActorID []uint64 `json:"actorID"` + Resource string `json:"resource"` + Action string `json:"action"` + Limit uint `json:"limit"` } loggableMetaValue interface { diff --git a/store/actionlog.yaml b/store/actionlog.yaml index 9a26f29c2..165b4d97a 100644 --- a/store/actionlog.yaml +++ b/store/actionlog.yaml @@ -33,6 +33,8 @@ rdbms: ActorID: { column: actor_id } search: + enablePaging: false + enableSorting: false enableFilterCheckFunction: false update: diff --git a/store/rdbms/actionlog.gen.go b/store/rdbms/actionlog.gen.go index cec7467e3..c7cec8116 100644 --- a/store/rdbms/actionlog.gen.go +++ b/store/rdbms/actionlog.gen.go @@ -14,7 +14,6 @@ import ( "github.com/Masterminds/squirrel" "github.com/cortezaproject/corteza-server/pkg/actionlog" "github.com/cortezaproject/corteza-server/pkg/errors" - "github.com/cortezaproject/corteza-server/pkg/filter" "github.com/cortezaproject/corteza-server/store" ) @@ -37,147 +36,11 @@ func (s Store) SearchActionlogs(ctx context.Context, f actionlog.Filter) (action return err } - // Paging enabled - // {search: {enablePaging:true}} - // Cleanup unwanted cursors (only relevant is f.PageCursor, next&prev are reset and returned) - f.PrevPage, f.NextPage = nil, nil - - if f.PageCursor != nil { - // Page cursor exists so we need to validate it against used sort - if f.Sort, err = f.PageCursor.Sort(f.Sort); err != nil { - return err - } - } - - if len(f.Sort) == 0 { - f.Sort = filter.SortExprSet{} - } - - // Make sure results are always sorted at least by primary keys - if f.Sort.Get("id") == nil { - f.Sort = append(f.Sort, &filter.SortExpr{Column: "id", Descending: true}) - } - - sort := f.Sort.Clone() - - // When cursor for a previous page is used it's marked as reversed - // This tells us to flip the descending flag on all used sort keys - if f.PageCursor != nil && f.PageCursor.Reverse { - sort.Reverse() - } - - // Apply sorting expr from filter to query - if q, err = setOrderBy(q, sort, s.sortableActionlogColumns()); err != nil { - return err - } - - set, f.PrevPage, f.NextPage, err = s.fetchFullPageOfActionlogs(ctx, q, sort.Columns(), sort.Reversed(), f.PageCursor, f.Limit, nil) - if err != nil { - return err - } - - f.PageCursor = nil - return nil + set, err = s.QueryActionlogs(ctx, q, nil) + return err }() } -// fetchFullPageOfActionlogs collects all requested results. -// -// Function applies: -// - cursor conditions (where ...) -// - limit -// -// Main responsibility of this function is to perform additional sequential queries in case when not enough results -// are collected due to failed check on a specific row (by check fn). -// -// Function then moves cursor to the last item fetched -func (s Store) fetchFullPageOfActionlogs( - ctx context.Context, - q squirrel.SelectBuilder, - sortColumns []string, - sortDesc bool, - cursor *filter.PagingCursor, - limit uint, - check func(*actionlog.Action) (bool, error), -) (set []*actionlog.Action, prev, next *filter.PagingCursor, err error) { - var ( - aux []*actionlog.Action - - // When cursor for a previous page is used it's marked as reversed - // This tells us to flip the descending flag on all used sort keys - reversedOrder = cursor != nil && cursor.Reverse - - // copy of the select builder - tryQuery squirrel.SelectBuilder - - fetched uint - ) - - set = make([]*actionlog.Action, 0, DefaultSliceCapacity) - - if cursor != nil { - cursor.Reverse = sortDesc - } - - for try := 0; try < MaxRefetches; try++ { - tryQuery = setCursorCond(q, cursor) - if limit > 0 { - tryQuery = tryQuery.Limit(uint64(limit + 1)) - } - - if aux, err = s.QueryActionlogs(ctx, tryQuery, check); err != nil { - return nil, nil, nil, err - } - - fetched = uint(len(aux)) - if cursor != nil && prev == nil && fetched > 0 { - // Cursor for previous page is calculated only when cursor is used (so, not on first page) - prev = s.collectActionlogCursorValues(aux[0], sortColumns...) - } - - // Point cursor to the last fetched element - if fetched > limit && limit > 0 { - next = s.collectActionlogCursorValues(aux[limit-1], sortColumns...) - - // we should use only as much as requested - set = append(set, aux[:limit]...) - break - } else { - set = append(set, aux...) - } - - // if limit is not set or we've already collected enough items - // we can break the loop right away - if limit == 0 || fetched == 0 || fetched <= limit { - break - } - - // In case limit is set very low and we've missed records in the first fetch, - // make sure next fetch limit is a bit higher - if limit < MinEnsureFetchLimit { - limit = MinEnsureFetchLimit - } - - // @todo improve strategy for collecting next page with lower limit - } - - if reversedOrder { - // Fetched set needs to be reversed because we've forced a descending order to get the previous page - for i, j := 0, len(set)-1; i < j; i, j = i+1, j-1 { - set[i], set[j] = set[j], set[i] - } - - // and flip prev/next cursors too - prev, next = next, prev - } - - if prev != nil { - prev.Reverse = true - } - - return set, prev, next, nil -} - // QueryActionlogs queries the database, converts and checks each row and // returns collected set // @@ -324,16 +187,7 @@ func (Store) actionlogColumns(aa ...string) []string { } } -// {true true false true true false} - -// sortableActionlogColumns returns all Actionlog columns flagged as sortable -// -// With optional string arg, all columns are returned aliased -func (Store) sortableActionlogColumns() map[string]string { - return map[string]string{ - "id": "id", - } -} +// {true true false false false false} // internalActionlogEncoder encodes fields from actionlog.Action to store.Payload (map) // @@ -343,46 +197,6 @@ func (s Store) internalActionlogEncoder(res *actionlog.Action) store.Payload { return s.encodeActionlog(res) } -// collectActionlogCursorValues collects values from the given resource that and sets them to the cursor -// to be used for pagination -// -// Values that are collected must come from sortable, unique or primary columns/fields -// At least one of the collected columns must be flagged as unique, otherwise fn appends primary keys at the end -// -// Known issue: -// when collecting cursor values for query that sorts by unique column with partial index (ie: unique handle on -// undeleted items) -func (s Store) collectActionlogCursorValues(res *actionlog.Action, cc ...string) *filter.PagingCursor { - var ( - cursor = &filter.PagingCursor{} - - hasUnique bool - - // All known primary key columns - - pkId bool - - collect = func(cc ...string) { - for _, c := range cc { - switch c { - case "id": - cursor.Set(c, res.ID, false) - - pkId = true - - } - } - } - ) - - collect(cc...) - if !hasUnique || !(pkId && true) { - collect("id") - } - - return cursor -} - // checkActionlogConstraints performs lookups (on valid) resource to check if any of the values on unique fields // already exists in the store // diff --git a/store/rdbms/actionlog.go b/store/rdbms/actionlog.go index 30cc20398..58222d5e2 100644 --- a/store/rdbms/actionlog.go +++ b/store/rdbms/actionlog.go @@ -10,12 +10,19 @@ import ( func (s Store) convertActionlogFilter(f actionlog.Filter) (query squirrel.SelectBuilder, err error) { query = s.actionlogsSelectBuilder() - if f.From != nil { - query = query.Where(squirrel.GtOrEq{"ts": f.From}) + // Always sort by ID descending + query = query.OrderBy("id DESC") + + if f.BeforeActionID > 0 { + query = query.Where(squirrel.Lt{"id": f.BeforeActionID}) } - if f.To != nil { - query = query.Where(squirrel.LtOrEq{"ts": f.To}) + if f.FromTimestamp != nil { + query = query.Where(squirrel.GtOrEq{"ts": f.FromTimestamp}) + } + + if f.ToTimestamp != nil { + query = query.Where(squirrel.LtOrEq{"ts": f.ToTimestamp}) } if len(f.ActorID) > 0 { @@ -30,6 +37,12 @@ func (s Store) convertActionlogFilter(f actionlog.Filter) (query squirrel.Select query = query.Where(squirrel.Eq{"action": f.Action}) } + if f.Limit == 0 || f.Limit > MaxLimit { + f.Limit = MaxLimit + } + + query = query.Limit(uint64(f.Limit)) + return } diff --git a/store/rdbms/rdbms.go b/store/rdbms/rdbms.go index cc885e37f..1544e836c 100644 --- a/store/rdbms/rdbms.go +++ b/store/rdbms/rdbms.go @@ -67,6 +67,8 @@ const ( MinEnsureFetchLimit = 10 MaxRefetches = 100 + + MaxLimit = 1000 ) func Connect(ctx context.Context, cfg *Config) (s *Store, err error) { diff --git a/store/tests/actionlog_test.go b/store/tests/actionlog_test.go index 96d832f2e..84f290871 100644 --- a/store/tests/actionlog_test.go +++ b/store/tests/actionlog_test.go @@ -3,12 +3,10 @@ package tests import ( "context" "strconv" - "strings" "testing" "time" "github.com/cortezaproject/corteza-server/pkg/actionlog" - "github.com/cortezaproject/corteza-server/pkg/rand" "github.com/cortezaproject/corteza-server/store" _ "github.com/joho/godotenv/autoload" "github.com/stretchr/testify/require" @@ -18,15 +16,13 @@ func testActionlog(t *testing.T, s store.Actionlogs) { var ( ctx = context.Background() - makeNew = func(id uint64, dd ...string) *actionlog.Action { + makeNew = func(id uint64) *actionlog.Action { // minimum data set for new user - desc := strings.Join(dd, "") return &actionlog.Action{ - ID: id, - Timestamp: *now(), - Action: "test-action", - Resource: "test-resource", - Description: desc, + ID: id, + Timestamp: *now(), + Action: "test-action", + Resource: "test-resource", } } @@ -51,7 +47,7 @@ func testActionlog(t *testing.T, s store.Actionlogs) { set := make([]*actionlog.Action, l) for i := 0; i < l; i++ { - set[i] = makeNew(uint64(i), string(rand.Bytes(10))) + set[i] = makeNew(uint64(i)) } req.NoError(s.CreateActionlog(ctx, set...)) @@ -93,79 +89,40 @@ func testActionlog(t *testing.T, s store.Actionlogs) { req.NoError(s.TruncateActionlogs(ctx)) set := []*actionlog.Action{ - makeNew(1, "01"), - makeNew(2, "02"), - makeNew(3, "03"), - makeNew(4, "04"), - makeNew(5, "05"), - makeNew(6, "06"), - makeNew(7, "07"), - makeNew(8, "08"), - makeNew(9, "09"), - makeNew(10, "10"), + makeNew(1), + makeNew(2), + makeNew(3), + makeNew(4), + makeNew(5), + makeNew(6), + makeNew(7), + makeNew(8), + makeNew(9), } req.NoError(s.CreateActionlog(ctx, set...)) f := actionlog.Filter{} // Fetch first page - f.Limit = 3 + f.Limit = 1 set, f, err := store.SearchActionlogs(ctx, s, f) req.NoError(err) - req.Len(set, 3) - req.NotNil(f.NextPage) - req.Nil(f.PrevPage) - req.Equal("10..8", stringifySetRange(set)) + req.Len(set, int(f.Limit)) + req.Equal("9", stringifySetRange(set)) - // 2nd page - f.Limit = 6 - f.PageCursor = f.NextPage - set, f, err = store.SearchActionlogs(ctx, s, f) - req.NoError(err) - req.Len(set, 6) - req.NotNil(f.NextPage) - req.NotNil(f.PrevPage) - req.Equal("7..2", stringifySetRange(set)) - - // 3rd, last page (1 item left) - f.Limit = 2 - f.PageCursor = f.NextPage - set, f, err = store.SearchActionlogs(ctx, s, f) - req.NoError(err) - req.Len(set, 1) - req.Nil(f.NextPage) - req.NotNil(f.PrevPage) - req.Equal("1", stringifySetRange(set)) - - // now, in reverse, last 3 items f.Limit = 3 - f.PageCursor = f.PrevPage + f.BeforeActionID = set[0].ID set, f, err = store.SearchActionlogs(ctx, s, f) req.NoError(err) - req.Len(set, 3) - req.NotNil(f.NextPage) - req.NotNil(f.PrevPage) - req.Equal("4..2", stringifySetRange(set)) + req.Len(set, int(f.Limit)) + req.Equal("8..6", stringifySetRange(set)) - // still in reverse, next 6 items - f.Limit = 5 - f.PageCursor = f.PrevPage + f.Limit = 9 + f.BeforeActionID = set[2].ID set, f, err = store.SearchActionlogs(ctx, s, f) req.NoError(err) req.Len(set, 5) - req.NotNil(f.NextPage) - req.NotNil(f.PrevPage) - req.Equal("9..5", stringifySetRange(set)) - - // still in reverse, last 5 items (actually, we'll only get 1) - f.Limit = 5 - f.PageCursor = f.PrevPage - set, f, err = store.SearchActionlogs(ctx, s, f) - req.NoError(err) - req.Len(set, 1) - req.Nil(f.PrevPage) - req.NotNil(f.NextPage) - req.Equal("10", stringifySetRange(set)) + req.Equal("5..1", stringifySetRange(set)) }) }) diff --git a/system/rest.yaml b/system/rest.yaml index b530a47df..b4a2d8f78 100644 --- a/system/rest.yaml +++ b/system/rest.yaml @@ -1207,6 +1207,10 @@ endpoints: type: "*time.Time" required: false title: To + - name: beforeActionID + type: uint64 + required: false + title: Entries before specified action ID - name: resource required: false title: Resource @@ -1222,6 +1226,3 @@ endpoints: - type: uint name: limit title: Limit - - type: string - name: pageCursor - title: Page cursor diff --git a/system/rest/actionlog.go b/system/rest/actionlog.go index 666797e2c..397165d2c 100644 --- a/system/rest/actionlog.go +++ b/system/rest/actionlog.go @@ -40,18 +40,15 @@ func (ctrl *Actionlog) List(ctx context.Context, r *request.ActionlogList) (inte var ( err error f = actionlog.Filter{ - From: r.From, - To: r.To, - ActorID: payload.ParseUint64s(r.ActorID), - Resource: r.Resource, - Action: r.Action, + FromTimestamp: r.From, + ToTimestamp: r.To, + ActorID: payload.ParseUint64s(r.ActorID), + Resource: r.Resource, + Action: r.Action, + Limit: r.Limit, } ) - if f.Paging, err = filter.NewPaging(r.Limit, r.PageCursor); err != nil { - return nil, err - } - ee, f, err := ctrl.actionSvc.Find(ctx, f) return ctrl.makeFilterPayload(ctx, ee, f, err) diff --git a/system/rest/request/actionlog.go b/system/rest/request/actionlog.go index d8770f164..d79b4825e 100644 --- a/system/rest/request/actionlog.go +++ b/system/rest/request/actionlog.go @@ -41,6 +41,11 @@ type ( // To To *time.Time + // BeforeActionID GET parameter + // + // Entries before specified action ID + BeforeActionID uint64 `json:",string"` + // Resource GET parameter // // Resource @@ -60,11 +65,6 @@ type ( // // Limit Limit uint - - // PageCursor GET parameter - // - // Page cursor - PageCursor string } ) @@ -76,13 +76,13 @@ func NewActionlogList() *ActionlogList { // Auditable returns all auditable/loggable parameters func (r ActionlogList) Auditable() map[string]interface{} { return map[string]interface{}{ - "from": r.From, - "to": r.To, - "resource": r.Resource, - "action": r.Action, - "actorID": r.ActorID, - "limit": r.Limit, - "pageCursor": r.PageCursor, + "from": r.From, + "to": r.To, + "beforeActionID": r.BeforeActionID, + "resource": r.Resource, + "action": r.Action, + "actorID": r.ActorID, + "limit": r.Limit, } } @@ -96,6 +96,11 @@ func (r ActionlogList) GetTo() *time.Time { return r.To } +// Auditable returns all auditable/loggable parameters +func (r ActionlogList) GetBeforeActionID() uint64 { + return r.BeforeActionID +} + // Auditable returns all auditable/loggable parameters func (r ActionlogList) GetResource() string { return r.Resource @@ -116,11 +121,6 @@ func (r ActionlogList) GetLimit() uint { return r.Limit } -// Auditable returns all auditable/loggable parameters -func (r ActionlogList) GetPageCursor() string { - return r.PageCursor -} - // Fill processes request and fills internal variables func (r *ActionlogList) Fill(req *http.Request) (err error) { if strings.ToLower(req.Header.Get("content-type")) == "application/json" { @@ -150,6 +150,12 @@ func (r *ActionlogList) Fill(req *http.Request) (err error) { return err } } + if val, ok := tmp["beforeActionID"]; ok && len(val) > 0 { + r.BeforeActionID, err = payload.ParseUint64(val[0]), nil + if err != nil { + return err + } + } if val, ok := tmp["resource"]; ok && len(val) > 0 { r.Resource, err = val[0], nil if err != nil { @@ -179,12 +185,6 @@ func (r *ActionlogList) Fill(req *http.Request) (err error) { return err } } - if val, ok := tmp["pageCursor"]; ok && len(val) > 0 { - r.PageCursor, err = val[0], nil - if err != nil { - return err - } - } } return err