From d7139358ceda5db069cd693353cb857bb218fa73 Mon Sep 17 00:00:00 2001 From: Tom Coupland Date: Fri, 29 Jun 2018 19:14:13 +0100 Subject: [PATCH] List Cursor management moved into datastore layer. (#1102) * Don't try to delete an app that wasn't successfully created in the case of failure * Allow datastore implementations to inject additional annotations on objects * Allow for datastores transparently adding annotations on apps, fns and triggers. Change NameIn filter to Name for apps. * Move *List types including JSON annotations for App, Fn and Trigger into models * Change return types for GetApps, GetFns and GetTriggers on datastore to be models.*List and ove cursor generation into datastore * Trigger cursor handling fixed into db layer Also changes the name generation so that it is not in the same order as the id (well is random), this means we are now testing our name ordering. * GetFns now respects cursors * Apps now feeds cursor back * Mock fixes * Fixing up api level cursor decoding * Tidy up treatment of cursors in the db layer * Adding conditions for non nil items lists * fix mock test --- api/datastore/datastoretest/test.go | 350 ++++++++++++------ .../internal/datastoreutil/metrics.go | 6 +- .../internal/datastoreutil/validator.go | 6 +- api/datastore/mock.go | 95 +++-- api/datastore/sql/sql.go | 100 +++-- api/models/annotations.go | 4 + api/models/app.go | 24 +- api/models/datastore.go | 10 +- api/models/fn.go | 27 ++ api/models/trigger.go | 19 + api/models/trigger_test.go | 13 + api/server/apps_list.go | 21 +- api/server/apps_test.go | 2 +- api/server/apps_v1_list.go | 8 +- api/server/fns_list.go | 12 +- api/server/fns_test.go | 17 +- api/server/server.go | 27 +- api/server/trigger_list.go | 14 +- api/server/trigger_test.go | 3 +- fnext/datastore.go | 4 +- 20 files changed, 522 insertions(+), 240 deletions(-) diff --git a/api/datastore/datastoretest/test.go b/api/datastore/datastoretest/test.go index 7dded2ae4..631f874f2 100644 --- a/api/datastore/datastoretest/test.go +++ b/api/datastore/datastoretest/test.go @@ -9,6 +9,8 @@ import ( "context" "fmt" "log" + "math/rand" + "sort" "sync/atomic" "testing" "time" @@ -50,7 +52,7 @@ type ResourceProvider interface { // BasicResourceProvider supplies simple objects and can be used as a base for custom resource providers type BasicResourceProvider struct { - idCount int32 + idCount uint32 } // DataStoreFunc provides an instance of a data store @@ -60,8 +62,8 @@ func NewBasicResourceProvider() ResourceProvider { return &BasicResourceProvider{} } -func (brp *BasicResourceProvider) NextID() int32 { - return atomic.AddInt32(&brp.idCount, 1) +func (brp *BasicResourceProvider) NextID() uint32 { + return atomic.AddUint32(&brp.idCount, rand.Uint32()) } func (brp *BasicResourceProvider) DefaultCtx() context.Context { @@ -173,6 +175,7 @@ func (h *Harness) GivenTriggerInDb(validTrigger *models.Trigger) *models.Trigger return trigger } + func (h *Harness) AppForDeletion(app *models.App) { h.appIds = append(h.appIds, app.ID) } @@ -185,6 +188,30 @@ func NewHarness(t *testing.T, ctx context.Context, ds models.Datastore) *Harness } } +type AppByName []*models.App + +func (a AppByName) Len() int { return len(a) } +func (a AppByName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a AppByName) Less(i, j int) bool { return a[i].Name < a[j].Name } + +type FnByName []*models.Fn + +func (f FnByName) Len() int { return len(f) } +func (f FnByName) Swap(i, j int) { f[i], f[j] = f[j], f[i] } +func (f FnByName) Less(i, j int) bool { return f[i].Name < f[j].Name } + +type TriggerByName []*models.Trigger + +func (f TriggerByName) Len() int { return len(f) } +func (f TriggerByName) Swap(i, j int) { f[i], f[j] = f[j], f[i] } +func (f TriggerByName) Less(i, j int) bool { return f[i].Name < f[j].Name } + +type RouteByPath []*models.Route + +func (f RouteByPath) Len() int { return len(f) } +func (f RouteByPath) Swap(i, j int) { f[i], f[j] = f[j], f[i] } +func (f RouteByPath) Less(i, j int) bool { return f[i].Path < f[j].Path } + func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { ds := dsf(t) @@ -206,11 +233,10 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { start := time.Now() returnedApp, err := ds.InsertApp(ctx, rp.ValidApp()) - - h.AppForDeletion(returnedApp) if err != nil { t.Fatalf("Expected succcess, got %s", err) } + h.AppForDeletion(returnedApp) if !time.Time(returnedApp.CreatedAt).After(start) { t.Fatalf("expected created to be set %s", returnedApp.CreatedAt) @@ -274,7 +300,7 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("error when updating app: %v", err) } expected := &models.App{ID: testApp.ID, Name: testApp.Name, Config: map[string]string{"TEST": "1"}} - if !updated.Equals(expected) { + if !expected.EqualsWithAnnotationSubset(updated) { t.Fatalf("expected updated `%v` but got `%v`", expected, updated) } }) @@ -297,7 +323,7 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("error when updating app: %v", err) } expected := &models.App{Name: testApp.Name, ID: testApp.ID, Config: map[string]string{"TEST": "1", "OTHER": "TEST"}} - if !updated.Equals(expected) { + if !expected.EqualsWithAnnotationSubset(updated) { t.Fatalf("expected updated `%v` but got `%v`", expected, updated) } }) @@ -345,7 +371,7 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("error when updating app: %v", err) } expected := &models.App{Name: testApp.Name, ID: testApp.ID, Config: map[string]string{"OTHER": "TEST"}} - if !updated.Equals(expected) { + if !expected.EqualsWithAnnotationSubset(updated) { t.Fatalf("expected updated `%#v` but got `%#v`", expected, updated) } }) @@ -376,22 +402,43 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Run("List apps", func(t *testing.T) { h := NewHarness(t, ctx, ds) defer h.Cleanup() - testApp := h.GivenAppInDb(rp.ValidApp()) + + apps, err := ds.GetApps(ctx, &models.AppFilter{PerPage: 100}) + if err != nil { + t.Fatalf("not expecting err %s", err) + } + + if len(apps.Items) != 0 { + t.Fatalf("expecting 0 results, got %d", len(apps.Items)) + } + if apps.Items == nil { + t.Fatalf("response items must not be nil") + } + + a1 := h.GivenAppInDb(rp.ValidApp()) + h.GivenAppInDb(rp.ValidApp()) // Testing list apps - apps, err := ds.GetApps(ctx, &models.AppFilter{PerPage: 100}) + apps, err = ds.GetApps(ctx, &models.AppFilter{PerPage: 100}) if err != nil { t.Fatalf("unexpected error %v", err) } - if len(apps) == 0 { - t.Fatal("expected result count to be greater than 0") + if len(apps.Items) != 2 { + t.Fatalf("expected result count to be 2, got %d", len(apps.Items)) } - for _, app := range apps { - if app.Name == testApp.Name { + apps, err = ds.GetApps(ctx, &models.AppFilter{PerPage: 100, Name: a1.Name}) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + if len(apps.Items) != 1 { + t.Fatalf("expected result count to be 1, got %d", len(apps.Items)) + } + for _, app := range apps.Items { + if app.Name == a1.Name { return } } - t.Fatalf("expected app list to contain app %s, got %#v", testApp.Name, apps) + t.Fatalf("expected app list to contain app %s, got %#v", a1.Name, apps) }) t.Run("Simple Pagination", func(t *testing.T) { @@ -402,38 +449,43 @@ func RunAppsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { a2 := h.GivenAppInDb(rp.ValidApp()) a3 := h.GivenAppInDb(rp.ValidApp()) + gendApps := []*models.App{a1, a2, a3} + sort.Sort(AppByName(gendApps)) + apps, err := ds.GetApps(ctx, &models.AppFilter{PerPage: 1}) if err != nil { t.Fatalf(" error: %s", err) } - if len(apps) != 1 { - t.Fatalf(" expected result count to be 1 but got %d", len(apps)) - } else if apps[0].Name != a1.Name { - t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", a1.Name, apps[0].Name) + if len(apps.Items) != 1 { + t.Fatalf(" expected result count to be 1 but got %d", len(apps.Items)) + } else if apps.Items[0].Name != gendApps[0].Name { + t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", gendApps[0].Name, apps.Items[0].Name) } - apps, err = ds.GetApps(ctx, &models.AppFilter{PerPage: 100, Cursor: apps[0].Name}) + apps, err = ds.GetApps(ctx, &models.AppFilter{PerPage: 100, Cursor: apps.NextCursor}) if err != nil { t.Fatalf(" error: %s", err) } - if len(apps) != 2 { - t.Fatalf(" expected result count to be 2 but got %d", len(apps)) - } else if apps[0].Name != a2.Name { - t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", a2.Name, apps[0].Name) - } else if apps[1].Name != a3.Name { - t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", a3.Name, apps[1].Name) + if len(apps.Items) != 2 { + t.Fatalf(" expected result count to be 2 but got %d", len(apps.Items)) + } else if apps.Items[0].Name != gendApps[1].Name { + t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", gendApps[1].Name, apps.Items[0].Name) + } else if apps.Items[1].Name != gendApps[2].Name { + t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", gendApps[2].Name, apps.Items[1].Name) } a4 := h.GivenAppInDb(rp.ValidApp()) + gendApps = append(gendApps, a4) + sort.Sort(AppByName(gendApps)) apps, err = ds.GetApps(ctx, &models.AppFilter{PerPage: 100}) if err != nil { t.Fatalf(" error: %s", err) } - if len(apps) != 4 { - t.Fatalf(" expected result count to be 4 but got %d", len(apps)) - } else if apps[3].Name != a4.Name { - t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", a4.Name, apps[0].Name) + if len(apps.Items) != 4 { + t.Fatalf(" expected result count to be 4 but got %d", len(apps.Items)) + } else if apps.Items[3].Name != gendApps[3].Name { + t.Fatalf(" expected `app.Name` to be `%s` but it was `%s`", gendApps[4].Name, apps.Items[0].Name) } }) @@ -714,7 +766,7 @@ func RunRoutesTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { } }) - t.Run("pagination on routes return rotues in order ", func(t *testing.T) { + t.Run("pagination on routes return routes in order ", func(t *testing.T) { h := NewHarness(t, ctx, ds) defer h.Cleanup() testApp := h.GivenAppInDb(rp.ValidApp()) @@ -723,14 +775,17 @@ func RunRoutesTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { r2 := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) r3 := h.GivenRouteInDb(rp.ValidRoute(testApp.ID)) + gendRoutes := []*models.Route{r1, r2, r3} + sort.Sort(RouteByPath(gendRoutes)) + routes, err := ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 1}) if err != nil { t.Fatalf("error: %s", err) } if len(routes) != 1 { t.Fatalf("expected result count to be 1 but got %d", len(routes)) - } else if routes[0].Path != r1.Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", r1.Path, routes[0].Path) + } else if routes[0].Path != gendRoutes[0].Path { + t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[0].Path, routes[0].Path) } routes, err = ds.GetRoutesByApp(ctx, testApp.ID, &models.RouteFilter{PerPage: 2, Cursor: routes[0].Path}) @@ -740,10 +795,10 @@ func RunRoutesTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { if len(routes) != 2 { t.Fatalf("expected result count to be 2 but got %d", len(routes)) - } else if routes[0].Path != r2.Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", r2.Path, routes[0].Path) - } else if routes[1].Path != r3.Path { - t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", r3.Path, routes[1].Path) + } else if routes[0].Path != gendRoutes[1].Path { + t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[1].Path, routes[0].Path) + } else if routes[1].Path != gendRoutes[2].Path { + t.Fatalf("expected `route.Path` to be `%s` but it was `%s`", gendRoutes[2].Path, routes[1].Path) } r4 := rp.ValidRoute(testApp.ID) @@ -882,7 +937,7 @@ func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { if err != nil { t.Fatalf("unexpected error %v : %s", err, testFn.ID) } - if !fn.Equals(testFn) { + if !testFn.EqualsWithAnnotationSubset(fn) { t.Fatalf("expected to get the right func:\n%v\nbut got:\n%v", testFn, fn) } }) @@ -927,7 +982,7 @@ func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { "THIRD": "3", }, } - if !updated.Equals(expected) { + if !expected.EqualsWithAnnotationSubset(updated) { t.Fatalf("expected updated `%#v` but got `%#v`", expected, updated) } @@ -979,7 +1034,7 @@ func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { "THIRD": "3", }, } - if !updated.Equals(expected) { + if !expected.EqualsWithAnnotationSubset(updated) { t.Fatalf("expected updated:\n`%v`\nbut got:\n`%v`", expected, updated) } }) @@ -993,9 +1048,12 @@ func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { if err != nil { t.Fatalf("unexpected error %v", err) } - if len(fns) != 0 { + if len(fns.Items) != 0 { t.Fatal("expected result count to be 0") } + if fns.Items == nil { + t.Fatal("response items must not be nil") + } }) t.Run("basic pagination with funcs", func(t *testing.T) { @@ -1006,36 +1064,39 @@ func RunFnsTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { f2 := h.GivenFnInDb(rp.ValidFn(testApp.ID)) f3 := h.GivenFnInDb(rp.ValidFn(testApp.ID)) + gendFns := []*models.Fn{f1, f2, f3} + sort.Sort(FnByName(gendFns)) + // Testing list fns fns, err := ds.GetFns(ctx, &models.FnFilter{AppID: testApp.ID}) if err != nil { t.Fatalf("unexpected error %v", err) } - if len(fns) != 3 { - t.Fatalf("expected result count to be 3, but was %d", len(fns)) + if len(fns.Items) != 3 { + t.Fatalf("expected result count to be 3, but was %d", len(fns.Items)) } fns, err = ds.GetFns(ctx, &models.FnFilter{AppID: testApp.ID, PerPage: 1}) if err != nil { t.Fatalf("unexpected error %v", err) } - if len(fns) != 1 { - t.Fatalf("expected result count to be 1, but was %d", len(fns)) + if len(fns.Items) != 1 { + t.Fatalf("expected result count to be 1, but was %d", len(fns.Items)) } - if !f1.Equals(fns[0]) { - t.Fatalf("Expecting function to be %#v but was %#v", f1, fns[0]) + if !gendFns[0].EqualsWithAnnotationSubset(fns.Items[0]) { + t.Fatalf("Expecting function to be %#v but was %#v", gendFns[0], fns.Items[0]) } - fns, err = ds.GetFns(ctx, &models.FnFilter{AppID: testApp.ID, PerPage: 2, Cursor: fns[0].Name}) + fns, err = ds.GetFns(ctx, &models.FnFilter{AppID: testApp.ID, PerPage: 2, Cursor: fns.NextCursor}) if err != nil { t.Fatalf("error: %s", err) } - if len(fns) != 2 { - t.Fatalf("expected result count to be 2 but got %d", len(fns)) - } else if !fns[0].Equals(f2) { - t.Fatalf("expected `func.Name` to be `%#v` but it was `%#v`", f2, fns[0]) - } else if !fns[1].Equals(f3) { - t.Fatalf("expected `func.Name` to be `%#v` but it was `%#v`", f3, fns[1]) + if len(fns.Items) != 2 { + t.Fatalf("expected result count to be 2 but got %d", len(fns.Items)) + } else if !gendFns[1].EqualsWithAnnotationSubset(fns.Items[0]) { + t.Fatalf("expected `func.Name` to be `%#v` but it was `%#v`", gendFns[1].Name, fns.Items[0].Name) + } else if !gendFns[2].EqualsWithAnnotationSubset(fns.Items[1]) { + t.Fatalf("expected `func.Name` to be `%#v` but it was `%#v`", gendFns[2], fns.Items[1]) } }) @@ -1105,7 +1166,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("error when storing new trigger: %s", err) } newTrigger.ID = insertedTrigger.ID - if !insertedTrigger.Equals(newTrigger) { + if !newTrigger.EqualsWithAnnotationSubset(insertedTrigger) { t.Errorf("Expecting returned trigger %#v to equal %#v", insertedTrigger, newTrigger) } @@ -1131,7 +1192,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("No ID ") } newTrigger.ID = insertedTrigger.ID - if !insertedTrigger.Equals(newTrigger) { + if !newTrigger.EqualsWithAnnotationSubset(insertedTrigger) { t.Errorf("Expecting returned trigger %#v to equal %#v", insertedTrigger, newTrigger) } }) @@ -1157,7 +1218,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { } newTrigger.ID = insertedTrigger.ID - if !gotTrigger.Equals(newTrigger) { + if !newTrigger.EqualsWithAnnotationSubset(gotTrigger) { t.Errorf("Expecting returned trigger %#v to equal %#v", gotTrigger, newTrigger) } }) @@ -1173,8 +1234,8 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Run("non-existant app", func(t *testing.T) { nonMatchingFilter := &models.TriggerFilter{AppID: "notexist"} triggers, err := ds.GetTriggers(ctx, nonMatchingFilter) - if len(triggers) != 0 && err == nil { - t.Fatalf("expected empty trigger list and no error, but got list [%v] and err %s", triggers, err) + if len(triggers.Items) != 0 && err == nil { + t.Fatalf("expected empty trigger list and no error, but got list [%v] and err %s", triggers.Items, err) } }) @@ -1193,6 +1254,116 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { } }) + t.Run("page triggers", func(t *testing.T) { + h := NewHarness(t, ctx, ds) + defer h.Cleanup() + testApp := h.GivenAppInDb(rp.ValidApp()) + testFn := h.GivenFnInDb(rp.ValidFn(testApp.ID)) + + var storedTriggers []*models.Trigger + + for i := 0; i < 10; i++ { + trigger := rp.ValidTrigger(testApp.ID, testFn.ID) + trigger.Source = fmt.Sprintf("src_%v", i) + storedTriggers = append(storedTriggers, h.GivenTriggerInDb(trigger)) + } + + sort.Sort(TriggerByName(storedTriggers)) + + appIDFilter := &models.TriggerFilter{AppID: testApp.ID} + triggers, err := ds.GetTriggers(ctx, appIDFilter) + if err != nil { + t.Fatalf("Test GetTriggers(page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 10 { + t.Fatalf("Test GetTriggers(page triggers), expecting 10 results, got %d", len(triggers.Items)) + } + + for i := 1; i < 10; i++ { + if triggers.Items[i-1].Name > triggers.Items[i].Name { + t.Fatalf("Test GetTriggers(page triggers), names out of order, %s, %s", triggers.Items[i-1], triggers.Items[i]) + } + } + + fiveFilter := &models.TriggerFilter{AppID: testApp.ID, PerPage: 5} + triggers, err = ds.GetTriggers(ctx, fiveFilter) + if err != nil { + t.Fatalf("Test GetTriggers(page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 5 { + t.Fatalf("Test GetTriggers(page triggers), expecting 5 results, got %d", len(triggers.Items)) + } + + for i := 0; i < 5; i++ { + if !triggers.Items[i].EqualsWithAnnotationSubset(storedTriggers[i]) { + t.Fatalf("Test GetTriggers(first five page triggers), expect equal, %s, %s", triggers.Items[i], storedTriggers[i]) + } + } + + if triggers.NextCursor == "" { + t.Fatalf("Test GetTriggers(first five page triggers), expected Cursor but got nothing") + } + + secondFiveFilter := &models.TriggerFilter{AppID: testApp.ID, PerPage: 5, Cursor: triggers.NextCursor} + triggers, err = ds.GetTriggers(ctx, secondFiveFilter) + if err != nil { + t.Fatalf("Test GetTriggers(second five page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 5 { + t.Fatalf("Test GetTriggers(second five page triggers), expecting 5 results, got %d", len(triggers.Items)) + } + + for i := 0; i < 5; i++ { + if !triggers.Items[i].EqualsWithAnnotationSubset(storedTriggers[i+5]) { + t.Fatalf("Test GetTriggers(second five page triggers), expect equal, %s, %s", triggers.Items[i], storedTriggers[i+5]) + } + } + + zeroFilter := &models.TriggerFilter{AppID: testApp.ID, PerPage: 0} + triggers, err = ds.GetTriggers(ctx, zeroFilter) + if err != nil { + t.Fatalf("Test GetTriggers(zero page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 10 { + t.Fatalf("Test GetTriggers(zero page triggers), expecting 10 results, got %d", len(triggers.Items)) + } + + if triggers.NextCursor != "" { + t.Fatalf("Test GetTriggers(zero page triggers), expected no NextCursor, got %s", triggers.NextCursor) + } + + negativeFilter := &models.TriggerFilter{AppID: testApp.ID, PerPage: -10} + triggers, err = ds.GetTriggers(ctx, negativeFilter) + if err != nil { + t.Fatalf("Test GetTriggers(negative page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 10 { + t.Fatalf("Test GetTriggers(negative page triggers), expecting 10 results, got %d", len(triggers.Items)) + } + + if triggers.NextCursor != "" { + t.Fatalf("Test GetTriggers(negative page triggers), expected no NextCursor, got %s", triggers.NextCursor) + } + + emptyListFilter := &models.TriggerFilter{AppID: "notexist"} + triggers, err = ds.GetTriggers(ctx, emptyListFilter) + if err != nil { + t.Fatalf("Test GetTriggers(notexist page triggers), not expecting err %s", err) + } + + if len(triggers.Items) != 0 { + t.Fatalf("Test GetTriggers(notexist page triggers), expecting 0 results, got %d", len(triggers.Items)) + } + if triggers.Items == nil { + t.Fatalf("Test GetTriggers(notexist page triggers), response items must not be nil") + } + }) + t.Run("filter triggers", func(t *testing.T) { h := NewHarness(t, ctx, ds) defer h.Cleanup() @@ -1210,7 +1381,10 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { trigger := rp.ValidTrigger(testApp.ID, testFn2.ID) trigger.Source = fmt.Sprintf("src_%v", 11) - h.GivenTriggerInDb(trigger) + trigger = h.GivenTriggerInDb(trigger) + storedTriggers = append(storedTriggers, trigger) + + sort.Sort(TriggerByName(storedTriggers)) appIDFilter := &models.TriggerFilter{AppID: testApp.ID} triggers, err := ds.GetTriggers(ctx, appIDFilter) @@ -1218,14 +1392,13 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("Test GetTriggers(get all triggers for app), not expecting err %s", err) } - if len(triggers) != 11 { - t.Fatalf("Test GetTriggers(get all triggers for app), expecting 10 results, got %d", len(triggers)) + if len(triggers.Items) != 11 { + t.Fatalf("Test GetTriggers(get all triggers for app), expecting 10 results, got %d", len(triggers.Items)) } - for i := 1; i < 10; i++ { - if !storedTriggers[i].Equals(triggers[i]) { - t.Fatalf("expecting ordered by names, but aren't: %s, %s", storedTriggers[i].Name, triggers[i].Name) - + for i := 0; i < 11; i++ { + if !storedTriggers[i].EqualsWithAnnotationSubset(triggers.Items[i]) { + t.Fatalf("Test GetTriggers(get all triggers for app), expecting ordered by names, but aren't: %+v, %+v", storedTriggers[i], triggers.Items[i]) } } @@ -1235,41 +1408,12 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("Test GetTriggers(filter by name), not expecting err %s", err) } - if len(triggers) != 1 { - t.Fatalf("Test GetTriggers(filter by name), expecting 1 results, got %d", len(triggers)) + if len(triggers.Items) != 1 { + t.Fatalf("Test GetTriggers(filter by name), expecting 1 results, got %d", len(triggers.Items)) } - if !triggers[0].Equals(storedTriggers[0]) { - t.Fatalf("expect single result to equal first stored result : %#v != %#v", triggers[4], storedTriggers[4]) - } - - appIDPagedFilter := &models.TriggerFilter{AppID: testApp.ID, PerPage: 5} - triggers, err = ds.GetTriggers(ctx, appIDPagedFilter) - if err != nil { - t.Fatalf("Test GetTriggers(page triggers for app), not expecting err %s", err) - } - - if len(triggers) != 5 { - t.Fatalf("Test GetTriggers(get all triggers for app), expecting 5 results, got %d", len(triggers)) - } - - if !triggers[4].Equals(storedTriggers[4]) { - t.Fatalf("expect 5th result to equal 5th stored result : %#v != %#v", triggers[4], storedTriggers[4]) - } - - appIDPagedFilter.Cursor = triggers[4].ID - triggers, err = ds.GetTriggers(ctx, appIDPagedFilter) - - if err != nil { - t.Fatalf("Test GetTriggers(page triggers for app), not expecting err %s", err) - } - - if len(triggers) != 5 { - t.Fatalf("Test GetTriggers(get all triggers for app), expecting 5 results, got %d", len(triggers)) - } - - if !triggers[4].Equals(storedTriggers[9]) { - t.Fatalf("expect 5th result to equal 9th stored result : %#v != %#v", triggers[4], storedTriggers[9]) + if !storedTriggers[0].EqualsWithAnnotationSubset(triggers.Items[0]) { + t.Fatalf("expect single result to equal first stored result : %#v != %#v", triggers.Items[4], storedTriggers[4]) } // components are AND'd @@ -1278,8 +1422,8 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { if err != nil { t.Fatalf("Test GetTriggers(AND filtering), not expecting err %s", err) } - if len(triggers) != 10 { - t.Fatalf("Test GetTriggers(AND filtering), expecting 10 results, got %d", len(triggers)) + if len(triggers.Items) != 10 { + t.Fatalf("Test GetTriggers(AND filtering), expecting 10 results, got %d", len(triggers.Items)) } }) @@ -1300,7 +1444,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { t.Fatalf("error when updating trigger: %s", err) } - if !gotTrigger.Equals(testTrigger) { + if !testTrigger.EqualsWithAnnotationSubset(gotTrigger) { t.Fatalf("expecting returned triggers equal, got : %#v : %#v", testTrigger, gotTrigger) } @@ -1308,7 +1452,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) { if err != nil { t.Fatalf("wasn't expecting an error : %s", err) } - if !gotTrigger.Equals(testTrigger) { + if !testTrigger.EqualsWithAnnotationSubset(gotTrigger) { t.Fatalf("expecting fetch trigger to be updated got : %v : %v", testTrigger, gotTrigger) } diff --git a/api/datastore/internal/datastoreutil/metrics.go b/api/datastore/internal/datastoreutil/metrics.go index 433046ead..f261cb3e8 100644 --- a/api/datastore/internal/datastoreutil/metrics.go +++ b/api/datastore/internal/datastoreutil/metrics.go @@ -28,7 +28,7 @@ func (m *metricds) GetAppByID(ctx context.Context, appID string) (*models.App, e return m.ds.GetAppByID(ctx, appID) } -func (m *metricds) GetApps(ctx context.Context, filter *models.AppFilter) ([]*models.App, error) { +func (m *metricds) GetApps(ctx context.Context, filter *models.AppFilter) (*models.AppList, error) { ctx, span := trace.StartSpan(ctx, "ds_get_apps") defer span.End() return m.ds.GetApps(ctx, filter) @@ -107,7 +107,7 @@ func (m *metricds) GetTriggerByID(ctx context.Context, triggerID string) (*model return m.ds.GetTriggerByID(ctx, triggerID) } -func (m *metricds) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([]*models.Trigger, error) { +func (m *metricds) GetTriggers(ctx context.Context, filter *models.TriggerFilter) (*models.TriggerList, error) { ctx, span := trace.StartSpan(ctx, "ds_get_triggers") defer span.End() return m.ds.GetTriggers(ctx, filter) @@ -125,7 +125,7 @@ func (m *metricds) UpdateFn(ctx context.Context, fn *models.Fn) (*models.Fn, err return m.ds.UpdateFn(ctx, fn) } -func (m *metricds) GetFns(ctx context.Context, filter *models.FnFilter) ([]*models.Fn, error) { +func (m *metricds) GetFns(ctx context.Context, filter *models.FnFilter) (*models.FnList, error) { ctx, span := trace.StartSpan(ctx, "ds_get_funcs") defer span.End() return m.ds.GetFns(ctx, filter) diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go index 2cf6435f2..08ab05b4a 100644 --- a/api/datastore/internal/datastoreutil/validator.go +++ b/api/datastore/internal/datastoreutil/validator.go @@ -31,7 +31,7 @@ func (v *validator) GetAppByID(ctx context.Context, appID string) (*models.App, return v.Datastore.GetAppByID(ctx, appID) } -func (v *validator) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { +func (v *validator) GetApps(ctx context.Context, appFilter *models.AppFilter) (*models.AppList, error) { return v.Datastore.GetApps(ctx, appFilter) } @@ -151,7 +151,7 @@ func (v *validator) UpdateTrigger(ctx context.Context, trigger *models.Trigger) return v.Datastore.UpdateTrigger(ctx, trigger) } -func (v *validator) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([]*models.Trigger, error) { +func (v *validator) GetTriggers(ctx context.Context, filter *models.TriggerFilter) (*models.TriggerList, error) { if filter.AppID == "" { return nil, models.ErrTriggerMissingAppID @@ -195,7 +195,7 @@ func (v *validator) GetFnByID(ctx context.Context, fnID string) (*models.Fn, err return v.Datastore.GetFnByID(ctx, fnID) } -func (v *validator) GetFns(ctx context.Context, filter *models.FnFilter) ([]*models.Fn, error) { +func (v *validator) GetFns(ctx context.Context, filter *models.FnFilter) (*models.FnList, error) { if filter.AppID == "" { return nil, models.ErrFnsMissingAppID diff --git a/api/datastore/mock.go b/api/datastore/mock.go index 17b95bfd7..cf482bee9 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -2,9 +2,9 @@ package datastore import ( "context" + "encoding/base64" "sort" "strings" - "time" "github.com/fnproject/fn/api/common" @@ -12,6 +12,7 @@ import ( "github.com/fnproject/fn/api/id" "github.com/fnproject/fn/api/logs" "github.com/fnproject/fn/api/models" + "github.com/sirupsen/logrus" ) type mock struct { @@ -75,33 +76,43 @@ func (s sortA) Len() int { return len(s) } func (s sortA) Less(i, j int) bool { return strings.Compare(s[i].Name, s[j].Name) < 0 } func (s sortA) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (m *mock) GetApps(ctx context.Context, appFilter *models.AppFilter) ([]*models.App, error) { +func (m *mock) GetApps(ctx context.Context, filter *models.AppFilter) (*models.AppList, error) { // sort them all first for cursoring (this is for testing, n is small & mock is not concurrent..) sort.Sort(sortA(m.Apps)) - var apps []*models.App + var cursor string + if filter.Cursor != "" { + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return nil, err + } + logrus.Error(s) + cursor = string(s) + } + + apps := []*models.App{} for _, a := range m.Apps { - if len(apps) == appFilter.PerPage { + if len(apps) == filter.PerPage { break } - if len(appFilter.NameIn) > 0 { - var found bool - for _, fn := range appFilter.NameIn { - if fn == a.Name { - found = true - break - } - } - if !found { + if strings.Compare(cursor, a.Name) < 0 { + if filter.Name != "" && filter.Name != a.Name { continue } - } - if strings.Compare(appFilter.Cursor, a.Name) < 0 { apps = append(apps, a.Clone()) } } - return apps, nil + var nextCursor string + if len(apps) > 0 && len(apps) == filter.PerPage { + last := []byte(apps[len(apps)-1].Name) + nextCursor = base64.RawURLEncoding.EncodeToString(last) + } + + return &models.AppList{ + NextCursor: nextCursor, + Items: apps, + }, nil } func (m *mock) InsertApp(ctx context.Context, newApp *models.App) (*models.App, error) { @@ -315,25 +326,44 @@ func (s sortF) Len() int { return len(s) } func (s sortF) Less(i, j int) bool { return strings.Compare(s[i].Name, s[j].Name) < 0 } func (s sortF) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (m *mock) GetFns(ctx context.Context, filter *models.FnFilter) ([]*models.Fn, error) { +func (m *mock) GetFns(ctx context.Context, filter *models.FnFilter) (*models.FnList, error) { // sort them all first for cursoring (this is for testing, n is small & mock is not concurrent..) sort.Sort(sortF(m.Fns)) funcs := []*models.Fn{} + var cursor string + if filter.Cursor != "" { + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return nil, err + } + cursor = string(s) + } + for _, f := range m.Fns { if filter.PerPage > 0 && len(funcs) == filter.PerPage { break } - if strings.Compare(filter.Cursor, f.Name) < 0 && + if strings.Compare(cursor, f.Name) < 0 && (filter.AppID == "" || filter.AppID == f.AppID) && (filter.Name == "" || filter.Name == f.Name) { funcs = append(funcs, f) } } - return funcs, nil + + var nextCursor string + if len(funcs) > 0 && len(funcs) == filter.PerPage { + last := []byte(funcs[len(funcs)-1].Name) + nextCursor = base64.RawURLEncoding.EncodeToString(last) + } + + return &models.FnList{ + NextCursor: nextCursor, + Items: funcs, + }, nil } func (m *mock) GetFnByID(ctx context.Context, fnID string) (*models.Fn, error) { @@ -438,12 +468,21 @@ func (m *mock) GetTriggerByID(ctx context.Context, triggerId string) (*models.Tr type sortT []*models.Trigger func (s sortT) Len() int { return len(s) } -func (s sortT) Less(i, j int) bool { return strings.Compare(s[i].ID, s[j].ID) < 0 } +func (s sortT) Less(i, j int) bool { return strings.Compare(s[i].Name, s[j].Name) < 0 } func (s sortT) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (m *mock) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([]*models.Trigger, error) { +func (m *mock) GetTriggers(ctx context.Context, filter *models.TriggerFilter) (*models.TriggerList, error) { sort.Sort(sortT(m.Triggers)) + var cursor string + if filter.Cursor != "" { + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return nil, err + } + cursor = string(s) + } + res := []*models.Trigger{} for _, t := range m.Triggers { if filter.PerPage > 0 && len(res) == filter.PerPage { @@ -451,7 +490,7 @@ func (m *mock) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([ } matched := true - if filter.Cursor != "" && t.ID <= filter.Cursor { + if filter.Cursor != "" && t.Name <= cursor { matched = false } @@ -470,7 +509,17 @@ func (m *mock) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([ res = append(res, t) } } - return res, nil + + var nextCursor string + if len(res) > 0 && len(res) == filter.PerPage { + last := []byte(res[len(res)-1].Name) + nextCursor = base64.RawURLEncoding.EncodeToString(last) + } + + return &models.TriggerList{ + NextCursor: nextCursor, + Items: res, + }, nil } func (m *mock) RemoveTrigger(ctx context.Context, triggerID string) error { diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index f597bb093..dc476c2d8 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "database/sql" + "encoding/base64" "fmt" "io" "net/url" @@ -508,12 +509,8 @@ func (ds *SQLStore) GetAppByID(ctx context.Context, appID string) (*models.App, } // GetApps retrieves an array of apps according to a specific filter. -func (ds *SQLStore) GetApps(ctx context.Context, filter *models.AppFilter) ([]*models.App, error) { - res := []*models.App{} // for JSON empty list - - if filter.NameIn != nil && len(filter.NameIn) == 0 { // this basically makes sure it doesn't return ALL apps - return res, nil - } +func (ds *SQLStore) GetApps(ctx context.Context, filter *models.AppFilter) (*models.AppList, error) { + res := &models.AppList{Items: []*models.App{}} query, args, err := buildFilterAppQuery(filter) if err != nil { @@ -535,7 +532,12 @@ func (ds *SQLStore) GetApps(ctx context.Context, filter *models.AppFilter) ([]*m } return res, err } - res = append(res, &app) + res.Items = append(res.Items, &app) + } + + if len(res.Items) > 0 && len(res.Items) == filter.PerPage { + last := []byte(res.Items[len(res.Items)-1].Name) + res.NextCursor = base64.RawURLEncoding.EncodeToString(last) } if err := rows.Err(); err != nil { @@ -844,13 +846,16 @@ func (ds *SQLStore) UpdateFn(ctx context.Context, fn *models.Fn) (*models.Fn, er return fn, nil } -func (ds *SQLStore) GetFns(ctx context.Context, filter *models.FnFilter) ([]*models.Fn, error) { - res := []*models.Fn{} // for JSON empty list +func (ds *SQLStore) GetFns(ctx context.Context, filter *models.FnFilter) (*models.FnList, error) { + res := &models.FnList{Items: []*models.Fn{}} if filter == nil { filter = new(models.FnFilter) } - filterQuery, args := buildFilterFnQuery(filter) + filterQuery, args, err := buildFilterFnQuery(filter) + if err != nil { + return res, err + } query := fmt.Sprintf("%s %s", fnSelector, filterQuery) query = ds.db.Rebind(query) @@ -869,14 +874,19 @@ func (ds *SQLStore) GetFns(ctx context.Context, filter *models.FnFilter) ([]*mod if err != nil { continue } - res = append(res, &fn) + res.Items = append(res.Items, &fn) } + + if len(res.Items) > 0 && len(res.Items) == filter.PerPage { + last := []byte(res.Items[len(res.Items)-1].Name) + res.NextCursor = base64.RawURLEncoding.EncodeToString(last) + } + if err := rows.Err(); err != nil { if err == sql.ErrNoRows { return res, nil // no error for empty list } } - return res, nil } @@ -1066,16 +1076,20 @@ func buildFilterAppQuery(filter *models.AppFilter) (string, []interface{}, error var b bytes.Buffer - // where("name LIKE ?%", filter.Name) // TODO needs escaping? - args = where(&b, args, "name>?", filter.Cursor) - args = where(&b, args, "name IN (?)", filter.NameIn) + if filter.Cursor != "" { + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return "", args, err + } + args = where(&b, args, "name>?", string(s)) + } + if filter.Name != "" { + args = where(&b, args, "name=?", filter.Name) + } fmt.Fprintf(&b, ` ORDER BY name ASC`) // TODO assert this is indexed fmt.Fprintf(&b, ` LIMIT ?`) args = append(args, filter.PerPage) - if len(filter.NameIn) > 0 { - return sqlx.In(b.String(), args...) - } return b.String(), args, nil } @@ -1103,23 +1117,30 @@ func buildFilterCallQuery(filter *models.CallFilter) (string, []interface{}) { return b.String(), args } -func buildFilterFnQuery(filter *models.FnFilter) (string, []interface{}) { +func buildFilterFnQuery(filter *models.FnFilter) (string, []interface{}, error) { if filter == nil { - return "", nil + return "", nil, nil } var b bytes.Buffer var args []interface{} // where(fmt.Sprintf("image LIKE '%s%%'"), filter.Image) // TODO needs escaping, prob we want prefix query to ignore tags args = where(&b, args, "app_id=? ", filter.AppID) - args = where(&b, args, "name>?", filter.Cursor) + + if filter.Cursor != "" { + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return "", args, err + } + args = where(&b, args, "name>?", string(s)) + } fmt.Fprintf(&b, ` ORDER BY name ASC`) if filter.PerPage > 0 { fmt.Fprintf(&b, ` LIMIT ?`) args = append(args, filter.PerPage) } - return b.String(), args + return b.String(), args, nil } func where(b *bytes.Buffer, args []interface{}, colOp string, val interface{}) []interface{} { @@ -1305,7 +1326,7 @@ func (ds *SQLStore) GetTriggerByID(ctx context.Context, triggerID string) (*mode return &trigger, nil } -func buildFilterTriggerQuery(filter *models.TriggerFilter) (string, []interface{}) { +func buildFilterTriggerQuery(filter *models.TriggerFilter) (string, []interface{}, error) { var b bytes.Buffer var args []interface{} @@ -1323,29 +1344,35 @@ func buildFilterTriggerQuery(filter *models.TriggerFilter) (string, []interface{ } if filter.Cursor != "" { - fmt.Fprintf(&b, ` AND id > ?`) - args = append(args, filter.Cursor) + s, err := base64.RawURLEncoding.DecodeString(filter.Cursor) + if err != nil { + return "", nil, err + } + + fmt.Fprintf(&b, ` AND name > ?`) + args = append(args, string(s)) } fmt.Fprintf(&b, ` ORDER BY name ASC`) - if filter.PerPage != 0 { + if filter.PerPage > 0 { fmt.Fprintf(&b, ` LIMIT ?`) args = append(args, filter.PerPage) } - return b.String(), args + return b.String(), args, nil } -func (ds *SQLStore) GetTriggers(ctx context.Context, filter *models.TriggerFilter) ([]*models.Trigger, error) { - res := []*models.Trigger{} // for JSON empty list +func (ds *SQLStore) GetTriggers(ctx context.Context, filter *models.TriggerFilter) (*models.TriggerList, error) { + res := &models.TriggerList{Items: []*models.Trigger{}} if filter == nil { filter = new(models.TriggerFilter) } - filterQuery, args := buildFilterTriggerQuery(filter) - - logrus.Error(filterQuery, args) + filterQuery, args, err := buildFilterTriggerQuery(filter) + if err != nil { + return res, err + } query := fmt.Sprintf("%s WHERE %s", triggerSelector, filterQuery) query = ds.db.Rebind(query) @@ -1364,14 +1391,19 @@ func (ds *SQLStore) GetTriggers(ctx context.Context, filter *models.TriggerFilte if err != nil { continue } - res = append(res, &trigger) + res.Items = append(res.Items, &trigger) } + + if len(res.Items) > 0 && len(res.Items) == filter.PerPage { + last := []byte(res.Items[len(res.Items)-1].Name) + res.NextCursor = base64.RawURLEncoding.EncodeToString(last) + } + if err := rows.Err(); err != nil { if err == sql.ErrNoRows { return res, nil // no error for empty list } } - return res, nil } diff --git a/api/models/annotations.go b/api/models/annotations.go index b9a0abf57..c1307e46e 100644 --- a/api/models/annotations.go +++ b/api/models/annotations.go @@ -32,6 +32,10 @@ func (m Annotations) Equals(other Annotations) bool { if len(m) != len(other) { return false } + return m.Subset(other) +} + +func (m Annotations) Subset(other Annotations) bool { for k1, v1 := range m { v2, _ := other[k1] if v2 == nil { diff --git a/api/models/app.go b/api/models/app.go index c0a9c2a5e..c994b43b6 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -135,6 +135,22 @@ func (a1 *App) Equals(a2 *App) bool { return eq } +func (a1 *App) EqualsWithAnnotationSubset(a2 *App) bool { + // start off equal, check equivalence of each field. + // the RHS of && won't eval if eq==false so config checking is lazy + + eq := true + eq = eq && a1.ID == a2.ID + eq = eq && a1.Name == a2.Name + eq = eq && a1.Config.Equals(a2.Config) + eq = eq && a1.Annotations.Subset(a2.Annotations) + // NOTE: datastore tests are not very fun to write with timestamp checks, + // and these are not values the user may set so we kind of don't care. + //eq = eq && time.Time(a1.CreatedAt).Equal(time.Time(a2.CreatedAt)) + //eq = eq && time.Time(a1.UpdatedAt).Equal(time.Time(a2.UpdatedAt)) + return eq +} + // Update adds entries from patch to a.Config and a.Annotations, and removes entries with empty values. func (a *App) Update(patch *App) { original := a.Clone() @@ -176,8 +192,12 @@ func (e ErrInvalidSyslog) Error() string { return string(e) } // AppFilter is the filter used for querying apps type AppFilter struct { - // NameIn will filter by all names in the list (IN query) - NameIn []string + Name string PerPage int Cursor string } + +type AppList struct { + NextCursor string `json:"next_cursor,omitempty"` + Items []*App `json:"items"` +} diff --git a/api/models/datastore.go b/api/models/datastore.go index b6664603e..d58cd9ff2 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -15,9 +15,9 @@ type Datastore interface { // Returns ErrAppsNotFound if no app is found. GetAppID(ctx context.Context, appName string) (string, error) - // GetApps gets a slice of Apps, optionally filtered by name. + // GetApps gets a slice of Apps, optionally filtered by name, and a cursor. // Missing filter or empty name will match all Apps. - GetApps(ctx context.Context, filter *AppFilter) ([]*App, error) + GetApps(ctx context.Context, filter *AppFilter) (*AppList, error) // InsertApp inserts an App. Returns ErrDatastoreEmptyApp when app is nil, and // ErrDatastoreEmptyAppName when app.Name is empty. @@ -63,8 +63,8 @@ type Datastore interface { // ErrMissingName is func.Name is empty. UpdateFn(ctx context.Context, fn *Fn) (*Fn, error) - // GetFns returns a list of funcs, applying any additional filters provided. - GetFns(ctx context.Context, filter *FnFilter) ([]*Fn, error) + // GetFns returns a list of funcs, and a cursor, applying any additional filters provided. + GetFns(ctx context.Context, filter *FnFilter) (*FnList, error) // GetFnByID returns a function by ID. Returns ErrDatastoreEmptyFnID if fnID is empty. // Returns ErrFnsNotFound if a fn is not found. @@ -91,7 +91,7 @@ type Datastore interface { // GetTriggers gets a list of triggers that match the specified filter // Return ErrDatastoreEmptyAppId if no AppID set in the filter - GetTriggers(ctx context.Context, filter *TriggerFilter) ([]*Trigger, error) + GetTriggers(ctx context.Context, filter *TriggerFilter) (*TriggerList, error) // implements io.Closer to shutdown io.Closer diff --git a/api/models/fn.go b/api/models/fn.go index 76f208eac..e9bfa2535 100644 --- a/api/models/fn.go +++ b/api/models/fn.go @@ -231,6 +231,28 @@ func (f1 *Fn) Equals(f2 *Fn) bool { return eq } +func (f1 *Fn) EqualsWithAnnotationSubset(f2 *Fn) bool { + // start off equal, check equivalence of each field. + // the RHS of && won't eval if eq==false so config/headers checking is lazy + + eq := true + eq = eq && f1.ID == f2.ID + eq = eq && f1.Name == f2.Name + eq = eq && f1.AppID == f2.AppID + eq = eq && f1.Image == f2.Image + eq = eq && f1.Memory == f2.Memory + eq = eq && f1.Format == f2.Format + eq = eq && f1.Timeout == f2.Timeout + eq = eq && f1.IdleTimeout == f2.IdleTimeout + eq = eq && f1.Config.Equals(f2.Config) + eq = eq && f1.Annotations.Subset(f2.Annotations) + // NOTE: datastore tests are not very fun to write with timestamp checks, + // and these are not values the user may set so we kind of don't care. + //eq = eq && time.Time(f1.CreatedAt).Equal(time.Time(f2.CreatedAt)) + //eq = eq && time.Time(f2.UpdatedAt).Equal(time.Time(f2.UpdatedAt)) + return eq +} + // Update updates fields in f with non-zero field values from new, and sets // updated_at if any of the fields change. 0-length slice Header values, and // empty-string Config values trigger removal of map entry. @@ -279,3 +301,8 @@ type FnFilter struct { Cursor string PerPage int } + +type FnList struct { + NextCursor string `json:"next_cursor,omitempty"` + Items []*Fn `json:"items"` +} diff --git a/api/models/trigger.go b/api/models/trigger.go index 802737a25..61218212c 100644 --- a/api/models/trigger.go +++ b/api/models/trigger.go @@ -36,6 +36,20 @@ func (t *Trigger) Equals(t2 *Trigger) bool { return eq } +func (t *Trigger) EqualsWithAnnotationSubset(t2 *Trigger) bool { + eq := true + eq = eq && t.ID == t2.ID + eq = eq && t.Name == t2.Name + eq = eq && t.AppID == t2.AppID + eq = eq && t.FnID == t2.FnID + + eq = eq && t.Type == t2.Type + eq = eq && t.Source == t2.Source + eq = eq && t.Annotations.Subset(t2.Annotations) + + return eq +} + var triggerTypes = []string{"http"} func ValidTriggerTypes() []string { @@ -178,3 +192,8 @@ type TriggerFilter struct { Cursor string PerPage int } + +type TriggerList struct { + NextCursor string `json:"next_cursor,omitempty"` + Items []*Trigger `json:"items"` +} diff --git a/api/models/trigger_test.go b/api/models/trigger_test.go index f814366f6..53fb67f31 100644 --- a/api/models/trigger_test.go +++ b/api/models/trigger_test.go @@ -26,6 +26,19 @@ func TestTriggerJsonMarshalling(t *testing.T) { } } +func TestTriggerListJsonMarshalling(t *testing.T) { + emptyList := &TriggerList{Items: []*Trigger{}} + expected := "{\"items\":[]}" + + v, err := json.Marshal(emptyList) + if err != nil { + t.Fatalf("Failed to marshal json into %s: %v", expected, err) + } + if string(v) != expected { + t.Errorf("Invalid trigger value, expected %s, got %s", expected, string(v)) + } +} + var httpTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "http", Source: "baz"} var invalidTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "error", Source: "baz"} diff --git a/api/server/apps_list.go b/api/server/apps_list.go index 0348b6e54..6bbab1008 100644 --- a/api/server/apps_list.go +++ b/api/server/apps_list.go @@ -1,7 +1,6 @@ package server import ( - "encoding/base64" "net/http" "github.com/fnproject/fn/api/models" @@ -12,11 +11,10 @@ func (s *Server) handleAppList(c *gin.Context) { ctx := c.Request.Context() filter := &models.AppFilter{} - filter.Cursor, filter.PerPage = pageParams(c, true) - name := c.Query("name") - if name != "" { - filter.NameIn = []string{name} - } + + filter.Cursor, filter.PerPage = pageParamsV2(c) + + filter.Name = c.Query("name") apps, err := s.datastore.GetApps(ctx, filter) if err != nil { @@ -24,14 +22,5 @@ func (s *Server) handleAppList(c *gin.Context) { return } - var nextCursor string - if len(apps) > 0 && len(apps) == filter.PerPage { - last := []byte(apps[len(apps)-1].Name) - nextCursor = base64.RawURLEncoding.EncodeToString(last) - } - - c.JSON(http.StatusOK, appListResponse{ - NextCursor: nextCursor, - Items: apps, - }) + c.JSON(http.StatusOK, apps) } diff --git a/api/server/apps_test.go b/api/server/apps_test.go index f105d256c..3b18b33df 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -209,7 +209,7 @@ func TestAppList(t *testing.T) { } else { // normal path - var resp appListResponse + var resp models.AppList err := json.NewDecoder(rec.Body).Decode(&resp) if err != nil { t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) diff --git a/api/server/apps_v1_list.go b/api/server/apps_v1_list.go index f4973510c..bc6e9e1ad 100644 --- a/api/server/apps_v1_list.go +++ b/api/server/apps_v1_list.go @@ -13,7 +13,7 @@ func (s *Server) handleV1AppList(c *gin.Context) { ctx := c.Request.Context() filter := &models.AppFilter{} - filter.Cursor, filter.PerPage = pageParams(c, true) + filter.Cursor, filter.PerPage = pageParamsV2(c) apps, err := s.datastore.GetApps(ctx, filter) if err != nil { @@ -22,14 +22,14 @@ func (s *Server) handleV1AppList(c *gin.Context) { } var nextCursor string - if len(apps) > 0 && len(apps) == filter.PerPage { - last := []byte(apps[len(apps)-1].Name) + if len(apps.Items) > 0 && len(apps.Items) == filter.PerPage { + last := []byte(apps.Items[len(apps.Items)-1].Name) nextCursor = base64.RawURLEncoding.EncodeToString(last) } c.JSON(http.StatusOK, appsV1Response{ Message: "Successfully listed applications", NextCursor: nextCursor, - Apps: apps, + Apps: apps.Items, }) } diff --git a/api/server/fns_list.go b/api/server/fns_list.go index 955a345f4..73930ea9b 100644 --- a/api/server/fns_list.go +++ b/api/server/fns_list.go @@ -11,7 +11,7 @@ func (s *Server) handleFnList(c *gin.Context) { ctx := c.Request.Context() var filter models.FnFilter - filter.Cursor, filter.PerPage = pageParams(c, false) + filter.Cursor, filter.PerPage = pageParamsV2(c) filter.AppID = c.Query("app_id") filter.Name = c.Query("name") @@ -21,13 +21,5 @@ func (s *Server) handleFnList(c *gin.Context) { return } - var nextCursor string - if len(fns) > 0 && len(fns) == filter.PerPage { - nextCursor = fns[len(fns)-1].Name - } - - c.JSON(http.StatusOK, fnListResponse{ - NextCursor: nextCursor, - Items: fns, - }) + c.JSON(http.StatusOK, fns) } diff --git a/api/server/fns_test.go b/api/server/fns_test.go index c01733cef..d22d29bf4 100644 --- a/api/server/fns_test.go +++ b/api/server/fns_test.go @@ -2,6 +2,7 @@ package server import ( "bytes" + "encoding/base64" "encoding/json" "fmt" "net/http" @@ -233,6 +234,10 @@ func TestFnList(t *testing.T) { srv := testServer(ds, &mqs.Mock{}, fnl, rnr, ServerTypeFull) + fn1b := base64.RawURLEncoding.EncodeToString([]byte(fn1)) + fn2b := base64.RawURLEncoding.EncodeToString([]byte(fn2)) + fn3b := base64.RawURLEncoding.EncodeToString([]byte(fn3)) + for i, test := range []struct { path string body string @@ -244,11 +249,11 @@ func TestFnList(t *testing.T) { }{ {"/v2/fns", "", http.StatusBadRequest, models.ErrFnsMissingAppID, 0, ""}, {fmt.Sprintf("/v2/fns?app_id=%s", app1.ID), "", http.StatusOK, nil, 3, ""}, - {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1", app1.ID), "", http.StatusOK, nil, 1, fn1}, - {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn1), "", http.StatusOK, nil, 1, fn2}, - {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn2), "", http.StatusOK, nil, 1, fn3}, - {fmt.Sprintf("/v2/fns?app_id=%s&per_page=100&cursor=%s", app1.ID, fn3), "", http.StatusOK, nil, 0, ""}, // cursor is empty if per_page > len(results) - {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn3), "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page + {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1", app1.ID), "", http.StatusOK, nil, 1, fn1b}, + {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn1b), "", http.StatusOK, nil, 1, fn2b}, + {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn2b), "", http.StatusOK, nil, 1, fn3b}, + {fmt.Sprintf("/v2/fns?app_id=%s&per_page=100&cursor=%s", app1.ID, fn3b), "", http.StatusOK, nil, 0, ""}, // cursor is empty if per_page > len(results) + {fmt.Sprintf("/v2/fns?app_id=%s&per_page=1&cursor=%s", app1.ID, fn3b), "", http.StatusOK, nil, 0, ""}, // cursor could point to empty page } { _, rec := routerRequest(t, srv.Router, "GET", test.path, nil) @@ -269,7 +274,7 @@ func TestFnList(t *testing.T) { } else { // normal path - var resp fnListResponse + var resp models.FnList err := json.NewDecoder(rec.Body).Decode(&resp) if err != nil { t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) diff --git a/api/server/server.go b/api/server/server.go index 59de3aa8a..bc6fc2c46 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -1013,6 +1013,18 @@ func pageParams(c *gin.Context, base64d bool) (cursor string, perPage int) { return cursor, perPage } +func pageParamsV2(c *gin.Context) (cursor string, perPage int) { + cursor = c.Query("cursor") + + perPage, _ = strconv.Atoi(c.Query("per_page")) + if perPage > 100 { + perPage = 100 + } else if perPage <= 0 { + perPage = 30 + } + return cursor, perPage +} + type appResponse struct { Message string `json:"message"` App *models.App `json:"app"` @@ -1046,18 +1058,3 @@ type callsResponse struct { NextCursor string `json:"next_cursor"` Calls []*models.Call `json:"calls"` } - -type appListResponse struct { - NextCursor string `json:"next_cursor"` - Items []*models.App `json:"items"` -} - -type fnListResponse struct { - NextCursor string `json:"next_cursor"` - Items []*models.Fn `json:"items"` -} - -type triggerListResponse struct { - NextCursor string `json:"next_cursor"` - Items []*models.Trigger `json:"items"` -} diff --git a/api/server/trigger_list.go b/api/server/trigger_list.go index b3f192a68..b8f080e1f 100644 --- a/api/server/trigger_list.go +++ b/api/server/trigger_list.go @@ -1,7 +1,6 @@ package server import ( - "encoding/base64" "net/http" "github.com/fnproject/fn/api/models" @@ -12,7 +11,7 @@ func (s *Server) handleTriggerList(c *gin.Context) { ctx := c.Request.Context() filter := &models.TriggerFilter{} - filter.Cursor, filter.PerPage = pageParams(c, true) + filter.Cursor, filter.PerPage = pageParamsV2(c) filter.AppID = c.Query("app_id") @@ -29,14 +28,5 @@ func (s *Server) handleTriggerList(c *gin.Context) { return } - var nextCursor string - if len(triggers) > 0 && len(triggers) == filter.PerPage { - last := []byte(triggers[len(triggers)-1].ID) - nextCursor = base64.RawURLEncoding.EncodeToString(last) - } - - c.JSON(http.StatusOK, triggerListResponse{ - NextCursor: nextCursor, - Items: triggers, - }) + c.JSON(http.StatusOK, triggers) } diff --git a/api/server/trigger_test.go b/api/server/trigger_test.go index 0d62e031b..b1f64eefa 100644 --- a/api/server/trigger_test.go +++ b/api/server/trigger_test.go @@ -221,6 +221,7 @@ func TestTriggerList(t *testing.T) { }{ {"/v2/triggers?per_page", "", http.StatusBadRequest, nil, 0, ""}, {"/v2/triggers?app_id=app_id1", "", http.StatusOK, nil, 4, ""}, + {"/v2/triggers?app_id=app_id2", "", http.StatusOK, nil, 1, ""}, {"/v2/triggers?app_id=app_id1&name=trigger1", "", http.StatusOK, nil, 1, ""}, {"/v2/triggers?app_id=app_id1&fn_id=fn_id1", "", http.StatusOK, nil, 3, ""}, {"/v2/triggers?app_id=app_id1&fn_id=fn_id1&per_page", "", http.StatusOK, nil, 3, ""}, @@ -249,7 +250,7 @@ func TestTriggerList(t *testing.T) { } else { // normal path - var resp triggerListResponse + var resp models.TriggerList err := json.NewDecoder(rec.Body).Decode(&resp) if err != nil { t.Errorf("Test %d: Expected response body to be a valid json object. err: %v", i, err) diff --git a/fnext/datastore.go b/fnext/datastore.go index fc05d4c26..5758f0b98 100644 --- a/fnext/datastore.go +++ b/fnext/datastore.go @@ -130,7 +130,7 @@ func (e *extds) RemoveApp(ctx context.Context, appName string) error { return e.al.AfterAppDelete(ctx, &app) } -func (e *extds) GetApps(ctx context.Context, filter *models.AppFilter) ([]*models.App, error) { +func (e *extds) GetApps(ctx context.Context, filter *models.AppFilter) (*models.AppList, error) { err := e.al.BeforeAppsList(ctx, filter) if err != nil { return nil, err @@ -141,7 +141,7 @@ func (e *extds) GetApps(ctx context.Context, filter *models.AppFilter) ([]*model return nil, err } - err = e.al.AfterAppsList(ctx, apps) + err = e.al.AfterAppsList(ctx, apps.Items) return apps, err }